Skip to content

Add timing-aware spec splitting to RunSpecs#29

Merged
tycooon merged 5 commits into
masterfrom
timing-aware-split
Jul 20, 2026
Merged

Add timing-aware spec splitting to RunSpecs#29
tycooon merged 5 commits into
masterfrom
timing-aware-split

Conversation

@tycooon

@tycooon tycooon commented Jul 14, 2026

Copy link
Copy Markdown
Member

Summary

RunSpecs splits 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.yml is 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 an after(: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

  • Whole-file items are passed to rspec as file paths: shorter command line, and before(:context) hooks stop running on multiple nodes.
  • First real-world run (20-node suite, 6.7k examples): max node 373s -> 329s, min 169s -> 182s with first-generation timings.
  • Version bumped to 0.11.0.

@tycooon-review-bot tycooon-review-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by Codex.

Comment thread lib/ci_helper/commands/run_specs.rb Outdated
Comment thread lib/ci_helper/commands/run_specs.rb Outdated
tycooon added 2 commits July 20, 2026 18:12
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 tycooon-review-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by Codex.

Comment thread lib/ci_helper/commands/run_specs.rb Outdated
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 tycooon-review-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by Codex.

Comment thread lib/ci_helper/tools/spec_timings_recorder.rb Outdated
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.

@tycooon-review-bot tycooon-review-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved by Codex.

@tycooon
tycooon merged commit f4a24ad into master Jul 20, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants