Merge branch 'backstage/gb/migrations-tests-schema-version' into 'master'9-5-stable-patch-3

Improve migrations / background migrations testing strategy

Closes #36303

See merge request !13589

5 files changed, 47 insertions, 28 deletions

diff --git a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb
index 87f45619e7a..0d5fffa38ff 100644
--- a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb
+++ b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb
@@ -210,7 +210,11 @@ describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads::Event do
   end
 end
 
-describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads do
+##
+# The background migration relies on a temporary table, hence we're migrating
+# to a specific version of the database where said table is still present.
+#
+describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads, :migration, schema: 20170608152748 do
   let(:migration) { described_class.new }
   let(:project) { create(:project_empty_repo) }
   let(:author) { create(:user) }
@@ -229,21 +233,6 @@ describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads do
     )
   end
 
-  # The background migration relies on a temporary table, hence we're migrating
-  # to a specific version of the database where said table is still present.
-  before :all do
-    ActiveRecord::Migration.verbose = false
-
-    ActiveRecord::Migrator
-      .migrate(ActiveRecord::Migrator.migrations_paths, 20170608152748)
-  end
-
-  after :all do
-    ActiveRecord::Migrator.migrate(ActiveRecord::Migrator.migrations_paths)
-
-    ActiveRecord::Migration.verbose = true
-  end
-
   describe '#perform' do
     it 'returns if data should not be migrated' do
       allow(migration).to receive(:migrate?).and_return(false)
diff --git a/spec/migrations/README.md b/spec/migrations/README.md
index 05d4f35db72..45cf25b96de 100644
--- a/spec/migrations/README.md
+++ b/spec/migrations/README.md
@@ -28,6 +28,14 @@ The `after` hook will migrate the database **up** and reinstitutes the latest
 schema version, so that the process does not affect subsequent specs and
 ensures proper isolation.
 
+## Testing a class that is not an ActiveRecord::Migration
+
+In order to test a class that is not a migration itself, 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.
+
+Example: `describe SomeClass, :migration, schema: 20170608152748`.
+
 ## Available helpers
 
 Use `table` helper to create a temporary `ActiveRecord::Base` derived model
@@ -80,8 +88,6 @@ end
 
 ## Best practices
 
-1. Use only one test example per migration unless there is a good reason to
-use more.
 1. Note that this type of tests do not run within the transaction, we use
 a truncation database cleanup strategy. Do not depend on transaction being
 present.
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index d35c82b293c..2580776afb0 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -136,17 +136,12 @@ RSpec.configure do |config|
     Sidekiq.redis(&:flushall)
   end
 
-  config.before(:example, :migration) do
-    ActiveRecord::Migrator
-      .migrate(migrations_paths, previous_migration.version)
-
-    reset_column_in_migration_models
+  config.before(:each, :migration) do
+    schema_migrate_down!
   end
 
-  config.after(:example, :migration) do
-    ActiveRecord::Migrator.migrate(migrations_paths)
-
-    reset_column_in_migration_models
+  config.after(:context, :migration) do
+    schema_migrate_up!
   end
 
   config.around(:each, :nested_groups) do |example|
diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb
index 7f5769209bb..b0f520d08e8 100644
--- a/spec/support/db_cleaner.rb
+++ b/spec/support/db_cleaner.rb
@@ -20,7 +20,7 @@ RSpec.configure do |config|
   end
 
   config.before(:each, :migration) do
-    DatabaseCleaner.strategy = :truncation
+    DatabaseCleaner.strategy = :truncation, { cache_tables: false }
   end
 
   config.before(:each) do
diff --git a/spec/support/migrations_helpers.rb b/spec/support/migrations_helpers.rb
index ef9d15016d1..4ca019c1b05 100644
--- a/spec/support/migrations_helpers.rb
+++ b/spec/support/migrations_helpers.rb
@@ -33,6 +33,35 @@ module MigrationsHelpers
     end
   end
 
+  def migration_schema_version
+    self.class.metadata[:schema] || previous_migration.version
+  end
+
+  def schema_migrate_down!
+    disable_migrations_output do
+      ActiveRecord::Migrator.migrate(migrations_paths,
+                                     migration_schema_version)
+    end
+
+    reset_column_in_migration_models
+  end
+
+  def schema_migrate_up!
+    disable_migrations_output do
+      ActiveRecord::Migrator.migrate(migrations_paths)
+    end
+
+    reset_column_in_migration_models
+  end
+
+  def disable_migrations_output
+    ActiveRecord::Migration.verbose = false
+
+    yield
+  ensure
+    ActiveRecord::Migration.verbose = true
+  end
+
   def migrate!
     ActiveRecord::Migrator.up(migrations_paths) do |migration|
       migration.name == described_class.name