mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
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.
<details>
<summary>Where the 8 minutes is</summary>
`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.
</details>
<details>
<summary>npm cache, per job (install step, against the uncached
baseline)</summary>
| 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.
</details>
<details>
<summary>Over-subscribing the worker pool: tried on this PR,
reverted</summary>
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.
</details>
<details>
<summary>What actually reaches 1 minute</summary>
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.
</details>
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user