Add parallel CI support for DeprecationTracker#176
Conversation
Add node_index option to support running deprecation tracking across parallel CI nodes. Each node writes to a shard file (e.g., *.node-0.json) in save mode, and compares only its own buckets in compare mode. Also adds default values for shitlist_path and mode, and a detect_node_index class method that auto-detects common CI env vars (CircleCI, Buildkite, Semaphore, GitLab CI).
Users know their CI env vars — they can pass node_index directly (e.g., node_index: ENV["CIRCLE_NODE_INDEX"]).
The constructor calls mode.to_sym which crashes with NoMethodError
when mode is nil. This happens when calling DeprecationTracker.new
directly without a mode argument (e.g., DeprecationTracker.new("path",
nil, nil)). The init_tracker method has its own fallback, but the
constructor is public and should handle nil gracefully.
Now falls back to :save when mode is nil, matching the documented
default behavior.
In compare mode, normalized_deprecation_messages was called multiple times: once in compare to build buckets_to_check, and again in diff via create_temp_file. Each call triggered a read_json disk read and JSON parse of the same file. Since this method is only called after the test run completes (via after_run), the data cannot change between calls. Memoizing it eliminates redundant file I/O.
The diff method interpolates shitlist_path directly into a backtick shell command. File paths containing spaces or special characters cause git diff to fail silently, producing an empty diff in the error message. Now uses Shellwords.shellescape to properly quote both file paths before passing them to the shell.
Consistent with the other renames in this branch that moved away from the "shitlist" naming in method and variable names.
Array#any? has been available since Ruby 1.8 and is the idiomatic way to check for a non-empty collection in Ruby.
The ensure keyword inside a block (do...end) without an explicit begin is only supported in Ruby 2.5+. Wrap with begin/ensure/end for compatibility with Ruby 2.4 and older.
Removing the Shellwords change and its test to keep this PR focused on parallel CI support and renames. The spaces-in-path fix can be addressed in a separate PR.
Previously hardcoded .json in chomp/append, which would produce incorrect shard paths for files with non-.json extensions. Now uses File.extname to handle any file extension correctly.
JuanVqz
force-pushed
the
feature/parallel-ci-deprecation-tracker
branch
from
a061cd7 to
f8c203d
Compare
etagwerker
left a comment
etagwerker
left a comment
There was a problem hiding this comment.
@JuanVqz These changes look good but I wonder how you're planning to handle the merge process?
@etagwerker Was thinking on releasing a minor version or probably I can release a dev/pre release version (have to investigate that) to test and once we confirm it works release a stable version? is that what you mean? |
|
@JuanVqz oh, no, I'm sorry. I meant to ask how you're gonna handle the merge process for the JSON files. Is there gonna be utility class or rake task to merge them once the user gets them from CI? |
@etagwerker Now I get it, yes I'm going to create a new class to do that I already started it but wanted you to code review this first to don't n make a huge PR and kind of checking if you were interested in the feature first. I've seeing some work around it I. The past and I know it is a bit complicated. For first version wanted to merge them locally probably with a command line, then one it's stable, let's try to implementó something that runs in CI. Thoughts? |
etagwerker
left a comment
etagwerker
left a comment
There was a problem hiding this comment.
@JuanVqz These changes look good but I wonder how you're planning to handle the merge process?
2 participants
Description
Add support for running deprecation tracking across parallel CI nodes. Each node writes to its own shard file, and compare mode only checks the buckets that the current node ran.
Also adds sensible default values, fixes bugs, and improves naming throughout
DeprecationTracker.Changes
Parallel CI support
node_indexoption to constructor andinit_trackernode_index, write to shard file (e.g.,*.node-0.json)node_index, only check buckets this node exercisedtarget_pathmethod for write path resolutionDefault values
shitlist_pathdefaults tospec/support/deprecation_warning.shitlist.jsonmodedefaults toENV["DEPRECATION_TRACKER"]then:saveBug fixes
NoMethodErrorwhenmodeisnilin constructornormalized_deprecation_messagesto avoid repeated disk readsRefactors
read_shitlist→read_json,create_temp_shitlist→create_temp_filenew_shitlist→temp_file,shitlist→storedDEFAULT_SHITLIST_PATH→DEFAULT_PATHany?instead oflength > 0Usage
How Has This Been Tested?
I will abide by the code of conduct