feat(quality): detect forgotten sibling tests in PRs (#9530) (#10009)

Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-08-10 18:16:04 -03:00
committed by GitHub
parent 696ad182cd
commit 995618d27a
9 changed files with 563 additions and 29 deletions

View File

@@ -0,0 +1,293 @@
#!/usr/bin/env node
import { execFileSync } from "node:child_process";
import fs from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";
import { globSync } from "tinyglobby";
import { resolveImport } from "../quality/build-test-impact-map.mjs";
const DEFAULT_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
const SOURCE_ROOTS = ["src/", "open-sse/", "bin/"];
const SOURCE_GLOBS = [
"src/**/*.{ts,tsx,mts,js,mjs}",
"open-sse/**/*.{ts,tsx,mts,js,mjs}",
"bin/**/*.{ts,tsx,mts,js,mjs}",
];
const IGNORE = [
"**/__tests__/**",
"**/*.test.*",
"**/*.spec.*",
"**/fixtures/**",
"**/generated/**",
];
const STATIC_IMPORT_RE =
/(?:import|export)[^'"()]*from\s*['"]([^'"]+)['"]|require\(\s*['"]([^'"]+)['"]\s*\)/g;
const DYNAMIC_IMPORT_RE = /import\(\s*['"]([^'"]+)['"]\s*\)/g;
const TEST_MASK_RE =
/^\+.*(?:\b(?:it|test|describe)\.(?:skip|todo)\b|\b(?:xit|xtest|xdescribe)\s*\()/;
const REFERENCE_RE = /^(?:#\d+|https:\/\/github\.com\/[^/]+\/[^/]+\/(?:issues|pull)\/\d+)$/;
function normalize(file) {
return file.split(path.sep).join("/");
}
function isProduction(file) {
return (
SOURCE_ROOTS.some((root) => file.startsWith(root)) &&
!IGNORE.some((pattern) => {
const token = pattern.replaceAll("**/", "").replaceAll("/**", "").replaceAll("*", "");
return token && file.includes(token);
})
);
}
function isBarrel(file, code) {
return /(?:^|\/)index\.[cm]?[jt]sx?$/.test(file) && /\bexport\s+(?:\*|\{)/.test(code);
}
function importEdges(root) {
const edges = [];
const files = globSync(SOURCE_GLOBS, { cwd: root, absolute: true, ignore: IGNORE });
for (const absolute of files) {
const consumer = normalize(path.relative(root, absolute));
const code = fs.readFileSync(absolute, "utf8");
for (const match of code.matchAll(STATIC_IMPORT_RE)) {
const resolved = resolveImport(match[1] || match[2], absolute, root);
if (resolved) {
edges.push({
module: normalize(path.relative(root, resolved)),
consumer,
kind: isBarrel(consumer, code) ? "barrel" : "static",
});
}
}
for (const match of code.matchAll(DYNAMIC_IMPORT_RE)) {
const resolved = resolveImport(match[1], absolute, root);
if (resolved) {
edges.push({
module: normalize(path.relative(root, resolved)),
consumer,
kind: "dynamic-import",
});
}
}
}
return edges.sort((a, b) =>
`${a.module}\0${a.consumer}\0${a.kind}`.localeCompare(`${b.module}\0${b.consumer}\0${b.kind}`)
);
}
export function validateAllowlist(value) {
const entries = Array.isArray(value) ? value : value?.entries;
if (!Array.isArray(entries))
throw new Error("forgotten-sibling allowlist must contain an entries array");
return entries.map((entry, index) => {
for (const field of ["consumer", "candidateTest", "rationale", "reference"]) {
if (typeof entry?.[field] !== "string" || !entry[field].trim()) {
throw new Error(`forgotten-sibling allowlist entry ${index} requires ${field}`);
}
}
if (entry.rationale.trim().length < 20) {
throw new Error(`forgotten-sibling allowlist entry ${index} rationale must be specific`);
}
if (!REFERENCE_RE.test(entry.reference.trim())) {
throw new Error(
`forgotten-sibling allowlist entry ${index} reference must be a GitHub issue or PR`
);
}
return {
consumer: normalize(entry.consumer.trim()),
candidateTest: normalize(entry.candidateTest.trim()),
rationale: entry.rationale.trim(),
reference: entry.reference.trim(),
};
});
}
export function analyzeForgottenSiblingTests({
root = DEFAULT_ROOT,
changedEntries,
impactMap,
allowlist,
changedSymbolsByFile = {},
addedTestLines = [],
}) {
const changed = new Map(changedEntries.map((entry) => [normalize(entry.file), entry.status]));
const changedModules = [...changed.keys()].filter(isProduction).sort();
const maskingAdded = addedTestLines.some((line) => TEST_MASK_RE.test(line));
const allow = new Map(
allowlist.map((entry) => [`${entry.consumer}\0${entry.candidateTest}`, entry])
);
const findings = [];
const diagnostics = [];
const suppressed = [];
const maskingRisks = [];
for (const edge of importEdges(root)) {
if (!changedModules.includes(edge.module)) continue;
const tests = [...new Set(impactMap.sources?.[edge.consumer] || [])].sort();
if (edge.kind !== "static") {
diagnostics.push({
changedModule: edge.module,
consumer: edge.consumer,
kind: edge.kind,
message: `${edge.kind} resolution is advisory and never blocks`,
});
continue;
}
for (const candidateTest of tests) {
const status = changed.get(candidateTest);
const masking = status === "D" || (status && maskingAdded);
if (masking) {
maskingRisks.push({
changedModule: edge.module,
consumer: edge.consumer,
candidateTest,
reason:
status === "D"
? "candidate sibling test was deleted"
: "candidate sibling test adds skip/todo masking",
});
continue;
}
if (status) continue;
const finding = {
changedModule: edge.module,
changedSymbols: [...(changedSymbolsByFile[edge.module] || [])].sort(),
consumer: edge.consumer,
candidateTest,
reason: "candidate sibling test is absent from the PR diff",
};
const exception = allow.get(`${edge.consumer}\0${candidateTest}`);
if (exception) suppressed.push({ ...finding, exception });
else findings.push(finding);
}
}
return { mode: "advisory", findings, diagnostics, suppressed, maskingRisks };
}
function arg(name, fallback = "") {
const index = process.argv.indexOf(name);
return index >= 0 && process.argv[index + 1] ? process.argv[index + 1] : fallback;
}
function git(root, args) {
return execFileSync("git", args, { cwd: root, encoding: "utf8" });
}
function changedEntries(root, base) {
return git(root, ["diff", "--name-status", "--diff-filter=ACMRD", `${base}...HEAD`])
.trim()
.split(/\r?\n/)
.filter(Boolean)
.map((line) => {
const [status, ...files] = line.split("\t");
return { status: status[0], file: files.at(-1) };
});
}
function changedSymbols(root, base, entries) {
const result = {};
const declaration =
/^\+\s*(?:export\s+)?(?:async\s+)?(?:function|class|const|let|var|interface|type|enum)\s+([A-Za-z_$][\w$]*)/;
for (const entry of entries.filter(({ file }) => isProduction(file))) {
const diff = git(root, ["diff", "--unified=0", `${base}...HEAD`, "--", entry.file]);
result[entry.file] = [
...new Set(
diff
.split(/\r?\n/)
.map((line) => line.match(declaration)?.[1])
.filter(Boolean)
),
];
}
return result;
}
function markdown(result, base) {
const lines = [
"## Forgotten sibling tests (advisory)",
"",
`Base: \`${base}\``,
`Unallowlisted findings: ${result.findings.length}`,
`Reviewed exceptions: ${result.suppressed.length}`,
`Resolution diagnostics: ${result.diagnostics.length}`,
`Masking/deletion risks (owned by blocking sibling gates): ${result.maskingRisks.length}`,
"",
];
if (result.findings.length) {
lines.push("### Candidate tests absent from this diff", "");
for (const item of result.findings) {
const symbol = item.changedSymbols.length ? ` (${item.changedSymbols.join(", ")})` : "";
lines.push(
`- \`${item.changedModule}\`${symbol} -> \`${item.consumer}\` -> \`${item.candidateTest}\``
);
}
lines.push("", "> Report-only calibration: these findings do not fail the job.", "");
}
for (const [heading, items] of [
["Resolution diagnostics", result.diagnostics],
["Test masking/deletion risks", result.maskingRisks],
]) {
if (!items.length) continue;
lines.push(`### ${heading}`, "");
for (const item of items)
lines.push(
`- \`${item.changedModule}\` -> \`${item.consumer}\`${item.candidateTest ? ` -> \`${item.candidateTest}\`` : ""}: ${item.reason || item.message}`
);
lines.push("");
}
return `${lines.join("\n")}\n`;
}
function main() {
const root = DEFAULT_ROOT;
const base = arg(
"--base",
process.env.GITHUB_BASE_SHA ||
(process.env.GITHUB_BASE_REF ? `origin/${process.env.GITHUB_BASE_REF}` : "HEAD~1")
);
const mapPath = arg("--impact-map", path.join(root, "config/quality/test-impact-map.json"));
const allowlistPath = arg(
"--allowlist",
path.join(root, "config/quality/forgotten-sibling-allowlist.json")
);
const summaryPath = arg("--summary-file", "");
const jsonPath = arg("--json-file", "");
const entries = changedEntries(root, base);
const impactMap = JSON.parse(fs.readFileSync(mapPath, "utf8"));
const allowlist = validateAllowlist(JSON.parse(fs.readFileSync(allowlistPath, "utf8")));
const addedTestLines = git(root, ["diff", "--unified=0", `${base}...HEAD`, "--", "tests/"])
.split(/\r?\n/)
.filter((line) => line.startsWith("+") && !line.startsWith("+++"));
const result = analyzeForgottenSiblingTests({
root,
changedEntries: entries,
impactMap,
allowlist,
changedSymbolsByFile: changedSymbols(root, base, entries),
addedTestLines,
});
const report = markdown(result, base);
process.stdout.write(report);
for (const [target, contents] of [
[summaryPath, report],
[jsonPath, `${JSON.stringify(result, null, 2)}\n`],
]) {
if (!target) continue;
fs.mkdirSync(path.dirname(target), { recursive: true });
fs.writeFileSync(target, contents);
}
}
if (fileURLToPath(import.meta.url) === path.resolve(process.argv[1] || "")) {
try {
main();
} catch (error) {
console.error(
`forgotten-sibling-tests: ${error instanceof Error ? error.message : String(error)}`
);
process.exit(1);
}
}

View File

@@ -9,11 +9,11 @@ const IMPORT_RE =
/(?:import|export)[^'"]*from\s*['"]([^'"]+)['"]|require\(\s*['"]([^'"]+)['"]\s*\)|import\(\s*['"]([^'"]+)['"]\s*\)/g;
const EXTS = [".ts", ".tsx", ".mts", ".js", ".mjs"];
function resolveImport(spec, fromFile) {
export function resolveImport(spec, fromFile, root = ROOT) {
let base;
if (spec.startsWith("@/")) base = path.join(ROOT, "src", spec.slice(2));
if (spec.startsWith("@/")) base = path.join(root, "src", spec.slice(2));
else if (spec.startsWith("@omniroute/open-sse"))
base = path.join(ROOT, "open-sse", spec.replace(/^@omniroute\/open-sse\/?/, ""));
base = path.join(root, "open-sse", spec.replace(/^@omniroute\/open-sse\/?/, ""));
else if (spec.startsWith(".")) base = path.resolve(path.dirname(fromFile), spec);
else return null;
for (const e of EXTS) {
@@ -26,7 +26,7 @@ function resolveImport(spec, fromFile) {
return fs.existsSync(base) && fs.statSync(base).isFile() ? base : null;
}
function sourceDepsOf(entry) {
export function sourceDepsOf(entry, root = ROOT) {
const seen = new Set();
const stack = [entry];
const sources = new Set();
@@ -43,9 +43,9 @@ function sourceDepsOf(entry) {
for (const m of code.matchAll(IMPORT_RE)) {
const spec = m[1] || m[2] || m[3];
if (!spec) continue;
const r = resolveImport(spec, f);
const r = resolveImport(spec, f, root);
if (!r) continue;
const rel = path.relative(ROOT, r);
const rel = path.relative(root, r);
if (SRC_ROOTS.some((s) => rel.startsWith(s + path.sep))) sources.add(rel);
stack.push(r);
}
@@ -59,27 +59,35 @@ function sourceDepsOf(entry) {
// e2e/integration tests, which can't run under node:test (they 99-false-failed before).
// Mirror EXACTLY the package.json `test:unit` / `test:unit:ci` globs (incl. memory,
// usage, combo, dashboard, serial, and *.test.mjs). Drift here → false __RUN_ALL__.
const testFiles = globSync(
[
"tests/unit/*.test.ts",
"tests/unit/{api,auth,authz,build,cli,cli-helper,combo,compression,correctness,cors,db,db-adapters,docs,gamification,guardrails,lib,mcp,memory,runtime,security,services,settings,shared,ui,usage}/**/*.test.ts",
"tests/unit/**/*.test.mjs",
"tests/unit/dashboard/**/*.test.ts",
// Quarentena serial (P0.3): também são node:test — a TIA precisa mapeá-los.
"tests/unit/serial/**/*.test.ts",
],
{ cwd: ROOT, absolute: true }
);
const map = {};
for (const tf of testFiles) {
const relTest = path.relative(ROOT, tf);
for (const src of sourceDepsOf(tf)) {
(map[src] ||= []).push(relTest);
export function buildTestImpactMap(root = ROOT) {
const testFiles = globSync(
[
"tests/unit/*.test.ts",
"tests/unit/{api,auth,authz,build,cli,cli-helper,combo,compression,correctness,cors,db,db-adapters,docs,gamification,guardrails,lib,mcp,memory,runtime,security,services,settings,shared,ui,usage}/**/*.test.ts",
"tests/unit/**/*.test.mjs",
"tests/unit/dashboard/**/*.test.ts",
// Quarentena serial (P0.3): também são node:test — a TIA precisa mapeá-los.
"tests/unit/serial/**/*.test.ts",
],
{ cwd: root, absolute: true }
);
const map = {};
for (const tf of testFiles) {
const relTest = path.relative(root, tf);
for (const src of sourceDepsOf(tf, root)) {
(map[src] ||= []).push(relTest);
}
}
for (const k of Object.keys(map)) map[k].sort();
return { generatedFrom: "import-graph", sources: map, testFileCount: testFiles.length };
}
if (fileURLToPath(import.meta.url) === path.resolve(process.argv[1] || "")) {
const result = buildTestImpactMap();
const { testFileCount, ...map } = result;
const out = path.join(ROOT, "config/quality/test-impact-map.json");
fs.writeFileSync(out, JSON.stringify(map, null, 2) + "\n");
console.log(
`test-impact-map: ${Object.keys(map.sources).length} source files mapped from ${testFileCount} test files`
);
}
for (const k of Object.keys(map)) map[k].sort();
const out = path.join(ROOT, "config/quality/test-impact-map.json");
fs.writeFileSync(out, JSON.stringify({ generatedFrom: "import-graph", sources: map }, null, 2) + "\n");
console.log(
`test-impact-map: ${Object.keys(map).length} source files mapped from ${testFiles.length} test files`
);