Skip to content

Commit ae30abe

Browse files
authored
ci(viewer): shard by a hash of the path, so membership is stable (#3068)
Raised by a reviewer, and it breaks something I built plus a piece of evidence I used. I sharded with `awk 'NR % n == s'` over the sorted file list, so shard membership depends on a file's INDEX. Adding or removing any test file shifts every file after it. Measured on the real tree: adding one file reassigned 534 of 534. Hashing the path instead reassigns 0 of 534. Two consequences, and the second is the one that caught me. A flaky file wanders between shards, so "shard 2 is the slow one" or "the flake lives in shard 1" decays with every added test. And cross-PR comparison is invalid. While chasing the useSandbox.runSupersession flake I argued that #3038's shard 1 passing on the same base showed #3027's shard-1 failure was unrelated. Different PRs, different file counts, different shard composition, so that is not the same set of files and the argument does not carry. The conclusion still stands on the rest of the evidence (neither diff can reach that test, it passes locally 3 of 3, five PRs hit it under concurrency 4), but I stated an invalid argument as proof and it should not have been in the pile. Partition verified exact rather than assumed: 124+147+127+136 = 534 files, 534 distinct paths across the four shards, and with no env set the script still selects all 534, so a local `pnpm test` is unchanged.
1 parent efcbd96 commit ae30abe

2 files changed

Lines changed: 10 additions & 1 deletion

File tree

.github/workflows/test.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,15 @@ jobs:
373373
# knows what good looks like rather than only what broken looked like:
374374
# 2:29, 2:01, 1:50 against a 25 minute cap.
375375
#
376+
# Shard membership is a hash of the file PATH, not its index in the sorted
377+
# list. With `NR % n` every file after an added or removed test moves to a
378+
# different shard: measured, adding ONE file reassigned 534 of 534. Two
379+
# things that breaks. A flaky file wanders between shards, and "shard 1
380+
# passed on PR X" stops being evidence about shard 1 on PR Y, because they
381+
# are not the same set of files. I used exactly that cross-PR comparison as
382+
# proof while chasing a flake, and it did not hold. With the hash, adding a
383+
# file reassigns 0 of 534.
384+
#
376385
# Parallelism comes from the SHARDS, not from concurrency inside a shard.
377386
# `--test-concurrency=1` is deliberate: at 4 a timing-sensitive test races
378387
# three neighbours, and `useSandbox.runSupersession.test.tsx` failed on three

apps/viewer/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"build": "vite build && node ../../scripts/check-tla-chunk-await.mjs",
1515
"typecheck": "tsc --noEmit",
1616
"preview": "vite preview",
17-
"test": "tsx --import ./src/test/vite-module-hooks.mjs --test --test-timeout=120000 --test-concurrency=1 $(find src -type f \\( -name '*.test.ts' -o -name '*.test.tsx' \\) | sort | awk -v s=\"${TEST_SHARD:-0}\" -v n=\"${TEST_SHARDS:-1}\" 'NR % n == s')",
17+
"test": "tsx --import ./src/test/vite-module-hooks.mjs --test --test-timeout=120000 --test-concurrency=1 $(find src -type f \\( -name '*.test.ts' -o -name '*.test.tsx' \\) | sort | awk -v s=\"${TEST_SHARD:-0}\" -v n=\"${TEST_SHARDS:-1}\" '{h=0; for(i=1;i<=length($0);i++) h=(h*31+index(\" !\\\"#$%&()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_abcdefghijklmnopqrstuvwxyz{|}~\", substr($0,i,1)))%1000003} h%n==s')",
1818
"check:templates": "tsc -p src/lib/scripts/templates/tsconfig.json --noEmit"
1919
},
2020
"dependencies": {

0 commit comments

Comments
 (0)