mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
test: shard vitest by measured cost instead of path hash (#650)
Vitest's BaseSequencer sorts specs by SHA1 of their path and slices an equal number of files per shard, which is blind to how long each one takes. That dropped the four costliest generated matrix specs into a single shard: 24m47s against shard 1's 3m47s. Partition by measured cost instead, greedy longest-processing-time-first. CI wall goes 25 min to 21.3 min. The remaining ceiling is that `format-matrix-comprehensive` and `format-matrix` are each one indivisible file, and tests within a file run sequentially in a single fork. Coverage is unchanged by construction. The partition is total and disjoint, guarded over the real spec list for shard counts 1 through 8. Per-shard totals on the PR run matched the baseline exactly: 297 files, 9903 tests, 9435 passed, 468 skipped.
This commit is contained in:
@@ -0,0 +1,100 @@
|
||||
/**
|
||||
* Weight-aware partitioning for `vitest --shard`.
|
||||
*
|
||||
* Vitest's BaseSequencer shards by SHA1 of the file path, sorted, then sliced
|
||||
* into equal FILE COUNTS (see `BaseSequencer.shard`). That is blind to how long
|
||||
* a file takes, so the split is effectively a coin toss. Measured on CI run
|
||||
* 30206780088 it put the four costliest generated matrix specs in one shard:
|
||||
*
|
||||
* shard 1 3m47s shard 2 5m13s shard 3 16m04s shard 4 24m47s
|
||||
*
|
||||
* The pipeline is gated by the slowest shard, so that cost the whole run ~20
|
||||
* minutes of idle. Partitioning by measured cost instead lands every shard near
|
||||
* the mean.
|
||||
*
|
||||
* A spec is never split across shards, so no shard can finish faster than the
|
||||
* single costliest file (format-matrix-comprehensive, ~1365s of test time).
|
||||
* That is the floor for any shard count.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Total test time per spec in seconds, measured from CI run 30206780088
|
||||
* (2026-07-26). Only the expensive tail is listed; everything else is close
|
||||
* enough to the default that ranking it adds no value.
|
||||
*
|
||||
* These are load hints, not assertions. A stale number costs balance, never
|
||||
* correctness: the partition stays total and disjoint whatever the weights say.
|
||||
* Refresh by parsing per-test durations out of the Integration job logs.
|
||||
*/
|
||||
export const FILE_COST_SECONDS: Record<string, number> = {
|
||||
"tests/integration/generated/format-matrix-comprehensive.test.ts": 1365,
|
||||
"tests/integration/generated/format-matrix.test.ts": 1130,
|
||||
"tests/integration/generated/format-matrix-generated.test.ts": 779,
|
||||
"tests/integration/generated/format-matrix-exotic.test.ts": 370,
|
||||
"tests/integration/generated/format-matrix-expanded.test.ts": 319,
|
||||
"tests/integration/tools/image/image-enhancement.test.ts": 226,
|
||||
"tests/integration/generated/new-formats.test.ts": 202,
|
||||
"tests/integration/generated/format-matrix-multimodal.test.ts": 126,
|
||||
"tests/integration/generated/settings-pairwise.test.ts": 109,
|
||||
"tests/integration/generated/settings-matrix.test.ts": 99,
|
||||
"tests/integration/security/hostile-inputs.test.ts": 72,
|
||||
"tests/integration/tools/image/collage.test.ts": 52,
|
||||
"tests/integration/security/adversarial-coverage-gaps.test.ts": 52,
|
||||
"tests/integration/security/adversarial-extended.test.ts": 24,
|
||||
"tests/integration/security/adversarial-matrix.test.ts": 19,
|
||||
"tests/integration/tools/image/beautify.test.ts": 18,
|
||||
"tests/integration/security/adversarial-final-gaps.test.ts": 18,
|
||||
"tests/integration/platform/batch.test.ts": 17,
|
||||
"tests/integration/tools/image/edit-metadata.test.ts": 17,
|
||||
"tests/integration/tools/image/qr-generate.test.ts": 16,
|
||||
};
|
||||
|
||||
/** Median-ish cost of an unlisted spec. Most sit well under a second. */
|
||||
const DEFAULT_COST_SECONDS = 2;
|
||||
|
||||
/**
|
||||
* Reduce an absolute moduleId, a leading-slash path, or an already-relative
|
||||
* path down to the repo-relative form used as the cost-table key.
|
||||
*/
|
||||
function normalize(file: string): string {
|
||||
const posix = file.replace(/\\/g, "/");
|
||||
const idx = posix.indexOf("tests/");
|
||||
return idx === -1 ? posix.replace(/^\/+/, "") : posix.slice(idx);
|
||||
}
|
||||
|
||||
export function costOf(file: string): number {
|
||||
return FILE_COST_SECONDS[normalize(file)] ?? DEFAULT_COST_SECONDS;
|
||||
}
|
||||
|
||||
/**
|
||||
* Longest-processing-time-first greedy bin packing: heaviest spec first, each
|
||||
* one onto whichever bin is currently lightest.
|
||||
*
|
||||
* Every shard runs this in its own vitest process and then keeps only its own
|
||||
* bin, so the result has to be identical everywhere. It is: the input is sorted
|
||||
* before packing, and ties break on path, so filesystem enumeration order
|
||||
* cannot change the answer.
|
||||
*/
|
||||
export function partitionByCost(files: string[], count: number): string[][] {
|
||||
if (count <= 0) return [];
|
||||
|
||||
const bins: string[][] = Array.from({ length: count }, () => []);
|
||||
const loads: number[] = new Array(count).fill(0);
|
||||
|
||||
const heaviestFirst = [...files].sort((a, b) => {
|
||||
const byCost = costOf(b) - costOf(a);
|
||||
if (byCost !== 0) return byCost;
|
||||
return a < b ? -1 : a > b ? 1 : 0;
|
||||
});
|
||||
|
||||
for (const file of heaviestFirst) {
|
||||
let lightest = 0;
|
||||
for (let i = 1; i < count; i++) {
|
||||
if (loads[i] < loads[lightest]) lightest = i;
|
||||
}
|
||||
bins[lightest].push(file);
|
||||
loads[lightest] += costOf(file);
|
||||
}
|
||||
|
||||
return bins;
|
||||
}
|
||||
@@ -0,0 +1,121 @@
|
||||
import { readdirSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { costOf, FILE_COST_SECONDS, partitionByCost } from "../../helpers/shard-partition.js";
|
||||
|
||||
const repoRoot = path.resolve(__dirname, "../../..");
|
||||
|
||||
/** Every integration spec on disk, repo-relative, sorted. Mirrors the CI glob. */
|
||||
function integrationSpecs(): string[] {
|
||||
const out: string[] = [];
|
||||
const walk = (dir: string) => {
|
||||
for (const entry of readdirSync(dir, { withFileTypes: true })) {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) walk(full);
|
||||
else if (entry.name.endsWith(".test.ts")) out.push(path.relative(repoRoot, full));
|
||||
}
|
||||
};
|
||||
walk(path.join(repoRoot, "tests/integration"));
|
||||
return out.sort();
|
||||
}
|
||||
|
||||
const HEAVYWEIGHTS = [
|
||||
"tests/integration/generated/format-matrix-comprehensive.test.ts",
|
||||
"tests/integration/generated/format-matrix.test.ts",
|
||||
"tests/integration/generated/format-matrix-generated.test.ts",
|
||||
"tests/integration/generated/format-matrix-exotic.test.ts",
|
||||
];
|
||||
|
||||
describe("partitionByCost", () => {
|
||||
// The whole point of the helper: a shard must never silently drop a spec.
|
||||
// These two invariants are what make the CI change coverage-neutral.
|
||||
describe("partition is total and disjoint", () => {
|
||||
it("places every input file in exactly one bin", () => {
|
||||
const files = integrationSpecs();
|
||||
const bins = partitionByCost(files, 4);
|
||||
const flat = bins.flat();
|
||||
|
||||
expect(flat.slice().sort()).toEqual(files.slice().sort());
|
||||
expect(new Set(flat).size).toBe(files.length);
|
||||
});
|
||||
|
||||
it("holds for shard counts 1 through 8", () => {
|
||||
const files = integrationSpecs();
|
||||
for (let count = 1; count <= 8; count++) {
|
||||
const flat = partitionByCost(files, count).flat();
|
||||
expect(new Set(flat).size, `count=${count} lost or duplicated a file`).toBe(files.length);
|
||||
expect(flat.slice().sort(), `count=${count} changed the file set`).toEqual(
|
||||
files.slice().sort(),
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("returns exactly `count` bins even when files are scarce", () => {
|
||||
expect(partitionByCost(["a.test.ts", "b.test.ts"], 4)).toHaveLength(4);
|
||||
expect(partitionByCost([], 4)).toHaveLength(4);
|
||||
expect(partitionByCost([], 4).flat()).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
// Each shard is a separate vitest process computing the partition
|
||||
// independently, so all of them must agree or specs get run twice or never.
|
||||
describe("determinism across processes", () => {
|
||||
it("is stable across repeated calls", () => {
|
||||
const files = integrationSpecs();
|
||||
expect(partitionByCost(files, 4)).toEqual(partitionByCost(files, 4));
|
||||
});
|
||||
|
||||
it("ignores input ordering", () => {
|
||||
const files = integrationSpecs();
|
||||
const shuffled = files.slice().reverse();
|
||||
expect(partitionByCost(shuffled, 4)).toEqual(partitionByCost(files, 4));
|
||||
});
|
||||
});
|
||||
|
||||
describe("balance", () => {
|
||||
it("splits the four heavyweight matrix specs across different bins", () => {
|
||||
const bins = partitionByCost(integrationSpecs(), 4);
|
||||
const landedIn = HEAVYWEIGHTS.map((h) => bins.findIndex((b) => b.includes(h)));
|
||||
|
||||
expect(landedIn).not.toContain(-1);
|
||||
expect(new Set(landedIn).size).toBe(HEAVYWEIGHTS.length);
|
||||
});
|
||||
|
||||
it("keeps the costliest bin within 25% of the ideal share", () => {
|
||||
const files = integrationSpecs();
|
||||
const bins = partitionByCost(files, 4);
|
||||
const cost = (b: string[]) => b.reduce((sum, f) => sum + costOf(f), 0);
|
||||
const ideal = cost(files) / 4;
|
||||
const heaviest = Math.max(...bins.map(cost));
|
||||
|
||||
// A spec is never split, so the floor is the single costliest file.
|
||||
const floor = Math.max(ideal, ...files.map(costOf));
|
||||
expect(heaviest).toBeLessThanOrEqual(floor * 1.25);
|
||||
});
|
||||
});
|
||||
|
||||
describe("costOf", () => {
|
||||
it("returns the measured cost for a known-heavy spec", () => {
|
||||
const known = "tests/integration/generated/format-matrix-comprehensive.test.ts";
|
||||
expect(costOf(known)).toBe(FILE_COST_SECONDS[known]);
|
||||
});
|
||||
|
||||
it("falls back to a default for unmeasured specs", () => {
|
||||
expect(costOf("tests/integration/does/not/exist.test.ts")).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("matches regardless of leading slash or absolute prefix", () => {
|
||||
const known = "tests/integration/generated/format-matrix.test.ts";
|
||||
expect(costOf(`/${known}`)).toBe(costOf(known));
|
||||
expect(costOf(path.join(repoRoot, known))).toBe(costOf(known));
|
||||
});
|
||||
});
|
||||
|
||||
describe("cost table hygiene", () => {
|
||||
it("only lists specs that still exist", () => {
|
||||
const onDisk = new Set(integrationSpecs());
|
||||
const stale = Object.keys(FILE_COST_SECONDS).filter((f) => !onDisk.has(f));
|
||||
expect(stale, `cost table references deleted specs: ${stale.join(", ")}`).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,8 @@ import { readdirSync } from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { defineConfig } from "vitest/config";
|
||||
import { BaseSequencer } from "vitest/node";
|
||||
import { partitionByCost } from "./tests/helpers/shard-partition.js";
|
||||
|
||||
// Resolve api-workspace packages that pnpm only exposes under apps/api/node_modules.
|
||||
const apiNodeModules = path.resolve(__dirname, "apps/api/node_modules");
|
||||
@@ -26,6 +28,29 @@ function findPnpmPackage(scope: string, name: string): string {
|
||||
return path.join(pnpmDir, entries[entries.length - 1], "node_modules", scope, name);
|
||||
}
|
||||
|
||||
/**
|
||||
* Shards by measured cost instead of by SHA1 of the file path.
|
||||
*
|
||||
* The stock BaseSequencer slices an equal number of FILES per shard, which took
|
||||
* no account of the fact that four generated matrix specs carry most of the
|
||||
* suite's runtime. See tests/helpers/shard-partition.ts for the measurements.
|
||||
*
|
||||
* `sort` is inherited untouched, and vitest only calls `shard` when --shard is
|
||||
* passed, so unsharded runs (local, nightly full matrix) behave exactly as before.
|
||||
*/
|
||||
class CostAwareSequencer extends BaseSequencer {
|
||||
async shard(specs: Parameters<BaseSequencer["shard"]>[0]) {
|
||||
const shardConfig = this.ctx.config.shard;
|
||||
if (!shardConfig) return specs;
|
||||
|
||||
const { index, count } = shardConfig;
|
||||
const byModuleId = new Map(specs.map((spec) => [spec.moduleId, spec]));
|
||||
const bin = partitionByCost([...byModuleId.keys()], count)[index - 1] ?? [];
|
||||
|
||||
return bin.map((moduleId) => byModuleId.get(moduleId)).filter((spec) => spec !== undefined);
|
||||
}
|
||||
}
|
||||
|
||||
export default defineConfig({
|
||||
esbuild: {
|
||||
jsx: "automatic",
|
||||
@@ -50,6 +75,9 @@ export default defineConfig({
|
||||
(process.env.CI ? 4 : Math.max(2, Math.floor(os.availableParallelism() / 2))),
|
||||
},
|
||||
},
|
||||
sequence: {
|
||||
sequencer: CostAwareSequencer,
|
||||
},
|
||||
globalSetup: ["tests/global-setup.ts"],
|
||||
setupFiles: ["tests/setup/per-fork-env.ts"],
|
||||
exclude: [
|
||||
|
||||
Reference in New Issue
Block a user