Repository navigation
ci(test): reduce test shards to three - #1481
Conversation
juliopolycarpo
added
type: test
github-actions
Bot
added
the
area: build
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good to merge from a config-consistency standpoint. No prior review threads; this is the first pass on d039bbdc vs stacked base perf/unit-coverage-parallel-workers (74f557a5).
What changed: SHARD_COUNT, the shard matrix, and the job name all moved from 4 → 3 together. The 240s watchdog is intentionally unchanged. Docs record that this count is a projection from run 38202725009, not a hosted 3-vs-4 bake-off, and why two shards is out of scope. Existing workflow tests already derive the expected matrix and job-name denominator from SHARD_COUNT, so no new regression test was required.
Correctness: The three-line change contract is satisfied. Merge/metrics still take $SHARD_COUNT, so a missing shard still fails as partial rather than a quiet green. Watchdog math still fits timeout-minutes: 10 (2 × 240s + ~60s other steps). The 25s / 55s / 60s projection is internally consistent with the table (unit scaled ~2.5×, integration and root scripts redistributed over three).
Risk (accepted, already written down): worst-shard integration imbalance. Shard 4 was already 73s of serial integration; three shards can still land a step well above the ~80s mean. Rejection criteria (step ≥ 147s, or Test completion clearly worse than four shards) are the right gate. Stacked targeting means this PR’s current check suite is not evidence the projection holds — that only happens after #1480 merges and this is retargeted.
Nit, not blocking: the new watchdog comment in .github/workflows/test.yml wraps mid-sentence (…bound is kept. Both attempts have to fit this job's on one line). Wrap it if you touch the file again.
No P0/P1. No inline threads.
Sent by Cursor Automation: Mango Ecosystem CodeReview
Reduce the Test matrix from four shards to three. With the api unit lane at four in-process workers per shard, that lane no longer bounds the shard test step: the serial, unisolated integration lane does, with root scripts next. Projected from the per-lane times of a four-shard run on main, three shards land near an 80s step, about where four stood at one unit worker, for one runner and one job setup fewer. Set `SHARD_COUNT`, the matrix and the job name to three. The 240s watchdog bound is kept: it was sized to the 120s worst step at four shards, and the projected step is shorter. Record the projection, its assumptions and why two shards is not the next step in the testing reference. The hosted 4/6/8 comparison stays as the record of the previous count; it ran the unit lane at one worker and does not rank three. Signed-off-by: Julio Polycarpo <julio@polycarpo.dev>
Record the first hosted four-worker baseline and revise the three-shard estimate to about 90 seconds. Preserve the initial estimate as historical evidence and require a matched hosted comparison before claiming a timing gain. Keep the 147-second rejection limit, the integration isolation gate, and count-independent watchdog comments visible. Signed-off-by: Julio Polycarpo <julio@polycarpo.dev>
d039bbd to
a5754c2
Compare
|
PR head: CI duration (wall clock, critical path, slowest jobs)CI DurationReport-only timing from the Actions jobs API; runner variance does not affect the gate.
Since previous PR run: n/a — Five slowest head jobs
QA Gate — Coverage & QualityBase: ✅ No attention signals — collected metrics look healthy against base. LoC (code): ⚪ ▲ = 0 • Line coverage (all workspaces): 🟢 ▲ +0.01pp • Quick check: pass • Duplication: ⚪ ▲ = 0 • Bundle gzip: ⚪ ▲ = 0 • Locked deps: ⚪ ▲ = 0 • Tests passed: ⚪ ▲ = 0 Metric details (coverage, LoC, bundle, dependencies, tests, duplication, tooling)CoverageAPI/shared branches and statements are source-derived from LCOV line hits because Bun LCOV does not emit branch or statement records. Rust crates: line, function and region coverage from
Lines of CodeAuthored source: production and test files of every discovered component. Generated, fixture, config and docs lines are recorded in the envelope but not counted here.
Frontend Bundle
Dependencies
TestsSingle full-suite pass (unit + integration, from the coverage run).
Test lanesOne row per lane, deduplicated by test.
Code Duplication (jscpd)
Repo Tooling
|
Commits — 2 commitsBase
Full commit messages
|
1 participant
Summary
Reduce the Test matrix from four shards to three after #1480 enabled four isolated API unit workers per shard. This removes one runner job and its fixed setup. The wall-clock tradeoff remains a projection pending a hosted three-versus-four comparison.
Changes
SHARD_COUNT, the matrix and the job-name denominator to three.Test Plan
bun run checkpassesbun run testpassesbun run buildpasses@mangostudio/shared/i18n(no hardcoded strings)Local validation at
a5754c27:bun run check && bun run testpassed, plus 181 focused tests covering workflow/lane guards, qualification, shard/timing merges, metrics and watchdog retries. Built the runtime and fake agent from this checkout with--locked; the runtime checksum remained unchanged after the fixture build.Local
bun run buildand manual UI smoke were not run. No UI strings changed. Current-head hosted Test jobs and the coverage/timings merge passed: 14,136 tests passed, 82 skipped, no failures or retries. The three shard steps took 108s, 107s and 53s, all below 147s. Broader CI is still running. This single first-attempt sample does not establish a paired three-versus-four performance benefit.Screenshots / GIFs
Not applicable.
Notes
Ref #889. This changes CI shard count; the broader worker defaults and override remain outside this PR.
Timing evidence and limits
The earlier hosted 4/6/8 comparison ran API unit coverage at one worker per shard and does not rank three. The initial projection used this four-shard main run, 38202725009, at
a39ca7econ Bun 1.4.3:Assuming about 2.5x for unit coverage on a shared runner gave a three-shard estimate near 80s.
The parent's first four-worker run, 38208603805, at
74f557a5on Bun 1.4.3, showed root scripts bounding two shards:Redistributing these totals over three shards gives about 31s unit, 58s integration and 69s root scripts. Unit and integration run sequentially, so the revised estimate is near 90s; an imbalanced shard could exceed 110s. These source revisions differ, and the observations are neither a paired speedup nor a distribution for three shards.
Reject three shards if any shard test step reaches 147s or Test completion is clearly worse than four over several first-attempt runs on matched source, worker width and timing baseline. Keep the 240s watchdog.
Two shards remains a later candidate, projecting to about 133s before imbalance from the newer totals. Integration parallelism needs separate qualification while oven-sh/bun#39709 remains open; unit-lane soak does not clear that gate.