Apply reviewer feedback

- Use `let` instead of local variable
- Add comment to make the consequences of `validate: false`
explicit

lib/gitlab/database/migration_helpers.rb

@@ -198,6 +198,8 @@ module Gitlab
         # long time to complete, but fortunately does not lock the source table
         # while running.
         # Disable this check by passing `validate: false` to the method call
+        # The check will be enforced for new data (inserts) coming in,
+        # but validating existing data is delayed.
         #
         # Note this is a no-op in case the constraint is VALID already
 

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -327,21 +327,21 @@ describe Gitlab::Database::MigrationHelpers do
       end
 
       describe 'validate option' do
-        args = [:projects, :users]
-        options = { column: :user_id, on_delete: nil }
+        let(:args) { [:projects, :users] }
+        let(:options) { { column: :user_id, on_delete: nil } }
 
         context 'when validate is supplied with a falsey value' do
-          it_behaves_like 'skips validation', args, options.merge(validate: false)
-          it_behaves_like 'skips validation', args, options.merge(validate: nil)
+          it_behaves_like 'skips validation', validate: false
+          it_behaves_like 'skips validation', validate: nil
         end
 
         context 'when validate is supplied with a truthy value' do
-          it_behaves_like 'performs validation', args, options.merge(validate: true)
-          it_behaves_like 'performs validation', args, options.merge(validate: :yes)
+          it_behaves_like 'performs validation', validate: true
+          it_behaves_like 'performs validation', validate: :whatever
         end
 
         context 'when validate is not supplied' do
-          it_behaves_like 'performs validation', args, options
+          it_behaves_like 'performs validation', {}
         end
       end
     end

spec/support/shared_examples/migration_helpers_examples.rb

 # frozen_string_literal: true
 
-shared_examples 'skips validation' do |args, options|
+shared_examples 'skips validation' do |validation_option|
   it 'skips validation' do
     expect(model).not_to receive(:disable_statement_timeout)
     expect(model).to receive(:execute).with(/ADD CONSTRAINT/)
     expect(model).not_to receive(:execute).with(/VALIDATE CONSTRAINT/)
 
-    model.add_concurrent_foreign_key(*args, **options)
+    model.add_concurrent_foreign_key(*args, **options.merge(validation_option))
   end
 end
 
-shared_examples 'performs validation' do |args, options|
+shared_examples 'performs validation' do |validation_option|
   it 'performs validation' do
     expect(model).to receive(:disable_statement_timeout).and_call_original
     expect(model).to receive(:execute).with(/statement_timeout/)
@@ -18,6 +18,6 @@ shared_examples 'performs validation' do |args, options|
     expect(model).to receive(:execute).ordered.with(/VALIDATE CONSTRAINT/)
     expect(model).to receive(:execute).with(/RESET ALL/)
 
-    model.add_concurrent_foreign_key(*args, **options)
+    model.add_concurrent_foreign_key(*args, **options.merge(validation_option))
   end
 end