From 4fe0f72195028d87e23867c79704bb0b40643ce6 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Tue, 16 Jun 2026 00:44:03 +0100 Subject: [PATCH] test: fix env-dependent and parallel-process test failures Two orthogonal local test failures, unrelated to product behavior: 1. /tmp/gh-aw transport-file race. create_pull_request.test.cjs and push_to_pull_request_branch.test.cjs (and generate_git_bundle.test.cjs) share the process-global /tmp/gh-aw directory. Each test's beforeEach globbed and deleted every aw-*.patch/aw-*.bundle file in that directory, so when vitest ran the files in parallel processes they deleted each other's in-flight transport files mid-test. Track only the paths each test file creates and delete just those. 2. handle_noop_message.test.cjs depended on an ambient RUNNER_TEMP. getPromptPath() throws unless GH_AW_PROMPTS_DIR or RUNNER_TEMP is set; RUNNER_TEMP is present on CI but not in local dev, so 16 tests threw before reaching their assertions. Set GH_AW_PROMPTS_DIR explicitly in beforeEach (fs.readFileSync is already mocked, so the path is irrelevant). --- actions/setup/js/create_pull_request.test.cjs | 27 ++++++++++++------- actions/setup/js/handle_noop_message.test.cjs | 6 +++++ .../js/push_to_pull_request_branch.test.cjs | 27 ++++++++++++------- 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index c9ee905a6ff..fee0b962b2a 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -14,22 +14,31 @@ const { getBundlePathForBranch, getBundlePathForBranchInRepo } = require("./gene // The privileged handler derives patch/bundle paths from `branch` (and `repo`) // via resolveTransportPaths, so tests must write transport files at the // canonical derived location and let the handler discover them. +// +// `/tmp/gh-aw` is a process-global path shared by every test file. Vitest runs +// test files in parallel processes, so a cleanup that globbed the whole +// directory would delete another file's in-flight transport files mid-test. +// Track only the paths this file created and delete just those. +const createdTransportPaths = new Set(); function canonicalPatchPath(branch, repo) { fs.mkdirSync("/tmp/gh-aw", { recursive: true }); - return repo ? getPatchPathForBranchInRepo(branch, repo) : getPatchPathForBranch(branch); + const p = repo ? getPatchPathForBranchInRepo(branch, repo) : getPatchPathForBranch(branch); + createdTransportPaths.add(p); + return p; } function canonicalBundlePath(branch, repo) { fs.mkdirSync("/tmp/gh-aw", { recursive: true }); - return repo ? getBundlePathForBranchInRepo(branch, repo) : getBundlePathForBranch(branch); + const p = repo ? getBundlePathForBranchInRepo(branch, repo) : getBundlePathForBranch(branch); + createdTransportPaths.add(p); + return p; } function cleanupCanonicalTransports() { - try { - for (const f of fs.readdirSync("/tmp/gh-aw")) { - if (/^aw-.*\.(patch|bundle)$/.test(f)) { - fs.rmSync(`/tmp/gh-aw/${f}`, { force: true }); - } - } - } catch {} + for (const p of createdTransportPaths) { + try { + fs.rmSync(p, { force: true }); + } catch {} + } + createdTransportPaths.clear(); } beforeEach(() => { diff --git a/actions/setup/js/handle_noop_message.test.cjs b/actions/setup/js/handle_noop_message.test.cjs index c6adf5de507..5f21d0a07e6 100644 --- a/actions/setup/js/handle_noop_message.test.cjs +++ b/actions/setup/js/handle_noop_message.test.cjs @@ -20,6 +20,12 @@ describe("handle_noop_message", () => { // Create temp directory for test files tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "handle-noop-test-")); + // getPromptPath() throws unless GH_AW_PROMPTS_DIR or RUNNER_TEMP is set. + // On CI RUNNER_TEMP is ambient, but it is not set in local dev, so set the + // prompts dir explicitly to make the suite environment-independent. The + // actual path is irrelevant because fs.readFileSync is mocked below. + process.env.GH_AW_PROMPTS_DIR = tempDir; + // Mock fs.readFileSync to return template content originalReadFileSync = fs.readFileSync; fs.readFileSync = vi.fn((filePath, encoding) => { diff --git a/actions/setup/js/push_to_pull_request_branch.test.cjs b/actions/setup/js/push_to_pull_request_branch.test.cjs index 7eaaa380acc..30245d667b9 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -9,22 +9,31 @@ const { getBundlePathForBranch, getBundlePathForBranchInRepo } = require("./gene // The privileged handler derives patch/bundle paths from `branch` (and `repo`) // via resolveTransportPaths, so tests must write transport files at the // canonical derived location and let the handler discover them. +// +// `/tmp/gh-aw` is a process-global path shared by every test file. Vitest runs +// test files in parallel processes, so a cleanup that globbed the whole +// directory would delete another file's in-flight transport files mid-test. +// Track only the paths this file created and delete just those. +const createdTransportPaths = new Set(); function canonicalPatchPath(branch, repo) { fs.mkdirSync("/tmp/gh-aw", { recursive: true }); - return repo ? getPatchPathForBranchInRepo(branch, repo) : getPatchPathForBranch(branch); + const p = repo ? getPatchPathForBranchInRepo(branch, repo) : getPatchPathForBranch(branch); + createdTransportPaths.add(p); + return p; } function canonicalBundlePath(branch, repo) { fs.mkdirSync("/tmp/gh-aw", { recursive: true }); - return repo ? getBundlePathForBranchInRepo(branch, repo) : getBundlePathForBranch(branch); + const p = repo ? getBundlePathForBranchInRepo(branch, repo) : getBundlePathForBranch(branch); + createdTransportPaths.add(p); + return p; } function cleanupCanonicalTransports() { - try { - for (const f of fs.readdirSync("/tmp/gh-aw")) { - if (/^aw-.*\.(patch|bundle)$/.test(f)) { - fs.rmSync(`/tmp/gh-aw/${f}`, { force: true }); - } - } - } catch {} + for (const p of createdTransportPaths) { + try { + fs.rmSync(p, { force: true }); + } catch {} + } + createdTransportPaths.clear(); } beforeEach(() => {