Merge branch '40744-idempotent-ids' into 'master'

Use the DatabaseCleaner 'deletion' strategy instead of 'truncation'

Closes #30783

See merge request gitlab-org/gitlab-ce!16516

spec/migrations/rename_users_with_renamed_namespace_spec.rb

 require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20170518200835_rename_users_with_renamed_namespace.rb')
-describe RenameUsersWithRenamedNamespace, truncate: true do
+describe RenameUsersWithRenamedNamespace, :delete do
   it 'renames a user that had their namespace renamed to the namespace path' do
     other_user = create(:user, username: 'kodingu')
     other_user1 = create(:user, username: 'api0')

spec/migrations/update_retried_for_ci_build_spec.rb

 require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20170503004427_update_retried_for_ci_build.rb')
-describe UpdateRetriedForCiBuild, truncate: true do
+describe UpdateRetriedForCiBuild, :delete do
   let(:pipeline) { create(:ci_pipeline) }
   let!(:build_old) { create(:ci_build, pipeline: pipeline, name: 'test') }
   let!(:build_new) { create(:ci_build, pipeline: pipeline, name: 'test') }

spec/models/concerns/avatarable_spec.rb

 require 'spec_helper'
 describe Avatarable do
-  subject { create(:project, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) }
+  set(:project) { create(:project, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) }
   let(:gitlab_host) { "https://gitlab.example.com" }
   let(:relative_url_root) { "/gitlab" }
-  let(:asset_host) { "https://gitlab-assets.example.com" }
+  let(:asset_host) { 'https://gitlab-assets.example.com' }
   before do
     stub_config_setting(base_url: gitlab_host)
@@ -15,29 +15,32 @@ describe Avatarable do
   describe '#avatar_path' do
     using RSpec::Parameterized::TableSyntax
-    where(:has_asset_host, :visibility_level, :only_path, :avatar_path) do
-      true  | Project::PRIVATE  | true  | [gitlab_host, relative_url_root, subject.avatar.url]
-      true  | Project::PRIVATE  | false | [gitlab_host, relative_url_root, subject.avatar.url]
-      true  | Project::INTERNAL | true  | [gitlab_host, relative_url_root, subject.avatar.url]
-      true  | Project::INTERNAL | false | [gitlab_host, relative_url_root, subject.avatar.url]
-      true  | Project::PUBLIC   | true  | [subject.avatar.url]
-      true  | Project::PUBLIC   | false | [asset_host, subject.avatar.url]
-      false | Project::PRIVATE  | true  | [relative_url_root, subject.avatar.url]
-      false | Project::PRIVATE  | false | [gitlab_host, relative_url_root, subject.avatar.url]
-      false | Project::INTERNAL | true  | [relative_url_root, subject.avatar.url]
-      false | Project::INTERNAL | false | [gitlab_host, relative_url_root, subject.avatar.url]
-      false | Project::PUBLIC   | true  | [relative_url_root, subject.avatar.url]
-      false | Project::PUBLIC   | false | [gitlab_host, relative_url_root, subject.avatar.url]
+    where(:has_asset_host, :visibility_level, :only_path, :avatar_path_prefix) do
+      true  | Project::PRIVATE  | true  | [gitlab_host, relative_url_root]
+      true  | Project::PRIVATE  | false | [gitlab_host, relative_url_root]
+      true  | Project::INTERNAL | true  | [gitlab_host, relative_url_root]
+      true  | Project::INTERNAL | false | [gitlab_host, relative_url_root]
+      true  | Project::PUBLIC   | true  | []
+      true  | Project::PUBLIC   | false | [asset_host]
+      false | Project::PRIVATE  | true  | [relative_url_root]
+      false | Project::PRIVATE  | false | [gitlab_host, relative_url_root]
+      false | Project::INTERNAL | true  | [relative_url_root]
+      false | Project::INTERNAL | false | [gitlab_host, relative_url_root]
+      false | Project::PUBLIC   | true  | [relative_url_root]
+      false | Project::PUBLIC   | false | [gitlab_host, relative_url_root]
     end
     with_them do
       before do
-        allow(ActionController::Base).to receive(:asset_host).and_return(has_asset_host ? asset_host : nil)
-        subject.visibility_level = visibility_level
+        allow(ActionController::Base).to receive(:asset_host) { has_asset_host && asset_host }
+
+        project.visibility_level = visibility_level
       end
+      let(:avatar_path) { (avatar_path_prefix + [project.avatar.url]).join }
+
       it 'returns the expected avatar path' do
-        expect(subject.avatar_path(only_path: only_path)).to eq(avatar_path.join)
+        expect(project.avatar_path(only_path: only_path)).to eq(avatar_path)
       end
     end
   end

