Merge branch 'fix_next_instance_of_rspec_mock_helper' into 'master'

Fix the `_next_instance_of` mock helpers

See merge request gitlab-org/gitlab!40763

doc/development/gotchas.md

@@ -106,7 +106,7 @@ end
        Using `any_instance` to stub a method (elasticsearch_indexing) that has been defined on a prepended module (EE::ApplicationSetting) is not supported.
   ```
 
-### Alternative: `expect_next_instance_of` or `allow_next_instance_of`
+### Alternative: `expect_next_instance_of`, `allow_next_instance_of`, `expect_next_found_instance_of` or `allow_next_found_instance_of`
 
 Instead of writing:
 
@@ -130,8 +130,21 @@ end
 allow_next_instance_of(Project) do |project|
   allow(project).to receive(:add_import_job)
 end
+
+# Do this:
+expect_next_found_instance_of(Project) do |project|
+  expect(project).to receive(:add_import_job)
+end
+
+# Do this:
+allow_next_found_instance_of(Project) do |project|
+  allow(project).to receive(:add_import_job)
+end
 ```
 
+_**Note:** Since Active Record is not calling the `.new` method on model classes to instantiate the objects,
+you should use `expect_next_found_instance_of` or `allow_next_found_instance_of` mock helpers to setup mock on objects returned by Active Record query & finder methods._
+
 If we also want to initialize the instance with some particular arguments, we
 could also pass it like:
 

spec/spec_helper.rb

@@ -115,6 +115,7 @@ RSpec.configure do |config|
   config.include StubExperiments
   config.include StubGitlabCalls
   config.include StubGitlabData
+  config.include NextFoundInstanceOf
   config.include NextInstanceOf
   config.include TestEnv
   config.include Devise::Test::ControllerHelpers, type: :controller

spec/support/helpers/next_found_instance_of.rb

+# frozen_string_literal: true
+
+module NextFoundInstanceOf
+  ERROR_MESSAGE = 'NextFoundInstanceOf mock helpers can only be used with ActiveRecord targets'
+
+  def expect_next_found_instance_of(klass)
+    check_if_active_record!(klass)
+
+    stub_allocate(expect(klass)) do |expectation|
+      yield(expectation)
+    end
+  end
+
+  def allow_next_found_instance_of(klass)
+    check_if_active_record!(klass)
+
+    stub_allocate(allow(klass)) do |allowance|
+      yield(allowance)
+    end
+  end
+
+  private
+
+  def check_if_active_record!(klass)
+    raise ArgumentError.new(ERROR_MESSAGE) unless klass < ActiveRecord::Base
+  end
+
+  def stub_allocate(target)
+    target.to receive(:allocate).and_wrap_original do |method|
+      method.call.tap { |allocation| yield(allocation) }
+    end
+  end
+end

spec/workers/new_note_worker_spec.rb

@@ -54,14 +54,10 @@ RSpec.describe NewNoteWorker do
     let(:note) { create(:note) }
 
     before do
-      # TODO: `allow_next_instance_of` helper method is not working
-      # because ActiveRecord is directly calling `.allocate` on model
-      # classes and bypasses the `.new` method call.
-      # Fix the `allow_next_instance_of` helper and change these to mock
-      # the next instance of `Note` model class.
-      allow(Note).to receive(:find_by).with(id: note.id).and_return(note)
+      allow_next_found_instance_of(Note) do |note|
         allow(note).to receive(:skip_notification?).and_return(true)
       end
+    end
 
     it 'does not create a new note notification' do
       expect_any_instance_of(NotificationService).not_to receive(:new_note)