From 7ebd0b25e231ac5a3590c80106a69e06844dc46e Mon Sep 17 00:00:00 2001
From: Peter Leitzen <pleitzen@gitlab.com>
Date: Thu, 1 Jun 2023 20:57:01 +0200
Subject: [PATCH] Refactor Migration/UpdateColumnInBatches specs
Use Pathname API as much as possible and resolve all RuboCop offenses.
---
.rubocop_todo/layout/line_length.yml | 1 -
.../naming/heredoc_delimiter_naming.yml | 1 -
.rubocop_todo/rspec/context_wording.yml | 1 -
.rubocop_todo/rspec/empty_line_after_hook.yml | 1 -
.rubocop_todo/rspec/instance_variable.yml | 1 -
.../rspec/missing_feature_category.yml | 1 -
.rubocop_todo/rspec/scattered_let.yml | 1 -
.../cop/migration/update_column_in_batches.rb | 2 +-
.../update_column_in_batches_spec.rb | 70 ++++++++-----------
9 files changed, 30 insertions(+), 49 deletions(-)
diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml
index 16b3419ef029b..d9f18726897ae 100644
--- a/.rubocop_todo/layout/line_length.yml
+++ b/.rubocop_todo/layout/line_length.yml
@@ -4692,7 +4692,6 @@ Layout/LineLength:
- 'spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb'
- 'spec/rubocop/cop/lint/last_keyword_argument_spec.rb'
- 'spec/rubocop/cop/migration/safer_boolean_column_spec.rb'
- - 'spec/rubocop/cop/migration/update_column_in_batches_spec.rb'
- 'spec/rubocop/cop/performance/readlines_each_spec.rb'
- 'spec/rubocop/cop/rspec/env_assignment_spec.rb'
- 'spec/rubocop/cop/rspec/expect_gitlab_tracking_spec.rb'
diff --git a/.rubocop_todo/naming/heredoc_delimiter_naming.yml b/.rubocop_todo/naming/heredoc_delimiter_naming.yml
index 9b6d26a53ca47..02cc7d2a85c7f 100644
--- a/.rubocop_todo/naming/heredoc_delimiter_naming.yml
+++ b/.rubocop_todo/naming/heredoc_delimiter_naming.yml
@@ -95,7 +95,6 @@ Naming/HeredocDelimiterNaming:
- 'spec/models/concerns/ci/maskable_spec.rb'
- 'spec/models/integrations/asana_spec.rb'
- 'spec/models/ssh_host_key_spec.rb'
- - 'spec/rubocop/cop/migration/update_column_in_batches_spec.rb'
- 'spec/services/ci/create_downstream_pipeline_service_spec.rb'
- 'spec/services/ci/create_pipeline_service/cache_spec.rb'
- 'spec/services/ci/create_pipeline_service/evaluate_runner_tags_spec.rb'
diff --git a/.rubocop_todo/rspec/context_wording.yml b/.rubocop_todo/rspec/context_wording.yml
index f6fbdd31463d7..d29aedff63fc5 100644
--- a/.rubocop_todo/rspec/context_wording.yml
+++ b/.rubocop_todo/rspec/context_wording.yml
@@ -2578,7 +2578,6 @@ RSpec/ContextWording:
- 'spec/rubocop/cop/migration/safer_boolean_column_spec.rb'
- 'spec/rubocop/cop/migration/schedule_async_spec.rb'
- 'spec/rubocop/cop/migration/timestamps_spec.rb'
- - 'spec/rubocop/cop/migration/update_column_in_batches_spec.rb'
- 'spec/rubocop/cop/migration/versioned_migration_class_spec.rb'
- 'spec/rubocop/cop/performance/ar_exists_and_present_blank_spec.rb'
- 'spec/rubocop/cop/qa/ambiguous_page_object_name_spec.rb'
diff --git a/.rubocop_todo/rspec/empty_line_after_hook.yml b/.rubocop_todo/rspec/empty_line_after_hook.yml
index fbfb7e8bfe0ae..7dc3fd235a4a4 100644
--- a/.rubocop_todo/rspec/empty_line_after_hook.yml
+++ b/.rubocop_todo/rspec/empty_line_after_hook.yml
@@ -41,7 +41,6 @@ RSpec/EmptyLineAfterHook:
- 'spec/requests/api/pages/internal_access_spec.rb'
- 'spec/requests/api/pages/private_access_spec.rb'
- 'spec/requests/api/pages/public_access_spec.rb'
- - 'spec/rubocop/cop/migration/update_column_in_batches_spec.rb'
- 'spec/services/merge_requests/execute_approval_hooks_service_spec.rb'
- 'spec/services/notes/create_service_spec.rb'
- 'spec/services/notes/quick_actions_service_spec.rb'
diff --git a/.rubocop_todo/rspec/instance_variable.yml b/.rubocop_todo/rspec/instance_variable.yml
index fe632cdd57b81..a5a0cec68d4b5 100644
--- a/.rubocop_todo/rspec/instance_variable.yml
+++ b/.rubocop_todo/rspec/instance_variable.yml
@@ -136,7 +136,6 @@ RSpec/InstanceVariable:
- 'spec/requests/git_http_spec.rb'
- 'spec/requests/openid_connect_spec.rb'
- 'spec/requests/projects/issues/discussions_spec.rb'
- - 'spec/rubocop/cop/migration/update_column_in_batches_spec.rb'
- 'spec/services/ci/process_sync_events_service_spec.rb'
- 'spec/services/labels/update_service_spec.rb'
- 'spec/services/members/destroy_service_spec.rb'
diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml
index a5cee560ef19a..d4d20e2cf6b49 100644
--- a/.rubocop_todo/rspec/missing_feature_category.yml
+++ b/.rubocop_todo/rspec/missing_feature_category.yml
@@ -5338,7 +5338,6 @@ RSpec/MissingFeatureCategory:
- 'spec/rubocop/cop/migration/schema_addition_methods_no_post_spec.rb'
- 'spec/rubocop/cop/migration/sidekiq_queue_migrate_spec.rb'
- 'spec/rubocop/cop/migration/timestamps_spec.rb'
- - 'spec/rubocop/cop/migration/update_column_in_batches_spec.rb'
- 'spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb'
- 'spec/rubocop/cop/migration/with_lock_retries_with_change_spec.rb'
- 'spec/rubocop/cop/performance/active_record_subtransaction_methods_spec.rb'
diff --git a/.rubocop_todo/rspec/scattered_let.yml b/.rubocop_todo/rspec/scattered_let.yml
index 573991418f742..592f11ef77218 100644
--- a/.rubocop_todo/rspec/scattered_let.yml
+++ b/.rubocop_todo/rspec/scattered_let.yml
@@ -223,7 +223,6 @@ RSpec/ScatteredLet:
- 'spec/requests/api/notes_spec.rb'
- 'spec/requests/api/project_clusters_spec.rb'
- 'spec/requests/jira_routing_spec.rb'
- - 'spec/rubocop/cop/migration/update_column_in_batches_spec.rb'
- 'spec/serializers/build_details_entity_spec.rb'
- 'spec/serializers/ci/job_entity_spec.rb'
- 'spec/serializers/merge_requests/pipeline_entity_spec.rb'
diff --git a/rubocop/cop/migration/update_column_in_batches.rb b/rubocop/cop/migration/update_column_in_batches.rb
index 309a2f414534d..e97f51361a9e2 100644
--- a/rubocop/cop/migration/update_column_in_batches.rb
+++ b/rubocop/cop/migration/update_column_in_batches.rb
@@ -48,7 +48,7 @@ def checksum_filenames(pattern)
digest = Digest::SHA256.new
rails_root.glob(pattern) do |path|
- digest.update(path.to_path)
+ digest.update(path.relative_path_from(rails_root).to_path)
end
digest.hexdigest
diff --git a/spec/rubocop/cop/migration/update_column_in_batches_spec.rb b/spec/rubocop/cop/migration/update_column_in_batches_spec.rb
index d48b42247f235..131a9a2712b17 100644
--- a/spec/rubocop/cop/migration/update_column_in_batches_spec.rb
+++ b/spec/rubocop/cop/migration/update_column_in_batches_spec.rb
@@ -4,50 +4,42 @@
require_relative '../../../../rubocop/cop/migration/update_column_in_batches'
-RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches do
+RSpec.describe RuboCop::Cop::Migration::UpdateColumnInBatches, feature_category: :database do
let(:tmp_rails_root) { Pathname.new(rails_root_join('tmp', 'rails_root')) }
let(:migration_code) do
- <<-END
- def up
- update_column_in_batches(:projects, :name, "foo") do |table, query|
- query.where(table[:name].eq(nil))
+ <<~RUBY
+ def up
+ update_column_in_batches(:projects, :name, "foo") do |table, query|
+ query.where(table[:name].eq(nil))
+ end
end
- end
- END
+ RUBY
end
+ let(:spec_filepath) { 'spec/migrations/my_super_migration_spec.rb' }
+
before do
+ tmp_rails_root.mkpath
allow(cop).to receive(:rails_root).and_return(tmp_rails_root)
end
+
after do
- FileUtils.rm_rf(tmp_rails_root)
+ tmp_rails_root.rmtree
end
- let(:spec_filepath) { File.join(tmp_rails_root, 'spec', 'migrations', 'my_super_migration_spec.rb') }
-
- context 'outside of a migration' do
+ context 'when outside of a migration' do
it 'does not register any offenses' do
expect_no_offenses(migration_code)
end
end
- shared_context 'with a migration file' do
+ shared_examples 'a migration file with no spec file' do
before do
- FileUtils.mkdir_p(File.dirname(migration_filepath))
- @migration_file = File.new(migration_filepath, 'w+')
- end
- after do
- @migration_file.close
+ touch_file(migration_filepath)
end
- end
-
- shared_examples 'a migration file with no spec file' do
- include_context 'with a migration file'
-
- let(:relative_spec_filepath) { Pathname.new(spec_filepath).relative_path_from(tmp_rails_root) }
it 'registers an offense when using update_column_in_batches' do
- expect_offense(<<~RUBY, @migration_file)
+ expect_offense(<<~RUBY, tmp_rails_root.join(migration_filepath).to_path)
def up
update_column_in_batches(:projects, :name, "foo") do |table, query|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migration running `update_column_in_batches` [...]
@@ -59,14 +51,9 @@ def up
end
shared_examples 'a migration file with a spec file' do
- include_context 'with a migration file'
-
before do
- FileUtils.mkdir_p(File.dirname(spec_filepath))
- @spec_file = File.new(spec_filepath, 'w+')
- end
- after do
- @spec_file.close
+ touch_file(migration_filepath)
+ touch_file(spec_filepath)
end
it 'does not register any offenses' do
@@ -75,31 +62,31 @@ def up
end
context 'when in migration' do
- let(:migration_filepath) { File.join(tmp_rails_root, 'db', 'migrate', '20121220064453_my_super_migration.rb') }
+ let(:migration_filepath) { 'db/migrate/20121220064453_my_super_migration.rb' }
it_behaves_like 'a migration file with no spec file'
it_behaves_like 'a migration file with a spec file'
end
context 'when in a post migration' do
- let(:migration_filepath) { File.join(tmp_rails_root, 'db', 'post_migrate', '20121220064453_my_super_migration.rb') }
+ let(:migration_filepath) { 'db/post_migrate/20121220064453_my_super_migration.rb' }
it_behaves_like 'a migration file with no spec file'
it_behaves_like 'a migration file with a spec file'
end
- context 'EE migrations' do
- let(:spec_filepath) { File.join(tmp_rails_root, 'ee', 'spec', 'migrations', 'my_super_migration_spec.rb') }
+ context 'for EE migrations' do
+ let(:spec_filepath) { 'ee/spec/migrations/my_super_migration_spec.rb' }
context 'when in a migration' do
- let(:migration_filepath) { File.join(tmp_rails_root, 'ee', 'db', 'migrate', '20121220064453_my_super_migration.rb') }
+ let(:migration_filepath) { 'ee/db/migrate/20121220064453_my_super_migration.rb' }
it_behaves_like 'a migration file with no spec file'
it_behaves_like 'a migration file with a spec file'
end
context 'when in a post migration' do
- let(:migration_filepath) { File.join(tmp_rails_root, 'ee', 'db', 'post_migrate', '20121220064453_my_super_migration.rb') }
+ let(:migration_filepath) { 'ee/db/post_migrate/20121220064453_my_super_migration.rb' }
it_behaves_like 'a migration file with no spec file'
it_behaves_like 'a migration file with a spec file'
@@ -116,12 +103,13 @@ def up
end
# The computed SHA from sorted list of filenames above
- it { is_expected.to eq('687c9239e00d9e7744131f3d1de27901cfe2d21e64969f5c30b6ea2817aac801') }
+ it { is_expected.to eq('833525c0d9c95d066dbfc8d973153b44a1f8a42694b54de3aaa854cb9f72a6bd') }
+ end
- private
+ private
- def touch_file(path)
- full_path = tmp_rails_root.join(path)
+ def touch_file(path)
+ tmp_rails_root.join(path).tap do |full_path|
full_path.dirname.mkpath
full_path.write('')
end
--
GitLab