From 5ff247b99ec1e1bdcf3136d6131b31d219caa52b Mon Sep 17 00:00:00 2001 From: Mason Huang Date: Thu, 2 Jul 2026 10:46:28 +0800 Subject: [PATCH] test: prefer auto-cleaning temp dir helper (#93209) Summary: - The branch adds `useAutoCleanupTempDirTracker()`, broadens the temp-dir warning reporter to flag new manual helper imports/usages, updates docs, and migrates two script tests to the new helper. - PR surface: Tests +301, Docs +1, Other +248. Total +550 across 8 files. - Reproducibility: not applicable. this is test/tooling cleanup, and the changed behavior is exercised through helper/reporter tests and CI evidence rather than a user reproduction path. Automerge notes: - PR branch already contained follow-up commit before automerge: test: harden temp dir helper guard - PR branch already contained follow-up commit before automerge: test: clarify auto cleanup temp dir helper name - PR branch already contained follow-up commit before automerge: test: cover existing mkdtemp temp dir forms - PR branch already contained follow-up commit before automerge: test: read staged temp helper source from index Validation: - ClawSweeper review passed for head 1fdd7d2a9aed342397bfef20348a1c5fd50fa5ff. - Required merge gates passed before the squash merge. Prepared head SHA: 1fdd7d2a9aed342397bfef20348a1c5fd50fa5ff Review: https://github.com/openclaw/openclaw/pull/93209#issuecomment-4705653665 Co-authored-by: Mason Huang Approved-by: hxy91819 --- docs/help/testing.md | 33 ++- scripts/report-test-temp-creations.mjs | 264 ++++++++++++++++- src/scripts/test-projects.test.ts | 2 + test/helpers/temp-dir.test.ts | 29 +- test/helpers/temp-dir.ts | 20 ++ ...llels-package-log-progress-extract.test.ts | 18 +- .../report-test-temp-creations.test.ts | 280 +++++++++++++++++- test/scripts/runtime-postbuild-stamp.test.ts | 34 +-- 8 files changed, 615 insertions(+), 65 deletions(-) diff --git a/docs/help/testing.md b/docs/help/testing.md index d7edb2f9ad13..ed972497d24e 100644 --- a/docs/help/testing.md +++ b/docs/help/testing.md @@ -50,12 +50,9 @@ temporary directories. They make ownership explicit and keep cleanup in the same test lifecycle: ```ts -import { afterEach } from "vitest"; -import { createTempDirTracker } from "../helpers/temp-dir.js"; +import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js"; -const tempDirs = createTempDirTracker(); - -afterEach(tempDirs.cleanup); +const tempDirs = useAutoCleanupTempDirTracker(); it("uses a temp workspace", () => { const workspace = tempDirs.make("openclaw-example-"); @@ -63,11 +60,14 @@ it("uses a temp workspace", () => { }); ``` -Use `makeTempDir(tempDirs, prefix)` and `cleanupTempDirs(tempDirs)` when a test -already owns an array or set of paths. Avoid new bare `fs.mkdtemp*` calls in -tests unless a case is explicitly verifying raw temp-dir behavior. Add an -auditable allow comment with a concrete reason when a test intentionally needs a -bare temp directory: +`useAutoCleanupTempDirTracker()` intentionally exposes no manual cleanup method; Vitest +owns cleanup after each test. Existing lower-level helpers remain for tests that +have not moved yet, but new and migrated tests should use the auto-cleaning +tracker. Avoid new manual `makeTempDir`, `cleanupTempDirs`, or +`createTempDirTracker` usage and avoid new bare `fs.mkdtemp*` calls in tests +unless a case is explicitly verifying raw temp-dir behavior. Add an auditable +allow comment with a concrete reason when a test intentionally needs a bare temp +directory: ```ts // openclaw-temp-dir: allow verifies raw fs cleanup behavior @@ -75,12 +75,13 @@ const workspace = fs.mkdtempSync(prefix); ``` For migration visibility, `node scripts/report-test-temp-creations.mjs` reports -new bare temp-dir creation in added diff lines without blocking existing cleanup -styles. Its file scope intentionally follows the same test-path classification -used by `scripts/changed-lanes.mjs` instead of maintaining a separate test-helper -filename heuristic, while skipping the shared helper implementation itself. -`check:changed` runs this report for changed test paths as a warning-only CI -signal; findings are GitHub warning annotations, not failures. +new bare temp-dir creation and new manual shared-helper usage in added diff +lines without blocking existing cleanup styles. Its file scope intentionally +follows the same test-path classification used by `scripts/changed-lanes.mjs` +instead of maintaining a separate test-helper filename heuristic, while skipping +the shared helper implementation itself. `check:changed` runs this report for +changed test paths as a warning-only CI signal; findings are GitHub warning +annotations, not failures. When debugging real providers/models (requires real creds): diff --git a/scripts/report-test-temp-creations.mjs b/scripts/report-test-temp-creations.mjs index 86b2af00338d..4e7325235a72 100644 --- a/scripts/report-test-temp-creations.mjs +++ b/scripts/report-test-temp-creations.mjs @@ -1,6 +1,9 @@ #!/usr/bin/env node import { execFileSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import ts from "typescript"; import { isChangedLaneTestPath } from "./changed-lanes.mjs"; import { booleanFlag, parseFlagArgs, stringFlag } from "./lib/arg-utils.mjs"; import { runAsScript } from "./lib/ts-guard-utils.mjs"; @@ -8,6 +11,8 @@ import { runAsScript } from "./lib/ts-guard-utils.mjs"; const DEFAULT_BASE_REF = "origin/main"; const DEFAULT_HEAD_REF = "HEAD"; const TEMP_DIR_HELPER_PATH = "test/helpers/temp-dir.ts"; +const TEMP_DIR_HELPER_TEST_PATH = "test/helpers/temp-dir.test.ts"; +const MANUAL_TEMP_DIR_HELPERS = new Set(["cleanupTempDirs", "createTempDirTracker", "makeTempDir"]); const FINDING_PATTERNS = [ { pattern: /\bmkdtemp(?:Sync)?\s*\(/u, @@ -25,9 +30,9 @@ function usage() { return `Usage: node scripts/report-test-temp-creations.mjs [options] Description: - Reports new bare test temp-directory creation patterns in added diff lines. + Reports new test temp-directory migration warnings in added diff lines. This is a low-noise migration aid, not a cleanup data-flow checker. It does - not scan existing lines and does not decide whether cleanup is sufficient. + not scan existing lines for bare temp dirs and does not decide whether cleanup is sufficient. Add "openclaw-temp-dir: allow " in a same-line or immediately preceding added comment when a test intentionally needs bare temp creation. File scope intentionally reuses scripts/changed-lanes.mjs test-path @@ -64,6 +69,11 @@ function shouldInspectFile(filePath) { return normalizedPath !== TEMP_DIR_HELPER_PATH && isChangedLaneTestPath(normalizedPath); } +function shouldInspectManualHelperUsage(filePath) { + const normalizedPath = normalizePath(filePath); + return normalizedPath !== TEMP_DIR_HELPER_TEST_PATH && shouldInspectFile(normalizedPath); +} + function isTruthyEnvFlag(value) { const normalized = String(value ?? "") .trim() @@ -93,7 +103,7 @@ export function formatGithubWarning(finding) { const file = escapeGithubCommandProperty(finding.file); const line = escapeGithubCommandProperty(finding.line); const message = escapeGithubCommandValue( - `${finding.reason}: prefer test/helpers/temp-dir.ts for new test-owned temp directories.`, + `${finding.reason}: prefer useAutoCleanupTempDirTracker() from test/helpers/temp-dir.ts for new test-owned temp directories.`, ); return `::warning file=${file},line=${line}::${message}`; } @@ -133,8 +143,228 @@ function readDiff(args, cwd = process.cwd()) { }); } -export function collectTempCreationFindingsFromDiff(diffText) { +function readWorktreeSource(filePath, cwd) { + try { + return fs.readFileSync(path.join(cwd, filePath), "utf8"); + } catch { + return ""; + } +} + +function readStagedSource(filePath, cwd) { + try { + return execFileSync("git", ["show", `:${filePath}`], { + cwd, + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + stdio: ["ignore", "pipe", "pipe"], + }); + } catch { + return ""; + } +} + +function readSourceForDiff(filePath, args, cwd) { + // Staged checks must parse the index blob. Reading the worktree mixes in + // unstaged edits and can warn on code that will not be committed. + return args.staged ? readStagedSource(filePath, cwd) : readWorktreeSource(filePath, cwd); +} + +function stripKnownExtension(filePath) { + return filePath.replace(/\.(?:c|m)?[jt]sx?$/u, ""); +} + +function isTempDirHelperImportSpec(filePath, specifier) { + const normalizedSpecifier = normalizePath(specifier); + const resolvedPath = normalizedSpecifier.startsWith(".") + ? path.posix.normalize(path.posix.join(path.posix.dirname(filePath), normalizedSpecifier)) + : normalizedSpecifier; + return stripKnownExtension(resolvedPath) === stripKnownExtension(TEMP_DIR_HELPER_PATH); +} + +function scriptKindForFile(filePath) { + if (/\.[cm]?tsx$/u.test(filePath)) { + return ts.ScriptKind.TSX; + } + if (/\.[cm]?jsx$/u.test(filePath)) { + return ts.ScriptKind.JSX; + } + if (/\.[cm]?js$/u.test(filePath)) { + return ts.ScriptKind.JS; + } + return ts.ScriptKind.TS; +} + +function createSourceFile(filePath, sourceText) { + return ts.createSourceFile( + filePath, + sourceText, + ts.ScriptTarget.Latest, + true, + scriptKindForFile(filePath), + ); +} + +function lineForNode(sourceFile, node) { + return sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)).line + 1; +} + +function sourceLineText(sourceFile, line) { + const lineStarts = sourceFile.getLineStarts(); + const start = lineStarts[line - 1] ?? 0; + const end = lineStarts[line] ?? sourceFile.text.length; + return sourceFile.text.slice(start, end).trim(); +} + +function nodeOverlapsAddedLine(sourceFile, node, addedLineNumbers) { + const startLine = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)).line + 1; + const endLine = sourceFile.getLineAndCharacterOfPosition(node.getEnd()).line + 1; + for (let line = startLine; line <= endLine; line += 1) { + if (addedLineNumbers.has(line)) { + return true; + } + } + return false; +} + +function normalizeFileTextMap(fileTextByPath) { + if (!fileTextByPath) { + return null; + } + if (fileTextByPath instanceof Map) { + return fileTextByPath; + } + return new Map(Object.entries(fileTextByPath)); +} + +function readCurrentSource(filePath, options, fileTextByPath) { + if (fileTextByPath?.has(filePath)) { + return fileTextByPath.get(filePath) ?? ""; + } + if (typeof options.readFile === "function") { + return options.readFile(filePath) ?? ""; + } + return ""; +} + +function collectManualTempDirHelperImports(sourceFile, filePath, addedLineNumbers = null) { + const imports = []; + const localNames = new Set(); + for (const statement of sourceFile.statements) { + if ( + !ts.isImportDeclaration(statement) || + !statement.importClause?.namedBindings || + !ts.isStringLiteral(statement.moduleSpecifier) || + !isTempDirHelperImportSpec(filePath, statement.moduleSpecifier.text) || + !ts.isNamedImports(statement.importClause.namedBindings) + ) { + continue; + } + let importWarningLine = null; + for (const element of statement.importClause.namedBindings.elements) { + const imported = element.propertyName?.text ?? element.name.text; + if (!MANUAL_TEMP_DIR_HELPERS.has(imported)) { + continue; + } + localNames.add(element.name.text); + if ( + importWarningLine === null && + (!addedLineNumbers || nodeOverlapsAddedLine(sourceFile, element, addedLineNumbers)) + ) { + importWarningLine = lineForNode(sourceFile, element); + } + } + if (importWarningLine !== null) { + imports.push({ + line: importWarningLine, + source: statement.getText(sourceFile).trim().replace(/\s+/gu, " "), + }); + } + } + return { imports, localNames }; +} + +function findManualHelperUsageFindings(filePath, sourceText, addedLines) { + const addedLineNumbers = new Set(addedLines.map((line) => line.line)); + const sourceFile = createSourceFile(filePath, sourceText); + const { imports, localNames } = collectManualTempDirHelperImports( + sourceFile, + filePath, + addedLineNumbers, + ); + const findings = imports.map((manualImport) => ({ + file: filePath, + line: manualImport.line, + reason: "new manual temp-dir helper import", + source: manualImport.source, + })); + if (localNames.size === 0) { + return findings; + } + const visit = (node) => { + if ( + ts.isCallExpression(node) && + ts.isIdentifier(node.expression) && + localNames.has(node.expression.text) && + nodeOverlapsAddedLine(sourceFile, node.expression, addedLineNumbers) + ) { + findings.push({ + file: filePath, + line: lineForNode(sourceFile, node.expression), + reason: "new manual temp-dir helper usage", + source: sourceLineText(sourceFile, lineForNode(sourceFile, node.expression)), + }); + } + ts.forEachChild(node, visit); + }; + visit(sourceFile); + return findings; +} + +function collectAddedLinesByFile(diffText) { + const addedLinesByFile = new Map(); + let currentFile = null; + let currentLine = 0; + + for (const line of diffText.split(/\r?\n/u)) { + const fileMatch = line.match(/^\+\+\+ b\/(.+)$/u); + if (fileMatch) { + currentFile = normalizePath(fileMatch[1]); + continue; + } + if (line === "+++ /dev/null") { + currentFile = null; + continue; + } + + const hunkMatch = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/u); + if (hunkMatch) { + currentLine = Number.parseInt(hunkMatch[1], 10); + continue; + } + + if (line.startsWith("+") && !line.startsWith("+++")) { + if (currentFile && shouldInspectFile(currentFile)) { + const lines = addedLinesByFile.get(currentFile) ?? []; + lines.push({ line: currentLine, source: line.slice(1) }); + addedLinesByFile.set(currentFile, lines); + } + currentLine += 1; + continue; + } + + if (line.startsWith(" ") || line === "") { + currentLine += 1; + } + } + + return addedLinesByFile; +} + +export function collectTempCreationFindingsFromDiff(diffText, options = {}) { const findings = []; + const addedLinesByFile = collectAddedLinesByFile(diffText); + const fileTextByPath = normalizeFileTextMap(options.fileTextByPath); let currentFile = null; let currentLine = 0; let allowNextLine = null; @@ -192,6 +422,17 @@ export function collectTempCreationFindingsFromDiff(diffText) { } } + for (const [file, addedLines] of addedLinesByFile) { + if (!shouldInspectManualHelperUsage(file)) { + continue; + } + const sourceText = readCurrentSource(file, options, fileTextByPath); + if (!sourceText) { + continue; + } + findings.push(...findManualHelperUsageFindings(file, sourceText, addedLines)); + } + return findings; } @@ -205,21 +446,28 @@ export async function main(argv, io) { return 0; } - const findings = collectTempCreationFindingsFromDiff(readDiff(args)); + const cwd = process.cwd(); + const findings = collectTempCreationFindingsFromDiff(readDiff(args, cwd), { + readFile(filePath) { + return readSourceForDiff(filePath, args, cwd); + }, + }); if (args.json) { stdout.write(`${JSON.stringify(findings, null, 2)}\n`); } else if (findings.length === 0) { - stderr.write("No new bare test temp-directory creation patterns found.\n"); + stderr.write("No new test temp-directory migration warnings found.\n"); } else if (isTruthyEnvFlag(env.GITHUB_ACTIONS)) { for (const finding of findings) { stderr.write(`${formatGithubWarning(finding)}\n`); } } else { - stderr.write("New bare test temp-directory creation patterns:\n"); + stderr.write("New test temp-directory migration warnings:\n"); for (const finding of findings) { stderr.write(`- ${finding.file}:${finding.line} ${finding.reason}: ${finding.source}\n`); } - stderr.write("Prefer test/helpers/temp-dir.ts for new test-owned temp directories.\n"); + stderr.write( + "Prefer useAutoCleanupTempDirTracker() from test/helpers/temp-dir.ts for new test-owned temp directories.\n", + ); } return args.failOnFindings && findings.length > 0 ? 1 : 0; diff --git a/src/scripts/test-projects.test.ts b/src/scripts/test-projects.test.ts index a622b237dc5b..2b2e4ccf37fa 100644 --- a/src/scripts/test-projects.test.ts +++ b/src/scripts/test-projects.test.ts @@ -913,6 +913,7 @@ describe("test-projects args", () => { "test/scripts/native-app-i18n.test.ts", "test/scripts/onboard-config-fixtures.test.ts", "test/scripts/parallels-lib-helpers.test.ts", + "test/scripts/parallels-package-log-progress-extract.test.ts", "test/scripts/parallels-smoke-model.test.ts", "test/scripts/plugin-package-dependencies.test.ts", "test/scripts/plugins-assertions.test.ts", @@ -921,6 +922,7 @@ describe("test-projects args", () => { "test/scripts/release-preflight.test.ts", "test/scripts/render-maturity-docs.test.ts", "test/scripts/report-test-temp-creations.test.ts", + "test/scripts/runtime-postbuild-stamp.test.ts", "test/scripts/test-install-sh-docker.test.ts", "test/scripts/test-projects.test.ts", "test/test-env.test.ts", diff --git a/test/helpers/temp-dir.test.ts b/test/helpers/temp-dir.test.ts index 9077b7ea2ff6..bc835dabcf45 100644 --- a/test/helpers/temp-dir.test.ts +++ b/test/helpers/temp-dir.test.ts @@ -1,7 +1,12 @@ import fs from "node:fs"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { cleanupTempDirs, createTempDirTracker, makeTempDir } from "./temp-dir.js"; +import { + cleanupTempDirs, + createTempDirTracker, + makeTempDir, + useAutoCleanupTempDirTracker, +} from "./temp-dir.js"; const tempDirs = new Set(); @@ -39,4 +44,26 @@ describe("temp-dir test helpers", () => { expect(fs.existsSync(dir)).toBe(false); expect([...tempDirs]).toEqual([]); }); + + describe("auto-cleaning tracker", () => { + const createdDirs: string[] = []; + + afterEach(() => { + for (const dir of createdDirs.splice(0)) { + expect(fs.existsSync(dir)).toBe(false); + } + expect([...autoCleanupTracker.dirs]).toEqual([]); + }); + + const autoCleanupTracker = useAutoCleanupTempDirTracker(); + + it("tracks temp dirs with Vitest cleanup", () => { + const autoCleanedDir = autoCleanupTracker.make("openclaw-temp-dir-auto-"); + createdDirs.push(autoCleanedDir); + fs.writeFileSync(path.join(autoCleanedDir, "artifact.txt"), "artifact\n", "utf8"); + + expect(fs.existsSync(autoCleanedDir)).toBe(true); + expect("cleanup" in autoCleanupTracker).toBe(false); + }); + }); }); diff --git a/test/helpers/temp-dir.ts b/test/helpers/temp-dir.ts index 7db37f4587ed..868f60ca58a5 100644 --- a/test/helpers/temp-dir.ts +++ b/test/helpers/temp-dir.ts @@ -2,6 +2,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { afterEach } from "vitest"; // Synchronous temporary directory helpers for tests. @@ -13,6 +14,11 @@ export interface TestTempDirTracker { cleanup(): void; } +export interface AutoCleanupTempDirTracker { + readonly dirs: ReadonlySet; + make(prefix: string): string; +} + /** Create a temp dir and register it in an array or set for cleanup. */ export function makeTempDir(tempDirs: TempDirCollection, prefix: string): string { const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); @@ -47,3 +53,17 @@ export function createTempDirTracker(): TestTempDirTracker { }, }; } + +/** Create a temp dir tracker that Vitest cleans up after each test. */ +export function useAutoCleanupTempDirTracker(): AutoCleanupTempDirTracker { + const tracker = createTempDirTracker(); + afterEach(() => { + tracker.cleanup(); + }); + return { + dirs: tracker.dirs, + make(prefix: string): string { + return tracker.make(prefix); + }, + }; +} diff --git a/test/scripts/parallels-package-log-progress-extract.test.ts b/test/scripts/parallels-package-log-progress-extract.test.ts index 1cace0240bc7..14097a3f655c 100644 --- a/test/scripts/parallels-package-log-progress-extract.test.ts +++ b/test/scripts/parallels-package-log-progress-extract.test.ts @@ -1,17 +1,15 @@ // Parallels Package Log Progress Extract tests cover parallels package log progress extract script behavior. import { spawnSync } from "node:child_process"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { writeFileSync } from "node:fs"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { describe, expect, it } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js"; const SCRIPT_PATH = "scripts/e2e/lib/parallels-package/log-progress-extract.mjs"; -const tempRoots: string[] = []; +const tempRoots = useAutoCleanupTempDirTracker(); function makeTempRoot(): string { - const root = mkdtempSync(path.join(tmpdir(), "openclaw-parallels-progress-")); - tempRoots.push(root); - return root; + return tempRoots.make("openclaw-parallels-progress-"); } function runExtract(logPath?: string) { @@ -20,12 +18,6 @@ function runExtract(logPath?: string) { }); } -afterEach(() => { - for (const root of tempRoots.splice(0)) { - rmSync(root, { force: true, recursive: true }); - } -}); - describe("parallels package log progress extractor", () => { it("prints a blank status when the log is absent", () => { const result = runExtract(path.join(makeTempRoot(), "missing.log")); diff --git a/test/scripts/report-test-temp-creations.test.ts b/test/scripts/report-test-temp-creations.test.ts index 55c91d872076..b018bc77e30e 100644 --- a/test/scripts/report-test-temp-creations.test.ts +++ b/test/scripts/report-test-temp-creations.test.ts @@ -1,15 +1,15 @@ import { execFileSync, spawnSync } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { describe, expect, it } from "vitest"; import { collectTempCreationFindingsFromDiff, formatGithubWarning, } from "../../scripts/report-test-temp-creations.mjs"; -import { createTempDirTracker } from "../helpers/temp-dir.js"; +import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js"; const repoRoot = process.cwd(); -const tempDirs = createTempDirTracker(); +const tempDirs = useAutoCleanupTempDirTracker(); const nestedGitEnvKeys = [ "GIT_ALTERNATE_OBJECT_DIRECTORIES", "GIT_DIR", @@ -31,10 +31,6 @@ function createNestedGitEnv(): NodeJS.ProcessEnv { return env; } -afterEach(() => { - tempDirs.cleanup(); -}); - describe("report-test-temp-creations", () => { it("keeps a non-executed warning fixture for changed-gate proof", () => { // openclaw-temp-dir: allow test fixture for the temp warning report @@ -118,6 +114,32 @@ describe("report-test-temp-creations", () => { ]); }); + it("reports repository-observed mkdtemp call forms", () => { + const sources = [ + ["const root = await fs.promises.", "mkdtemp", '(path.join(os.tmpdir(), "case-"));'].join(""), + ["const root = await fs.", "mkdtemp", '(path.join(os.tmpdir(), "case-"));'].join(""), + ["const root = await fsPromises.", "mkdtemp", '("/tmp/openclaw-case-");'].join(""), + ["const root = await ", "mkdtemp", '(path.join(tmpdir(), "case-"));'].join(""), + ["const root = ", "mkdtemp", 'Sync(join(tmpdir(), "case-"));'].join(""), + ]; + const diff = [ + "diff --git a/test/scripts/temp-patterns.test.ts b/test/scripts/temp-patterns.test.ts", + "--- a/test/scripts/temp-patterns.test.ts", + "+++ b/test/scripts/temp-patterns.test.ts", + "@@ -1,0 +1,5 @@", + ...sources.map((source) => `+${source}`), + ].join("\n"); + + expect(collectTempCreationFindingsFromDiff(diff)).toEqual( + sources.map((source, index) => ({ + file: "test/scripts/temp-patterns.test.ts", + line: index + 1, + reason: "new mkdtemp temp directory creation", + source, + })), + ); + }); + it("honors explicit allow comments with reasons", () => { const mkdtempCall = ["fs.", "mkdtemp", 'Sync("case-")'].join(""); const tmpDirCall = ["tmp.", "dir", 'Sync({ prefix: "case-" })'].join(""); @@ -166,6 +188,182 @@ describe("report-test-temp-creations", () => { ]); }); + it("reports added imports and calls for manual temp-dir helpers", () => { + const file = "test/scripts/manual-temp.test.ts"; + const source = [ + 'import { afterEach } from "vitest";', + 'import { cleanupTempDirs, makeTempDir } from "../helpers/temp-dir.js";', + "const tempDirs = new Set();", + "afterEach(() => cleanupTempDirs(tempDirs));", + 'const workspace = makeTempDir(tempDirs, "case-");', + ].join("\n"); + const diff = [ + "diff --git a/test/scripts/manual-temp.test.ts b/test/scripts/manual-temp.test.ts", + "--- a/test/scripts/manual-temp.test.ts", + "+++ b/test/scripts/manual-temp.test.ts", + "@@ -1,0 +1,5 @@", + '+import { afterEach } from "vitest";', + '+import { cleanupTempDirs, makeTempDir } from "../helpers/temp-dir.js";', + "+const tempDirs = new Set();", + "+afterEach(() => cleanupTempDirs(tempDirs));", + '+const workspace = makeTempDir(tempDirs, "case-");', + ].join("\n"); + + expect( + collectTempCreationFindingsFromDiff(diff, { fileTextByPath: { [file]: source } }), + ).toEqual([ + { + file, + line: 2, + reason: "new manual temp-dir helper import", + source: 'import { cleanupTempDirs, makeTempDir } from "../helpers/temp-dir.js";', + }, + { + file, + line: 4, + reason: "new manual temp-dir helper usage", + source: "afterEach(() => cleanupTempDirs(tempDirs));", + }, + { + file, + line: 5, + reason: "new manual temp-dir helper usage", + source: 'const workspace = makeTempDir(tempDirs, "case-");', + }, + ]); + }); + + it("reports multiline imports from the shared temp-dir helper", () => { + const file = "src/example.test.ts"; + const source = [ + "import {", + " createTempDirTracker,", + '} from "../test/helpers/temp-dir.js";', + "const tempDirs = createTempDirTracker();", + ].join("\n"); + const diff = [ + "diff --git a/src/example.test.ts b/src/example.test.ts", + "--- a/src/example.test.ts", + "+++ b/src/example.test.ts", + "@@ -1,0 +1,4 @@", + "+import {", + "+ createTempDirTracker,", + '+} from "../test/helpers/temp-dir.js";', + "+const tempDirs = createTempDirTracker();", + ].join("\n"); + + expect( + collectTempCreationFindingsFromDiff(diff, { fileTextByPath: { [file]: source } }), + ).toEqual([ + { + file, + line: 2, + reason: "new manual temp-dir helper import", + source: 'import { createTempDirTracker, } from "../test/helpers/temp-dir.js";', + }, + { + file, + line: 4, + reason: "new manual temp-dir helper usage", + source: "const tempDirs = createTempDirTracker();", + }, + ]); + }); + + it("reports manual helpers added to existing multiline imports", () => { + const file = "test/scripts/manual-temp.test.ts"; + const source = [ + "import {", + " useAutoCleanupTempDirTracker,", + " makeTempDir,", + '} from "../helpers/temp-dir.js";', + "const tempDirs = useAutoCleanupTempDirTracker();", + ].join("\n"); + const diff = [ + "diff --git a/test/scripts/manual-temp.test.ts b/test/scripts/manual-temp.test.ts", + "--- a/test/scripts/manual-temp.test.ts", + "+++ b/test/scripts/manual-temp.test.ts", + "@@ -1,3 +1,4 @@", + " import {", + " useAutoCleanupTempDirTracker,", + "+ makeTempDir,", + ' } from "../helpers/temp-dir.js";', + ].join("\n"); + + expect( + collectTempCreationFindingsFromDiff(diff, { fileTextByPath: { [file]: source } }), + ).toEqual([ + { + file, + line: 3, + reason: "new manual temp-dir helper import", + source: + 'import { useAutoCleanupTempDirTracker, makeTempDir, } from "../helpers/temp-dir.js";', + }, + ]); + }); + + it("allows the auto-cleaning temp-dir helper", () => { + const file = "test/scripts/auto-temp.test.ts"; + const source = [ + 'import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";', + "const tempDirs = useAutoCleanupTempDirTracker();", + 'const workspace = tempDirs.make("case-");', + ].join("\n"); + const diff = [ + "diff --git a/test/scripts/auto-temp.test.ts b/test/scripts/auto-temp.test.ts", + "--- a/test/scripts/auto-temp.test.ts", + "+++ b/test/scripts/auto-temp.test.ts", + "@@ -1,0 +1,3 @@", + '+import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";', + "+const tempDirs = useAutoCleanupTempDirTracker();", + '+const workspace = tempDirs.make("case-");', + ].join("\n"); + + expect( + collectTempCreationFindingsFromDiff(diff, { fileTextByPath: { [file]: source } }), + ).toEqual([]); + }); + + it("ignores manual helper fixture strings and the helper test file", () => { + const fixtureFile = "test/scripts/report-test-temp-creations.test.ts"; + const helperTestFile = "test/helpers/temp-dir.test.ts"; + const fixtureSource = [ + 'import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";', + 'const fixture = "makeTempDir(tempDirs, \\"case-\\")";', + ].join("\n"); + const helperTestSource = [ + 'import { createTempDirTracker } from "./temp-dir.js";', + "const tempDirs = createTempDirTracker();", + ].join("\n"); + const diff = [ + "diff --git a/test/scripts/report-test-temp-creations.test.ts b/test/scripts/report-test-temp-creations.test.ts", + "--- a/test/scripts/report-test-temp-creations.test.ts", + "+++ b/test/scripts/report-test-temp-creations.test.ts", + "@@ -1,0 +1,5 @@", + '+const importFixture = "import { makeTempDir } from \\"../helpers/temp-dir.js\\";";', + "+const callFixture = [", + '+ "makeTempDir",', + '+ "(tempDirs, \\"case-\\")",', + '+].join("");', + "diff --git a/test/helpers/temp-dir.test.ts b/test/helpers/temp-dir.test.ts", + "--- a/test/helpers/temp-dir.test.ts", + "+++ b/test/helpers/temp-dir.test.ts", + "@@ -1,0 +1,2 @@", + '+import { createTempDirTracker } from "./temp-dir.js";', + "+const tempDirs = createTempDirTracker();", + ].join("\n"); + + expect( + collectTempCreationFindingsFromDiff(diff, { + fileTextByPath: { + [fixtureFile]: fixtureSource, + [helperTestFile]: helperTestSource, + }, + }), + ).toEqual([]); + }); + it("prints help with usage, outputs, and examples", () => { const output = execFileSync( process.execPath, @@ -192,10 +390,76 @@ describe("report-test-temp-creations", () => { source: "const tempRoot = fs.mkdtempSync();", }), ).toBe( - "::warning file=test/helpers/temp%2Cfixture.ts,line=12::new mkdtemp temp directory creation: prefer test/helpers/temp-dir.ts for new test-owned temp directories.", + "::warning file=test/helpers/temp%2Cfixture.ts,line=12::new mkdtemp temp directory creation: prefer useAutoCleanupTempDirTracker() from test/helpers/temp-dir.ts for new test-owned temp directories.", ); }); + it("reads staged source for manual helper scans", () => { + const root = tempDirs.make("openclaw-temp-report-staged-source-"); + const env = createNestedGitEnv(); + execFileSync("git", ["init", "-q", "--initial-branch=main"], { cwd: root, env }); + execFileSync( + "git", + [ + "-c", + "user.email=test@example.com", + "-c", + "user.name=Test User", + "commit", + "--allow-empty", + "-q", + "-m", + "initial", + ], + { cwd: root, env }, + ); + + fs.mkdirSync(path.join(root, "test", "scripts"), { recursive: true }); + const stagedManualFile = path.join(root, "test", "scripts", "staged-manual.test.ts"); + const stagedAutoFile = path.join(root, "test", "scripts", "staged-auto.test.ts"); + const manualSource = [ + 'import { makeTempDir } from "../helpers/temp-dir.js";', + "const tempDirs = new Set();", + 'const workspace = makeTempDir(tempDirs, "case-");', + ].join("\n"); + const autoSource = [ + 'import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";', + "const tempDirs = useAutoCleanupTempDirTracker();", + 'const workspace = tempDirs.make("case-");', + ].join("\n"); + fs.writeFileSync(stagedManualFile, `${manualSource}\n`, "utf8"); + fs.writeFileSync(stagedAutoFile, `${autoSource}\n`, "utf8"); + execFileSync("git", ["add", "test/scripts"], { cwd: root, env }); + fs.writeFileSync(stagedManualFile, `${autoSource}\n`, "utf8"); + fs.writeFileSync(stagedAutoFile, `${manualSource}\n`, "utf8"); + + const result = spawnSync( + process.execPath, + [path.join(repoRoot, "scripts", "report-test-temp-creations.mjs"), "--staged", "--json"], + { + cwd: root, + encoding: "utf8", + env, + }, + ); + + expect(result.status).toBe(0); + expect(JSON.parse(result.stdout)).toEqual([ + { + file: "test/scripts/staged-manual.test.ts", + line: 1, + reason: "new manual temp-dir helper import", + source: 'import { makeTempDir } from "../helpers/temp-dir.js";', + }, + { + file: "test/scripts/staged-manual.test.ts", + line: 3, + reason: "new manual temp-dir helper usage", + source: 'const workspace = makeTempDir(tempDirs, "case-");', + }, + ]); + }); + it("exits non-zero for staged findings when requested", () => { const root = tempDirs.make("openclaw-temp-report-"); const env = createNestedGitEnv(); diff --git a/test/scripts/runtime-postbuild-stamp.test.ts b/test/scripts/runtime-postbuild-stamp.test.ts index 18ea07aba7f8..61a3df0838be 100644 --- a/test/scripts/runtime-postbuild-stamp.test.ts +++ b/test/scripts/runtime-postbuild-stamp.test.ts @@ -1,30 +1,26 @@ // Runtime Postbuild Stamp tests cover runtime postbuild stamp script behavior. import fs from "node:fs"; -import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; import { RUNTIME_POSTBUILD_STAMP_FILE } from "../../scripts/lib/local-build-metadata-paths.mjs"; import { writeRuntimePostBuildStamp } from "../../scripts/runtime-postbuild-stamp.mjs"; +import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js"; describe("runtime-postbuild-stamp script", () => { - it("writes dist/.runtime-postbuildstamp with the current git head", () => { - const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-runtime-postbuild-stamp-")); - try { - const stampPath = writeRuntimePostBuildStamp({ - cwd: rootDir, - now: () => 123, - spawnSync: () => ({ status: 0, stdout: "abc123\n" }), - }); + const tempDirs = useAutoCleanupTempDirTracker(); - expect(path.relative(rootDir, stampPath)).toBe( - path.join("dist", RUNTIME_POSTBUILD_STAMP_FILE), - ); - expect(JSON.parse(fs.readFileSync(stampPath, "utf8"))).toEqual({ - syncedAt: 123, - head: "abc123", - }); - } finally { - fs.rmSync(rootDir, { recursive: true, force: true }); - } + it("writes dist/.runtime-postbuildstamp with the current git head", () => { + const rootDir = tempDirs.make("openclaw-runtime-postbuild-stamp-"); + const stampPath = writeRuntimePostBuildStamp({ + cwd: rootDir, + now: () => 123, + spawnSync: () => ({ status: 0, stdout: "abc123\n" }), + }); + + expect(path.relative(rootDir, stampPath)).toBe(path.join("dist", RUNTIME_POSTBUILD_STAMP_FILE)); + expect(JSON.parse(fs.readFileSync(stampPath, "utf8"))).toEqual({ + syncedAt: 123, + head: "abc123", + }); }); });