Merge branch '335814-fix-spec-violations-cross-modification' into 'master'

Fix spec violations for internal_ids cross-modification

See merge request gitlab-org/gitlab!75316

spec/factories/ci/pipelines.rb

@@ -18,15 +18,13 @@ FactoryBot.define do
     transient { child_of { nil } }
     transient { upstream_of { nil } }
 
-    before(:create) do |pipeline, evaluator|
-      pipeline.ensure_project_iid!
-    end
-
     after(:build) do |pipeline, evaluator|
       if evaluator.child_of
         pipeline.project = evaluator.child_of.project
         pipeline.source = :parent_pipeline
       end
+
+      pipeline.ensure_project_iid!
     end
 
     after(:create) do |pipeline, evaluator|

spec/requests/api/commit_statuses_spec.rb

@@ -14,8 +14,19 @@ RSpec.describe API::CommitStatuses do
     let(:get_url) { "/projects/#{project.id}/repository/commits/#{sha}/statuses" }
 
     context 'ci commit exists' do
-      let!(:master) { project.ci_pipelines.create!(source: :push, sha: commit.id, ref: 'master', protected: false) }
-      let!(:develop) { project.ci_pipelines.create!(source: :push, sha: commit.id, ref: 'develop', protected: false) }
+      let!(:master) do
+        project.ci_pipelines.build(source: :push, sha: commit.id, ref: 'master', protected: false).tap do |p|
+          p.ensure_project_iid! # Necessary to avoid cross-database modification error
+          p.save!
+        end
+      end
+
+      let!(:develop) do
+        project.ci_pipelines.build(source: :push, sha: commit.id, ref: 'develop', protected: false).tap do |p|
+          p.ensure_project_iid! # Necessary to avoid cross-database modification error
+          p.save!
+        end
+      end
 
       context "reporter user" do
         let(:statuses_id) { json_response.map { |status| status['id'] } }
@@ -310,8 +321,19 @@ RSpec.describe API::CommitStatuses do
         end
 
         context 'when a pipeline id is specified' do
-          let!(:first_pipeline) { project.ci_pipelines.create!(source: :push, sha: commit.id, ref: 'master', status: 'created') }
-          let!(:other_pipeline) { project.ci_pipelines.create!(source: :push, sha: commit.id, ref: 'master', status: 'created') }
+          let!(:first_pipeline) do
+            project.ci_pipelines.build(source: :push, sha: commit.id, ref: 'master', status: 'created').tap do |p|
+              p.ensure_project_iid! # Necessary to avoid cross-database modification error
+              p.save!
+            end
+          end
+
+          let!(:other_pipeline) do
+            project.ci_pipelines.build(source: :push, sha: commit.id, ref: 'master', status: 'created').tap do |p|
+              p.ensure_project_iid! # Necessary to avoid cross-database modification error
+              p.save!
+            end
+          end
 
           subject do
             post api(post_url, developer), params: {

spec/support/database/cross-database-modification-allowlist.yml

@@ -46,7 +46,6 @@
 - "./spec/models/ci/group_variable_spec.rb"
 - "./spec/models/ci/job_artifact_spec.rb"
 - "./spec/models/ci/job_variable_spec.rb"
-- "./spec/models/ci/pipeline_spec.rb"
 - "./spec/models/ci/runner_spec.rb"
 - "./spec/models/ci/variable_spec.rb"
 - "./spec/models/clusters/applications/runner_spec.rb"
@@ -63,7 +62,6 @@
 - "./spec/models/user_status_spec.rb"
 - "./spec/requests/api/ci/pipeline_schedules_spec.rb"
 - "./spec/requests/api/ci/pipelines_spec.rb"
-- "./spec/requests/api/commit_statuses_spec.rb"
 - "./spec/requests/api/commits_spec.rb"
 - "./spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb"
 - "./spec/requests/api/resource_access_tokens_spec.rb"

spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb

@@ -80,15 +80,22 @@ RSpec.shared_examples 'AtomicInternalId' do |validate_presence: true|
       it 'calls InternalId.generate_next and sets internal id attribute' do
         iid = rand(1..1000)
 
-        expect(InternalId).to receive(:generate_next).with(instance, scope_attrs, usage, any_args).and_return(iid)
+        # Need to do this before evaluating instance otherwise it gets set
+        # already in factory
+        allow(InternalId).to receive(:generate_next).and_return(iid)
+
         subject
         expect(read_internal_id).to eq(iid)
+
+        expect(InternalId).to have_received(:generate_next).with(instance, scope_attrs, usage, any_args)
       end
 
       it 'does not overwrite an existing internal id' do
         write_internal_id(4711)
 
-        expect { subject }.not_to change { read_internal_id }
+        allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/347091') do
+          expect { subject }.not_to change { read_internal_id }
+        end
       end
 
       context 'when the instance has an internal ID set' do
@@ -101,6 +108,7 @@ RSpec.shared_examples 'AtomicInternalId' do |validate_presence: true|
             .to receive(:track_greatest)
             .with(instance, scope_attrs, usage, internal_id, any_args)
             .and_return(internal_id)
+
           subject
         end
       end
@@ -110,7 +118,11 @@ RSpec.shared_examples 'AtomicInternalId' do |validate_presence: true|
       context 'when the internal id has been changed' do
         context 'when the internal id is automatically set' do
           it 'clears it on the instance' do
-            expect_iid_to_be_set_and_rollback
+            write_internal_id(nil)
+
+            allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/347091') do
+              expect_iid_to_be_set_and_rollback
+            end
 
             expect(read_internal_id).to be_nil
           end
@@ -120,7 +132,9 @@ RSpec.shared_examples 'AtomicInternalId' do |validate_presence: true|
           it 'does not clear it on the instance' do
             write_internal_id(100)
 
-            expect_iid_to_be_set_and_rollback
+            allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/347091') do
+              expect_iid_to_be_set_and_rollback
+            end
 
             expect(read_internal_id).not_to be_nil
           end