From b570f4c484917196a5b50e0c5ab4f56ba07a9bb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20=C5=81=C4=85tka?= Date: Wed, 9 Sep 2026 20:18:12 +0200 Subject: [PATCH] perf(ci): take typechecking off the unit-test critical path (#1110) `Unit Tests` is the entire PR wall clock: **468s of run + 61s of queue**, while the other five gating jobs finish in 7-118s. Typechecking is 64s of that, sits in series ahead of the tests, and needs none of their output. | | before | after | |---|---|---| | typecheck (64s) | serial, inside the test job | own job, runs alongside - **116s, parallel** | | npm cache | none on any gating job | `cache: npm`, except knip | | superseded PR runs | keep holding a runner | cancelled | This is scaffolding, not the fix. The job is dominated by `tool-server/test/flows`, and that needs its own PR - measurements and a rejected shortcut below. `Typecheck` is a new check. It needs adding to the required status checks on `main`, or typecheck failures stop blocking merge.
Where the 8 minutes is `Unit Tests` run `34367669322`, per-step: | step | time | |---|---| | setup + checkout + node | 8s | | npm ci | 12s | | tsc --build | 29s | | Typecheck tests (13 serial `tsc`) | 62s | | **Run tests** | **350s** | | everything else | 5s | `Run tests`, per package: tool-server 317.7s, the other 12 packages 28s combined. Inside tool-server (368 local test files): `test/flows` is 591s across 54 files, everything else 51s across 314. Half the suite is 5 files; 80% is 13, all flows. Those files drive the flow engine through its real `SETTLE_TIMEOUT_MS` (3000), `DEFAULT_ACTION_TIMEOUT_MS` (7500) and `POST_LAUNCH_SETTLE_MS` (1500). The five worst run in isolation at **87.4s wall, 7% CPU**. `flow-gesture-settle.test.ts` is 37 tests at a 3.02s median - the settle window waited out per test.
npm cache, per job (install step, against the uncached baseline) | job | before | after | |---|---|---| | ESLint (docs tree) | 26s | 16s | | Prettier check | 17s | 11s | | Unit tests | 12s | 10s | | Knip | 6s | 8s + 1s save | Knip installs with `--ignore-scripts` and pulls the least of any job here, so restore and save cost more than the download they replace. It keeps the uncached path.
Over-subscribing the worker pool: tried on this PR, reverted Flooring `maxWorkers` at 8 was in the first commit here. On this PR's own CI run it halved the suite - **317.7s -> 152.1s** - and broke one test. `flow-composition.test.ts` went **26.5s -> 86.8s** and blew the 60s budget it sets for itself. It pumps a fake clock in a busy loop rather than sleeping, so it is CPU-bound where the rest of `test/flows` is idle, and two workers per core starve it superlinearly. Suite-wide, `import` went 106s -> 210s for the same reason. Reverted in the second commit. Over-subscription is only a way to hide the sleeping; once the flow tests stop waiting in real time, one worker per core is the right number and nothing is starved.
What actually reaches 1 minute Vitest parallelises per file and `flow-gesture-settle.test.ts` alone is 86.5s, so bin-packing the real per-file durations gives 161s at 4 workers and 87s at 8, 16, 32 or 64. **No runner size or CI matrix shard beats ~87s of test time.** Getting under a minute needs the flow engine's timing made injectable, with those tests injecting small values. 13 flow files assert absolute durations and need individual attention. Two shortcuts do not work: - Scaling the engine's timing constants 10x down reaches 131s of test time and 52.9s wall at 4 workers, but is unstable: a rerun of the same config hung 3 files to ~600s. The fixtures' own values (`readDelayMs = 400`, flow `timeout: 600`, `DEFAULT_LONG_PRESS_MS` 800) do not scale with it, so the relationships invert. - `vi.useFakeTimers({ shouldAdvanceTime: true })` passes 37/37 but gives no speedup - `shouldAdvanceTime` advances fake time 1:1 with real time by design. `flow-composition.test.ts` already demonstrates the pattern that does work: fake clock plus an explicit pump loop.
Not caching `dist/`: this repo has a history of dist-skew making assertions pass vacuously, and a stale build artifact is a worse failure than a slow job. No docs update needed: CI-internal, no user-facing capability changed. --- .github/workflows/format.yml | 7 +++++ .github/workflows/knip.yml | 6 +++++ .github/workflows/lint.yml | 10 +++++++ .github/workflows/lockfile.yml | 6 +++++ .github/workflows/repo-hygiene.yml | 6 +++++ .github/workflows/unit-tests.yml | 42 +++++++++++++++++++++++++----- 6 files changed, 71 insertions(+), 6 deletions(-) diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index a8aa554bd..195d3bc69 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -8,6 +8,12 @@ on: branches: - main +# Supersede a PR's in-flight run rather than let it hold a runner for a verdict +# nobody wants. A push to main keeps its run: that is the commit's only record. +concurrency: + group: format-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: format: name: Prettier check @@ -21,6 +27,7 @@ jobs: uses: actions/setup-node@v6 with: node-version: "20" + cache: npm - name: Install dependencies run: npm ci diff --git a/.github/workflows/knip.yml b/.github/workflows/knip.yml index 046a61dac..20a62777f 100644 --- a/.github/workflows/knip.yml +++ b/.github/workflows/knip.yml @@ -8,6 +8,12 @@ on: branches: - main +# Supersede a PR's in-flight run rather than let it hold a runner for a verdict +# nobody wants. A push to main keeps its run: that is the commit's only record. +concurrency: + group: knip-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: knip: name: Knip diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 03ab9f250..3be395288 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -8,6 +8,12 @@ on: branches: - main +# Supersede a PR's in-flight run rather than let it hold a runner for a verdict +# nobody wants. A push to main keeps its run: that is the commit's only record. +concurrency: + group: lint-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: lint: name: ESLint @@ -22,6 +28,10 @@ jobs: with: # ESLint 10 requires Node ^20.19 || ^22.13 || >=24. node-version: "24" + cache: npm + cache-dependency-path: | + package-lock.json + packages/docs/package-lock.json - name: Install dependencies run: npm ci diff --git a/.github/workflows/lockfile.yml b/.github/workflows/lockfile.yml index 38f737bdb..79180b47f 100644 --- a/.github/workflows/lockfile.yml +++ b/.github/workflows/lockfile.yml @@ -8,6 +8,12 @@ on: branches: - main +# Supersede a PR's in-flight run rather than let it hold a runner for a verdict +# nobody wants. A push to main keeps its run: that is the commit's only record. +concurrency: + group: lockfile-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: lockfile-sync: name: package-lock.json in sync diff --git a/.github/workflows/repo-hygiene.yml b/.github/workflows/repo-hygiene.yml index 86ee8100c..a8e15e2f2 100644 --- a/.github/workflows/repo-hygiene.yml +++ b/.github/workflows/repo-hygiene.yml @@ -8,6 +8,12 @@ on: branches: - main +# Supersede a PR's in-flight run rather than let it hold a runner for a verdict +# nobody wants. A push to main keeps its run: that is the commit's only record. +concurrency: + group: repo-hygiene-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: checks: name: Static checks diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index ba27a88b0..6bb3beba0 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -8,6 +8,12 @@ on: branches: - main +# Supersede a PR's in-flight run rather than let it hold a runner for a verdict +# nobody wants. A push to main keeps its run: that is the commit's only record. +concurrency: + group: unit-tests-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: unit-tests: name: Unit tests @@ -21,6 +27,7 @@ jobs: uses: actions/setup-node@v6 with: node-version: "20" + cache: npm - name: Install dependencies run: npm ci @@ -30,14 +37,37 @@ jobs: - name: Build workspace dependencies run: npm run build - - name: Typecheck tests - run: npm run typecheck:tests --workspaces --if-present - - - name: Typecheck scripts - run: npm run typecheck:scripts - - name: Run tests run: npm test --workspaces --if-present - name: Test root scripts run: npm run test:scripts + + # Typechecking needs the same install and build as the tests but none of their + # output, so running it alongside them takes its ~60s off the critical path + # rather than adding to it. + typecheck: + name: Typecheck + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v7 + + - name: Set up Node.js + uses: actions/setup-node@v6 + with: + node-version: "20" + cache: npm + + - name: Install dependencies + run: npm ci + + - name: Build workspace dependencies + run: npm run build + + - name: Typecheck tests + run: npm run typecheck:tests --workspaces --if-present + + - name: Typecheck scripts + run: npm run typecheck:scripts