diff --git a/scripts/check-cross-package-test-inputs.mjs b/scripts/check-cross-package-test-inputs.mjs index bdd2f265c5..dfdae22bfc 100644 --- a/scripts/check-cross-package-test-inputs.mjs +++ b/scripts/check-cross-package-test-inputs.mjs @@ -109,8 +109,9 @@ // node scripts/check-cross-package-test-inputs.mjs --list-escapes // node scripts/check-cross-package-test-inputs.mjs --self-test -import { readFileSync, readdirSync, statSync, existsSync, writeFileSync } from 'node:fs'; -import { join, resolve, relative, dirname, sep } from 'node:path'; +import { readFileSync, readdirSync, statSync, existsSync, writeFileSync, mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve, relative, dirname, sep, isAbsolute } from 'node:path'; import { fileURLToPath } from 'node:url'; import process from 'node:process'; @@ -1070,7 +1071,16 @@ function unionInto(listPath, changedPath) { if (!hit) continue; const dir = escapingDirs.get(name); if (!dir) continue; - items.push({ name, path: join(REPO_ROOT, dir) }); + // Repo-relative, because that is the convention `turbo ls` emits for every + // entry it wrote (measured on turbo 2.10.10: 0 of 77 items absolute). An + // absolute path here is not wrong for today's only consumer, but it makes a + // single document carry two conventions, and the obvious way to read such a + // document -- `join(REPO_ROOT, it.path)`, correct for every entry turbo + // wrote -- produces a garbage path for exactly these appended entries, which + // are the cross-package scans this function exists to keep running. One + // document, one convention; the consumer resolves it explicitly + // (partition-test-shards.mjs `packageDir()`). + items.push({ name, path: dir }); added.push(`${name} (declared glob matched ${hit})`); } // The push above changed the list's size, so the size the document DECLARES @@ -1416,6 +1426,30 @@ function selfTest() { ok('the write never invents items', written({ count: 0, items: [] }).items.length === 0); ok('the write leaves turbo\'s other fields alone', JSON.parse(serializePackageList({ packageManager: 'pnpm9', packages: { count: 0, items: [] } })).packageManager === 'pnpm9'); + // The path convention this function appends in. `turbo ls` writes every entry + // of this document repo-relative; an entry appended in the other convention + // is not wrong for today's consumer but it makes one array carry two rules, + // and the obvious way to read it -- `join(REPO_ROOT, it.path)` -- then breaks + // on exactly the appended entries. End-to-end through the real `unionInto()` + // and the real serializer, on the fixture that first measured the divergence: + // a diff touching `scripts/**` pulls @objectstack/spec in by its declaration. + const unionDir = mkdtempSync(join(tmpdir(), 'os-union-into-')); + const unionList = join(unionDir, 'turbo-ls.json'); + const unionChanged = join(unionDir, 'changed-files.txt'); + writeFileSync(unionList, JSON.stringify({ packageManager: 'pnpm9', packages: { count: 0, items: [] } })); + writeFileSync(unionChanged, 'scripts/sync-template-versions.mjs\n'); + unionInto(unionList, unionChanged); + const unioned = JSON.parse(readFileSync(unionList, 'utf8')).packages.items; + ok('the union appends the package its declaration matched', unioned.length > 0); + ok( + 'every appended path is repo-relative, the convention `turbo ls` emits', + unioned.length > 0 && unioned.every((i) => !isAbsolute(i.path)), + ); + ok( + 'and each one still names a real directory once resolved against the repo root', + unioned.length > 0 && unioned.every((i) => existsSync(resolve(REPO_ROOT, i.path))), + ); + const failed = cases.filter((c) => !c.cond); for (const c of cases) console.log(`${c.cond ? 'ok ' : 'FAIL'} ${c.label}`); if (failed.length) { diff --git a/scripts/partition-test-shards.mjs b/scripts/partition-test-shards.mjs index a34002d26a..b55b44c1e5 100644 --- a/scripts/partition-test-shards.mjs +++ b/scripts/partition-test-shards.mjs @@ -22,6 +22,43 @@ // all ties broken by name, so every shard computes the identical split from // the same input without coordinating. // +// HOW WELL THAT PROXY HOLDS, MEASURED (2026-08-20, queue builds, not a guess). +// Per-package durations read out of the turbo task groups in `merge_group` CI +// logs, against each package's weight in the same tree: +// +// @objectstack/cli 135 files 548.6s / 474.4s ~3.5-4.1 s/file +// @objectstack/spec 414 files 496.4s ~1.20 s/file +// @objectstack/service-automation 83 files 118.9s ~1.43 s/file +// @objectstack/driver-turso 39 files 53.5s ~1.37 s/file +// @objectstack/client 24 files 34.7s ~1.45 s/file +// @objectstack/example-showcase 21 files 21.6s ~1.03 s/file +// +// So the proxy tracks within roughly +-20% across the big packages and is off +// by ~2.8x on ONE of them (@objectstack/cli). The binning is not the problem: +// on the full package list a queue build hands this script, the three bins come +// out 783/783/782 -- a one-file spread across 2348, far inside LPT's <=4/3 +// bound. Duration spread between shards in the same builds was up to 3.4x, and +// that gap is the proxy's, not the algorithm's. +// +// ⛔ THE HARD LIMIT, AND THE REASON RE-WEIGHTING ALONE CANNOT MEET #4859. +// Sharding is BY PACKAGE, so a shard can never finish faster than its single +// heaviest package. Two packages are already over #4859's "Test Core 最慢分片 +// <= ~7min" threshold on their own: @objectstack/spec at 496s (8m16s) and +// @objectstack/cli at 548s (9m09s). Measured consequence -- in run +// 32352993803, Test Core shard 1 took 8m17.77s wall and @objectstack/spec's own +// suite accounted for 8m16.4s of it: the shard IS that one package. No weight +// function and no shard count changes that; only splitting those suites below +// package granularity, or moving the threshold, does. Anyone arriving here to +// swap the weight input should read that bound first (#10149). +// +// Run-to-run variance is a SEPARATE and equally large effect, and it is not +// placement: this job's Turbo cache key is namespaced per shard +// (`...-turbo---...`) and only main `push` runs write it, so +// each shard's cache ages independently. Measured legs of the same shard index +// ranged from 79/79 tasks cached (866ms, ">>> FULL TURBO") to 0/85 cached +// (10m08s). A single build's shard spread therefore says nothing about +// placement on its own -- compare legs at the same cache state or not at all. +// // Usage: // node scripts/partition-test-shards.mjs --shard N/M \ // [--exclude ]... @@ -37,8 +74,32 @@ import { readFileSync, readdirSync } from 'node:fs'; import path from 'node:path'; +import { fileURLToPath } from 'node:url'; import process from 'node:process'; +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); + +// Where a `packages.items[].path` actually points. +// +// The document has two writers -- `turbo ls`, which emits repo-relative paths, +// and `--union-into` in check-cross-package-test-inputs.mjs -- so the base this +// resolves against must be stated, not inherited. `process.cwd()` is the base +// you get by saying nothing, and it is the one base that can be wrong: CI runs +// this from the repo root, so a relative entry happens to land on the right +// directory, and the day something runs it from anywhere else countTestFiles() +// reads nothing, returns 0, and the partitioner absorbs the zero without +// complaint. Measured before this was pinned -- same document, same tree, cwd +// `/`: `shard 1/1: 1/1 packages, weight 0`. That failure mode is not a red +// step, it is a shard matrix that quietly stops balancing. +// +// `path.resolve` is also the reason this stays correct for both conventions: +// given an already-absolute entry it returns that entry unchanged, so an old +// document written by the previous absolute-path union step still resolves to +// the same directory it always did. +export function packageDir(itemPath) { + return path.resolve(REPO_ROOT, itemPath); +} + const TEST_FILE = /\.test\.[cm]?[jt]sx?$/; const SKIP_DIRS = new Set(['node_modules', 'dist', 'coverage', '.turbo', '.next']); @@ -173,6 +234,34 @@ function selfTest() { if (!threw(() => readPackageItems(doc({ count: 0, items: {} }), 'f'))) throw new Error('payload: a non-array items was accepted'); if (!threw(() => readPackageItems({}, 'f'))) throw new Error('payload: a document with no packages key was accepted'); + // Path resolution. `it.path` reaches this script in two conventions and the + // weight it produces must not depend on where the process happens to stand. + // The cwd leg is the one that matters: it is the exact measurement that made + // this a defect rather than a style question, and it fails SILENTLY (weight 0, + // package still assigned) rather than loudly, so nothing but an assertion can + // hold it. + if (packageDir('packages/spec') !== path.join(REPO_ROOT, 'packages', 'spec')) { + throw new Error('path: a repo-relative entry did not resolve against the repo root'); + } + const absolute = path.join(REPO_ROOT, 'packages', 'spec'); + if (packageDir(absolute) !== absolute) { + throw new Error('path: an already-absolute entry was not left alone'); + } + const hereWeight = countTestFiles(packageDir('packages/spec')); + if (hereWeight === 0) throw new Error('path: fixture package `packages/spec` has no test files to weigh'); + const cwdBefore = process.cwd(); + try { + process.chdir(path.parse(REPO_ROOT).root); + if (packageDir('packages/spec') !== absolute) { + throw new Error('path: resolution moved with the cwd'); + } + if (countTestFiles(packageDir('packages/spec')) !== hereWeight) { + throw new Error('path: weight changed with the cwd -- the silent weight-0 regression is back'); + } + } finally { + process.chdir(cwdBefore); + } + console.log('partition-test-shards: self-test OK'); } @@ -210,7 +299,7 @@ function main() { throw new Error(`${listPath}: package entry missing name/path: ${JSON.stringify(it)}`); } if (excluded.has(it.name)) continue; - weighted.push({ name: it.name, weight: countTestFiles(it.path) }); + weighted.push({ name: it.name, weight: countTestFiles(packageDir(it.path)) }); } const bins = partition(weighted, shardCount); const mine = bins[shardIndex - 1];