diff --git a/test/cli-coverage-sequencer.test.ts b/test/cli-coverage-sequencer.test.ts index c7c8bbcb176..30d73285fc2 100644 --- a/test/cli-coverage-sequencer.test.ts +++ b/test/cli-coverage-sequencer.test.ts @@ -8,17 +8,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 +44,35 @@ function sequencer(index: number, count: number): CliCoverageSequencer { } as unknown as Vitest); } -describe("CLI coverage duration-aware sharding", () => { +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: 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 = + projectName === "cli" + ? `src/lib/fixture-${index}.test.ts` + : `test/fixture-${index}.test.ts`; + return { + key: `${projectName}:${file}`, + weightMs: cliTestTimingHints.defaultDurationMs, + value: file, + }; + }); + }, + ); + + return [...measured, ...ordinary]; +} + +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 +90,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 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(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, };