Merge branch 'automatically-use-latest-schema-in-background-migrations' into 'master'

Automatically use latest schema in background migrations

See merge request gitlab-org/gitlab!27122

doc/development/background_migrations.md

@@ -301,12 +301,13 @@ It is required to write tests for:
 - The background migration itself.
 - A cleanup migration.
 
-You can use the `:migration` RSpec tag when testing the migrations.
+The `:migration` and `schema: :latest` RSpec tags are automatically set for
+background migration specs.
 See the
 [Testing Rails migrations](testing_guide/testing_migrations_guide.md#testing-a-non-activerecordmigration-class)
 style guide.
 
-When you do that, keep in mind that `before` and `after` RSpec hooks are going
+Keep in mind that `before` and `after` RSpec hooks are going
 to migrate you database down and up, which can result in other background
 migrations being called. That means that using `spy` test doubles with
 `have_received` is encouraged, instead of using regular test doubles, because

doc/development/testing_guide/testing_migrations_guide.md

@@ -158,7 +158,9 @@ end
 
 To test a non-`ActiveRecord::Migration` test (a background migration),
 you will need to manually provide a required schema version. Please add a
-schema tag to a context that you want to switch the database schema within.
+`schema` tag to a context that you want to switch the database schema within.
+
+If not set, `schema` defaults to `:latest`.
 
 Example:
 

ee/spec/lib/ee/gitlab/background_migration/backfill_version_data_from_gitaly_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 # rubocop:disable RSpec/FactoriesInMigrationSpecs
-describe Gitlab::BackgroundMigration::BackfillVersionDataFromGitaly, schema: :latest do
+describe Gitlab::BackgroundMigration::BackfillVersionDataFromGitaly do
   let(:issue) { create(:issue) }
 
   def perform_worker

ee/spec/lib/ee/gitlab/background_migration/migrate_approver_to_approval_rules_check_progress_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRulesCheckProgress, schema: :latest do
+describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRulesCheckProgress do
   context 'when there is MigrateApproverToApprovalRulesInBatch jobs' do
     it 'reschedules check' do
       allow(Gitlab::BackgroundMigration).to receive(:exists?).with('MigrateApproverToApprovalRulesInBatch').and_return(true)

ee/spec/lib/ee/gitlab/background_migration/migrate_approver_to_approval_rules_in_batch_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 # rubocop:disable RSpec/FactoriesInMigrationSpecs
-describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRulesInBatch, schema: :latest do
+describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRulesInBatch do
   context 'when there is no more MigrateApproverToApprovalRules jobs' do
     let(:job) { double(:job) }
     let(:project) { create(:project) }

ee/spec/lib/ee/gitlab/background_migration/migrate_approver_to_approval_rules_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 # rubocop:disable RSpec/FactoriesInMigrationSpecs
-describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules, schema: :latest do
+describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules do
   def create_skip_sync(*args)
     build(*args) do |record|
       allow(record).to receive(:schedule_approval_migration)

ee/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb

@@ -5,7 +5,7 @@ require './db/post_migrate/20191115115043_migrate_epic_mentions_to_db'
 require './db/post_migrate/20191115115522_migrate_epic_notes_mentions_to_db'
 require './db/post_migrate/20200124110831_migrate_design_notes_mentions_to_db'
 
-describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMention, schema: :latest do
+describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMention do
   include MigrationsHelpers
 
   context 'when migrating data' do

lib/quality/test_level.rb

@@ -7,6 +7,8 @@ module Quality
     TEST_LEVEL_FOLDERS = {
       migration: %w[
         migrations
+      ],
+      background_migration: %w[
         lib/gitlab/background_migration
         lib/ee/gitlab/background_migration
       ],
@@ -70,7 +72,7 @@ module Quality
       case file_path
       # Detect migration first since some background migration tests are under
       # spec/lib/gitlab/background_migration and tests under spec/lib are unit by default
-      when regexp(:migration)
+      when regexp(:migration), regexp(:background_migration)
         :migration
       when regexp(:unit)
         :unit
@@ -83,6 +85,10 @@ module Quality
       end
     end
 
+    def background_migration?(file_path)
+      !!(file_path =~ regexp(:background_migration))
+    end
+
     private
 
     def folders_pattern(level)

spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 # rubocop:disable RSpec/FactoriesInMigrationSpecs
-describe Gitlab::BackgroundMigration::BackfillProjectRepositories, schema: :latest do
+describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
   let(:group) { create(:group, name: 'foo', path: 'foo') }
 
   describe described_class::ShardFinder do

spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb

@@ -2,7 +2,7 @@
 require 'spec_helper'
 
 # rubocop: disable RSpec/FactoriesInMigrationSpecs
-describe Gitlab::BackgroundMigration::LegacyUploadMover, schema: :latest do
+describe Gitlab::BackgroundMigration::LegacyUploadMover do
   let(:test_dir) { FileUploader.options['storage_path'] }
   let(:filename) { 'image.png' }
 

spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb

@@ -2,7 +2,7 @@
 require 'spec_helper'
 
 # rubocop: disable RSpec/FactoriesInMigrationSpecs
-describe Gitlab::BackgroundMigration::LegacyUploadsMigrator, schema: :latest do
+describe Gitlab::BackgroundMigration::LegacyUploadsMigrator do
   let(:test_dir) { FileUploader.options['storage_path'] }
 
   let!(:hashed_project) { create(:project) }

spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::BackgroundMigration::MergeRequestAssigneesMigrationProgressCheck, schema: :latest do
+describe Gitlab::BackgroundMigration::MergeRequestAssigneesMigrationProgressCheck do
   context 'rescheduling' do
     context 'when there are ongoing and no dead jobs' do
       it 'reschedules check' do

spec/lib/gitlab/background_migration_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::BackgroundMigration, schema: :latest do
+describe Gitlab::BackgroundMigration do
   describe '.queue' do
     it 'returns background migration worker queue' do
       expect(described_class.queue)

spec/lib/quality/test_level_spec.rb

@@ -28,7 +28,14 @@ RSpec.describe Quality::TestLevel do
     context 'when level is migration' do
       it 'returns a pattern' do
         expect(subject.pattern(:migration))
-          .to eq("spec/{migrations,lib/gitlab/background_migration,lib/ee/gitlab/background_migration}{,/**/}*_spec.rb")
+          .to eq("spec/{migrations}{,/**/}*_spec.rb")
+      end
+    end
+
+    context 'when level is background_migration' do
+      it 'returns a pattern' do
+        expect(subject.pattern(:background_migration))
+          .to eq("spec/{lib/gitlab/background_migration,lib/ee/gitlab/background_migration}{,/**/}*_spec.rb")
       end
     end
 
@@ -89,7 +96,14 @@ RSpec.describe Quality::TestLevel do
     context 'when level is migration' do
       it 'returns a regexp' do
         expect(subject.regexp(:migration))
-          .to eq(%r{spec/(migrations|lib/gitlab/background_migration|lib/ee/gitlab/background_migration)})
+          .to eq(%r{spec/(migrations)})
+      end
+    end
+
+    context 'when level is background_migration' do
+      it 'returns a regexp' do
+        expect(subject.regexp(:background_migration))
+          .to eq(%r{spec/(lib/gitlab/background_migration|lib/ee/gitlab/background_migration)})
       end
     end
 
@@ -160,4 +174,26 @@ RSpec.describe Quality::TestLevel do
           %r{Test level for spec/unknown/foo_spec.rb couldn't be set. Please rename the file properly or change the test level detection regexes in .+/lib/quality/test_level.rb.})
     end
   end
+
+  describe '#background_migration?' do
+    it 'returns false for a unit test' do
+      expect(subject.background_migration?('spec/models/abuse_report_spec.rb')).to be(false)
+    end
+
+    it 'returns true for a migration test' do
+      expect(subject.background_migration?('spec/migrations/add_default_and_free_plans_spec.rb')).to be(false)
+    end
+
+    it 'returns true for a background migration test' do
+      expect(subject.background_migration?('spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb')).to be(true)
+    end
+
+    it 'returns true for a geo migration test' do
+      expect(described_class.new('ee/').background_migration?('ee/spec/migrations/geo/migrate_ci_job_artifacts_to_separate_registry_spec.rb')).to be(false)
+    end
+
+    it 'returns true for a EE-namespaced background migration test' do
+      expect(described_class.new('ee/').background_migration?('ee/spec/lib/ee/gitlab/background_migration/prune_orphaned_geo_events_spec.rb')).to be(true)
+    end
+  end
 end

spec/spec_helper.rb

@@ -81,6 +81,11 @@ RSpec.configure do |config|
       metadata[:migration] = true if metadata[:level] == :migration
     end
 
+    # Do not overwrite schema if it's already set
+    unless metadata.key?(:schema)
+      metadata[:schema] = :latest if quality_level.background_migration?(location)
+    end
+
     # Do not overwrite type if it's already set
     unless metadata.key?(:type)
       match = location.match(%r{/spec/([^/]+)/})