spec/models/member_spec.rb

@@ -488,7 +488,7 @@ describe Member do
       member.accept_invite!(user)
     end
-    it "refreshes user's authorized projects", :truncate do
+    it "refreshes user's authorized projects", :delete do
       project = member.source
       expect(user.authorized_projects).not_to include(project)
@@ -523,7 +523,7 @@ describe Member do
     end
   end
-  describe "destroying a record", :truncate do
+  describe "destroying a record", :delete do
     it "refreshes user's authorized projects" do
       project = create(:project, :private)
       user    = create(:user)

spec/models/project_group_link_spec.rb

@@ -30,7 +30,7 @@ describe ProjectGroupLink do
     end
   end
-  describe "destroying a record", :truncate do
+  describe "destroying a record", :delete do
     it "refreshes group users' authorized projects" do
       project     = create(:project, :private)
       group       = create(:group)

spec/models/user_spec.rb

@@ -1569,7 +1569,7 @@ describe User do
     it { is_expected.to eq([private_group]) }
   end
-  describe '#authorized_projects', :truncate do
+  describe '#authorized_projects', :delete do
     context 'with a minimum access level' do
       it 'includes projects for which the user is an owner' do
         user = create(:user)

spec/support/db_cleaner.rb

+require 'database_cleaner/active_record/deletion'
+
+module FakeInformationSchema
+  # Work around a bug in DatabaseCleaner when using the deletion strategy:
+  # https://github.com/DatabaseCleaner/database_cleaner/issues/347
+  #
+  # On MySQL, if the information schema is said to exist, we use an inaccurate
+  # row count leading to some tables not being cleaned when they should
+  def information_schema_exists?(_connection)
+    false
+  end
+end
+
+DatabaseCleaner::ActiveRecord::Deletion.prepend(FakeInformationSchema)
+
 RSpec.configure do |config|
+  # Ensure all sequences are reset at the start of the suite run
   config.before(:suite) do
     DatabaseCleaner.clean_with(:truncation)
   end
   config.append_after(:context) do
-    DatabaseCleaner.clean_with(:truncation, cache_tables: false)
+    DatabaseCleaner.clean_with(:deletion, cache_tables: false)
   end
   config.before(:each) do
@@ -12,15 +28,15 @@ RSpec.configure do |config|
   end
   config.before(:each, :js) do
-    DatabaseCleaner.strategy = :truncation
+    DatabaseCleaner.strategy = :deletion
   end
-  config.before(:each, :truncate) do
-    DatabaseCleaner.strategy = :truncation
+  config.before(:each, :delete) do
+    DatabaseCleaner.strategy = :deletion
   end
   config.before(:each, :migration) do
-    DatabaseCleaner.strategy = :truncation, { cache_tables: false }
+    DatabaseCleaner.strategy = :deletion, { cache_tables: false }
   end
   config.before(:each) do

spec/support/features/discussion_comments_shared_example.rb

@@ -143,15 +143,17 @@ shared_examples 'discussion comments' do |resource_name|
         end
         if resource_name == 'merge request'
+          let(:note_id) { find("#{comments_selector} .note", match: :first)['data-note-id'] }
+
           it 'shows resolved discussion when toggled' do
             click_button "Resolve discussion"
-            expect(page).to have_selector('.note-row-1', visible: true)
+            expect(page).to have_selector(".note-row-#{note_id}", visible: true)
             refresh
             click_button "Toggle discussion"
-            expect(page).to have_selector('.note-row-1', visible: true)
+            expect(page).to have_selector(".note-row-#{note_id}", visible: true)
           end
         end
       end

spec/uploaders/job_artifact_uploader_spec.rb

@@ -8,7 +8,7 @@ describe JobArtifactUploader do
   describe '#store_dir' do
     subject { uploader.store_dir }
-    let(:path) { "#{job_artifact.created_at.utc.strftime('%Y_%m_%d')}/#{job_artifact.project_id}/#{job_artifact.id}" }
+    let(:path) { "#{job_artifact.created_at.utc.strftime('%Y_%m_%d')}/#{job_artifact.job_id}/#{job_artifact.id}" }
     context 'when using local storage' do
       it { is_expected.to start_with(local_path) }
@@ -45,7 +45,7 @@ describe JobArtifactUploader do
     it { is_expected.to start_with(local_path) }
     it { is_expected.to include("/#{job_artifact.created_at.utc.strftime('%Y_%m_%d')}/") }
-    it { is_expected.to include("/#{job_artifact.project_id}/") }
+    it { is_expected.to include("/#{job_artifact.job_id}/#{job_artifact.id}/") }
     it { is_expected.to end_with("ci_build_artifacts.zip") }
   end
 end