From 6b3dcf09197d2598569c7b7efb4b0b2168714878 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Thu, 29 Oct 2026 15:11:45 +0100 Subject: [PATCH 1/2] test: close strong_migrations preboot note coverage gaps Follow-up to #3057's review, which found the spec too narrow for the invariants it claims: - the "other checks unchanged" example sampled only :add_index; it now asserts the note's absence on every key outside the preboot list - the "exactly once" example now also asserts the note is never the message prefix, which with the single-occurrence count proves the gem text comes first for all nine keys without hardcoding nine default wordings - no test covered the loud-failure design: an example reloads the initializer with :rename_column removed from error_messages and expects KeyError, so a future gem dropping a listed key cannot fail silently Mutation-checked: appending the note to every key fails the untouched-keys example, and dropping the append fails the ordering and KeyError ones. --- spec/config/strong_migrations_config_spec.rb | 26 +++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/spec/config/strong_migrations_config_spec.rb b/spec/config/strong_migrations_config_spec.rb index ba61b09dc..79e5c1f96 100644 --- a/spec/config/strong_migrations_config_spec.rb +++ b/spec/config/strong_migrations_config_spec.rb @@ -15,16 +15,30 @@ expect(message).to end_with("disable preboot for this deploy.\n") end - it 'carries the note exactly once on each destructive-operation message' do - counts = preboot_keys.index_with do |key| - StrongMigrations.error_messages.fetch(key).scan('disable preboot for this deploy').size - end + it 'carries the note exactly once, after the gem text, on each destructive-operation message' do + preboot_keys.each do |key| + message = StrongMigrations.error_messages.fetch(key) - expect(counts.values).to all(eq(1)) + expect(message.scan('disable preboot for this deploy').size).to eq(1) + expect(message).not_to start_with('Heroku preboot') + end end it 'leaves messages for other checks unchanged' do - expect(StrongMigrations.error_messages[:add_index]).not_to include('Heroku preboot') + untouched_keys = StrongMigrations.error_messages.keys.map(&:to_sym) - preboot_keys + + expect(untouched_keys).not_to be_empty + untouched_keys.each do |key| + expect(StrongMigrations.error_messages.fetch(key)).not_to include('Heroku preboot') + end + end + + it 'fails loudly when a future gem version drops a listed key' do + allow(StrongMigrations).to receive(:error_messages) + .and_return(StrongMigrations.error_messages.except(:rename_column)) + + expect { load Rails.root.join('config/initializers/strong_migrations.rb').to_s } + .to raise_error(KeyError, /rename_column/) end end end From 104a580510cefce54beba2bb0f5b75a9ac7d9df9 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Thu, 29 Oct 2026 15:29:35 +0100 Subject: [PATCH 2/2] test: pin note-last ordering per key after review Review of this PR found the exactly-once example could go green while the note came first: count == 1 plus not-start-with only rules out a literal 'Heroku preboot' prefix, so any other prefix before the note passes. Assert each message ends with the note; note-last plus the count pins gem-text-first for all nine keys. Also retitle the untouched-keys example to claim only what it asserts (no preboot note added), and capture the stub value before the allow call so the capture-before-stub order reads literally instead of depending on argument evaluation order (two reviewers mis-read it). --- spec/config/strong_migrations_config_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/config/strong_migrations_config_spec.rb b/spec/config/strong_migrations_config_spec.rb index 79e5c1f96..b3aa2e0c0 100644 --- a/spec/config/strong_migrations_config_spec.rb +++ b/spec/config/strong_migrations_config_spec.rb @@ -21,10 +21,11 @@ expect(message.scan('disable preboot for this deploy').size).to eq(1) expect(message).not_to start_with('Heroku preboot') + expect(message).to end_with("disable preboot for this deploy.\n") end end - it 'leaves messages for other checks unchanged' do + it 'does not add the preboot note to messages for other checks' do untouched_keys = StrongMigrations.error_messages.keys.map(&:to_sym) - preboot_keys expect(untouched_keys).not_to be_empty @@ -34,8 +35,8 @@ end it 'fails loudly when a future gem version drops a listed key' do - allow(StrongMigrations).to receive(:error_messages) - .and_return(StrongMigrations.error_messages.except(:rename_column)) + messages_without_rename = StrongMigrations.error_messages.except(:rename_column) + allow(StrongMigrations).to receive(:error_messages).and_return(messages_without_rename) expect { load Rails.root.join('config/initializers/strong_migrations.rb').to_s } .to raise_error(KeyError, /rename_column/)