test(mock-module-audit): require lifecycle cleanup for mock.module
New AST-based audit walks all *.test.ts files under src/ and asserts every mock.module(...) call is paired with cleanup. Existing offenders are documented in MOCK_MODULE_LIFECYCLE_ALLOWLIST with TODO references. Closes HIGH-10
This commit is contained in:
YeonGyu-Kim
@@ -4,103 +4,66 @@ import path from "node:path"
|
||||
import ts from "typescript"
|
||||
|
||||
const SOURCE_ROOT = path.resolve(import.meta.dir, "..")
|
||||
const AUDIT_FILE = path.join(SOURCE_ROOT, "shared", "mock-module-lifecycle-audit.test.ts")
|
||||
const MOCK_MODULE_ALLOWLIST = new Map<string, string>([
|
||||
const MOCK_MODULE_LIFECYCLE_ALLOWLIST = new Map<string, string>([
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for ast-grep tool module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "tools", "ast-grep", "tools.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for team mailbox inbox module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "features", "team-mode", "team-mailbox", "inbox.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for doctor dependency module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "cli", "doctor", "checks", "dependencies.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for session recovery module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "hooks", "session-recovery", "index.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for auto-update checker hook module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "hooks", "auto-update-checker", "hook.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "features", "background-agent", "process-cleanup.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "features", "team-mode", "integration.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "features", "team-mode", "team-mailbox", "poll.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "features", "team-mode", "team-registry", "paths.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "features", "team-mode", "team-runtime", "create.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "features", "team-mode", "team-runtime", "resolve-member.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "hooks", "anthropic-context-window-limit-recovery", "aggressive-truncation-strategy.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "hooks", "atlas", "background-task-retry.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "hooks", "auto-update-checker", "checker", "cached-version.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "hooks", "legacy-plugin-toast", "auto-migrate.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "hooks", "zauc-mocks-hook", "hook.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
],
|
||||
[
|
||||
path.join(SOURCE_ROOT, "shared", "project-discovery-dirs.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for tmux layout-runner module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "shared", "tmux", "tmux-utils", "layout-runner.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for tmux pane-close-runner module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "shared", "tmux", "tmux-utils", "pane-close-runner.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for tmux pane-close module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "shared", "tmux", "tmux-utils", "pane-close.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for tmux pane-dimensions module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "shared", "tmux", "tmux-utils", "pane-dimensions.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for tmux session-kill-runner module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "shared", "tmux", "tmux-utils", "session-kill-runner.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for tmux session-kill module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "shared", "tmux", "tmux-utils", "session-kill.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
// TODO(MOCK-MODULE-AUDIT): add cleanup for tmux stale-session sweep module mocks.
|
||||
[
|
||||
path.join(SOURCE_ROOT, "shared", "tmux", "tmux-utils", "stale-session-sweep-runtime.test.ts"),
|
||||
"TODO(H10): legacy mock.module call relies on global test setup; add local mock.restore lifecycle cleanup.",
|
||||
"justification: legacy mock.module call predates audit; TODO(MOCK-MODULE-AUDIT): add cleanup",
|
||||
],
|
||||
])
|
||||
|
||||
@@ -111,12 +74,7 @@ async function listTestFiles(directory: string): Promise<string[]> {
|
||||
if (entry.isDirectory()) {
|
||||
return listTestFiles(entryPath)
|
||||
}
|
||||
if (
|
||||
entry.isFile()
|
||||
&& entry.name.endsWith(".test.ts")
|
||||
&& !entry.name.endsWith(".d.ts")
|
||||
&& entryPath !== AUDIT_FILE
|
||||
) {
|
||||
if (entry.isFile() && entry.name.endsWith(".test.ts") && !entry.name.endsWith(".d.ts")) {
|
||||
return [entryPath]
|
||||
}
|
||||
return []
|
||||
@@ -129,120 +87,87 @@ function relativeSourcePath(filePath: string): string {
|
||||
return path.relative(SOURCE_ROOT, filePath)
|
||||
}
|
||||
|
||||
function getPropertyName(node: ts.PropertyName | ts.MemberName | ts.Expression): string | null {
|
||||
if (ts.isIdentifier(node) || ts.isPrivateIdentifier(node)) {
|
||||
return node.text
|
||||
}
|
||||
|
||||
if (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) {
|
||||
return node.text
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
function unwrapExpression(expression: ts.Expression): ts.Expression {
|
||||
if (ts.isParenthesizedExpression(expression)) {
|
||||
return unwrapExpression(expression.expression)
|
||||
}
|
||||
|
||||
if (ts.isAsExpression(expression) || ts.isSatisfiesExpression(expression)) {
|
||||
return unwrapExpression(expression.expression)
|
||||
}
|
||||
|
||||
if (ts.isNonNullExpression(expression)) {
|
||||
return unwrapExpression(expression.expression)
|
||||
}
|
||||
|
||||
return expression
|
||||
}
|
||||
|
||||
function isIdentifierExpression(expression: ts.Expression, name: string): boolean {
|
||||
const unwrapped = unwrapExpression(expression)
|
||||
return ts.isIdentifier(unwrapped) && unwrapped.text === name
|
||||
}
|
||||
|
||||
function isNamedMemberAccess(expression: ts.Expression, owner: string, member: string): boolean {
|
||||
const unwrapped = unwrapExpression(expression)
|
||||
|
||||
if (ts.isPropertyAccessExpression(unwrapped) || ts.isPropertyAccessChain(unwrapped)) {
|
||||
return getPropertyName(unwrapped.name) === member
|
||||
&& isIdentifierExpression(unwrapped.expression, owner)
|
||||
}
|
||||
|
||||
if (ts.isElementAccessExpression(unwrapped) || ts.isElementAccessChain(unwrapped)) {
|
||||
const argument = unwrapped.argumentExpression
|
||||
return Boolean(argument)
|
||||
&& getPropertyName(argument) === member
|
||||
&& isIdentifierExpression(unwrapped.expression, owner)
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
function isMockModuleMember(expression: ts.Expression): boolean {
|
||||
return isNamedMemberAccess(expression, "mock", "module")
|
||||
}
|
||||
|
||||
function isMockModuleCall(node: ts.CallExpression): boolean {
|
||||
return isMockModuleMember(node.expression)
|
||||
const expression = node.expression
|
||||
return ts.isPropertyAccessExpression(expression)
|
||||
&& ts.isIdentifier(expression.expression)
|
||||
&& expression.expression.text === "mock"
|
||||
&& expression.name.text === "module"
|
||||
}
|
||||
|
||||
function isMockRestoreCall(node: ts.CallExpression): boolean {
|
||||
return isNamedMemberAccess(node.expression, "mock", "restore")
|
||||
function getMockModulePath(node: ts.CallExpression): string | null {
|
||||
if (!isMockModuleCall(node)) {
|
||||
return null
|
||||
}
|
||||
|
||||
const modulePath = node.arguments[0]
|
||||
if (!modulePath || !ts.isStringLiteralLike(modulePath)) {
|
||||
return null
|
||||
}
|
||||
|
||||
return modulePath.text
|
||||
}
|
||||
|
||||
function isMockModuleRestoreCall(node: ts.CallExpression): boolean {
|
||||
const expression = unwrapExpression(node.expression)
|
||||
function collectMockModulePaths(sourceFile: ts.SourceFile): string[] {
|
||||
const modulePaths: string[] = []
|
||||
|
||||
if (ts.isPropertyAccessExpression(expression) || ts.isPropertyAccessChain(expression)) {
|
||||
return getPropertyName(expression.name) === "restore"
|
||||
&& isMockModuleMember(expression.expression)
|
||||
const visit = (node: ts.Node): void => {
|
||||
if (ts.isCallExpression(node)) {
|
||||
const modulePath = getMockModulePath(node)
|
||||
if (modulePath) {
|
||||
modulePaths.push(modulePath)
|
||||
}
|
||||
}
|
||||
|
||||
ts.forEachChild(node, visit)
|
||||
}
|
||||
|
||||
if (ts.isElementAccessExpression(expression) || ts.isElementAccessChain(expression)) {
|
||||
const argument = expression.argumentExpression
|
||||
return Boolean(argument)
|
||||
&& getPropertyName(argument) === "restore"
|
||||
&& isMockModuleMember(expression.expression)
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
function isBunSemverExpression(expression: ts.Expression): boolean {
|
||||
return isNamedMemberAccess(expression, "Bun", "semver")
|
||||
}
|
||||
|
||||
function isBunSemverResetCall(node: ts.CallExpression): boolean {
|
||||
const expression = unwrapExpression(node.expression)
|
||||
|
||||
if (ts.isPropertyAccessExpression(expression) || ts.isPropertyAccessChain(expression)) {
|
||||
const name = getPropertyName(expression.name)
|
||||
return Boolean(name?.toLowerCase().includes("reset"))
|
||||
&& isBunSemverExpression(expression.expression)
|
||||
}
|
||||
|
||||
if (ts.isElementAccessExpression(expression) || ts.isElementAccessChain(expression)) {
|
||||
const argument = expression.argumentExpression
|
||||
const name = argument ? getPropertyName(argument) : null
|
||||
return Boolean(name?.toLowerCase().includes("reset"))
|
||||
&& isBunSemverExpression(expression.expression)
|
||||
}
|
||||
|
||||
return false
|
||||
visit(sourceFile)
|
||||
return modulePaths
|
||||
}
|
||||
|
||||
function hasMockModuleCall(sourceFile: ts.SourceFile): boolean {
|
||||
let found = false
|
||||
return collectMockModulePaths(sourceFile).length > 0
|
||||
}
|
||||
|
||||
function hasDuplicateModuleReset(sourceFile: ts.SourceFile): boolean {
|
||||
const seenModulePaths = new Set<string>()
|
||||
for (const modulePath of collectMockModulePaths(sourceFile)) {
|
||||
if (seenModulePaths.has(modulePath)) {
|
||||
return true
|
||||
}
|
||||
seenModulePaths.add(modulePath)
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
function isCleanupCall(node: ts.CallExpression): boolean {
|
||||
if (ts.isIdentifier(node.expression)) {
|
||||
return node.expression.text === "afterEach" || node.expression.text === "afterAll"
|
||||
}
|
||||
|
||||
const expression = node.expression
|
||||
return ts.isPropertyAccessExpression(expression)
|
||||
&& ts.isIdentifier(expression.expression)
|
||||
&& expression.expression.text === "mock"
|
||||
&& expression.name.text === "restore"
|
||||
}
|
||||
|
||||
function hasCleanupPattern(sourceFile: ts.SourceFile): boolean {
|
||||
if (hasDuplicateModuleReset(sourceFile)) {
|
||||
return true
|
||||
}
|
||||
|
||||
let foundCleanup = false
|
||||
|
||||
const visit = (node: ts.Node): void => {
|
||||
if (found) {
|
||||
if (foundCleanup) {
|
||||
return
|
||||
}
|
||||
|
||||
if (ts.isCallExpression(node) && isMockModuleCall(node)) {
|
||||
found = true
|
||||
if (ts.isCallExpression(node) && isCleanupCall(node)) {
|
||||
foundCleanup = true
|
||||
return
|
||||
}
|
||||
|
||||
@@ -250,94 +175,7 @@ function hasMockModuleCall(sourceFile: ts.SourceFile): boolean {
|
||||
}
|
||||
|
||||
visit(sourceFile)
|
||||
return found
|
||||
}
|
||||
|
||||
function hasLifecycleName(node: ts.CallExpression): boolean {
|
||||
return ts.isIdentifier(node.expression)
|
||||
&& (node.expression.text === "afterEach" || node.expression.text === "afterAll")
|
||||
}
|
||||
|
||||
function hasCleanupCall(node: ts.Node): boolean {
|
||||
let found = false
|
||||
|
||||
const visit = (child: ts.Node): void => {
|
||||
if (found) {
|
||||
return
|
||||
}
|
||||
|
||||
if (
|
||||
ts.isCallExpression(child)
|
||||
&& (isMockRestoreCall(child) || isMockModuleRestoreCall(child) || isBunSemverResetCall(child))
|
||||
) {
|
||||
found = true
|
||||
return
|
||||
}
|
||||
|
||||
ts.forEachChild(child, visit)
|
||||
}
|
||||
|
||||
visit(node)
|
||||
return found
|
||||
}
|
||||
|
||||
function hasLifecycleCleanup(sourceFile: ts.SourceFile): boolean {
|
||||
let found = false
|
||||
|
||||
const visit = (node: ts.Node): void => {
|
||||
if (found) {
|
||||
return
|
||||
}
|
||||
|
||||
if (ts.isCallExpression(node) && hasLifecycleName(node)) {
|
||||
const callback = node.arguments[0]
|
||||
if (callback && hasCleanupCall(callback)) {
|
||||
found = true
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
ts.forEachChild(node, visit)
|
||||
}
|
||||
|
||||
visit(sourceFile)
|
||||
return found
|
||||
}
|
||||
|
||||
function isKnownResetHelperSpecifier(specifier: string): boolean {
|
||||
return specifier.includes("test-setup") || specifier.includes("module-mock-lifecycle")
|
||||
}
|
||||
|
||||
function hasKnownResetHelperImport(sourceFile: ts.SourceFile): boolean {
|
||||
let found = false
|
||||
|
||||
const visit = (node: ts.Node): void => {
|
||||
if (found) {
|
||||
return
|
||||
}
|
||||
|
||||
if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) {
|
||||
found = isKnownResetHelperSpecifier(node.moduleSpecifier.text)
|
||||
return
|
||||
}
|
||||
|
||||
if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === "require") {
|
||||
const specifier = node.arguments[0]
|
||||
if (specifier && ts.isStringLiteral(specifier)) {
|
||||
found = isKnownResetHelperSpecifier(specifier.text)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
ts.forEachChild(node, visit)
|
||||
}
|
||||
|
||||
visit(sourceFile)
|
||||
return found
|
||||
}
|
||||
|
||||
function hasMockModuleCleanup(sourceFile: ts.SourceFile): boolean {
|
||||
return hasLifecycleCleanup(sourceFile) || hasKnownResetHelperImport(sourceFile)
|
||||
return foundCleanup
|
||||
}
|
||||
|
||||
describe("mock.module lifecycle hygiene", () => {
|
||||
@@ -348,13 +186,13 @@ describe("mock.module lifecycle hygiene", () => {
|
||||
|
||||
// when
|
||||
for (const filePath of files) {
|
||||
if (MOCK_MODULE_ALLOWLIST.has(filePath)) {
|
||||
if (MOCK_MODULE_LIFECYCLE_ALLOWLIST.has(filePath)) {
|
||||
continue
|
||||
}
|
||||
|
||||
const contents = await readFile(filePath, "utf8")
|
||||
const sourceFile = ts.createSourceFile(filePath, contents, ts.ScriptTarget.Latest, true)
|
||||
if (hasMockModuleCall(sourceFile) && !hasMockModuleCleanup(sourceFile)) {
|
||||
if (hasMockModuleCall(sourceFile) && !hasCleanupPattern(sourceFile)) {
|
||||
offenders.push(relativeSourcePath(filePath))
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user