From 5fdb764977c31fe300a684ea73c9e6f9bddc0ded Mon Sep 17 00:00:00 2001 From: Babissimo Date: Wed, 23 Sep 2026 16:36:29 +0100 Subject: [PATCH] Shard the dashboard's tests two ways Since #569 shortened the backend path, the dashboard's tests entry has been its equal: on CI runs since then its job took a median 88 s against 92 s for the slowest backend shard plus backend-coverage, queue-free, and it was the longer of the two in 2 of 7 runs. It set the gate outright on main run 35876349605. So no further backend saving reaches the gate while this entry sits beside it. A vitest shard loads only its own files, so unlike pytest's the split divides nearly all of the suite's time (about 8 s fixed against 92 s divisible, measured at three workers). Two shards partition the suite exactly, 58 files each and 497 + 857 = 1354 tests, and split its CPU time about evenly, which should put each entry near 55 s. test_web_build_matrix.py pinned each workspace script to exactly one entry. It now accepts a script that runs once per shard, provided the shards are 1..N of N exactly once each, and checks that the test step hands each entry its shard after npm's --. Without the separator npm keeps --shard as its own config and both entries run the whole suite, green. Nine mutations of the shard list, the step and the labels each turn it red. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 11 +++--- ONBOARDING.md | 3 +- backend/tests/test_web_build_matrix.py | 49 +++++++++++++++++++------- 3 files changed, 45 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c8ad2820e..dfa571b2f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -280,14 +280,17 @@ jobs: # The workspaces are independent; a failure in one should still report # the others rather than cancel them. Every workspace in package.json has # an entry, and each of its lint, typecheck, test and build scripts runs - # in exactly one (backend/tests/test_web_build_matrix.py). + # in exactly one, or once per shard (backend/tests/test_web_build_matrix.py). fail-fast: false matrix: # The dashboard's tests outlast its lint, typecheck and build together, - # so they run in an entry of their own rather than after them. + # so they run in entries of their own rather than after them. Unlike + # pytest's, a vitest shard loads only its own files, so splitting them + # divides nearly all of their time. include: - { workspace: dashboard, part: checks, label: dashboard checks } - - { workspace: dashboard, part: tests, label: dashboard tests } + - { workspace: dashboard, part: tests, shard: 1/2, label: dashboard tests 1/2 } + - { workspace: dashboard, part: tests, shard: 2/2, label: dashboard tests 2/2 } - { workspace: packages/shared } - { workspace: e2e } steps: @@ -309,7 +312,7 @@ jobs: # checks before a deploy depends on them. - run: npm run typecheck -w ${{ matrix.workspace }} if: matrix.part != 'tests' - - run: npm test -w ${{ matrix.workspace }} --if-present + - run: npm test -w ${{ matrix.workspace }} --if-present ${{ matrix.shard && format('-- --shard={0}', matrix.shard) || '' }} if: matrix.part != 'checks' # Only the console has a build. - run: npm run build -w ${{ matrix.workspace }} --if-present diff --git a/ONBOARDING.md b/ONBOARDING.md index 9eef70ca2..e068abc2f 100644 --- a/ONBOARDING.md +++ b/ONBOARDING.md @@ -294,7 +294,8 @@ CI runs on every PR, on push to `main`, and on demand through 1. Any PR, whatever its base: `backend-tests` (three shards) and `backend-coverage` behind them, `lint`, `web-build` (once per - workspace, with the dashboard's tests apart from its other checks), + workspace, with the dashboard's tests in two shards apart from its + other checks), `docker-build`, `env-parity`, plus an automated review. 2. Merge to `main` → deploy to **staging** → staging smoke + Playwright E2E → deploy to **production** → prod smoke + Playwright E2E. A merge that changes nothing the droplets serve skips that chain, which means diff --git a/backend/tests/test_web_build_matrix.py b/backend/tests/test_web_build_matrix.py index 5f521e789..bffc8b25a 100644 --- a/backend/tests/test_web_build_matrix.py +++ b/backend/tests/test_web_build_matrix.py @@ -1,12 +1,12 @@ """`web-build` runs each workspace's npm scripts from a matrix, and which entry runs which script is decided by step conditions. A mistake there fails nothing: -a script that runs in no entry goes unchecked, and one that runs in two quietly -lengthens the run. These pin the assignment. +a script that runs in no entry goes unchecked, one that runs in two quietly +lengthens the run, and a shard missing from the list drops its tests unseen. +These pin the assignment. """ import json import re -from collections import Counter from pathlib import Path import pytest @@ -38,6 +38,15 @@ def _runs(step: dict, entry: dict) -> bool: return (entry.get("part") == value) == (op == "==") +def _shard(entry) -> tuple[int, int] | None: + shard = entry.get("shard") + if shard is None: + return None + found = re.fullmatch(r"(\d+)/(\d+)", str(shard)) + assert found, f"{entry['workspace']}'s shard {shard!r} is not k/N" + return int(found.group(1)), int(found.group(2)) + + def _scripts_run(job, entry) -> list[str]: scripts = [] for step in job["steps"]: @@ -53,19 +62,32 @@ def test_every_workspace_has_an_entry(job): @pytest.mark.parametrize("workspace", WORKSPACES) -def test_each_script_a_workspace_defines_runs_in_exactly_one_entry(job, workspace): +def test_each_script_a_workspace_defines_runs_once_or_once_per_shard(job, workspace): defined = json.loads((ROOT / workspace / "package.json").read_text())["scripts"] - ran = Counter( - script for entry in _entries(job) if entry["workspace"] == workspace for script in _scripts_run(job, entry) - ) for script in ("lint", "typecheck", "test", "build"): - if script in defined: - assert ran[script] == 1, f"{workspace}'s {script} runs in {ran[script]} entries" + if script not in defined: + continue + shards = [_shard(e) for e in _entries(job) if e["workspace"] == workspace and script in _scripts_run(job, e)] + assert shards, f"{workspace}'s {script} runs in no entry" + if shards == [None]: + continue + assert None not in shards, f"{workspace}'s {script} runs in {len(shards)} entries, not all of them shards" + assert sorted(shards) == [(k, len(shards)) for k in range(1, len(shards) + 1)], ( + f"{workspace}'s {script} runs as shards {sorted(shards)}, not 1..N of N once each" + ) + + +def test_the_test_step_hands_each_entry_its_shard(job): + """Without it, every shard entry runs the whole suite. So it does without the + `--`, because npm keeps an option before that separator as its own config.""" + step = next((s for s in job["steps"] if re.match(r"npm (?:run )?test -w ", s.get("run", ""))), None) + assert step, "no step runs the test script" + assert re.search(r"format\('-- --shard=\{0\}', matrix\.shard\)", step["run"]), step["run"] def test_the_dashboard_is_split_into_its_tests_and_its_checks(job): - parts = sorted(e.get("part") for e in _entries(job) if e["workspace"] == "dashboard") - assert parts == ["checks", "tests"] + parts = {e.get("part") for e in _entries(job) if e["workspace"] == "dashboard"} + assert parts == {"checks", "tests"} def test_each_half_of_a_split_runs_only_its_own_scripts(job): @@ -80,7 +102,8 @@ def test_each_half_of_a_split_runs_only_its_own_scripts(job): def test_a_split_entry_is_named_for_its_part(job): - """The label is the job's name in the checks list, so it must say which half ran.""" + """The label is the job's name in the checks list, so it must say which half, and which shard, ran.""" for entry in _entries(job): if "part" in entry: - assert entry.get("label") == f"{entry['workspace']} {entry['part']}" + expected = " ".join(str(x) for x in (entry["workspace"], entry["part"], entry.get("shard")) if x) + assert entry.get("label") == expected