Deleting migration specs cause RuboCop offenses and broken master
Problem
In !121213 (merged) we've removed old migration specs which led to failing rubocop
job (Migration/UpdateColumnInBatches
offense - see master
(master:broken) and was fixed in !122143 (merged).
The RuboCop offenses were not caught in the initial MR because in https://gitlab.com/gitlab-org/gitlab/-/jobs/4375497171#L710 rubocop
was only checking changed files and did not check unchanged migrations.
!122282 (merged) helps to invalid RuboCop's cache for Migration/UpdateColumnInBatches
Proposed solution
Currently, RUBOCOP_TARGET_FILES
is a list of changed files. Deleted files are discarded.
Use tff
to map deleted spec files to existing migrations.
Define a new mapping migrations.yml
for tff
mapping:
- source: 'spec/migrations/(.+?)_spec\.rb'
test:
- 'db/post_migrate/*_%s.rb'
- 'db/migrate/*_%s.rb'
With
The tff patch
diff --git a/lib/test_file_finder/file_finder.rb b/lib/test_file_finder/file_finder.rb
index 14c585e..84a9207 100644
--- a/lib/test_file_finder/file_finder.rb
+++ b/lib/test_file_finder/file_finder.rb
@@ -22,7 +22,7 @@ module TestFileFinder
attr_reader :paths
def search
- file_path_guesses.select { |path| File.exist?(path) }
+ file_path_guesses.flat_map { |path| Dir.glob(path) }
end
def file_path_guesses
we now can run map (deleted) spec files to existing migrations:
tff -f migrations.yml spec/migrations/drop_packages_events_table_spec.rb spec/migrations/populate_releases_access_level_from_repository_spec.rb
db/post_migrate/20230316185746_drop_packages_events_table.rb
db/migrate/20221014034338_populate_releases_access_level_from_repository.rb
This resulting list can extend RUBOCOP_TARGET_FILES
which allows RuboCop to be run on migrations.
Refs
Discussion
The following discussion from !121213 (merged) should be addressed:
-
@icbd started a discussion: (+10 comments) bundle exec rubocop db/migrate/20221014034338_populate_releases_access_level_from_repository.rb Inspecting 1 file C Offenses: db/migrate/20221014034338_populate_releases_access_level_from_repository.rb:9:5: C: Migration/UpdateColumnInBatches: Migration running update_column_in_batches must have a spec file at spec/migrations/populate_releases_access_level_from_repository_spec.rb. update_column_in_batches( ... ^^^^^^^^^^^^^^^^^^^^^^^^^ 1 file inspected, 1 offense detected
Deleting the test will generate a rubocop error.
This breaks the pipeline of JH's default branch: https://jihulab.com/gitlab-cn/gitlab/-/pipelines/1256512/failures
I think this MR also needs to add a rubocop comment when deleting the test:
# rubocop:disable Migration/UpdateColumnInBatches