Files
software-mansion__argent/packages/tool-server/test/tmpdir-env.test.ts
Ignacy Łątka fd978f302d test(tool-server): stop a failed beforeEach from throwing again in teardown (#1091)
Fixes #1076

**Cause** - suites that redirect `os.tmpdir()` for a whole file hoist
the restorer, and the scratch directory it points at, into uninitialized
`let`s that `beforeEach` assigns partway through. Vitest still runs
`afterEach` when `beforeEach` throws, so teardown calls `undefined()`
and hands `undefined` to `fs.rm` - bogus `TypeError`s reported beside
the real failure, and a reader chases the wrong one.

**Fix** - initialize what the teardown reads (`= () => {}` / `= ""`;
`fs.rm("", { force: true })` is a no-op), plus a guard in
`tmpdir-env.test.ts` that scans the suite for the shape so a new
`redirectTmpdir` caller cannot reintroduce it.

**Verified** - injected a throw ahead of the assignment in each affected
`beforeEach`: before, the teardown `TypeError` rode alongside the
injected error; after, only the injected error is reported. The guard
lists all 9 declarations against `origin/main` and passes with the fix.
`npx tsc --build`, `typecheck:tests -w @argent/tool-server`, and the
full package suite (433 files, 6164 tests) are green.

Docs: none - test-only change, no tool, CLI, config key, flow file or
user-facing capability touched.

<details>
<summary>Repro, before and after</summary>

`packages/tool-server/test/react-profiler/dump.test.ts`, throw injected
as the first statement of `beforeEach`:

Before:
```
⎯⎯⎯⎯⎯⎯ Failed Tests 17 ⎯⎯⎯⎯⎯⎯⎯
Error: SIMULATED mkdtemp failure before assignment
TypeError: restoreTmpdir is not a function
```

With only the restorer initialized - the second half of the same hook
still throws:
```
Error: SIMULATED mkdtemp failure before assignment
TypeError: The "path" argument must be of type string or an instance of Buffer or URL. Received undefined
```

After (same for `screen-recording`, `profiler-load-list-e2e`,
`native-profiler-freshness-e2e`, `flow-visual`):
```
Error: SIMULATED failure before assignment
```
</details>

<details>
<summary>Guard against origin/main</summary>

```
- []
+ [
+   "android-perfetto/native-profiler-freshness-e2e.test.ts: let tempDir: string;",
+   "android-perfetto/native-profiler-freshness-e2e.test.ts: let restoreTmpdir: () => void;",
+   "android-perfetto/profiler-load-list-e2e.test.ts: let scratch: string;",
+   "android-perfetto/profiler-load-list-e2e.test.ts: let restoreTmpdir: () => void;",
+   "flows/flow-visual.test.ts: let tmpDir: string;",
+   "react-profiler/dump.test.ts: let restoreTmpdir: () => void;",
+   "react-profiler/dump.test.ts: let scratch: string;",
+   "screen-recording.test.ts: let restoreTmpdir: () => void;",
+   "screen-recording.test.ts: let scratch: string;",
+ ]
```
</details>

<details>
<summary>Class check - what was left alone</summary>

- `packages/telemetry/test/index.test.ts:89` `let restoreOptOut: () =>
void;` - same shape, no window: the assignment is the first statement of
the file's first `beforeEach`, so nothing can throw ahead of it.
- Roughly 90 other hoisted `let`s across tool-server tests are read in
`afterEach` without an initializer (`tmpDir`, `handle`, `exitSpy`, ...)
in suites that never redirect the tmpdir. That is a repo-wide pattern
for a lint rule, not for this fix; the guard is scoped to the
`redirectTmpdir` suites the issue reports.
- Local `npx eslint` on the changed files could not run:
`packages/docs/tsconfig.json` fails to resolve `@docusaurus/tsconfig`
because the root install skips that workspace. Pre-existing and
unrelated; CI's ESLint job is green on this branch.
</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-09-09 20:17:23 +02:00

119 lines
4.1 KiB
TypeScript

import { describe, it, expect, afterEach } from "vitest";
import { readdirSync, readFileSync, statSync } from "node:fs";
import { join, relative } from "node:path";
import { redirectTmpdir } from "./helpers/tmpdir-env";
// The variable os.tmpdir() consults is platform-dependent — POSIX reads TMPDIR,
// TMP, TEMP in that order and Windows reads TEMP then TMP — so a redirect that
// covers only the local platform's name leaves the other one resolving into the
// machine-wide temp directory. The files that scope the tmpdir to catch a leak
// then scan a directory nothing writes to and pass whatever the code does.
const LOOKUPS = {
posix: ["TMPDIR", "TMP", "TEMP"],
win32: ["TEMP", "TMP"],
};
describe("redirectTmpdir", () => {
const saved = new Map<string, string | undefined>();
for (const k of ["TMPDIR", "TEMP", "TMP"]) saved.set(k, process.env[k]);
afterEach(() => {
for (const [k, v] of saved) {
if (v === undefined) delete process.env[k];
else process.env[k] = v;
}
});
it.each(Object.entries(LOOKUPS))(
"redirects the whole %s lookup chain, not just its first entry",
(_platform, chain) => {
const restore = redirectTmpdir("/scratch/owned-by-this-run");
try {
for (const key of chain) {
expect(process.env[key]).toBe("/scratch/owned-by-this-run");
}
} finally {
restore();
}
}
);
it("restores an absent variable by deleting it rather than setting undefined", () => {
delete process.env.TEMP;
process.env.TMPDIR = "/before";
redirectTmpdir("/scratch")();
expect("TEMP" in process.env).toBe(false);
expect(process.env.TMPDIR).toBe("/before");
});
});
/**
* These suites call the restorer from `afterEach` unconditionally, and Vitest
* runs `afterEach` even when `beforeEach` threw. Whatever the teardown reads
* that the assigning hook never got to is then `undefined`, so the teardown
* throws a second TypeError beside the genuine failure - the one a reader
* chases first.
*/
const TEST_ROOT = __dirname;
function testFiles(dir: string, out: string[] = []): string[] {
for (const entry of readdirSync(dir)) {
const p = join(dir, entry);
if (statSync(p).isDirectory()) testFiles(p, out);
else if (p.endsWith(".test.ts")) out.push(p);
}
return out;
}
/** Body of every `name(...)` call in `src`, matched by balancing parentheses. */
function callBodies(src: string, name: string): string {
const bodies: string[] = [];
const calls = new RegExp(String.raw`\b${name}\s*\(`, "g");
for (let call = calls.exec(src); call; call = calls.exec(src)) {
const start = call.index + call[0].length;
let depth = 1;
let i = start;
for (; i < src.length && depth > 0; i++) {
if (src[i] === "(") depth++;
else if (src[i] === ")") depth--;
}
bodies.push(src.slice(start, i - 1));
}
return bodies.join("\n");
}
/** Hoisted `let`/`var` declarations carrying no initializer - `=>` is not one. */
function uninitialized(src: string): Map<string, string> {
const found = new Map<string, string>();
for (const [decl, name] of src.matchAll(/^[ \t]*(?:let|var)[ \t]+(\w+)[^;\n]*;$/gm)) {
if (!decl.replace(/=>/g, "").includes("=")) found.set(name, decl.trim());
}
return found;
}
const RESTORER = /(?:(const|let|var)[ \t]+)?\w+[ \t]*=[ \t]*redirectTmpdir\(/g;
describe("suites that redirect the tmpdir for a whole file", () => {
it("initialize every hoisted variable their teardown reads", () => {
const offenders: string[] = [];
let scanned = 0;
for (const file of testFiles(TEST_ROOT)) {
const src = readFileSync(file, "utf8");
// A block-scoped restorer cannot outlive the block that assigned it.
if (![...src.matchAll(RESTORER)].some((m) => !m[1])) continue;
scanned++;
const teardown = callBodies(src, "afterEach");
for (const [name, decl] of uninitialized(src))
if (new RegExp(String.raw`\b${name}\b`).test(teardown))
offenders.push(`${relative(TEST_ROOT, file)}: ${decl}`);
}
expect(scanned).toBeGreaterThan(0);
expect(offenders).toEqual([]);
});
});