Testing migrations with `update_column_in_batches`

We need to tests the migration better whenever update_column_in_batches is used, because the underlying query might not run if there's nothing to update at all.

We hit into this in !11115 (merged) and it was reported on the merge request, and one week later again as #32647 (closed), just before we're about to tag %9.2.

The problem is, we're not using MySQL so we never really tried it with real data. Perhaps we should have a staging running on MySQL?

/cc @yorickpeterse @stanhu @ayufan @smcgivern @rymai

@godfat It is easy to forget them. Do we need Rubocop rule to block merge if you add non-trivial migrations with update_column_in_batches?

@ayufan I realized that we're enforcing the migration test name: https://gitlab.com/gitlab-org/gitlab-ce/builds/16858721 so perhaps we could just check that if update_column_in_batches is used, a corresponding test should be written. If we don't want to write a test, add # rubocop:disable Migration/UpdateInBatchSpec. This way the reviewer would be aware.

Nice. That would be cool!

assigned to @godfat

I think that's a good idea to enforce the presence of a spec file for migrations that run update_column_in_batches.

@godfat I wanted to get my hands dirty with RuboCop and created a new cop for that! Have you started working on this?

@rymai Awesome! I have some codes written, but it's far from complete. I'll just pass my works to you.

assigned to @rymai and unassigned @godfat

@godfat Cool, I'm opening an MR so that you can see what I came up with.

changed milestone to %9.3

@rymai I'll just comment on your merge request to let you know what I've done. Actually I think I could just point out here. I was trying to merge all those method_name calls, but I haven't figured out how yet, as there are two different definitions. The other is that, are our current cops checking migrations, checking post-deployment migrations at all? We should probably also check post-deployment migrations.

mentioned in merge request !11611 (merged)

We should probably also check post-deployment migrations.

I've done that in my MR actually. :)

closed via merge request !11611 (merged)