From 4af914c25c44e88556b93ac5b08d63ee22e27c5c Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 6 Feb 2018 23:44:45 -0800 Subject: [PATCH 1/6] Disable caching of tables for migration spec that drops a temporary table This is to fix job failures, such as https://gitlab.com/gitlab-org/gitlab-ee/-/jobs/51409392. --- .../background_migration/populate_untracked_uploads_spec.rb | 2 +- spec/support/db_cleaner.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb index 8590522f3ef..5caf7e6cc4c 100644 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq do +describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :delete_no_cache do include TrackUntrackedUploadsHelpers subject { described_class.new } diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index 1809ae1d141..259e45ed8e2 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -35,6 +35,10 @@ RSpec.configure do |config| DatabaseCleaner.strategy = :deletion end + config.before(:each, :delete_no_cache) do + DatabaseCleaner.strategy = :deletion, { cache_tables: false } + end + config.before(:each, :migration) do DatabaseCleaner.strategy = :deletion, { cache_tables: false } end From d1eac99b92635429eb0ea4e11c6f61e0494f9eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 7 Feb 2018 10:31:56 +0100 Subject: [PATCH 2/6] Use the :migration metadata in migration specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .../populate_untracked_uploads_spec.rb | 7 ++++++- spec/support/db_cleaner.rb | 4 ---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb index 5caf7e6cc4c..fb3f29ff4c9 100644 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb @@ -1,6 +1,11 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :delete_no_cache do +# This migration is using UploadService, which sets uploads.secret that is only +# added to the DB schema in 20180129193323. Since the test isn't isolated, we +# just use the latest schema when testing this migration. +# Ideally, the test should not use factories nor UploadService, and rely on the +# `table` helper instead. +describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :migration, schema: 20180129193323 do include TrackUntrackedUploadsHelpers subject { described_class.new } diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index 259e45ed8e2..1809ae1d141 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -35,10 +35,6 @@ RSpec.configure do |config| DatabaseCleaner.strategy = :deletion end - config.before(:each, :delete_no_cache) do - DatabaseCleaner.strategy = :deletion, { cache_tables: false } - end - config.before(:each, :migration) do DatabaseCleaner.strategy = :deletion, { cache_tables: false } end From 5912d9b70b35769b7da468113ea6680ef8cbc309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 12 Feb 2018 15:33:39 +0100 Subject: [PATCH 3/6] Call .reset_column_in_all_models! before migrating in MigrationsHelpers.schema_migrate_up! MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/support/migrations_helpers.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/support/migrations_helpers.rb b/spec/support/migrations_helpers.rb index 06322aa0586..9a151439486 100644 --- a/spec/support/migrations_helpers.rb +++ b/spec/support/migrations_helpers.rb @@ -58,6 +58,8 @@ module MigrationsHelpers end def schema_migrate_up! + reset_column_in_all_models + disable_migrations_output do ActiveRecord::Migrator.migrate(migrations_paths) end From f6357b77ff8aa36b143794b43d42145f5a6bb642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 12 Feb 2018 15:31:12 +0100 Subject: [PATCH 4/6] Allow to use the latest migration in migration specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is useful for migration tests that relies on factories and that are very old and/or tedious to modify to not use factories. Signed-off-by: Rémy Coutable --- spec/support/migrations_helpers.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/support/migrations_helpers.rb b/spec/support/migrations_helpers.rb index 9a151439486..6bf976a2cf9 100644 --- a/spec/support/migrations_helpers.rb +++ b/spec/support/migrations_helpers.rb @@ -45,7 +45,13 @@ module MigrationsHelpers end def migration_schema_version - self.class.metadata[:schema] || previous_migration.version + metadata_schema = self.class.metadata[:schema] + + if metadata_schema == :latest + migrations.last.version + else + metadata_schema || previous_migration.version + end end def schema_migrate_down! From 1582fe782788b390c78ada258bb0e2f2b1a8f1c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 12 Feb 2018 15:32:37 +0100 Subject: [PATCH 5/6] Use the latest migration in spec/migrations/rename_reserved_project_names_spec.rb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We do that because it's using factories and it's not worth it to get rid of them. Signed-off-by: Rémy Coutable --- .../migrations/rename_reserved_project_names_spec.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/spec/migrations/rename_reserved_project_names_spec.rb b/spec/migrations/rename_reserved_project_names_spec.rb index e6555b1fe6b..34336d705b1 100644 --- a/spec/migrations/rename_reserved_project_names_spec.rb +++ b/spec/migrations/rename_reserved_project_names_spec.rb @@ -3,10 +3,14 @@ require 'spec_helper' require Rails.root.join('db', 'post_migrate', '20161221153951_rename_reserved_project_names.rb') -# This migration uses multiple threads, and thus different transactions. This -# means data created in this spec may not be visible to some threads. To work -# around this we use the DELETE cleaning strategy. -describe RenameReservedProjectNames, :delete do +# This migration is using factories, which set fields that don't actually +# exist in the DB schema previous to 20161221153951. Thus we just use the +# latest schema when testing this migration. +# This is ok-ish because: +# 1. This migration is a data migration +# 2. It only relies on very stable DB fields: routes.id, routes.path, namespaces.id, projects.namespace_id +# Ideally, the test should not use factories and rely on the `table` helper instead. +describe RenameReservedProjectNames, :migration, schema: :latest do let(:migration) { described_class.new } let!(:project) { create(:project) } From 957ae8734b6babc481e8f186de848ba1fe914c0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 12 Feb 2018 15:30:40 +0100 Subject: [PATCH 6/6] Fix a transient failure in db/post_migrate/20170717111152_cleanup_move_system_upload_folder_symlink.rb where symlink already exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .../20170717111152_cleanup_move_system_upload_folder_symlink.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/post_migrate/20170717111152_cleanup_move_system_upload_folder_symlink.rb b/db/post_migrate/20170717111152_cleanup_move_system_upload_folder_symlink.rb index 26b99b61424..c48f1c938d0 100644 --- a/db/post_migrate/20170717111152_cleanup_move_system_upload_folder_symlink.rb +++ b/db/post_migrate/20170717111152_cleanup_move_system_upload_folder_symlink.rb @@ -20,7 +20,7 @@ class CleanupMoveSystemUploadFolderSymlink < ActiveRecord::Migration def down if File.directory?(new_directory) say "Symlinking #{old_directory} -> #{new_directory}" - FileUtils.ln_s(new_directory, old_directory) + FileUtils.ln_s(new_directory, old_directory) unless File.exist?(old_directory) else say "#{new_directory} doesn't exist, skipping." end