Add timing-aware spec splitting to RunSpecs#29
Merged
Conversation
tycooon-review-bot
suggested changes
Jul 20, 2026
Registering the recorder with a command-line --format made RSpec drop the formatters the consuming suite configures via .rspec (in RSpec 3, any CLI --format replaces the configured list), silently removing documentation, custom, or JUnit formatters and their CI artifacts. Post-processing the report in the command also ran only on success, so a run with any failing example left timings_out as RSpec's nested report rather than the flat map. Ship an RSpecTimingsFormatter and register it via --require, so it is added on top of the suite's own formatters, and write the flat map from its dump_summary hook, which RSpec reaches even when examples fail.
tycooon-review-bot
suggested changes
Jul 20, 2026
An added formatter still occupies rspec's formatter list, and Formatters::Loader only installs the default progress formatter when that list is empty. So for the common suite with no explicit --format in .rspec, enabling --timings-out suppressed the normal progress and failure output entirely. Collect timings from an after(:suite) hook (via RSpec.world.all_examples) instead. It records after every example has finished, so it still captures timings when examples fail, and it never touches the formatter list, so rspec keeps both the suite's configured formatters and its default output.
tycooon-review-bot
suggested changes
Jul 20, 2026
A node runs a subset of example ids, but rspec loads the whole file, so RSpec.world.all_examples also returns examples filtered out on this node. Those never ran and have a nil run_time, which to_f turned into 0.0, so each node's map gained zero entries for other nodes' examples — and merging the per-node maps could overwrite a real timing with 0.0. Skip examples whose run_time is nil, keeping only those actually executed.
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
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.
Summary
RunSpecssplits individual examples across parallel nodes by count, so nodes get equal example counts but unequal runtimes (we observed a 2x+ spread on a large suite), and the whole pipeline waits for the unluckiest node. This PR adds opt-in timing-aware packing:--timings-file PATH- flat JSON map ({"./spec/a_spec.rb[1:1]": 0.42, ...}) of previously recorded timings. When the file is present and non-empty, nodes pack whole spec files by total recorded time (greedy LPT, deterministic, so every node computes the same partition); files heavier than--timings-split-threshold(default 30s) are packed example by example; examples missing from the map get the median recorded time. A missing or unreadable file falls back to the existing count-based split.spec/heavy_specs.ymlis ignored in this mode - timings subsume it.--timings-out PATH- records this run's per-example timings into the same flat map format, so CI can merge per-node files into the next run's--timings-file. Recording is wired in through rspec's--require: it runs in anafter(:suite)hook, so it writes even when examples fail, and — unlike an rspec formatter — it never joins the formatter list, so rspec keeps the suite's own configured formatters and its default progress/failure output intact.The example list still comes from the rspec dry-run, so new or renamed examples can never be silently dropped: everything enumerated is assigned to exactly one node.
Notes
before(:context)hooks stop running on multiple nodes.