From 206fa16cee1654aa74c46204183bdb89cffea20f Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 17 Aug 2017 11:54:54 +0000 Subject: Merge branch 'backstage/gb/migrations-tests-schema-version' into 'master' Improve migrations / background migrations testing strategy Closes #36303 See merge request !13589 --- .../migrate_events_to_push_event_payloads_spec.rb | 21 ++++------------ spec/migrations/README.md | 10 ++++++-- spec/spec_helper.rb | 13 +++------- spec/support/db_cleaner.rb | 2 +- spec/support/migrations_helpers.rb | 29 ++++++++++++++++++++++ 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 -- cgit v1.2.1