From 04dceb68f7978c9834b0f077e9a2df3f59bc84af Mon Sep 17 00:00:00 2001 From: Luke Hill Date: Fri, 18 Jul 2025 18:21:42 +0100 Subject: [PATCH 1/8] When there is no error, return nil instead of empty array Apportion an undiagnosable error when it occurs --- compatibility/support/cck/keys_checker.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compatibility/support/cck/keys_checker.rb b/compatibility/support/cck/keys_checker.rb index a86cdc235..bd92878eb 100644 --- a/compatibility/support/cck/keys_checker.rb +++ b/compatibility/support/cck/keys_checker.rb @@ -14,11 +14,11 @@ def initialize(detected, expected) end def compare - return [] if identical_keys? + return if identical_keys? errors << "Detected extra keys in message #{message_name}: #{extra_keys}" if extra_keys.any? errors << "Missing keys in message #{message_name}: #{missing_keys}" if missing_keys.any? - errors + errors << 'Undiagnosable error: Needs developer triage. Keys not identical, but nothing is identified erroneous' rescue StandardError => e ["Unexpected error: #{e.message}"] end From ba3b7538c144a60b53916c42481803235e4c2357 Mon Sep 17 00:00:00 2001 From: Luke Hill Date: Fri, 18 Jul 2025 18:25:48 +0100 Subject: [PATCH 2/8] Ensure both `nil` and `[]` are cleaned up in test response Avoid void context of items not being returned when erroneous --- compatibility/support/cck/keys_checker.rb | 6 +++--- compatibility/support/cck/messages_comparator.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compatibility/support/cck/keys_checker.rb b/compatibility/support/cck/keys_checker.rb index bd92878eb..2718434ce 100644 --- a/compatibility/support/cck/keys_checker.rb +++ b/compatibility/support/cck/keys_checker.rb @@ -15,10 +15,10 @@ def initialize(detected, expected) def compare return if identical_keys? + return "Detected extra keys in message #{message_name}: #{extra_keys}" if extra_keys.any? + return "Missing keys in message #{message_name}: #{missing_keys}" if missing_keys.any? - errors << "Detected extra keys in message #{message_name}: #{extra_keys}" if extra_keys.any? - errors << "Missing keys in message #{message_name}: #{missing_keys}" if missing_keys.any? - errors << 'Undiagnosable error: Needs developer triage. Keys not identical, but nothing is identified erroneous' + 'Undiagnosable error: Needs developer triage. Keys not identical, but nothing is identified erroneous' rescue StandardError => e ["Unexpected error: #{e.message}"] end diff --git a/compatibility/support/cck/messages_comparator.rb b/compatibility/support/cck/messages_comparator.rb index 1eac0e21a..105621584 100644 --- a/compatibility/support/cck/messages_comparator.rb +++ b/compatibility/support/cck/messages_comparator.rb @@ -12,7 +12,7 @@ def initialize(detected, expected) end def errors - all_errors.flatten + all_errors.compact.flatten end private From 53cdae7b8242c992da73fb31b9f40e8b36127960 Mon Sep 17 00:00:00 2001 From: Luke Hill Date: Fri, 18 Jul 2025 18:27:41 +0100 Subject: [PATCH 3/8] Remove redundant part of keyschecker --- compatibility/support/cck/keys_checker.rb | 24 ++++++++++------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/compatibility/support/cck/keys_checker.rb b/compatibility/support/cck/keys_checker.rb index 2718434ce..717e4f7e4 100644 --- a/compatibility/support/cck/keys_checker.rb +++ b/compatibility/support/cck/keys_checker.rb @@ -25,6 +25,10 @@ def compare private + def identical_keys? + detected_keys == expected_keys + end + def detected_keys @detected_keys ||= ordered_uniq_hash_keys(detected) end @@ -33,18 +37,18 @@ def expected_keys @expected_keys ||= ordered_uniq_hash_keys(expected) end - def identical_keys? - detected_keys == expected_keys - end - - def missing_keys - (expected_keys - detected_keys).reject { |key| meta_message? && key == :ci } + def ordered_uniq_hash_keys(object) + object.to_h(reject_nil_values: true).keys.sort end def extra_keys (detected_keys - expected_keys).reject { |key| meta_message? && key == :ci } end + def missing_keys + (expected_keys - detected_keys).reject { |key| meta_message? && key == :ci } + end + def meta_message? detected.instance_of?(Cucumber::Messages::Meta) end @@ -52,13 +56,5 @@ def meta_message? def message_name detected.class.name end - - def ordered_uniq_hash_keys(object) - object.to_h(reject_nil_values: true).keys.sort - end - - def errors - @errors ||= [] - end end end From 3a6be6f7cf62848e9796a47a255dd88fc94f81dd Mon Sep 17 00:00:00 2001 From: Luke Hill Date: Fri, 18 Jul 2025 18:33:12 +0100 Subject: [PATCH 4/8] Refactoring of bloated class now complete --- compatibility/support/cck/messages_comparator.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compatibility/support/cck/messages_comparator.rb b/compatibility/support/cck/messages_comparator.rb index 105621584..678cf2f3f 100644 --- a/compatibility/support/cck/messages_comparator.rb +++ b/compatibility/support/cck/messages_comparator.rb @@ -12,7 +12,7 @@ def initialize(detected, expected) end def errors - all_errors.compact.flatten + all_errors.flatten end private @@ -51,7 +51,6 @@ def compare_message(detected, expected) return if ignorable?(detected) return if incomparable?(detected) - # TODO: This needs refactoring as it's becoming rather large and bloated all_errors << CCK::KeysChecker.compare(detected, expected) compare_sub_messages(detected, expected) end From 3ebc70034f274390b2989e54da0b8e27320b73ce Mon Sep 17 00:00:00 2001 From: Luke Hill Date: Thu, 7 Aug 2025 17:11:40 +0100 Subject: [PATCH 5/8] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f32c2332..0f0cccbf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Please visit [cucumber/CONTRIBUTING.md](https://github.com/cucumber/cucumber/blo ## [Unreleased] ### Changed - Updated `cucumber-compatibility-kit` to v19 +- Optimised `compatibility` tests (That use the CCK), so that tests run slightly more optimal (Creating less empty arrays) ### Fixed - Fixed an issue where the html-formatter wasn't respecting the new structure for `StackTrace` cucumber messages ([#1790](https://github.com/cucumber/cucumber-ruby/pull/1790) [luke-hill](https://github.com/luke-hill)) From 493c9e91584d1526b6eb3beeccb6dbd698561ff9 Mon Sep 17 00:00:00 2001 From: Luke Hill Date: Thu, 7 Aug 2025 17:20:22 +0100 Subject: [PATCH 6/8] Fix situation for blank keys needing compacting Fix situation where extra keys are found but they are :ci inside a meta message --- compatibility/support/cck/keys_checker.rb | 3 +-- compatibility/support/cck/messages_comparator.rb | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/compatibility/support/cck/keys_checker.rb b/compatibility/support/cck/keys_checker.rb index 717e4f7e4..98177194d 100644 --- a/compatibility/support/cck/keys_checker.rb +++ b/compatibility/support/cck/keys_checker.rb @@ -16,9 +16,8 @@ def initialize(detected, expected) def compare return if identical_keys? return "Detected extra keys in message #{message_name}: #{extra_keys}" if extra_keys.any? - return "Missing keys in message #{message_name}: #{missing_keys}" if missing_keys.any? - 'Undiagnosable error: Needs developer triage. Keys not identical, but nothing is identified erroneous' + "Missing keys in message #{message_name}: #{missing_keys}" if missing_keys.any? rescue StandardError => e ["Unexpected error: #{e.message}"] end diff --git a/compatibility/support/cck/messages_comparator.rb b/compatibility/support/cck/messages_comparator.rb index 678cf2f3f..e5c4bac36 100644 --- a/compatibility/support/cck/messages_comparator.rb +++ b/compatibility/support/cck/messages_comparator.rb @@ -12,7 +12,7 @@ def initialize(detected, expected) end def errors - all_errors.flatten + all_errors.compact end private From ce853ad564feed9cf1fa3337de162f2f76f7b04c Mon Sep 17 00:00:00 2001 From: Luke Hill Date: Thu, 7 Aug 2025 17:35:25 +0100 Subject: [PATCH 7/8] Fix up tests to deal with nil response vs empty response Amend one test to signify we only return 1 error now not 2 --- compatibility/spec/cck/keys_checker_spec.rb | 29 +++++++++++---------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/compatibility/spec/cck/keys_checker_spec.rb b/compatibility/spec/cck/keys_checker_spec.rb index 94bfe140e..6849106b5 100644 --- a/compatibility/spec/cck/keys_checker_spec.rb +++ b/compatibility/spec/cck/keys_checker_spec.rb @@ -6,31 +6,32 @@ describe CCK::KeysChecker do describe '#compare' do - let(:expected_values) { Cucumber::Messages::Attachment.new(url: 'https://foo.com', file_name: 'file.extension') } - let(:erroneous_values) { Cucumber::Messages::Attachment.new(source: '1', test_step_id: '123') } - let(:wrong_values) { Cucumber::Messages::Attachment.new(url: 'https://otherfoo.com', file_name: 'file.other') } + let(:expected_kvps) { Cucumber::Messages::Attachment.new(url: 'https://foo.com', file_name: 'file.extension', test_step_id: 123_456) } + let(:missing_kvps) { Cucumber::Messages::Attachment.new(url: 'https://foo.com') } + let(:extra_kvps) { Cucumber::Messages::Attachment.new(url: 'https://foo.com', file_name: 'file.extension', test_step_id: 123_456, source: '1') } + let(:missing_and_extra_kvps) { Cucumber::Messages::Attachment.new(file_name: 'file.extension', test_step_id: 123_456, test_run_started_id: 123_456) } + let(:wrong_values) { Cucumber::Messages::Attachment.new(url: 'https://otherfoo.com', file_name: 'file.other', test_step_id: 456_789) } it 'finds missing keys' do - expect(described_class.compare(erroneous_values, expected_values)).to include( - 'Missing keys in message Cucumber::Messages::Attachment: [:file_name, :url]' + expect(described_class.compare(missing_kvps, expected_kvps)).to eq( + 'Missing keys in message Cucumber::Messages::Attachment: [:file_name, :test_step_id]' ) end it 'finds extra keys' do - expect(described_class.compare(erroneous_values, expected_values)).to include( - 'Detected extra keys in message Cucumber::Messages::Attachment: [:source, :test_step_id]' + expect(described_class.compare(extra_kvps, expected_kvps)).to eq( + 'Detected extra keys in message Cucumber::Messages::Attachment: [:source]' ) end - it 'finds extra and missing keys' do - expect(described_class.compare(erroneous_values, expected_values)).to contain_exactly( - 'Missing keys in message Cucumber::Messages::Attachment: [:file_name, :url]', - 'Detected extra keys in message Cucumber::Messages::Attachment: [:source, :test_step_id]' + it 'finds the extra keys first' do + expect(described_class.compare(missing_and_extra_kvps, expected_kvps)).to eq( + 'Detected extra keys in message Cucumber::Messages::Attachment: [:test_run_started_id]' ) end it 'does not care about the values' do - expect(described_class.compare(expected_values, wrong_values)).to be_empty + expect(described_class.compare(expected_kvps, wrong_values)).to be_nil end context 'when default values are omitted' do @@ -38,7 +39,7 @@ let(:default_not_set) { Cucumber::Messages::Duration.new(nanos: 12) } it 'does not raise an exception' do - expect(described_class.compare(default_set, default_not_set)).to be_empty + expect(described_class.compare(default_set, default_not_set)).to be_nil end end @@ -49,7 +50,7 @@ detected = Cucumber::Messages::Meta.new(ci: Cucumber::Messages::Ci.new(name: 'Some CI')) expected = Cucumber::Messages::Meta.new - expect(described_class.compare(detected, expected)).to be_empty + expect(described_class.compare(detected, expected)).to be_nil end end From 4ef9d370c94fc8456dd72479c062e37bc8698114 Mon Sep 17 00:00:00 2001 From: Luke Hill Date: Thu, 7 Aug 2025 17:42:02 +0100 Subject: [PATCH 8/8] Rubocop fix --- compatibility/spec/cck/keys_checker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compatibility/spec/cck/keys_checker_spec.rb b/compatibility/spec/cck/keys_checker_spec.rb index 6849106b5..2a7348a1d 100644 --- a/compatibility/spec/cck/keys_checker_spec.rb +++ b/compatibility/spec/cck/keys_checker_spec.rb @@ -10,7 +10,7 @@ let(:missing_kvps) { Cucumber::Messages::Attachment.new(url: 'https://foo.com') } let(:extra_kvps) { Cucumber::Messages::Attachment.new(url: 'https://foo.com', file_name: 'file.extension', test_step_id: 123_456, source: '1') } let(:missing_and_extra_kvps) { Cucumber::Messages::Attachment.new(file_name: 'file.extension', test_step_id: 123_456, test_run_started_id: 123_456) } - let(:wrong_values) { Cucumber::Messages::Attachment.new(url: 'https://otherfoo.com', file_name: 'file.other', test_step_id: 456_789) } + let(:wrong_values) { Cucumber::Messages::Attachment.new(url: 'https://otherfoo.com', file_name: 'file.other', test_step_id: 456_789) } it 'finds missing keys' do expect(described_class.compare(missing_kvps, expected_kvps)).to eq(