Repository navigation
build-test: remove the fixed per-package cost and run suites in parallel (CL-7484) - #674
Merged
Merged
Conversation
run-all.test.ts still hard-coded the test script as the one script the runner always caps at concurrency 1. Update its expectations first so the next commit's removal of that carve-out lands against a red-then- green suite rather than a change with no coverage of its own behavior.
Every DB-gated suite now resolves its own scratch database and every e2e suite mkdtemps its own HUB_DATA_DIR and binds an OS-assigned port (scripts/e2e/harness.ts's startHub/startSidecar/freePort), so nothing left in the test phase depends on running one package at a time. The test script now resolves concurrency exactly like typecheck: every core in CI, cores minus two locally, or WORKBENCH_CHECK_CONCURRENCY when set.
…ort chain Every package's migrations.test.ts / *.drizzle.test.ts imported scripts/e2e/harness.ts purely for e2eDatabaseUrl (to skip when DATABASE_URL is unset). harness.ts also imports @corbits/seeding and @corbits/workflows for pushWorkflowSource/workflowDeployBody, which pulls in @intx/types, @intx/hub-common, and @intx/agent — over a second of module-load cost paid on every build-test run of every one of those packages, whether or not the suite ever executes. Move the DATABASE_URL resolution (e2eDatabaseUrl, baseUrlToE2eUrl, parseEnvFileDatabaseUrl, REPO_ROOT) into scripts/e2e/database-url.ts, and the two workflow-source-publishing helpers into scripts/e2e/workflow-source.ts. harness.ts re-exports the former for its own heavier consumers and no longer imports seeding/workflows at all. Update every DB-gated suite to import e2eDatabaseUrl from database-url.ts directly, and the handful of full e2e suites (plus longevity-sim's stack.ts) to import pushWorkflowSource/ workflowDeployBody from workflow-source.ts.
A hosted CI runner's fixed core count caps how much package fan-out alone can cut build-test's wall time; the next commit lets the test script split across a matrix of runners instead. Add its test coverage first: --shard i/n partitions the stably name-sorted job list with no package dropped or run twice, rejects a malformed shard spec, and scripts/e2e/db-gate.test.ts covers the "build-test-shard" GITHUB_JOB name the matrix job actually reports.
--shard i/n round-robins the workspace's stably name-sorted package list across n shards and runs only shard i's assignment, so a script run can be split across several CI runners instead of one. Add "build-test-shard" next to "build-test" in dbGate's no-Postgres job set, since that's the actual GITHUB_JOB name a sharded matrix run carries.
A hosted GitHub Actions runner's 4 vCPUs meant scripts/run-all.ts's own package-level concurrency alone couldn't cut build-test's test step much past ~115s (its total CPU-seconds across every package's test script divided by 4 cores). Splitting the same total work across 3 runners' worth of vCPUs is what actually buys more parallelism. build-test-shard is now a 3-entry matrix, each running `bun run build` and `scripts/run-all.ts test --shard <n>/3`; shard 1 also carries the root-level scripts/*.test.ts suite, which only needs to run once. build-test itself becomes a summary job that needs build-test-shard and fails if any shard did, keeping the required check name every branch protection rule and PR template names unchanged.
build-test is now a no-op summary job over the build-test-shard matrix; the matrix job is the one that actually checks out the repo with fetch-depth: 0 for tool-package-freshness's merge-base diff.
Running the test suite at full concurrency (this stack's earlier commits) surfaced a genuine flake here: sweepIntervalMs=5 / idleSleepMs=10 / a 15ms wait window assume the event loop keeps up within single-digit milliseconds, which a CPU-contended CI runner running many package test scripts at once cannot guarantee. Scale every one of those constants up by 8x (5->40, 10->80, 15->120, 20->160) — same relative timing the tests already assert on, just wide enough margins to tolerate tens of milliseconds of scheduler jitter under load. idleSleepMs: 1_000 tests are unaffected; that window was already generous.
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
CI's
build-testjob took 261s. Two independent costs, both fixed:Fixed per-package cost (CL-7417). Every package's
migrations.test.ts/*.drizzle.test.tsimportedscripts/e2e/harness.tspurely fore2eDatabaseUrl(to skip cleanly whenDATABASE_URLis unset).harness.tsalso imported@corbits/seedingand@corbits/workflowsfor two workflow-publishing helpers most of its consumers never call — pulling in@intx/types,@intx/hub-common, and@intx/agent, over a second of pure module-load cost paid by ~48 files whether or not the suite ever ran. Split the DATABASE_URL resolution intoscripts/e2e/database-url.tsand the two heavy workflow-source helpers intoscripts/e2e/workflow-source.ts;harness.tsno longer imports seeding/workflows at all.packages/bench's test time dropped from 3.33s to 0.12s in isolation;packages/longevity-simfrom 2.89s to 1.4s.Sequential package fan-out.
scripts/run-all.tsforced thetestscript to concurrency 1, reasoning that suites spawn processes, bind sockets, and talk to Postgres and would flake under contention. Every suite that does any of that already isolates itself (OS-assigned ports, an explicit per-calldataDir, a scratch database,installDisposableHubDataDir()), and four consecutive full-concurrency local runs produced no failures, so the carve-out is dropped —testnow resolves concurrency exactly liketypecheck.Shard the CI job too. A hosted runner's 4 vCPUs still capped the test step at ~115s even after both fixes above — the total CPU-seconds across every package's test script, divided by only 4 cores.
scripts/run-all.tsnow supports--shard i/n(round-robins the stably name-sorted package list across shards, no package dropped or run twice), andbuild-testin CI is now a 3-way matrix (build-test-shard) plus abuild-testsummary job thatneedsit, keeping the required check name unchanged.Before / after
bun run testlocally: 261s (old CI) -> ~44s wall, three consecutive clean runs.build-testjob: 261s -> 143s (single job, harness fix + concurrency only) -> sharded 3-way matrix, watched green on this PR.Resolves CL-7484
Resolves CL-7417
Test plan
bun run typecheckbun run lintbun run test(three consecutive local runs, all green, ~44s each)bun run check:structuralgh pr checksgreen on this PR (build-test-shard matrix + build-test summary)