Deleting migration specs cause RuboCop offenses and broken master

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

Problem

In !121213 (merged) we've removed old migration specs which led to failing rubocop job (👮 Migration/UpdateColumnInBatches offense - see 🔴 https://gitlab.com/gitlab-org/gitlab/-/jobs/4380514365) on 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
Edited by 🤖 GitLab Bot 🤖