Skip to content

Draft: Run migration only on specified database

Rutger Wessels requested to merge 415824-specs-support-database-key into master

Run migration only on specified database

What does this MR do and why?

This MR changes the behaviour of rspec migration tests: the rspec tests are run using the database that is configured for the specified Gitlab Schema.

If you use RSpec.describe SomeMigration, database: :ci, the migrations (both the actual test and the surrounding up/down migrations) are run against ci

The same is valid when you specify a Gitlab schema: RSpec.describe SomeMigration, migration: :gitlab_ci

Running tests only on database will improve the overall runtime of the specs.

Issue #415824

Implementation

On master, we have two implementations for retrieving the database connection:

  • spec/support/migration.rb in config.around(:each, :migration) hook
  • spec/support/helpers/migrations_helpers.rb (def active_record_base)

The first did support migration key in RSpec.describe method calls, the second supported database key. I refactored active_record_base method:

  • Adding a database key to RSpec.describe will take precedence over migration: :<some schema>
  • We use for both places (migration helper and RSpec hooks) the very same implementation

Additionally, I removed special handling for Geo database. Migrations for geo now need migration: :gitlab_geo and not :geo. There is still one geo-specific code snippet (rescue ::Geo::TrackingBase::SecondaryNotConfigured) that should be refactored. We can use issue #415828 for that

TODO

  • Code cleanup (marked as TODO in the code)
  • There are still some failing specs (probably related to special handling of MODE_SINGLE_DATABASE_CI_CONNECTION)
  • if migration: :gitlab_main then migrations are still run on both main and ci. However, if migration: gitlab_ci, them migrations are only run on ci. Workaround: use database: :main
  • Geo migrations: no longer use :geo but use migration: :gitlab_geo
    • Find out: do we have a generator for these migrations
    • Involve geo db people (inform about this change)

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #415824

Edited by Rutger Wessels

Merge request reports