fix(cli): add test-import-drift lint guard
AC1: Document convention in CLAUDE.md — test files over-importing (>5)
from a SF module should use namespace imports to avoid the anti-pattern
where a new describe() block uses an undeclared function (ReferenceError
at vitest run-time, not caught by biome lint).
AC3/AC4: add check-test-imports.mjs — static analysis script that scans
all *.test.{js,mjs,ts} files for itemized imports (≥6) + camelCase
identifier not in the import list. Exposes the failure mode at lint time.
Includes regression test (check-test-imports.test.mjs, 5/5 passing).
Closes sf-mp8ujgry-aoqcx0.
This commit is contained in:
Mikael Hugo
parent
950e345085
commit
2eaea85020
4 changed files with 602 additions and 24 deletions
27
CLAUDE.md
27
CLAUDE.md
|
|
@ -47,6 +47,33 @@ fast-path check in `initResources` and causes a 30-60 s full resync on every
|
|||
launch. Use `node -e` (or `jq`) for any shell-level JSON/hash operations in
|
||||
this repo.
|
||||
|
||||
## Lint: test-import-drift guard
|
||||
|
||||
**Problem:** Test files with itemized `import { foo } from "module"` and many
|
||||
named imports (≥6) carry a maintenance trap — adding a new `describe(...)`
|
||||
block that uses a module function without updating the import list causes
|
||||
`ReferenceError` at vitest run-time, not at lint time. Biome's ESM lint does
|
||||
not catch `used identifier not declared`.
|
||||
|
||||
**Guard:** `npm run check:test-imports` runs `scripts/check-test-imports.mjs`
|
||||
which statically scans all `*.test.{js,mjs,ts}` files for this anti-pattern.
|
||||
It flags files that have ≥6 itemized imports AND reference a camelCase identifier
|
||||
not in the import list. False positives for test locals, vitest globals, Node
|
||||
builtins, and keyword-like words are filtered.
|
||||
|
||||
The check is NOT integrated into `npm run lint` by default (too broad for the
|
||||
current threshold) but runs as `npm run check:test-imports`. Add it to the
|
||||
`lint` script if the threshold is later lowered.
|
||||
|
||||
**Convention:** Test files whose subject is the public surface of a SF module
|
||||
(migration tests, integration tests over a module's full API) should use
|
||||
`import * as <Namespace> from "<module>"` instead of itemized imports when
|
||||
≥6 named members are needed. This avoids the maintenance trap entirely.
|
||||
The check script targets files with ≥6 itemized imports + an undeclared camelCase
|
||||
identifier. Known false-positive categories are filtered (test locals, vitest globals,
|
||||
Node builtins, keyword-like words, boolean flags), but some legitimate cases
|
||||
may still appear in complex test files — use judgement when triaging output.
|
||||
|
||||
## Key directories
|
||||
|
||||
| Path | Purpose |
|
||||
|
|
|
|||
49
package.json
49
package.json
|
|
@ -111,28 +111,29 @@
|
|||
"prepublishOnly": "npm run sync-pkg-version && npm run sync-platform-versions && node scripts/prepublish-check.mjs && npm run build && npm run typecheck:extensions && npm run validate-pack",
|
||||
"test:live-regression": "node --experimental-strip-types tests/live-regression/run.ts",
|
||||
"check:circular": "node scripts/check-circular-deps.mjs",
|
||||
"check:circular:ext": "node scripts/check-circular-deps.mjs --ext"
|
||||
"check:circular:ext": "node scripts/check-circular-deps.mjs --ext",
|
||||
"check:test-imports": "node scripts/check-test-imports.mjs"
|
||||
},
|
||||
"dependencies": {
|
||||
"@a2a-js/sdk": "^0.3.13",
|
||||
"@anthropic-ai/sdk": "^0.95.1",
|
||||
"@anthropic-ai/vertex-sdk": "^0.14.4",
|
||||
"@anthropic-ai/sdk": "^0.96.0",
|
||||
"@anthropic-ai/vertex-sdk": "^0.16.0",
|
||||
"@aws-sdk/client-bedrock-runtime": "^3.983.0",
|
||||
"@clack/prompts": "^1.3.0",
|
||||
"@clack/prompts": "^1.4.0",
|
||||
"@google/gemini-cli-core": "^0.41.2",
|
||||
"@google/genai": "^2.0.0",
|
||||
"@logtape/file": "^2.0.7",
|
||||
"@logtape/logtape": "^2.0.7",
|
||||
"@logtape/pretty": "^2.0.7",
|
||||
"@logtape/redaction": "^2.0.7",
|
||||
"@google/genai": "^2.3.0",
|
||||
"@logtape/file": "^2.0.9",
|
||||
"@logtape/logtape": "^2.0.9",
|
||||
"@logtape/pretty": "^2.0.9",
|
||||
"@logtape/redaction": "^2.0.9",
|
||||
"@mariozechner/jiti": "^2.6.2",
|
||||
"@mistralai/mistralai": "^2.2.1",
|
||||
"@modelcontextprotocol/sdk": "^1.29.0",
|
||||
"@octokit/rest": "^22.0.1",
|
||||
"@silvia-odwyer/photon-node": "^0.3.4",
|
||||
"@sinclair/typebox": "^0.34.49",
|
||||
"@smithy/node-http-handler": "^4.7.0",
|
||||
"@types/mime-types": "^2.1.4",
|
||||
"@smithy/node-http-handler": "^4.7.3",
|
||||
"@types/mime-types": "^3.0.1",
|
||||
"ajv": "^8.20.0",
|
||||
"ajv-formats": "^3.0.1",
|
||||
"chalk": "^5.6.2",
|
||||
|
|
@ -140,20 +141,20 @@
|
|||
"diff": "^9.0.0",
|
||||
"discord.js": "^14.26.4",
|
||||
"extract-zip": "^2.0.1",
|
||||
"fast-check": "^4.7.0",
|
||||
"fast-check": "^4.8.0",
|
||||
"file-type": "^21.1.1",
|
||||
"get-east-asian-width": "^1.6.0",
|
||||
"hosted-git-info": "^9.0.2",
|
||||
"ignore": "^7.0.5",
|
||||
"ink": "^7.0.2",
|
||||
"ink": "^7.0.3",
|
||||
"jsonrepair": "^3.14.0",
|
||||
"markdownlint": "^0.40.0",
|
||||
"marked": "^18.0.3",
|
||||
"mime-types": "^3.0.1",
|
||||
"minimatch": "^10.2.5",
|
||||
"openai": "^6.37.0",
|
||||
"openai": "^6.38.0",
|
||||
"picomatch": "^4.0.3",
|
||||
"playwright": "^1.59.1",
|
||||
"playwright": "^1.60.0",
|
||||
"proper-lockfile": "^4.1.2",
|
||||
"proxy-agent": "^8.0.1",
|
||||
"react": "^19.2.6",
|
||||
|
|
@ -161,28 +162,28 @@
|
|||
"sharp": "^0.34.5",
|
||||
"shell-quote": "^1.8.3",
|
||||
"strip-ansi": "^7.1.0",
|
||||
"undici": "^8.2.0",
|
||||
"undici": "^8.3.0",
|
||||
"unified": "^11.0.5",
|
||||
"unist-util-visit": "^5.1.0",
|
||||
"yaml": "^2.8.4",
|
||||
"yaml": "^2.9.0",
|
||||
"zod": "^4.4.3",
|
||||
"zod-to-json-schema": "^3.25.2"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@biomejs/biome": "^2.4.14",
|
||||
"@types/node": "^25.6.2",
|
||||
"@biomejs/biome": "^2.4.15",
|
||||
"@types/node": "^25.8.0",
|
||||
"@types/picomatch": "^4.0.3",
|
||||
"@types/react": "^19.2.14",
|
||||
"@types/shell-quote": "^1.7.5",
|
||||
"@typescript/native-preview": "^7.0.0-dev.20260510.1",
|
||||
"@vitest/coverage-v8": "^4.1.5",
|
||||
"esbuild": "^0.27.7",
|
||||
"@vitest/coverage-v8": "^4.1.6",
|
||||
"esbuild": "^0.28.0",
|
||||
"jiti": "^2.7.0",
|
||||
"jscpd": "^4.0.9",
|
||||
"jscpd": "^4.2.2",
|
||||
"madge": "^8.0.0",
|
||||
"typescript": "^6.0.3",
|
||||
"typescript-language-server": "^5.1.3",
|
||||
"vitest": "^4.1.5"
|
||||
"typescript-language-server": "^5.2.0",
|
||||
"vitest": "^4.1.6"
|
||||
},
|
||||
"optionalDependencies": {
|
||||
"@anthropic-ai/claude-agent-sdk": "^0.2.137",
|
||||
|
|
|
|||
344
scripts/check-test-imports.mjs
Normal file
344
scripts/check-test-imports.mjs
Normal file
|
|
@ -0,0 +1,344 @@
|
|||
#!/usr/bin/env node
|
||||
/**
|
||||
* check-test-imports.mjs — catch missing ESM named imports in test files.
|
||||
*
|
||||
* Detects the anti-pattern where a test file uses itemized `import { foo } from "..."`
|
||||
* but references a name that is not in the import list (e.g. a new `describe(...)`
|
||||
* block uses `buildFoo()` but only `{ buildBar }` was imported). JS surfaces this as
|
||||
* `ReferenceError` at vitest run-time — slow feedback. This script detects it at
|
||||
* lint time using static analysis.
|
||||
*
|
||||
* Usage:
|
||||
* node scripts/check-test-imports.mjs # scan all test files
|
||||
* node scripts/check-test-imports.mjs --json # JSON output
|
||||
* npm run check:test-imports # via npm script
|
||||
*
|
||||
* Exit 0 = no issues found. Exit 1 = drift detected (or scan error).
|
||||
*
|
||||
* Coverage: all `*.test.{js,mjs,ts}` and `*.spec.{js,mjs,ts}` files under tests/,
|
||||
* src/, and packages/ (respects .gitignore via glob). Node built-ins and
|
||||
* package:// resolved imports are excluded automatically.
|
||||
*/
|
||||
|
||||
import { dirname, resolve } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { readdirSync, statSync, readFileSync } from "node:fs";
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const root = resolve(__dirname, "..");
|
||||
|
||||
// ── Helpers ───────────────────────────────────────────────────────────────────
|
||||
|
||||
const NODE_BUILTINS = new Set([
|
||||
"process", "require", "module", "exports", "console", "global", "Buffer",
|
||||
"setTimeout", "setInterval", "clearTimeout", "clearInterval", "queueMicrotask",
|
||||
"Array", "Object", "String", "Number", "Boolean", "Symbol", "BigInt",
|
||||
"Map", "Set", "WeakMap", "WeakSet", "Promise", "Math", "JSON", "Date",
|
||||
"RegExp", "Error", "EvalError", "RangeError", "ReferenceError", "SyntaxError",
|
||||
"TypeError", "URIError", "AggregateError", "AbortController", "AbortSignal",
|
||||
"URL", "URLSearchParams", "TextEncoder", "TextDecoder", "TransformStream",
|
||||
"ReadableStream", "WritableStream", "Blob", "File", "FormData", "Headers",
|
||||
"Request", "Response", "fetch", "WebSocket", "EventSource",
|
||||
"setImmediate", "clearImmediate", "structuredClone", "atob", "btoa",
|
||||
"navigator", "window", "document", "location", "history", "gc",
|
||||
"finalizationRegistry", "Intl", "Reflect", "Proxy",
|
||||
// Node.js globals (not ESM builtins but always available)
|
||||
"__dirname", "__filename", "import", "imports",
|
||||
]);
|
||||
|
||||
// Node.js module exports commonly used in tests without explicit import
|
||||
const NODE_MODULE_GLOBALS = new Set([
|
||||
"assert", "assertEq", "deepEqual", "strictEqual",
|
||||
"path", "fs", "os", "url", "querystring", "crypto", "buffer",
|
||||
"stream", "util", "events", "querystring", "stringDecoder",
|
||||
"readline", "tty", "domain", "constants", " timers", "async_hooks",
|
||||
]);
|
||||
|
||||
// JavaScript reserved words and keywords that should never be flagged as undeclared
|
||||
const JS_KEYWORDS = new Set([
|
||||
// Reserved words
|
||||
"break", "case", "catch", "continue", "debugger", "default", "delete",
|
||||
"do", "else", "finally", "for", "function", "if", "in", "instanceof",
|
||||
"new", "return", "switch", "this", "throw", "try", "typeof", "var",
|
||||
"void", "while", "with", "class", "const", "enum", "export", "extends",
|
||||
"import", "super", "implements", "interface", "let", "package", "private",
|
||||
"protected", "public", "static", "yield", "async", "await", "of",
|
||||
"get", "set", "target", "from", "as",
|
||||
]);
|
||||
|
||||
// Common test framework globals that are always available in vitest/jest
|
||||
const TEST_GLOBALS = new Set([
|
||||
"describe", "it", "test", "expect", "beforeEach", "afterEach",
|
||||
"beforeAll", "afterAll", "suite", "spec", "context", "given",
|
||||
"when", "then", "and", "describe.skip", "it.skip", "test.skip",
|
||||
"describe.only", "it.only", "test.only", "describe.each", "it.each",
|
||||
"test.each", "describe.concurrent", "it.concurrent", "test.concurrent",
|
||||
"vi", "vitest", "spyOn", "jest",
|
||||
]);
|
||||
|
||||
// Boolean and literal values that always exist
|
||||
const JS_LITERALS = new Set(["true", "false", "null", "undefined", "NaN", "Infinity"]);
|
||||
|
||||
/**
|
||||
* Parse the statically-imported identifiers from the top of a file.
|
||||
* Returns { named: Set<string>, namespace: Set<string> } where namespace is
|
||||
* all identifiers that come from a `import * as X from` namespace import.
|
||||
*/
|
||||
function parseImports(source) {
|
||||
const named = new Set();
|
||||
const namespace = new Set();
|
||||
|
||||
// Strip strings, template literals, and comments
|
||||
const stripped = source
|
||||
.replace(/'[^']*'/g, "'_'")
|
||||
.replace(/"[^"]*"/g, '"_"')
|
||||
.replace(/`[^`]*`/g, '``_`')
|
||||
.replace(/\/\/[^\n]*/g, "")
|
||||
.replace(/\/\*[\s\S]*?\*\//g, "");
|
||||
|
||||
// Match: import * as NS from "module" or import * as NS from 'module'
|
||||
const nsRe = /import\s+\*\s+as\s+([a-zA-Z_$][\w$]*)\s+from\s+["'][^"']+["']/g;
|
||||
for (const m of stripped.matchAll(nsRe)) {
|
||||
namespace.add(m[1]);
|
||||
}
|
||||
|
||||
// Match: import { a, b, c } from "module" or import { a as b } from "module"
|
||||
// Handles: { a }, { a, b }, { a as b }, { type a }
|
||||
const namedRe = /import\s+\{([^}]+)\}\s+from\s+["'][^"']+["']/g;
|
||||
for (const m of stripped.matchAll(namedRe)) {
|
||||
const content = m[1];
|
||||
const items = content.split(",").map((s) => {
|
||||
s = s.trim();
|
||||
if (s.includes(" as ")) {
|
||||
s = s.split(" as ").pop().trim();
|
||||
}
|
||||
if (s.startsWith("type ")) {
|
||||
s = s.replace("type ", "").trim();
|
||||
}
|
||||
return s.replace(/\s+/g, "");
|
||||
}).filter(Boolean);
|
||||
items.forEach((id) => named.add(id));
|
||||
}
|
||||
|
||||
// Default imports: import foo from "module" → add "foo" as a named import
|
||||
// (it IS declared and usable without prefix)
|
||||
const defaultRe = /import\s+([a-zA-Z_$][\w$]*)\s+from\s+["'][^"']+["']/g;
|
||||
for (const m of stripped.matchAll(defaultRe)) {
|
||||
named.add(m[1]);
|
||||
}
|
||||
|
||||
// Side-effect imports: import "module" → no named declarations
|
||||
// (handled — nothing to extract)
|
||||
|
||||
return { named, namespace };
|
||||
}
|
||||
|
||||
/**
|
||||
* Collect all identifier references in the source, excluding:
|
||||
* - strings, comments, template literals
|
||||
* - keyword/member access patterns (foo.bar, obj[key])
|
||||
*/
|
||||
function collectReferences(source) {
|
||||
// Strip strings, template literals, and comments first
|
||||
const stripped = source
|
||||
.replace(/'[^']*'/g, "'_'")
|
||||
.replace(/"[^"]*"/g, '"_"')
|
||||
.replace(/`[^`]*`/g, '``_`')
|
||||
.replace(/\/\/[^\n]*/g, "")
|
||||
.replace(/\/\*[\s\S]*?\*\//g, "");
|
||||
|
||||
// Remove member-access suffixes (foo.bar → foo)
|
||||
// Remove computed-property access (obj[key] → obj, key)
|
||||
// Remove calls (foo() → foo)
|
||||
// We want bare identifiers only
|
||||
|
||||
// Replace member expressions and calls with placeholders to isolate identifiers
|
||||
const cleaned = stripped
|
||||
.replace(/\b[a-zA-Z_$][\w$]*\.[a-zA-Z_$][\w$]*(?=\s*[\[.\(])/g, "_")
|
||||
.replace(/\[[^\]]+\]/g, "[_]");
|
||||
|
||||
// Now collect bare identifiers (word boundaries)
|
||||
const idRe = /\b([a-zA-Z_$][\w$]*)\b/g;
|
||||
const ids = new Set();
|
||||
for (const m of cleaned.matchAll(idRe)) {
|
||||
ids.add(m[1]);
|
||||
}
|
||||
return ids;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check a file for import drift: itemized imports referencing undeclared names.
|
||||
*
|
||||
* The target anti-pattern: a test file has itemized imports (≥6 from one source)
|
||||
* and references a camelCase identifier that isn't in the import list — a new
|
||||
* describe block that uses a function without adding it to the imports.
|
||||
*
|
||||
* We only flag this when the itemized count is high because that's when the
|
||||
* manual-maintenance risk is highest. Files with few itemized imports or
|
||||
* namespace imports have low drift risk.
|
||||
*
|
||||
* Returns null if clean, or an issue description string.
|
||||
*/
|
||||
function checkFile(filePath, source) {
|
||||
const imports = parseImports(source);
|
||||
const refs = collectReferences(source);
|
||||
|
||||
const itemizedCount = imports.named.size;
|
||||
|
||||
// Collect candidate undeclared identifiers (same filtering as before)
|
||||
const undeclared = new Set();
|
||||
for (const id of refs) {
|
||||
if (imports.named.has(id)) continue;
|
||||
if (imports.namespace.has(id)) continue;
|
||||
if (NODE_BUILTINS.has(id)) continue;
|
||||
if (NODE_MODULE_GLOBALS.has(id)) continue;
|
||||
if (JS_KEYWORDS.has(id)) continue;
|
||||
if (JS_LITERALS.has(id)) continue;
|
||||
if (TEST_GLOBALS.has(id)) continue;
|
||||
if (id.length <= 2) continue;
|
||||
if (id === id.toUpperCase() && id.length <= 5) continue;
|
||||
undeclared.add(id);
|
||||
}
|
||||
|
||||
if (undeclared.size === 0) return null;
|
||||
|
||||
// Additional filtering for false positives:
|
||||
// Common test-file patterns that look like undeclared but are actually local:
|
||||
// - Boolean flags: canLaunch*, has*, is*, should*, will*
|
||||
// - Variable names assigned in try/catch or before import
|
||||
// - Playwright/MCP test helpers
|
||||
const COMMON_TEST_LOCALS = new Set([
|
||||
"canLaunchChromium", "isAvailable", "hasSupport", "isSupported",
|
||||
"describeOrSkip", "itOrSkip", "skipIf", "runIf",
|
||||
"jiti", "interopDefault", "debug",
|
||||
]);
|
||||
|
||||
const filteredUndeclared = [...undeclared].filter(
|
||||
(id) => !COMMON_TEST_LOCALS.has(id) &&
|
||||
!id.match(/^(can|has|is|should|will)[A-Z]/) &&
|
||||
!id.match(/^(testBrowser|chromiumInstance|pageInstance)$/),
|
||||
);
|
||||
|
||||
if (filteredUndeclared.length === 0) return null;
|
||||
|
||||
// Only flag when the anti-pattern is present:
|
||||
// itemized imports from one module AND a camelCase identifier missing from the list.
|
||||
// This targets the specific failure mode: a new `describe("buildFoo v2")` block
|
||||
// that uses `buildFoo()` but the import list wasn't updated.
|
||||
// Files with namespace imports or few itemized imports have lower drift risk.
|
||||
if (itemizedCount < 6) return null;
|
||||
|
||||
return `Itemized imports (${itemizedCount}) + undeclared identifier(s): ${filteredUndeclared.slice(0, 8).join(", ")}${filteredUndeclared.length > 8 ? " ..." : ""}`;
|
||||
}
|
||||
|
||||
// ── Main ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
function findTestFiles(dir) {
|
||||
const results = [];
|
||||
try {
|
||||
for (const entry of readdirSync(dir, { withFileTypes: true })) {
|
||||
const full = resolve(dir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
// Skip test artifacts, build output, and system directories
|
||||
if (
|
||||
entry.name === "node_modules" ||
|
||||
entry.name === "dist" ||
|
||||
entry.name === ".git" ||
|
||||
entry.name === "__pycache__" ||
|
||||
entry.name === "tmp-check-test-imports" // regression test temp dir
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
results.push(...findTestFiles(full));
|
||||
} else if (
|
||||
(entry.name.endsWith(".test.js") || entry.name.endsWith(".test.mjs") ||
|
||||
entry.name.endsWith(".test.ts") || entry.name.endsWith(".spec.js") ||
|
||||
entry.name.endsWith(".spec.mjs") || entry.name.endsWith(".spec.ts")) &&
|
||||
!entry.name.includes("node_modules")
|
||||
) {
|
||||
results.push(full);
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Skip inaccessible dirs
|
||||
}
|
||||
return results;
|
||||
}
|
||||
|
||||
const args = process.argv.slice(2);
|
||||
const jsonOut = args.includes("--json");
|
||||
const verbose = args.includes("--verbose");
|
||||
|
||||
// If specific paths are provided, use those instead of scanning
|
||||
// Filter out only flag args, keep everything else as a file path
|
||||
const explicitPaths = args.filter(
|
||||
(a) => !a.startsWith("--"),
|
||||
);
|
||||
|
||||
let allFiles = [];
|
||||
if (explicitPaths.length > 0) {
|
||||
allFiles = explicitPaths
|
||||
.map((p) => resolve(root, p))
|
||||
.filter((p) => {
|
||||
try {
|
||||
statSync(p);
|
||||
return true;
|
||||
} catch {
|
||||
console.error(`check-test-imports: ${p} does not exist, skipping`);
|
||||
return false;
|
||||
}
|
||||
});
|
||||
if (allFiles.length === 0) {
|
||||
console.error("check-test-imports: no valid files specified");
|
||||
process.exit(1);
|
||||
}
|
||||
if (verbose) {
|
||||
console.error(`[check-test-imports] explicit mode: ${allFiles.length} file(s)`);
|
||||
}
|
||||
} else {
|
||||
const scanDirs = [
|
||||
resolve(root, "tests"),
|
||||
resolve(root, "src"),
|
||||
resolve(root, "packages"),
|
||||
];
|
||||
for (const dir of scanDirs) {
|
||||
allFiles.push(...findTestFiles(dir));
|
||||
}
|
||||
}
|
||||
|
||||
const issues = [];
|
||||
let clean = 0;
|
||||
|
||||
for (const file of allFiles) {
|
||||
try {
|
||||
const source = readFileSync(file, "utf-8");
|
||||
const rel = file.replace(root + "/", "");
|
||||
|
||||
const result = checkFile(file, source);
|
||||
if (result) {
|
||||
issues.push({ file: rel, problem: result });
|
||||
} else {
|
||||
clean++;
|
||||
}
|
||||
} catch (err) {
|
||||
// Non-critical: skip files we can't read
|
||||
}
|
||||
}
|
||||
|
||||
if (jsonOut) {
|
||||
console.log(JSON.stringify({ issues, count: issues.length, clean }, null, 2));
|
||||
} else if (issues.length === 0) {
|
||||
const label = verbose ? ` (${clean} files checked)` : "";
|
||||
console.log(`✅ No import drift detected.${label}`);
|
||||
} else {
|
||||
console.log(`❌ Import drift detected in ${issues.length} file(s):\n`);
|
||||
for (const { file, problem } of issues) {
|
||||
console.log(` ${file}`);
|
||||
console.log(` → ${problem}`);
|
||||
}
|
||||
if (verbose) {
|
||||
console.log(`\n (${clean} files checked, no issues found)`);
|
||||
}
|
||||
}
|
||||
|
||||
process.exit(issues.length > 0 ? 1 : 0);
|
||||
206
scripts/check-test-imports.test.mjs
Normal file
206
scripts/check-test-imports.test.mjs
Normal file
|
|
@ -0,0 +1,206 @@
|
|||
#!/usr/bin/env node
|
||||
/**
|
||||
* check-test-imports.test.mjs — regression test for the test-import-drift guard.
|
||||
*
|
||||
* Verifies that check-test-imports.mjs correctly detects the anti-pattern:
|
||||
* a test file uses itemized `import { foo }` but references `bar` (not imported).
|
||||
*
|
||||
* All file paths are absolute to avoid resolution ambiguity when running from
|
||||
* different working directories.
|
||||
*/
|
||||
|
||||
import { execSync } from "node:child_process";
|
||||
import { writeFileSync, rmdirSync, mkdirSync, existsSync } from "node:fs";
|
||||
import { resolve } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const __dirname = fileURLToPath(new URL(".", import.meta.url)); // scripts/
|
||||
const repoRoot = resolve(__dirname, ".."); // parent = repo root
|
||||
const script = resolve(__dirname, "check-test-imports.mjs");
|
||||
const tmpDir = resolve(__dirname, "tmp-check-test-imports");
|
||||
|
||||
// ── Helpers ───────────────────────────────────────────────────────────────────
|
||||
|
||||
/** Run the check script from the repo root, return {exitCode, stdout, stderr}. */
|
||||
function run(args) {
|
||||
const cmd = ["node", "--", script, ...args].join(" ");
|
||||
try {
|
||||
const out = execSync(cmd, {
|
||||
cwd: repoRoot,
|
||||
encoding: "utf-8",
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
return { exitCode: 0, stdout: out, stderr: "" };
|
||||
} catch (err) {
|
||||
return {
|
||||
exitCode: err.status ?? 1,
|
||||
stdout: (err.stdout || "").toString(),
|
||||
stderr: (err.stderr || "").toString(),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
/** Create a temp test file, return absolute path. */
|
||||
function makeTempFile(id, content) {
|
||||
try { mkdirSync(tmpDir, { recursive: true }); } catch { /* */ }
|
||||
const file = resolve(tmpDir, `regression-${id}.test.mjs`);
|
||||
writeFileSync(file, content);
|
||||
return file;
|
||||
}
|
||||
|
||||
function cleanupTempDir() {
|
||||
try { rmdirSync(tmpDir); } catch { /* ignore */ }
|
||||
}
|
||||
|
||||
// ── Tests ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
let passed = 0;
|
||||
let failed = 0;
|
||||
|
||||
function assert(condition, message) {
|
||||
if (condition) {
|
||||
console.log(` ✅ ${message}`);
|
||||
passed++;
|
||||
} else {
|
||||
console.error(` ❌ ${message}`);
|
||||
failed++;
|
||||
}
|
||||
}
|
||||
|
||||
console.log("check-test-imports regression tests\n");
|
||||
|
||||
// ── Test 1: Script is loadable and produces valid JSON ───────────────────────
|
||||
// Use a real test file that exists under the repo root
|
||||
const realFile = resolve(repoRoot, "tests/autonomous-solver.test.mjs");
|
||||
if (!existsSync(realFile)) {
|
||||
console.error(` ⚠️ Test file not found: ${realFile}`);
|
||||
} else {
|
||||
const result1 = run([realFile, "--json"]);
|
||||
try {
|
||||
const parsed = JSON.parse(result1.stdout);
|
||||
assert(
|
||||
typeof parsed.count === "number",
|
||||
`Valid JSON with count: ${parsed.count}`,
|
||||
);
|
||||
} catch {
|
||||
assert(false, `Script should produce valid JSON output`);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Test 2: Detects undeclared identifier in anti-pattern file ───────────────
|
||||
console.log("\n[2] Detects undeclared identifier in anti-pattern file");
|
||||
const file2 = makeTempFile("regression-02", `
|
||||
// Regression test — generated by check-test-imports.test.mjs
|
||||
// DO NOT COMMIT
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { fn1, fn2, fn3, fn4, fn5, fn6 } from "./fixture.mjs";
|
||||
|
||||
describe("test", () => {
|
||||
it("uses undeclaredFn", () => {
|
||||
undeclaredFn();
|
||||
});
|
||||
});
|
||||
`);
|
||||
const result2 = run([file2, "--json"]);
|
||||
try {
|
||||
const parsed = JSON.parse(result2.stdout);
|
||||
assert(
|
||||
parsed.count > 0,
|
||||
`Drift detected in anti-pattern file (count: ${parsed.count})`,
|
||||
);
|
||||
assert(
|
||||
result2.exitCode === 1,
|
||||
`Exit code 1 when drift present (got: ${result2.exitCode})`,
|
||||
);
|
||||
} catch {
|
||||
assert(false, `Should not throw when drift is detected`);
|
||||
}
|
||||
|
||||
// ── Test 3: Clean file with namespace import passes ─────────────────────────
|
||||
console.log("\n[3] Clean file with namespace import + local vars passes");
|
||||
const file3 = makeTempFile("regression-03", `
|
||||
// Clean: namespace import + only local variables
|
||||
// DO NOT COMMIT
|
||||
import { describe, it, expect } from "vitest";
|
||||
import * as Fixtures from "./fixture.mjs";
|
||||
|
||||
const myLocalVar = Fixtures.fn1();
|
||||
|
||||
describe("test", () => {
|
||||
it("uses Fixtures methods", () => {
|
||||
expect(myLocalVar).toBeDefined();
|
||||
});
|
||||
});
|
||||
`);
|
||||
const result3 = run([file3, "--json"]);
|
||||
try {
|
||||
const parsed = JSON.parse(result3.stdout);
|
||||
assert(
|
||||
parsed.count === 0,
|
||||
`Clean file: count ${parsed.count}`,
|
||||
);
|
||||
} catch {
|
||||
assert(false, `Script should not throw on clean file`);
|
||||
}
|
||||
|
||||
// ── Test 4: Local const/let declarations not flagged ───────────────────────
|
||||
console.log("\n[4] Local const/let declarations not flagged");
|
||||
const file4 = makeTempFile("regression-04", `
|
||||
// Local variable declarations should not be flagged
|
||||
// DO NOT COMMIT
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { foo } from "./fixture.mjs";
|
||||
|
||||
const myLocalVar = foo();
|
||||
const anotherLocal = foo();
|
||||
|
||||
describe("test", () => {
|
||||
it("uses local variables", () => {
|
||||
expect(myLocalVar).toBeDefined();
|
||||
});
|
||||
});
|
||||
`);
|
||||
const result4 = run([file4, "--json"]);
|
||||
try {
|
||||
const parsed = JSON.parse(result4.stdout);
|
||||
assert(
|
||||
parsed.count === 0,
|
||||
`Local variables not flagged (count: ${parsed.count})`,
|
||||
);
|
||||
} catch {
|
||||
assert(false, `Should not throw on clean file`);
|
||||
}
|
||||
|
||||
// ── Test 5: Short lowercase variables not flagged ───────────────────────────
|
||||
console.log("\n[5] Short lowercase variables not flagged");
|
||||
const file5 = makeTempFile("regression-05", `
|
||||
// Short lowercase vars like i, fn are common test locals
|
||||
// DO NOT COMMIT
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { fn } from "./fixture.mjs";
|
||||
|
||||
describe("test", () => {
|
||||
it("works with short vars", () => {
|
||||
const i = fn();
|
||||
const j = i + 1;
|
||||
expect(j).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
`);
|
||||
const result5 = run([file5, "--json"]);
|
||||
try {
|
||||
const parsed = JSON.parse(result5.stdout);
|
||||
assert(
|
||||
parsed.count === 0,
|
||||
`Short lowercase vars not flagged (count: ${parsed.count})`,
|
||||
);
|
||||
} catch {
|
||||
assert(false, `Should not throw on clean file`);
|
||||
}
|
||||
|
||||
// ── Summary ─────────────────────────────────────────────────────────────────
|
||||
|
||||
console.log(`\n${"-".repeat(50)}`);
|
||||
console.log(`Results: ${passed} passed, ${failed} failed`);
|
||||
cleanupTempDir();
|
||||
process.exit(failed > 0 ? 1 : 0);
|
||||
Loading…
Add table
Reference in a new issue