From 687d597b11a68270bb2a5c496bbcfd666efcc920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Wed, 15 Apr 2026 16:49:00 -0600 Subject: [PATCH 1/6] Fix compare mode to check only buckets the current process ran Compare mode now always filters to buckets the current process actually ran, regardless of whether node_index is set. This fixes parallel test support (e.g. parallel_tests gem) where each process only sees a subset of specs but compares against the full shitlist. Previously, this filtering was gated behind the parallel? check which required node_index. Now node_index is only needed for save mode (to write shard files). --- CHANGELOG.md | 1 + README.md | 8 ++++---- lib/deprecation_tracker.rb | 10 ++++------ spec/deprecation_tracker_spec.rb | 24 ++++++++++++++++++++---- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1612a07..99fb906 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - [BUGFIX: example](https://github.com/fastruby/next_rails/pull/) +- [BUGFIX: Compare mode now checks only buckets the current process ran, fixing parallel test support](https://github.com/fastruby/next_rails/pull/) - [FEATURE: Add `deprecations merge` command to combine parallel CI shards](https://github.com/fastruby/next_rails/pull/177) - [FEATURE: Add parallel CI support for DeprecationTracker](https://github.com/fastruby/next_rails/pull/176) - [CHORE: Add Ruby 4.0 to the test matrix](https://github.com/fastruby/next_rails/pull/178) diff --git a/README.md b/README.md index 1612974..9b60c01 100644 --- a/README.md +++ b/README.md @@ -167,7 +167,7 @@ RSpec.configure do |config| end ``` -When `node_index` is set, the tracker writes to a shard file (e.g. `deprecation_warning.shitlist.node-0.json`) instead of the canonical file. +The `node_index` option is only used in save mode. When set, the tracker writes to a shard file (e.g. `deprecation_warning.shitlist.node-0.json`) instead of the canonical file. In compare mode, `node_index` is not needed because the tracker automatically checks only the buckets that the current process ran. #### Merging shards @@ -200,9 +200,9 @@ DEPRECATION_TRACKER=save CI_NODE_INDEX=$NODE bundle exec rspec # 2. Merge phase — fan-in step, runs once after all nodes finish deprecations merge --delete-shards -# 3. Compare phase — each parallel node checks its buckets -# against the merged canonical file -DEPRECATION_TRACKER=compare CI_NODE_INDEX=$NODE bundle exec rspec +# 3. Compare phase — each parallel node checks only its own buckets +# against the merged canonical file (no CI_NODE_INDEX needed) +DEPRECATION_TRACKER=compare bundle exec rspec ``` ### `deprecations` command diff --git a/lib/deprecation_tracker.rb b/lib/deprecation_tracker.rb index c171090..24befb3 100644 --- a/lib/deprecation_tracker.rb +++ b/lib/deprecation_tracker.rb @@ -180,12 +180,10 @@ def compare stored = read_json(shitlist_path) changed_buckets = [] - buckets_to_check = if parallel? - # In parallel mode, only check buckets that this node actually ran - normalized_deprecation_messages.select { |bucket, _| deprecation_messages.key?(bucket) } - else - normalized_deprecation_messages - end + # Only check buckets that this process actually ran. + # In single-process mode this is all buckets (no-op filter). + # In parallel mode this skips buckets belonging to other processes. + buckets_to_check = normalized_deprecation_messages.select { |bucket, _| deprecation_messages.key?(bucket) } buckets_to_check.each do |bucket, messages| if stored[bucket] != messages diff --git a/spec/deprecation_tracker_spec.rb b/spec/deprecation_tracker_spec.rb index 24fd311..823d219 100644 --- a/spec/deprecation_tracker_spec.rb +++ b/spec/deprecation_tracker_spec.rb @@ -361,7 +361,7 @@ def self.behavior end describe "#compare" do - it "only checks buckets that this node ran" do + it "only checks buckets that this process ran (without node_index)" do # Set up canonical shitlist with two buckets setup_tracker = DeprecationTracker.new(shitlist_path) setup_tracker.bucket = "bucket 1" @@ -370,21 +370,37 @@ def self.behavior setup_tracker.add("b") setup_tracker.save - # Parallel node only runs bucket 2 with matching deprecations - tracker = DeprecationTracker.new(shitlist_path, nil, :compare, node_index: "0") + # Process only runs bucket 2 with matching deprecations, no node_index needed + tracker = DeprecationTracker.new(shitlist_path, nil, :compare) tracker.bucket = "bucket 2" tracker.add("b") expect { tracker.compare }.not_to raise_error end - it "raises when this node's buckets have changed" do + it "ignores node_index and only checks buckets this process ran" do setup_tracker = DeprecationTracker.new(shitlist_path) setup_tracker.bucket = "bucket 1" setup_tracker.add("a") + setup_tracker.bucket = "bucket 2" + setup_tracker.add("b") setup_tracker.save + # node_index passed (real-world config uses same block for save and compare) tracker = DeprecationTracker.new(shitlist_path, nil, :compare, node_index: "0") + tracker.bucket = "bucket 2" + tracker.add("b") + + expect { tracker.compare }.not_to raise_error + end + + it "raises when this process's buckets have changed" do + setup_tracker = DeprecationTracker.new(shitlist_path) + setup_tracker.bucket = "bucket 1" + setup_tracker.add("a") + setup_tracker.save + + tracker = DeprecationTracker.new(shitlist_path, nil, :compare) tracker.bucket = "bucket 1" tracker.add("different") From eb7631eb2d2403894f4fdcfff0f983e6d1b7fa2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Wed, 15 Apr 2026 16:49:28 -0600 Subject: [PATCH 2/6] Update changelog with PR number for #179 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99fb906..25eacd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ - [BUGFIX: example](https://github.com/fastruby/next_rails/pull/) -- [BUGFIX: Compare mode now checks only buckets the current process ran, fixing parallel test support](https://github.com/fastruby/next_rails/pull/) +- [BUGFIX: Compare mode now checks only buckets the current process ran, fixing parallel test support](https://github.com/fastruby/next_rails/pull/179) - [FEATURE: Add `deprecations merge` command to combine parallel CI shards](https://github.com/fastruby/next_rails/pull/177) - [FEATURE: Add parallel CI support for DeprecationTracker](https://github.com/fastruby/next_rails/pull/176) - [CHORE: Add Ruby 4.0 to the test matrix](https://github.com/fastruby/next_rails/pull/178) From 9cefe2fd1d26413b436512f965439f61abba67d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Wed, 15 Apr 2026 16:50:50 -0600 Subject: [PATCH 3/6] Remove unnecessary bucket filter in compare mode The filter was a no-op: buckets not run by this process have identical values in both normalized and stored (both read from the same shitlist file), so they always compare equal. --- lib/deprecation_tracker.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/deprecation_tracker.rb b/lib/deprecation_tracker.rb index 24befb3..6f355ab 100644 --- a/lib/deprecation_tracker.rb +++ b/lib/deprecation_tracker.rb @@ -180,12 +180,8 @@ def compare stored = read_json(shitlist_path) changed_buckets = [] - # Only check buckets that this process actually ran. - # In single-process mode this is all buckets (no-op filter). - # In parallel mode this skips buckets belonging to other processes. - buckets_to_check = normalized_deprecation_messages.select { |bucket, _| deprecation_messages.key?(bucket) } - buckets_to_check.each do |bucket, messages| + normalized_deprecation_messages.each do |bucket, messages| if stored[bucket] != messages changed_buckets << bucket end From c298d26427a47fe5ae58fbacb97ca09f54439f8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Wed, 15 Apr 2026 16:51:12 -0600 Subject: [PATCH 4/6] Ignore node_index in compare mode to ensure target_path reads shitlist When node_index is set during compare, target_path would return the shard path (which does not exist after merge). This caused normalized_deprecation_messages to read an empty file instead of the full shitlist, breaking the comparison for non-run buckets. --- lib/deprecation_tracker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/deprecation_tracker.rb b/lib/deprecation_tracker.rb index 6f355ab..0145833 100644 --- a/lib/deprecation_tracker.rb +++ b/lib/deprecation_tracker.rb @@ -141,7 +141,7 @@ def initialize(shitlist_path, transform_message = nil, mode = :save, node_index: @transform_message = transform_message || -> (message) { message } @deprecation_messages = {} @mode = mode ? mode.to_sym : :save - @node_index = node_index + @node_index = @mode == :compare ? nil : node_index end def parallel? From 7cf9eef931541d9e289b5584d5b7cb137d999f12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Wed, 15 Apr 2026 16:55:56 -0600 Subject: [PATCH 5/6] Fix ternary precedence for Ruby 2.3 compatibility Ruby 2.3 parses '@mode == :compare ? nil : node_index' as '@mode == (:compare ? nil : node_index)'. Adding explicit parentheses fixes the precedence. --- lib/deprecation_tracker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/deprecation_tracker.rb b/lib/deprecation_tracker.rb index 0145833..6487a82 100644 --- a/lib/deprecation_tracker.rb +++ b/lib/deprecation_tracker.rb @@ -141,7 +141,7 @@ def initialize(shitlist_path, transform_message = nil, mode = :save, node_index: @transform_message = transform_message || -> (message) { message } @deprecation_messages = {} @mode = mode ? mode.to_sym : :save - @node_index = @mode == :compare ? nil : node_index + @node_index = (@mode == :compare) ? nil : node_index end def parallel? From 039977d63341785319f3af2ad57bfb4472fae170 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Tue, 28 Apr 2026 15:05:53 -0600 Subject: [PATCH 6/6] Enforce compare mode cannot use node_index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevents node_index in compare mode with error. Compare should only run on final merged shitlist, not on individual shards—shard-level compare is misleading because adding/removing tests changes file distribution. Co-Authored-By: Ariel Juodziulis Co-Authored-By: Ernesto Tagwerker --- README.md | 2 +- lib/deprecation_tracker.rb | 5 ++++- spec/deprecation_tracker_spec.rb | 18 ++++-------------- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 9b60c01..d0c51e0 100644 --- a/README.md +++ b/README.md @@ -167,7 +167,7 @@ RSpec.configure do |config| end ``` -The `node_index` option is only used in save mode. When set, the tracker writes to a shard file (e.g. `deprecation_warning.shitlist.node-0.json`) instead of the canonical file. In compare mode, `node_index` is not needed because the tracker automatically checks only the buckets that the current process ran. +The `node_index` option is only used in save mode. When set, the tracker writes to a shard file (e.g. `deprecation_warning.shitlist.node-0.json`) instead of the canonical file. Compare mode does not support `node_index` and will raise an error if passed—compare should only run after merging shards on the final canonical shitlist. #### Merging shards diff --git a/lib/deprecation_tracker.rb b/lib/deprecation_tracker.rb index 6487a82..08379c6 100644 --- a/lib/deprecation_tracker.rb +++ b/lib/deprecation_tracker.rb @@ -141,7 +141,10 @@ def initialize(shitlist_path, transform_message = nil, mode = :save, node_index: @transform_message = transform_message || -> (message) { message } @deprecation_messages = {} @mode = mode ? mode.to_sym : :save - @node_index = (@mode == :compare) ? nil : node_index + if @mode == :compare && node_index + raise ArgumentError, "node_index cannot be used with compare mode" + end + @node_index = node_index end def parallel? diff --git a/spec/deprecation_tracker_spec.rb b/spec/deprecation_tracker_spec.rb index 823d219..193afd0 100644 --- a/spec/deprecation_tracker_spec.rb +++ b/spec/deprecation_tracker_spec.rb @@ -378,20 +378,10 @@ def self.behavior expect { tracker.compare }.not_to raise_error end - it "ignores node_index and only checks buckets this process ran" do - setup_tracker = DeprecationTracker.new(shitlist_path) - setup_tracker.bucket = "bucket 1" - setup_tracker.add("a") - setup_tracker.bucket = "bucket 2" - setup_tracker.add("b") - setup_tracker.save - - # node_index passed (real-world config uses same block for save and compare) - tracker = DeprecationTracker.new(shitlist_path, nil, :compare, node_index: "0") - tracker.bucket = "bucket 2" - tracker.add("b") - - expect { tracker.compare }.not_to raise_error + it "raises error when node_index is passed with compare mode" do + expect do + DeprecationTracker.new(shitlist_path, nil, :compare, node_index: "0") + end.to raise_error(ArgumentError, "node_index cannot be used with compare mode") end it "raises when this process's buckets have changed" do