Merge branch 'pl-rspec-licensed-feature-cache' into 'master'

Clear cache for project's licensed features in EE specs [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!54215

doc/development/testing_guide/best_practices.md

@@ -521,35 +521,6 @@ let_it_be_with_refind(:project) { create(:project) }
 let_it_be(:project, refind: true) { create(:project) }
 ```
 
-### License stubbing with `let_it_be`
-
-`let_it_be_with_refind` is also useful when using `stub_licensed_features` in your tests:
-
-```ruby
-let_it_be_with_refind(:project) { create(:project) }
-# Project#licensed_feature_available? is memoized, and so we need to refind
-# the project for license updates to be applied.
-# An alternative is `project.clear_memoization(:licensed_feature_available)`.
-
-subject { project.allows_multiple_assignees? }
-
-context 'with license multiple_issue_assignees disabled' do
-  before do
-    stub_licensed_features(multiple_issue_assignees: true)
-  end
-
-  it { is_expected.to eq(true) }
-end
-
-context 'with license multiple_issue_assignees disabled' do
-  before do
-    stub_licensed_features(multiple_issue_assignees: false)
-  end
-
-  it { is_expected.to eq(false) }
-end
-```
-
 ### Time-sensitive tests
 
 [`ActiveSupport::Testing::TimeHelpers`](https://api.rubyonrails.org/v6.0.3.1/classes/ActiveSupport/Testing/TimeHelpers.html)

ee/spec/models/project_spec.rb

@@ -897,9 +897,11 @@ RSpec.describe Project do
 
     it 'only loads licensed availability once' do
       expect(project).to receive(:load_licensed_feature_available)
-                             .once.and_call_original
+        .once.and_call_original
 
-      2.times { project.feature_available?(:push_rules) }
+      with_license_feature_cache do
+        2.times { project.feature_available?(:push_rules) }
+      end
     end
 
     context 'when feature symbol is not included on Namespace features code' do

ee/spec/requests/api/project_mirror_spec.rb

@@ -293,7 +293,6 @@ RSpec.describe API::ProjectMirror do
           context 'when repository_mirrors feature is not available' do
             before do
               stub_licensed_features(repository_mirrors: false)
-              project_mirrored.clear_memoization(:licensed_feature_available)
             end
 
             it_behaves_like 'an API endpoint that does not trigger pull mirroring operation', :bad_request
@@ -302,7 +301,6 @@ RSpec.describe API::ProjectMirror do
           context 'when repository_mirrors feature is available' do
             before do
               stub_licensed_features(repository_mirrors: true)
-              project_mirrored.clear_memoization(:licensed_feature_available)
             end
 
             it_behaves_like 'an API endpoint that triggers pull mirroring operation'

ee/spec/services/ci/external_pull_requests/process_github_event_service_spec.rb

@@ -165,7 +165,6 @@ RSpec.describe Ci::ExternalPullRequests::ProcessGithubEventService do
       let(:source_sha) { double }
 
       before do
-        project.clear_memoization(:licensed_feature_available)
         allow(project).to receive(:mirror?).and_return(true)
         stub_licensed_features(ci_cd_projects: false, github_project_service_integration: false)
       end

ee/spec/services/dast/profiles/create_service_spec.rb

@@ -16,10 +16,6 @@ RSpec.describe Dast::Profiles::CreateService do
   subject { described_class.new(container: project, current_user: developer, params: params).execute }
 
   describe 'execute' do
-    before do
-      project.clear_memoization(:licensed_feature_available)
-    end
-
     context 'when on demand scan feature is disabled' do
       it 'communicates failure' do
         stub_licensed_features(security_on_demand_scans: true)

ee/spec/services/dast/profiles/destroy_service_spec.rb

@@ -18,10 +18,6 @@ RSpec.describe Dast::Profiles::DestroyService do
   end
 
   describe '#execute' do
-    before do
-      project.clear_memoization(:licensed_feature_available)
-    end
-
     context 'when the feature flag dast_saved_scans is disabled' do
       it 'communicates failure' do
         stub_licensed_features(security_on_demand_scans: true)

ee/spec/services/dast/profiles/update_service_spec.rb

@@ -30,10 +30,6 @@ RSpec.describe Dast::Profiles::UpdateService do
   end
 
   describe 'execute', :clean_gitlab_redis_shared_state do
-    before do
-      project.clear_memoization(:licensed_feature_available)
-    end
-
     context 'when on demand scan feature is disabled' do
       it 'communicates failure' do
         stub_licensed_features(security_on_demand_scans: true)

ee/spec/services/dast_site_validations/create_service_spec.rb

@@ -12,10 +12,6 @@ RSpec.describe DastSiteValidations::CreateService do
   subject { described_class.new(container: dast_site.project, params: params).execute }
 
   describe 'execute', :clean_gitlab_redis_shared_state do
-    before do
-      project.clear_memoization(:licensed_feature_available)
-    end
-
     context 'when on demand scan feature is disabled' do
       it 'communicates failure' do
         stub_licensed_features(security_on_demand_scans: true)

ee/spec/services/dast_site_validations/revoke_service_spec.rb

@@ -16,10 +16,6 @@ RSpec.describe DastSiteValidations::RevokeService do
   subject { described_class.new(container: project, params: params).execute }
 
   describe 'execute', :clean_gitlab_redis_shared_state do
-    before do
-      project.clear_memoization(:licensed_feature_available)
-    end
-
     context 'when on demand scan feature is disabled' do
       it 'communicates failure' do
         stub_licensed_features(security_on_demand_scans: true)

ee/spec/services/projects/prometheus/alerts/notify_service_spec.rb

@@ -16,11 +16,6 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do
 
   let(:subject) { service.execute(token_input) }
 
-  before do
-    # We use `let_it_be(:project)` so we make sure to clear caches
-    project.clear_memoization(:licensed_feature_available)
-  end
-
   context 'with valid payload' do
     let(:alert_firing) { create(:prometheus_alert, project: project) }
     let(:alert_resolved) { create(:prometheus_alert, project: project) }

ee/spec/support/helpers/ee/license_helpers.rb

@@ -31,6 +31,13 @@ module EE
         end
       end
 
+      # Do not clear license feature cache in this block.
+      #
+      # Useful for specs which rely on caching license features.
+      def with_license_feature_cache(&block)
+        ClearLicensedFeatureAvailableCache.without_clear_cache(&block)
+      end
+
       def enable_namespace_license_check!
         stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
         ::Gitlab::CurrentSettings.update!(check_namespace_plan: true)
@@ -42,6 +49,35 @@ module EE
         gl_license = create(:gitlab_license, options)
         create(:license, data: gl_license.export)
       end
+
+      ::Project.prepend ClearLicensedFeatureAvailableCache
+    end
+
+    # This patch helps `stub_licensed_features` to work properly
+    # without the need of clearing caches manually in `before` blocks or
+    # using `let_it_be_refind` deliberately.
+    #
+    # See https://gitlab.com/gitlab-org/gitlab/-/issues/10385
+    module ClearLicensedFeatureAvailableCache
+      class << self
+        attr_accessor :clear_cache
+
+        def without_clear_cache
+          self.clear_cache = false
+          yield
+        ensure
+          self.clear_cache = true
+        end
+      end
+
+      # Enabled by default but can be disabled via `without_clear_cache`.
+      self.clear_cache = true
+
+      def licensed_feature_available?(*)
+        clear_memoization(:licensed_feature_available) if ClearLicensedFeatureAvailableCache.clear_cache
+
+        super
+      end
     end
   end
 end

spec/support/shared_contexts/services_shared_context.rb

@@ -62,7 +62,6 @@ Service.available_services_names.each do |service|
 
       stub_licensed_features(licensed_feature => true)
       project.clear_memoization(:disabled_services)
-      project.clear_memoization(:licensed_feature_available)
     end
   end
 end