Repository navigation
fix(ci): stabilize nightly-mutation — guard tap.testFiles drift + anti-flake eps #5658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,121 @@ | ||||||||||||||||
| #!/usr/bin/env node | ||||||||||||||||
| // check-mutation-test-coverage — guards against tap.testFiles drift. | ||||||||||||||||
| // | ||||||||||||||||
| // WHY: Stryker (nightly-mutation) only runs the test files listed in | ||||||||||||||||
| // stryker.conf.json `tap.testFiles` against each mutant. When a NEW unit test | ||||||||||||||||
| // that covers a mutated module is added (or an existing one is split/renamed) | ||||||||||||||||
| // but NOT added to tap.testFiles, that test's kills stop counting. The mutants | ||||||||||||||||
| // it would kill then go COVERED-but-unkilled = SURVIVED on a cold run, so the | ||||||||||||||||
| // module's COVERED mutation score collapses and the blocking mutationScore | ||||||||||||||||
| // ratchet (nightly-mutation.yml) false-fails — but only on cold-cache nights, | ||||||||||||||||
| // because the warm incremental run reuses the older (passing) verdicts. The | ||||||||||||||||
| // pass/fail then tracks GitHub cache state, not code quality. Root cause: | ||||||||||||||||
| // tap.testFiles is a hand-maintained list with no drift guard. This is it. | ||||||||||||||||
| // | ||||||||||||||||
| // INVARIANT: every UNIT test (tests/unit/**) that imports a mutated module | ||||||||||||||||
| // (stryker.conf.json `mutate`) MUST be in `tap.testFiles`. Integration/e2e | ||||||||||||||||
| // tests are intentionally excluded (the tap-runner runs node:test units only). | ||||||||||||||||
| // | ||||||||||||||||
| // MODE: advisory by default (prints drift, exit 0). `--strict` exits 1 on drift | ||||||||||||||||
| // so CI can block. Skip-graceful (exit 0) if stryker.conf.json is absent. | ||||||||||||||||
| // | ||||||||||||||||
| // USAGE: | ||||||||||||||||
| // node scripts/check/check-mutation-test-coverage.mjs # advisory | ||||||||||||||||
| // node scripts/check/check-mutation-test-coverage.mjs --strict # blocking | ||||||||||||||||
|
|
||||||||||||||||
| import fs from "node:fs"; | ||||||||||||||||
| import { execFileSync } from "node:child_process"; | ||||||||||||||||
|
|
||||||||||||||||
| /** 3-segment path suffix without extension (unique enough; matches relative + alias imports). */ | ||||||||||||||||
| export function moduleFragment(modulePath) { | ||||||||||||||||
| return modulePath.replace(/\.ts$/, "").split("/").slice(-3).join("/"); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * True if `content` imports the module identified by `fragment`, in any form: | ||||||||||||||||
| * static `... from "…fragment…"`, dynamic `import("…fragment…")` (incl. split | ||||||||||||||||
| * across lines), or `require("…fragment…")`. The fragment must appear INSIDE the | ||||||||||||||||
| * import string literal — a bare mention in a comment does not match. | ||||||||||||||||
| */ | ||||||||||||||||
| export function testImportsModule(content, fragment) { | ||||||||||||||||
| const esc = fragment.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); | ||||||||||||||||
| const re = new RegExp( | ||||||||||||||||
| "(?:from\\s+|\\bimport\\s*\\(\\s*|\\brequire\\s*\\(\\s*)[\"'][^\"']*" + esc | ||||||||||||||||
| ); | ||||||||||||||||
| return re.test(content); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+40
to
+46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current regular expression only matches single ( We can update the regex to support backticks and also allow export function testImportsModule(content, fragmentOrRegExp) {
const re = fragmentOrRegExp instanceof RegExp
? fragmentOrRegExp
: new RegExp(
"(?:from\\s+|\\bimport\\s*\\(\\s*|\\brequire\\s*\\(\\s*)[\"'`][^\"'`]*" +
fragmentOrRegExp.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")
);
return re.test(content);
} |
||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * @param {{mutate: string[], tapTestFiles: string[], unitTests: {path:string, content:string}[]}} input | ||||||||||||||||
| * @returns {Record<string,string[]>} module -> covering unit tests NOT in tap.testFiles (drift) | ||||||||||||||||
| */ | ||||||||||||||||
| export function findCoverageDrift({ mutate, tapTestFiles, unitTests }) { | ||||||||||||||||
| const tap = new Set(tapTestFiles); | ||||||||||||||||
| const drift = {}; | ||||||||||||||||
| for (const mod of mutate) { | ||||||||||||||||
| if (mod.startsWith("_") || !mod.endsWith(".ts")) continue; // skip comment/non-ts entries | ||||||||||||||||
| const frag = moduleFragment(mod); | ||||||||||||||||
| const missing = unitTests | ||||||||||||||||
| .filter((t) => testImportsModule(t.content, frag) && !tap.has(t.path)) | ||||||||||||||||
| .map((t) => t.path) | ||||||||||||||||
| .sort(); | ||||||||||||||||
| if (missing.length > 0) drift[mod] = missing; | ||||||||||||||||
| } | ||||||||||||||||
| return drift; | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+52
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In We can pre-compile the export function findCoverageDrift({ mutate, tapTestFiles, unitTests }) {
const tap = new Set(tapTestFiles);
const drift = {};
for (const mod of mutate) {
if (mod.startsWith("_") || !mod.endsWith(".ts")) continue; // skip comment/non-ts entries
const frag = moduleFragment(mod);
const esc = frag.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const re = new RegExp(
"(?:from\\s+|\\bimport\\s*\\(\\s*|\\brequire\\s*\\(\\s*)[\"'`][^\"'`]*" + esc
);
const missing = unitTests
.filter((t) => testImportsModule(t.content, re) && !tap.has(t.path))
.map((t) => t.path)
.sort();
if (missing.length > 0) drift[mod] = missing;
}
return drift;
} |
||||||||||||||||
|
|
||||||||||||||||
| function listUnitTests() { | ||||||||||||||||
| // Static argv — no shell, no interpolation. | ||||||||||||||||
| const out = execFileSync("git", ["ls-files", "tests/unit"], { encoding: "utf8" }); | ||||||||||||||||
| return out | ||||||||||||||||
| .split("\n") | ||||||||||||||||
| .filter((f) => /\.test\.ts$/.test(f)) | ||||||||||||||||
| // Exclude tests/unit/build/: these test the build TOOLING (scripts/), not the | ||||||||||||||||
| // mutated runtime modules. They legitimately embed module paths as fixture | ||||||||||||||||
| // strings (e.g. this gate's own test), which would otherwise false-match. | ||||||||||||||||
| .filter((f) => !f.startsWith("tests/unit/build/")) | ||||||||||||||||
| .map((path) => ({ path, content: fs.readFileSync(path, "utf8") })); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+67
to
+78
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spawning a Since Node.js >= 22 is guaranteed, we can use the native function listUnitTests() {
return fs.readdirSync("tests/unit", { recursive: true })
.filter((f) => typeof f === "string" && /\.test\.ts$/.test(f))
.map((f) => `tests/unit/${f.replace(/\\/g, "/")}`)
// Exclude tests/unit/build/: these test the build TOOLING (scripts/), not the
// mutated runtime modules. They legitimately embed module paths as fixture
// strings (e.g. this gate's own test), which would otherwise false-match.
.filter((f) => !f.startsWith("tests/unit/build/"))
.map((path) => ({ path, content: fs.readFileSync(path, "utf8") }));
} |
||||||||||||||||
|
|
||||||||||||||||
| function main() { | ||||||||||||||||
| const STRICT = process.argv.includes("--strict"); | ||||||||||||||||
| let conf; | ||||||||||||||||
| try { | ||||||||||||||||
| conf = JSON.parse(fs.readFileSync("stryker.conf.json", "utf8")); | ||||||||||||||||
| } catch { | ||||||||||||||||
| console.warn("[mutation-test-coverage] stryker.conf.json not found — skipping (advisory)."); | ||||||||||||||||
| process.exit(0); | ||||||||||||||||
| } | ||||||||||||||||
| const mutate = (conf.mutate || []).filter((m) => typeof m === "string"); | ||||||||||||||||
| const tapTestFiles = conf.tap?.testFiles || []; | ||||||||||||||||
| const unitTests = listUnitTests(); | ||||||||||||||||
|
|
||||||||||||||||
| const drift = findCoverageDrift({ mutate, tapTestFiles, unitTests }); | ||||||||||||||||
| const modules = Object.keys(drift); | ||||||||||||||||
| const total = modules.reduce((n, m) => n + drift[m].length, 0); | ||||||||||||||||
|
|
||||||||||||||||
| console.log("Mutation test-coverage gate — tap.testFiles drift detection"); | ||||||||||||||||
| console.log("==========================================================="); | ||||||||||||||||
| console.log( | ||||||||||||||||
| `Scanned ${unitTests.length} unit test file(s) against ${mutate.filter((m) => m.endsWith(".ts")).length} mutated module(s).` | ||||||||||||||||
| ); | ||||||||||||||||
|
|
||||||||||||||||
| if (total === 0) { | ||||||||||||||||
| console.log("✓ No drift — every covering unit test is listed in tap.testFiles."); | ||||||||||||||||
| process.exit(0); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| for (const m of modules) { | ||||||||||||||||
| console.log(`\n ${m}`); | ||||||||||||||||
| for (const t of drift[m]) console.log(` + ${t}`); | ||||||||||||||||
| } | ||||||||||||||||
| const msg = `${total} covering unit test(s) across ${modules.length} module(s) are missing from stryker.conf.json tap.testFiles.`; | ||||||||||||||||
| if (STRICT) { | ||||||||||||||||
| console.error(`\n✗ ${msg} Add them so their mutant kills count (--strict).`); | ||||||||||||||||
| process.exit(1); | ||||||||||||||||
| } | ||||||||||||||||
| console.warn(`\n⚠ ${msg} Re-run with --strict to fail. Add them to tap.testFiles.`); | ||||||||||||||||
| process.exit(0); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| if (import.meta.url === `file://${process.argv[1]}`) main(); | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The check To ensure cross-platform compatibility, we should import
Suggested change
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import { test } from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
| import { | ||
| moduleFragment, | ||
| testImportsModule, | ||
| findCoverageDrift, | ||
| } from "../../../scripts/check/check-mutation-test-coverage.mjs"; | ||
|
|
||
| test("moduleFragment returns the 3-segment suffix without extension", () => { | ||
| assert.equal( | ||
| moduleFragment("open-sse/handlers/chatCore/headers.ts"), | ||
| "handlers/chatCore/headers" | ||
| ); | ||
| assert.equal(moduleFragment("src/sse/services/auth.ts"), "sse/services/auth"); | ||
| // shallow paths just use what is available | ||
| assert.equal(moduleFragment("a/b.ts"), "a/b"); | ||
| }); | ||
|
|
||
| test("testImportsModule matches static, dynamic and require imports of the module path", () => { | ||
| const frag = "handlers/chatCore/headers"; | ||
| // static import-from | ||
| assert.equal( | ||
| testImportsModule(`import { x } from "@omniroute/open-sse/handlers/chatCore/headers";`, frag), | ||
| true | ||
| ); | ||
| // dynamic await import, even split across lines | ||
| assert.equal( | ||
| testImportsModule(`const { y } = await import(\n "../../open-sse/handlers/chatCore/headers.ts"\n);`, frag), | ||
| true | ||
| ); | ||
| // require() | ||
| assert.equal( | ||
| testImportsModule(`const z = require("../../open-sse/handlers/chatCore/headers.ts");`, frag), | ||
| true | ||
| ); | ||
| // unrelated module is not matched | ||
| assert.equal( | ||
| testImportsModule(`import { a } from "@omniroute/open-sse/handlers/chatCore/idempotency";`, frag), | ||
| false | ||
| ); | ||
| // the fragment appearing only in a comment (not an import string) is NOT a match | ||
| assert.equal( | ||
| testImportsModule(`// see handlers/chatCore/headers for details\nconst a = 1;`, frag), | ||
| false | ||
| ); | ||
| }); | ||
|
|
||
| test("findCoverageDrift flags covering unit tests absent from tap.testFiles", () => { | ||
| const mutate = [ | ||
| "open-sse/handlers/chatCore/headers.ts", | ||
| "open-sse/handlers/chatCore/idempotency.ts", | ||
| "_a_comment_entry", | ||
| ]; | ||
| const tapTestFiles = ["tests/unit/chatcore-headers.test.ts"]; | ||
| const unitTests = [ | ||
| // covers headers, already in tap -> not drift | ||
| { path: "tests/unit/chatcore-headers.test.ts", content: `await import("../../open-sse/handlers/chatCore/headers.ts");` }, | ||
| // covers headers, NOT in tap -> drift | ||
| { path: "tests/unit/no-memory-header.test.ts", content: `const { isNoMemoryRequested } = await import("../../open-sse/handlers/chatCore/headers.ts");` }, | ||
| // covers idempotency, NOT in tap -> drift | ||
| { path: "tests/unit/idempo.test.ts", content: `import { x } from "@omniroute/open-sse/handlers/chatCore/idempotency";` }, | ||
| // covers nothing mutated -> ignored | ||
| { path: "tests/unit/unrelated.test.ts", content: `import { z } from "@/lib/foo";` }, | ||
| ]; | ||
| const drift = findCoverageDrift({ mutate, tapTestFiles, unitTests }); | ||
| assert.deepEqual(drift["open-sse/handlers/chatCore/headers.ts"], [ | ||
| "tests/unit/no-memory-header.test.ts", | ||
| ]); | ||
| assert.deepEqual(drift["open-sse/handlers/chatCore/idempotency.ts"], [ | ||
| "tests/unit/idempo.test.ts", | ||
| ]); | ||
| // comment-only mutate entries are skipped | ||
| assert.equal("_a_comment_entry" in drift, false); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import
fileURLToPathfromnode:urlto support cross-platform path comparison at the entry point check.