feat(shard): split runs across parallel CI jobs and merge the results - #17
Merged
Conversation
Splits a twd-cli run across parallel CI jobs via `--shard i/n`, has each shard write a machine-readable report plus raw Istanbul coverage, and adds `twd-cli merge <dir>` to join them back into one report. In-process Puppeteer parallelism was already tried and was too flaky — separate jobs share no CPU, dev server, or browser. What blocked that was the lack of any joinable output: every structured value runTests() builds is printed and discarded. Key decisions: - A merged report is shape-identical to a single-shard report, so merge is associative and existing formatters work on both. - Round-robin slicing needs no advance knowledge of the test count; each shard enumerates the suite itself, as runs already do. - discovery.fingerprint makes shards prove they saw the same test set, turning conditional test registration from a silent green into an error. - The coverage failure gate moves from shard level to merge level, so a red shard can no longer silently understate merged coverage. - Merge owns the final exit code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…onale --report only meant "write a report without sharding", which `--shard 1/1` already expresses. Cutting it removes a flag, the "--shard implies --report" rule, and a code path in parseArgs, for something no consumer needs yet. That cascade also removes selection.mode: with reports only existing under --shard, the field would be the constant "shard". Also records why a per-shard maxFailures budget is acceptable rather than merely unavoidable — with the suite divided N ways each shard hits its own limit fast, so the extra failures cost no noticeable time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A run without --shard must behave exactly as 1.4.0. The two changes that would have leaked into existing runs — relaxing the coverage failure gate and validating contracts after an early stop — are now gated on sharding, each reducing to today's expression when sharded is false. Adds the release plan: 1.5.0-beta.0 published under the beta dist-tag, which publish.yml already routes for prereleases, so `npm install twd-cli` keeps resolving to 1.4.0. Also adds explicit non-regression tests for the two touched conditionals, since inference is not coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The per-shard breakdown line needs to show which shard went red, and merged tests do not record which shard ran them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
11 TDD tasks, 78 steps, covering src/shard.js, src/runReport.js, src/reportFiles.js, src/mergeCoverage.js, src/mergeReports.js and src/mergeCommand.js, plus the index.js wiring, the merge subcommand, an end-to-end sharded CI job, and the 1.5.0-beta.0 bump. Two structural decisions the plan locks in beyond the spec: - mergeCommand.js is split from mergeReports.js so the merge stays pure and associative. Completeness checking cannot live inside the merge: a 2-of-3 merge is a legal intermediate value, so rejecting it there would make merge(merge(a,b),c) throw and destroy the property that proves no test is lost or doubled. - Each shard's contract markdown is suppressed. Under sharding every shard would overwrite the others with a fraction of the picture, so merge writes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
parseMergeArgs duplicated parseRunArgs's readValue closure verbatim, which violates the plan's own DRY constraint. Hoisted to module scope instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Promotes istanbul-lib-coverage to a runtime dependency (was only transitive via @vitest/coverage-v8) since the merge command runs in a consumer project where devDependencies aren't installed. Clones each shard's coverage object before CoverageMap.merge, since FileCoverage aliases a plain object instead of copying it, which would otherwise mutate the first shard's coverage in place once a later shard's counts for the same file are merged in.
Adds src/mergeReports.js: mergeRunReports combines shard reports while validating only cross-shard consistency (schemaVersion, fingerprint, shard total, duplicate index/test id). Completeness is intentionally left to the separately-exported findMissingShards, called later by the merge command, so a partial merge stays associative. Also exports reportTimings (wall vs compute time) and reportTotals (executed/notRun vs discovered count), both derived rather than stored.
…mment Review found the existing tests couldn't distinguish taking discovery/ selection/handlers/contracts.configured from the first shard report vs the last, since every fixture shared identical values for those fields. Adds a test that makes them differ (while keeping fingerprint/schemaVersion/ total consistent, since a partial merge is legal) and asserts first-wins. Also corrects a comment claiming the fingerprint check guarantees handler identity across shards — fingerprintTests only hashes the ordered test-id list and filters, not handler metadata, so that guarantee doesn't exist.
--shard takes a round-robin slice of the ordered test ids after --test filters resolve, then writes .twd/run/run.json (plus coverage.json when collected) for a later merge. Two gates are relaxed for sharded runs only, and both reduce to the original expression when shard is absent: - coverage: !hasFailures becomes (sharded || !hasFailures), because hasFailures is per shard and would let three green shards contribute coverage while a red fourth contributes none. - contracts: !stoppedEarly becomes (sharded || !stoppedEarly), with the result flagged partial so merge can say what is missing. A sharded run deliberately writes neither .nyc_output/out.json nor the contract markdown report: one shard's fraction sitting at either path would masquerade as the whole run's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Drop the shards.length !== 2 check (already enforced by findMissingShards before merged-run.json is even written, so it can never fire) and the file-existence check's implied claim of being a gate (the merge step has no if: always(), so a nonzero exit never reaches here). Replace with an executed-vs-merged-tests check that can actually catch a shard-slicing bug that silently drops tests.
… ids twd-js mints test ids with Math.random() at registration time (twd/src/runner.ts:52), so an id is a per-page-load nonce. Every shard boots its own browser, so no two shards ever agree on an id for the same test. The design built identity on those ids, which broke the feature three ways and made a fourth check a no-op: - discovery.fingerprint hashed the ordered id list, so it could never match across shards and `merge` refused every correct multi-shard run — blaming the user's app for conditional registration. - A merged report keeps only the first shard's handlers, so buildTestPath could not resolve anything from shards 2..n: every failed or retried test from them printed as a raw random id in the merged summary, the one output the merge exists to produce. - mergeRunReports' overlap check was keyed on ids that by construction never collide, so it looked like a guard while proving nothing. Identity is now two fields with distinct jobs. tests[].path is the "suite > test" string, resolved inside the shard that ran the test (the only place its handler map is valid); it is what the fingerprint hashes and what the summary displays. tests[].index is the test's position in the discovered order — deterministic across shards, and the identity key, because a path cannot serve: duplicate test names share one and may legally land in different shards. Fingerprinting paths is also strictly stronger than ids: a conditionally registered test still drops out of the ordered list. Two more findings from the same review: - discovery.totalTests was the unfiltered count while executed/notRun counted the filtered-and-sliced list, so `--test` with `--shard` printed a bogus "shard totals do not add up ... points at a shard-slicing bug". The report now records selection.selectedTests — what the shards actually divide — and reportTotals compares against that. - The "No mocks collected — ensure twd-js supports contract collection" hint fired for every shard whose slice exercised no mocks, advertising a version problem that does not exist on the happy path of a sharded CI run. Gated on !sharded. mergeRunReports also now validates schemaVersion against REPORT_SCHEMA_VERSION rather than only checking that shards agree: reports from a newer twd-cli agree with each other and were being mis-merged silently. REPORT_SCHEMA_VERSION 1 -> 2. The fingerprint's meaning changed, so a v1 and a v2 report would both claim to be mergeable and surface as "different test sets" instead of "run the same twd-cli version". Nothing is published yet. Packaging: add CHANGELOG.md to the files allowlist and document the allowlist in the 1.5.0-beta.0 entry (tarball 209 kB -> 33 kB, 99 files -> 25). No behavior change without --shard: a non-sharded run writes no report, so path/index never reach it, and formatRunComplete falls back to the handler lookup for entries that carry no path. Verified end to end against test-example-app with two real browsers: shards that share zero handler ids now produce identical fingerprints, `merge` succeeds and prints real "suite > test" paths for a shard-2 failure, and the pre-fix binary still reproduces the refusal on the same app. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TWD Contract Validation
23 passed · 41 failed · 3 warnings · 1 skipped Failed validations./contracts/users-3.0.json
./contracts/posts-3.1.json
./contracts/products-3.0.json
./contracts/events-3.1.json
|
Two follow-ups parked during the review. The design doc's canonical schema block still described v1 — schemaVersion 1, no selection.selectedTests, and tests[] without path or index — while a correction note 46 lines below explained that identity had moved to paths and positions. Anyone reading top-to-bottom got the broken shape first. The block now matches what buildRunReport actually emits. Every schemaVersion assertion in the suite derived from REPORT_SCHEMA_VERSION, so editing the constant left all 438 tests green. That is self-defeating for a value whose only job is to reject shards produced by mismatched twd-cli builds. One literal assertion now pins it; verified by flipping the constant to 3 and watching only that test fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TWD Contract Validation
23 passed · 41 failed · 3 warnings · 1 skipped Failed validations./contracts/users-3.0.json
./contracts/posts-3.1.json
./contracts/products-3.0.json
./contracts/events-3.1.json
|
bin/twd-cli.js imported src/index.js at the top level, so `twd-cli merge` pulled in puppeteer and openapi-mock-validator before it even read argv — for a command whose own import graph needs only fs, path, node:crypto and istanbul-lib-coverage. runTests and runMerge are now imported inside their branches. Verified by removing node_modules/puppeteer entirely: `merge` still runs and exits 1 with its usual message, while `run` fails loudly with "Cannot find package 'puppeteer'". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Anyone using BRIKEV/twd-cli/.github/actions/run could not shard: the action only ever ran `npx twd-cli run`, so sharded workflows had to inline every step themselves. It now takes `shard` (as <index>/<total>), plus `report-dir` and `upload-report`. The artifact upload is included and carries `if: always()`, since a red shard that uploads nothing leaves merge unable to tell "this shard failed" from "this shard never ran" — the single easiest thing to get wrong when wiring this by hand. The contract PR comment now skips itself with a notice when `shard` is set. A sharded run deliberately writes no contract markdown per shard, so that step belongs in the job that runs `twd-cli merge`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contract validation is the more important feature and now comes before sharding in both the table of contents and the body. Sharding was 85 lines of a 445-line README, so it moves to docs/sharding.md behind a short summary; the README is down to 384 lines. The doc also gains what the README never said: sharding only pays once test time dominates per-job setup, with the break-even and measured numbers from a 256-test suite, plus the note that a short suite comes out slower. That omission was a trap — this project's own 71-test suite goes from 25s to 41s when sharded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TWD Contract Validation
23 passed · 41 failed · 3 warnings · 1 skipped Failed validations./contracts/users-3.0.json
./contracts/posts-3.1.json
./contracts/products-3.0.json
./contracts/events-3.1.json
|
Sharding ships beta on purpose. It is strictly additive — a run without --shard writes the same files, prints the same output and exits the same way as 1.4.0 — so enabling it cannot disturb an existing pipeline. What is not yet a stable contract is which tests land in which shard: today each shard takes every nth test, and grouping by top-level describe so a suite always stays in one shard is the likely direction. Saying that up front makes the later change a documented evolution instead of a surprise. The beta status is now stated in the README section, the table of contents, the CHANGELOG entry, and `twd-cli` help, so it is visible wherever someone meets the flag rather than only in the docs. docs/sharding.md becomes a standalone guide: a complete copy-pasteable workflow using the bundled action rather than one abbreviated with elisions, a raw-CLI variant for people not using the action, the contract-report snippet for the merge job, and the three load-bearing conditions as a table naming what breaks without each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TWD Contract Validation
23 passed · 41 failed · 3 warnings · 1 skipped Failed validations./contracts/users-3.0.json
./contracts/posts-3.1.json
./contracts/products-3.0.json
./contracts/events-3.1.json
|
Releases as a normal version rather than a prerelease. The sharding *feature* is documented as beta — in the README, the docs, the CHANGELOG entry and `twd-cli` help — but the release itself is stable, so `npm install twd-cli` picks it up and nobody has to opt in through a dist-tag to get the rest of the version. Both package-lock version fields moved with it, regenerated through lock:linux. Also drops the implementation plan from the repo; the design spec is the document worth keeping. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TWD Contract Validation
23 passed · 41 failed · 3 warnings · 1 skipped Failed validations./contracts/users-3.0.json
./contracts/posts-3.1.json
./contracts/products-3.0.json
./contracts/events-3.1.json
|
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.
Splits a
twd-clirun across parallel CI jobs via--shard i/n, has each shard write a machine-readable report plus raw Istanbul coverage, and addstwd-cli merge <dir>to join them back into one report.In-process Puppeteer parallelism was tried before and was too flaky — several browsers contending for one runner's CPU. Separate jobs share no CPU, no dev server, and no browser. What blocked that was the lack of any joinable output: every structured value
runTests()built was printed and discarded.Usage
The
4is the job count, not the test count. Each shard enumerates the whole suite itself and takes every 4th test, so the suite can grow without a YAML edit.Key decisions
mergeRunReports. A 2-of-3 merge is a legal intermediate value; rejecting it inside the merge would destroy associativity, which is the property proving no test is lost or double-counted."suite > test"paths and positional indices, never on test ids. twd-js generates ids withMath.random()at registration, so ids differ per page load. Paths are stable; a positionalindexprovides uniqueness, since two tests can legitimately share a path.if: always(). A silent 3-of-4 merge would read as a complete green run..nyc_output/out.jsononly when the whole run is green. Applied per shard, one red shard would have silently understated merged coverage.No behavior change without
--shardA run without
--shardwrites the same files, prints the same output, and exits the same way as 1.4.0. The two conditionals that changed each reduce to their original whenshardedis false, and there are direct negative tests for both.Three workflow conditions that are load-bearing
fail-fast: falseon the matrix,if: always()on the shard's artifact upload, andif: ${{ !cancelled() }}on the merge job. Omitting any one breaks a sharded run differently — see the README section.Verification
438 unit tests. Verified end-to-end with real browsers against
test-example-app: two shards sharing 0 of 80 handler ids produce identical fingerprints,mergeexits 0 printingShards: 1 ✓36 | 2 ✓35, and an injected failure renders its realsuite > testpath.This PR is what runs
e2e-shardedande2e-mergefor the first time — they have never executed against real Actions.Ships as
1.5.0, a normal release —npm install twd-cligets it. Only the sharding feature is marked beta; the rest of the version is stable.🤖 Generated with Claude Code