docs: tell the truth about parallel test execution - #252
Merged
Conversation
The parallel-execution notes were wrong in a way that mattered: reporters.md promised a report that Rails' default parallel mode never writes, and thread_safety.md described internals that had since changed. Measured with a harness driving the real matcher/registry/reporter path (4 workers, repeated runs, both parallelization modes): - `parallelize(workers: N)` — Rails' default — writes NO report at all, and prints no summary line. Minitest skips `after_run` hooks in a forked child (`Minitest.allow_fork` is false), so the workers that hold the results never finalize and the parent that finalizes recorded nothing. reporters.md claimed "the last worker's results will be in the report"; there is no report. Test results and on-disk artifacts are unaffected. - `parallelize(with: :threads)` — the JRuby default — is genuinely clean: same failures and counts as a serial run. - One process per worker (parallel_tests, RSpec, CI sharding) is where last-writer-wins actually applies. Both replacement workarounds are verified, not inferred: finalizing from `parallelize_teardown` with a per-worker `output_path` preserves every failure, and `PARALLEL_WORKERS=1` restores the single complete report. The tempting `save_path` variant is called out as wrong — it also relocates the baselines. thread_safety.md additionally claimed SnapManager returns a fresh instance per call (it is fiber-memoized), documented a `registry` method now named `SnapDiff.session`, and never mentioned processes at all. It now carries the Vcs git-root memo, and a warning that screenshot names must be unique across tests: colliding names under concurrency silently skip comparisons — 18 to 34 of 64 assertions per measured run passed green having compared nothing.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reachedNext included review available in 48 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Reviewer's GuideUpdates documentation to accurately describe SnapDiff behavior under various parallel test execution modes, especially Rails’ forking parallelization, and adds guidance on safe configuration, shared state, and unique screenshot naming. Sequence diagram for report finalization under parallel test modessequenceDiagram
participant Suite as Test suite
participant Worker as Test worker
participant Reporter as HTML reporter
participant File as Report file
alt Serial or thread parallelism
Suite->>Worker: Run screenshot assertions
Worker->>Reporter: record(assertions)
Suite->>Reporter: finalize!
Reporter->>File: Write complete report
else Rails forked parallelism
Suite->>Worker: Fork and run assertions
Worker->>Reporter: record(assertions)
Note over Worker: Minitest.allow_fork is false
Worker--xReporter: after_run is skipped
Suite->>Reporter: finalize!
Reporter->>File: Write no report
else One process per worker
Suite->>Worker: Run assertions in each process
Worker->>Reporter: record(assertions)
Worker->>File: Write report on finalize
Note over File: Last process to finish overwrites earlier results
end
Flow diagram for restoring reports under Rails forked parallelismflowchart TD
A["Rails forks test workers"] --> B["parallelize_setup configures a worker-specific output_path"]
B --> C["Workers run screenshot assertions"]
C --> D["parallelize_teardown calls SnapDiff::Reporting.finalize!"]
D --> E["Each worker writes its own HTML report"]
C --> F["Set PARALLEL_WORKERS=1 instead"]
F --> G["One process writes one complete report"]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two docs made claims that a measurement contradicts. Since the code fix is deferred to 2.1, documentation truth is the whole 2.0 mitigation, so it has to be right.
What was wrong
docs/reporters.mdsaid that in parallel environments "each worker writes to the same file — the last worker's results will be in the report." That is true only for one-process-per-worker runners. Under Rails' defaultparallelize(workers: N), which forks, no report is written at all — and the[snap_diff] N screenshots compared …summary line disappears with it.Mechanism: Minitest skips its
after_runhooks in a forked child (Minitest.allow_forkisfalseby default — minitest 5.27.0lib/minitest.rb:64, guard at:79), and::Minitest.after_runis our only finalize trigger (lib/snap_diff/integrations/minitest.rb:69). The four workers hold every record and never finalize; the parent finalizes having recorded nothing.docs/thread_safety.mdwas stale on specifics: it claimedSnapManager"returns a new instance for each call" (it has been fiber-memoized since D7,lib/snap_diff/snap_manager.rb:112), documented aregistrymethod now namedSnapDiff.session, and never mentioned process parallelism at all — so a reader finished it believing all parallelism was fine.Evidence
A harness (scratch project,
path:-referencing this repo) drove the real matcher → snap manager → git baseline checkout → comparison → registry → reporter path with 8 test classes × 8 tests. The browser was the only stubbed layer.with: :threads, distinct namesworkers: 4, distinct namesreport=NOevery time; all.diff.pngartifacts presentBoth documented workarounds were run, not reasoned about:
parallelize_teardown { Reporting.finalize! }+ per-workeroutput_pathinparallelize_setup→ all 8 failures preserved across per-worker files, twice.PARALLEL_WORKERS=1→ single complete report.The tempting third option — a per-worker
save_path— is explicitly called out as wrong:save_pathalso determines where baselines are read from (SnapDiff.config.screenshot_area), so it would point comparisons at an empty baseline directory.Scope
Only
docs/reporters.md(its parallel section) anddocs/thread_safety.md(rewritten). No code, no CHANGELOG, no other docs — the 2.0.0 CHANGELOG entry for this limitation belongs to the release-readiness lane.Also new in
thread_safety.md: theSnapDiff::Vcsgit-root memo added by #250 (unsynchronized process-global Hash; idempotent values, but a plain Hash is not a concurrent container on JRuby), and a plain warning that screenshot names must be unique across tests.standardrb lib test— 151 files, no offenses.🤖 Generated with Claude Code
https://claude.ai/code/session_014BQJX6eWzBj2UTm5zQsjEs
Summary by Sourcery
Correct the parallel testing documentation so it accurately describes report generation, shared state, screenshot naming requirements, and mitigations for each execution mode.
New Features:
Enhancements:
Documentation: