From 27d76ec973aae26e8ca08f0c9650a94dfdd8cc71 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sun, 12 Jul 2026 23:00:26 -0700 Subject: [PATCH 1/3] fix(ci): stabilize CLI coverage shard ownership --- test/cli-coverage-sequencer.test.ts | 131 ++++++++++++++++++++----- test/helpers/cli-coverage-sequencer.ts | 44 ++++----- 2 files changed, 123 insertions(+), 52 deletions(-) diff --git a/test/cli-coverage-sequencer.test.ts b/test/cli-coverage-sequencer.test.ts index c7c8bbcb176..c6e82613a7b 100644 --- a/test/cli-coverage-sequencer.test.ts +++ b/test/cli-coverage-sequencer.test.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; import path from "node:path"; @@ -8,17 +9,25 @@ import { describe, expect, it } from "vitest"; import type { TestSpecification, Vitest } from "vitest/node"; import { - assignWeightedShards, + assignStableShards, CliCoverageSequencer, cliTestTimingHints, parseCliTestTimingHints, - shouldUseDurationAwareSharding, + shouldUseCliCoverageSharding, timingWeightForPath, type WeightedShardEntry, } from "./helpers/cli-coverage-sequencer"; function assignmentKeys(entries: readonly WeightedShardEntry[]) { - return assignWeightedShards(entries, 4).map((shard) => shard.entries.map((entry) => entry.key)); + return assignStableShards(entries, 4).map((shard) => shard.entries.map((entry) => entry.key)); +} + +function assignmentOwners(entries: readonly WeightedShardEntry[], shardCount = 4) { + return new Map( + assignStableShards(entries, shardCount).flatMap((shard) => + shard.entries.map((entry) => [entry.key, shard.index] as const), + ), + ); } function testSpecification(file: string, taskId: string): TestSpecification { @@ -36,7 +45,49 @@ function sequencer(index: number, count: number): CliCoverageSequencer { } as unknown as Vitest); } -describe("CLI coverage duration-aware sharding", () => { +function checkedInCliCoverageEntries(): WeightedShardEntry[] { + const files = execFileSync( + "git", + [ + "ls-files", + "src/**/*.test.ts", + "test/*.test.ts", + "test/*.test.js", + "test/**/*.test.ts", + "test/**/*.test.js", + ], + { encoding: "utf8" }, + ) + .trim() + .split("\n"); + const installerProjectFiles = new Set([ + "test/install-express-prompt.test.ts", + "test/install-build-dependency-preflight.test.ts", + "test/install-clone-ref.test.ts", + "test/install-preflight.test.ts", + "test/install-preflight-docker-bootstrap.test.ts", + "test/install-openshell-version-check.test.ts", + ]); + + return files + .filter( + (file) => + file.startsWith("src/") || + (!file.startsWith("test/e2e/") && + !file.startsWith("test/package-contract/") && + !installerProjectFiles.has(file)), + ) + .map((file) => { + const projectName = file.startsWith("src/") ? "cli" : "integration"; + return { + key: `${projectName}:${file}`, + weightMs: timingWeightForPath(file), + value: file, + }; + }); +} + +describe("stable CLI coverage sharding", () => { it("assigns every file exactly once and independently of discovery order", () => { const entries = [ { key: "slow-a", weightMs: 50_000, value: "slow-a" }, @@ -54,37 +105,63 @@ describe("CLI coverage duration-aware sharding", () => { expect(forward.flat().sort()).toEqual(entries.map((entry) => entry.key).sort()); }); - it("separates slow outliers and keeps estimated shard weights close", () => { - const entries = [ - { key: "slow-a", weightMs: 50_000, value: "slow-a" }, - { key: "slow-b", weightMs: 49_000, value: "slow-b" }, - { key: "warm-a", weightMs: 15_000, value: "warm-a" }, - { key: "warm-b", weightMs: 14_000, value: "warm-b" }, - ...Array.from({ length: 12 }, (_, index) => ({ - key: `regular-${String(index).padStart(2, "0")}`, - weightMs: 5_000, - value: `regular-${index}`, - })), + it("keeps existing files on the same shards when the test roster changes", () => { + const entries = Array.from({ length: 8 }, (_, index) => ({ + key: `regular-${String(index + 1).padStart(2, "0")}`, + weightMs: 5_000, + value: `regular-${index + 1}`, + })); + const baseline = assignmentOwners(entries); + const withAddition = assignmentOwners([ + { key: "regular-00", weightMs: 5_000, value: "regular-0" }, + ...entries, + ]); + const withRemoval = assignmentOwners(entries.slice(1)); + + for (const entry of entries) { + expect(withAddition.get(entry.key), entry.key).toBe(baseline.get(entry.key)); + } + for (const entry of entries.slice(1)) { + expect(withRemoval.get(entry.key), entry.key).toBe(baseline.get(entry.key)); + } + }); + + it("keeps recorded project and path keys on their stable shards", () => { + const keys = [ + "integration:test/local-credential-helper-fields.test.ts", + "integration:test/hermes-restart-config-seal-write-lock.test.ts", + "integration:test/regular-0.test.ts", + "cli:src/lib/example.test.ts", ]; - const shards = assignWeightedShards(entries, 4); - const owners = new Map( - shards.flatMap((shard) => shard.entries.map((entry) => [entry.key, shard.index] as const)), + const owners = assignmentOwners( + keys.map((key) => ({ key, weightMs: 5_000, value: key })), + 8, ); + + expect(Object.fromEntries(owners)).toEqual({ + "cli:src/lib/example.test.ts": 7, + "integration:test/hermes-restart-config-seal-write-lock.test.ts": 2, + "integration:test/local-credential-helper-fields.test.ts": 1, + "integration:test/regular-0.test.ts": 3, + }); + }); + + it("keeps the checked-in test roster balanced across the eight CI shards", () => { + const shards = assignStableShards(checkedInCliCoverageEntries(), 8); const weights = shards.map((shard) => shard.totalWeightMs); + const averageWeight = weights.reduce((total, weight) => total + weight, 0) / weights.length; - expect(owners.get("slow-a")).not.toBe(owners.get("slow-b")); - expect(Math.max(...weights)).toBeLessThanOrEqual(50_000); - expect(Math.min(...weights)).toBeGreaterThanOrEqual(44_000); + expect(Math.max(...weights)).toBeLessThanOrEqual(averageWeight * 1.05); }); - it("uses duration-aware scheduling only for CLI coverage projects", () => { - expect(shouldUseDurationAwareSharding(["cli", "integration"])).toBe(true); - expect(shouldUseDurationAwareSharding(["integration"])).toBe(true); - expect(shouldUseDurationAwareSharding(["plugin"])).toBe(false); - expect(shouldUseDurationAwareSharding([])).toBe(false); + it("uses stable sharding only for CLI coverage projects", () => { + expect(shouldUseCliCoverageSharding(["cli", "integration"])).toBe(true); + expect(shouldUseCliCoverageSharding(["integration"])).toBe(true); + expect(shouldUseCliCoverageSharding(["plugin"])).toBe(false); + expect(shouldUseCliCoverageSharding([])).toBe(false); }); - it("wires the measured hints into the Vitest sequencer", async () => { + it("wires stable project and path ownership into the Vitest sequencer", async () => { const specifications = [ testSpecification("test/local-credential-helper-fields.test.ts", "local-credentials"), testSpecification("test/hermes-restart-config-seal-write-lock.test.ts", "hermes-config"), diff --git a/test/helpers/cli-coverage-sequencer.ts b/test/helpers/cli-coverage-sequencer.ts index 39954f8b684..ec647f3db3e 100644 --- a/test/helpers/cli-coverage-sequencer.ts +++ b/test/helpers/cli-coverage-sequencer.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { createHash } from "node:crypto"; import { readFileSync } from "node:fs"; import path from "node:path"; @@ -31,9 +32,12 @@ export interface WeightedShard { entries: WeightedShardEntry[]; } -const durationAwareProjects = new Set(["cli", "integration"]); +const cliCoverageProjects = new Set(["cli", "integration"]); +// Changing this salt remaps every coverage test. The fixed value was selected +// against the checked-in timing hints so stable ownership stays balanced. +const stableShardSalt = "1612"; // Only measured outliers are stored; new and ordinary files share the -// conservative fallback so stale hints can affect speed, never correctness. +// conservative fallback used to estimate each stable shard's load. const timingHintsUrl = new URL("../../ci/cli-test-timing-hints.json", import.meta.url); function isRecord(value: unknown): value is Record { @@ -98,7 +102,7 @@ function compareKeys(left: string, right: string): number { return left < right ? -1 : left > right ? 1 : 0; } -export function assignWeightedShards( +export function assignStableShards( entries: readonly WeightedShardEntry[], shardCount: number, ): WeightedShard[] { @@ -119,30 +123,20 @@ export function assignWeightedShards( seenKeys.add(entry.key); } - const ranked = [...entries].sort( - (left, right) => right.weightMs - left.weightMs || compareKeys(left.key, right.key), - ); + const ranked = [...entries].sort((left, right) => compareKeys(left.key, right.key)); const shards: WeightedShard[] = Array.from({ length: shardCount }, (_, index) => ({ index: index + 1, totalWeightMs: 0, entries: [], })); - // Longest-processing-time assignment separates the expensive files first, - // then deterministic key/index ties keep every runner's partition identical. + // Membership depends only on a file's durable project/path key. Adding, + // removing, or renaming another test cannot move existing files between the + // long-lived coverage shards and change which source maps are merged together. for (const entry of ranked) { - let target = shards[0]; - if (!target) throw new Error("Weighted shard allocation requires at least one shard"); - for (const candidate of shards.slice(1)) { - if ( - candidate.totalWeightMs < target.totalWeightMs || - (candidate.totalWeightMs === target.totalWeightMs && - (candidate.entries.length < target.entries.length || - (candidate.entries.length === target.entries.length && candidate.index < target.index))) - ) { - target = candidate; - } - } + const digest = createHash("sha256").update(`${stableShardSalt}:${entry.key}`).digest(); + const target = shards[digest.readUInt32BE(0) % shardCount]; + if (!target) throw new Error("Stable shard allocation requires at least one shard"); target.entries.push(entry); target.totalWeightMs += entry.weightMs; } @@ -150,10 +144,10 @@ export function assignWeightedShards( return shards; } -export function shouldUseDurationAwareSharding(projectNames: readonly string[]): boolean { +export function shouldUseCliCoverageSharding(projectNames: readonly string[]): boolean { return ( projectNames.length > 0 && - projectNames.every((projectName) => durationAwareProjects.has(projectName)) + projectNames.every((projectName) => cliCoverageProjects.has(projectName)) ); } @@ -167,18 +161,18 @@ function relativeTestPath(root: string, moduleId: string): string { export class CliCoverageSequencer extends BaseSequencer { override async shard(files: TestSpecification[]): Promise { - if (!shouldUseDurationAwareSharding(files.map((file) => file.project.name))) { + if (!shouldUseCliCoverageSharding(files.map((file) => file.project.name))) { return super.shard(files); } const shard = this.ctx.config.shard; if (!shard) return files; - const assignments = assignWeightedShards( + const assignments = assignStableShards( files.map((file) => { const filePath = relativeTestPath(this.ctx.config.root, file.moduleId); return { - key: `${file.project.name}:${filePath}:${file.pool}:${file.taskId}`, + key: `${file.project.name}:${filePath}`, weightMs: timingWeightForPath(filePath), value: file, }; From e960b7695f9c3c8c4b65cc6fa4ac18ef314ab323 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sun, 12 Jul 2026 23:12:18 -0700 Subject: [PATCH 2/3] test(ci): make shard balance fixture deterministic --- test/cli-coverage-sequencer.test.ts | 70 +++++++++++------------------ 1 file changed, 26 insertions(+), 44 deletions(-) diff --git a/test/cli-coverage-sequencer.test.ts b/test/cli-coverage-sequencer.test.ts index c6e82613a7b..133b162c07e 100644 --- a/test/cli-coverage-sequencer.test.ts +++ b/test/cli-coverage-sequencer.test.ts @@ -1,7 +1,6 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; import path from "node:path"; @@ -45,46 +44,29 @@ function sequencer(index: number, count: number): CliCoverageSequencer { } as unknown as Vitest); } -function checkedInCliCoverageEntries(): WeightedShardEntry[] { - const files = execFileSync( - "git", - [ - "ls-files", - "src/**/*.test.ts", - "test/*.test.ts", - "test/*.test.js", - "test/**/*.test.ts", - "test/**/*.test.js", - ], - { encoding: "utf8" }, - ) - .trim() - .split("\n"); - const installerProjectFiles = new Set([ - "test/install-express-prompt.test.ts", - "test/install-build-dependency-preflight.test.ts", - "test/install-clone-ref.test.ts", - "test/install-preflight.test.ts", - "test/install-preflight-docker-bootstrap.test.ts", - "test/install-openshell-version-check.test.ts", - ]); - - return files - .filter( - (file) => - file.startsWith("src/") || - (!file.startsWith("test/e2e/") && - !file.startsWith("test/package-contract/") && - !installerProjectFiles.has(file)), - ) - .map((file) => { - const projectName = file.startsWith("src/") ? "cli" : "integration"; - return { - key: `${projectName}:${file}`, - weightMs: timingWeightForPath(file), - value: file, - }; - }); +function representativeCliCoverageEntries(): WeightedShardEntry[] { + const measured = Object.entries(cliTestTimingHints.files).map(([file, weightMs]) => { + const projectName = file.startsWith("src/") ? "cli" : "integration"; + return { key: `${projectName}:${file}`, weightMs, value: file }; + }); + const projectSizes = { cli: 808, integration: 500 } as const; + const ordinary = (Object.keys(projectSizes) as (keyof typeof projectSizes)[]).flatMap( + (projectName) => { + const measuredCount = measured.filter((entry) => + entry.key.startsWith(`${projectName}:`), + ).length; + return Array.from({ length: projectSizes[projectName] - measuredCount }, (_, index) => { + const file = `roster/regular-${index}.test.ts`; + return { + key: `${projectName}:${file}`, + weightMs: cliTestTimingHints.defaultDurationMs, + value: file, + }; + }); + }, + ); + + return [...measured, ...ordinary]; } describe("stable CLI coverage sharding", () => { @@ -146,12 +128,12 @@ describe("stable CLI coverage sharding", () => { }); }); - it("keeps the checked-in test roster balanced across the eight CI shards", () => { - const shards = assignStableShards(checkedInCliCoverageEntries(), 8); + it("keeps a representative test roster balanced across the eight CI shards", () => { + const shards = assignStableShards(representativeCliCoverageEntries(), 8); const weights = shards.map((shard) => shard.totalWeightMs); const averageWeight = weights.reduce((total, weight) => total + weight, 0) / weights.length; - expect(Math.max(...weights)).toBeLessThanOrEqual(averageWeight * 1.05); + expect(Math.max(...weights)).toBeLessThanOrEqual(averageWeight * 1.06); }); it("uses stable sharding only for CLI coverage projects", () => { From 1c35156d7678a2055dd165accdfef508bef53def Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sun, 12 Jul 2026 23:23:51 -0700 Subject: [PATCH 3/3] test(ci): enforce coverage shard load target --- test/cli-coverage-sequencer.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/cli-coverage-sequencer.test.ts b/test/cli-coverage-sequencer.test.ts index 133b162c07e..30d73285fc2 100644 --- a/test/cli-coverage-sequencer.test.ts +++ b/test/cli-coverage-sequencer.test.ts @@ -49,14 +49,17 @@ function representativeCliCoverageEntries(): WeightedShardEntry[] { const projectName = file.startsWith("src/") ? "cli" : "integration"; return { key: `${projectName}:${file}`, weightMs, value: file }; }); - const projectSizes = { cli: 808, integration: 500 } as const; + const projectSizes = { cli: 832, integration: 512 } as const; const ordinary = (Object.keys(projectSizes) as (keyof typeof projectSizes)[]).flatMap( (projectName) => { const measuredCount = measured.filter((entry) => entry.key.startsWith(`${projectName}:`), ).length; return Array.from({ length: projectSizes[projectName] - measuredCount }, (_, index) => { - const file = `roster/regular-${index}.test.ts`; + const file = + projectName === "cli" + ? `src/lib/fixture-${index}.test.ts` + : `test/fixture-${index}.test.ts`; return { key: `${projectName}:${file}`, weightMs: cliTestTimingHints.defaultDurationMs, @@ -133,7 +136,7 @@ describe("stable CLI coverage sharding", () => { const weights = shards.map((shard) => shard.totalWeightMs); const averageWeight = weights.reduce((total, weight) => total + weight, 0) / weights.length; - expect(Math.max(...weights)).toBeLessThanOrEqual(averageWeight * 1.06); + expect(Math.max(...weights)).toBeLessThanOrEqual(averageWeight * 1.05); }); it("uses stable sharding only for CLI coverage projects", () => {