mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-09 02:03:56 +00:00
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 head1fdd7d2a9a. - Required merge gates passed before the squash merge. Prepared head SHA:1fdd7d2a9aReview: https://github.com/openclaw/openclaw/pull/93209#issuecomment-4705653665 Co-authored-by: Mason Huang <masonxhuang@tencent.com> Approved-by: hxy91819
This commit is contained in:
@@ -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):
|
||||
|
||||
|
||||
@@ -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 <reason>" 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;
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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<string>();
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string>;
|
||||
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);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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"));
|
||||
|
||||
@@ -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<string>();",
|
||||
"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<string>();",
|
||||
"+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<string>();",
|
||||
'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();
|
||||
|
||||
@@ -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",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user