diff --git a/.archgate/adrs/ARCH-005-testing-standards.md b/.archgate/adrs/ARCH-005-testing-standards.md index f9566e23..60680cd4 100644 --- a/.archgate/adrs/ARCH-005-testing-standards.md +++ b/.archgate/adrs/ARCH-005-testing-standards.md @@ -53,6 +53,7 @@ Use Bun's built-in test runner (`bun test`) for all tests. Test files go in `tes - **Make large production thresholds injectable so tests can use a small value** — When production code only acts past a large numeric threshold (e.g., `SCOPE_FILE_WARN_THRESHOLD = 1000` in `src/engine/git-files.ts`), add an optional parameter that defaults to the module constant (e.g., `resolveScopedFiles(root, globs, { fileWarnThreshold })`). Tests inject a tiny value such as `5` and create only a handful of files to exercise the same code path. This keeps the test fast and deterministic on every platform instead of materializing thousands of fixture files. - **Only ever raise a per-test timeout override above the global, never below it** — the project global is `bun test --timeout 60000` (60s). A per-test `}, 30_000` override makes that test _more_ likely to time out than the default. Use a per-test override solely to grant a genuinely slow test more time than 60s; never set one shorter than the global. - **Mock first-party modules with `import * as mod` + `spyOn(mod, "fn")`, not `mock.module()`** — declare `import * as authMod from "../../src/helpers/auth"`, then in `beforeEach` install `spyOn(authMod, "requestDeviceCode").mockResolvedValue(...)` and restore with `mock.restore()` in `afterEach`. `spyOn` is scoped per-test and auto-restored, and it correctly reaches BOTH static named-import consumers (Bun backs named imports with live bindings) AND dynamic `await import()` consumers, because it mutates the single shared module instance. This is the same pattern used for `spyOn(pathsMod, "findProjectRoot")`. +- **When a test must redirect user-scope paths (home directory), mock `os.homedir()` — a runtime `HOME` env override does NOT work** — Bun caches `os.homedir()` on Linux, so setting `process.env.HOME`/`Bun.env.HOME` inside a test is silently ignored and the code under test resolves the REAL home directory. Declare `import * as os from "node:os"`, install `spyOn(os, "homedir").mockReturnValue(tempDir)` in `beforeEach`, and restore with `mockRestore()` in `afterEach` — the spy reaches named `import { homedir }` consumers in other modules via the same live-binding mechanism as first-party `spyOn`. Env-var overrides remain valid ONLY for code that reads `Bun.env.*` directly at call time (e.g., the `APPDATA` branch in `vscode-settings.ts`, or `paths.ts` helpers documented as "resolved at call time"). Production code MUST NOT be rewritten to read `Bun.env.HOME` just to make env overrides work — mock the implementation in the test instead. ### Don't @@ -70,6 +71,7 @@ Use Bun's built-in test runner (`bun test`) for all tests. Test files go in `tes - **Don't use `mock.module()` on a first-party module that any other test file imports** — Bun's `mock.module()` is process-global and retroactive: it replaces the module for ALL test files in the same Bun process and is NOT undone by `mock.restore()`. Symptom: a test that imports the real implementation intermittently receives a mocked one depending on file execution order — a flaky failure that passes in isolation. Do NOT work around this by splitting production code into a separate `-impl` file (or thin delegating wrapper) just so the "real" tests can import an un-mocked path — that contorts the production module layout to serve a test-tool quirk. Fix the test by switching to `spyOn` (see Do's above). `mock.module()` remains acceptable for third-party modules (e.g. `inquirer`, `node:readline`) that no first-party test needs the real implementation of. - **Don't place `mockRestore()` after assertions without `try/finally` protection** — if the assertion throws, the spy is never restored and contaminates subsequent tests. This is especially dangerous when spying on globals like `console.warn` or `globalThis.fetch`, where a leaked spy silently suppresses or redirects output for every test that follows. - **Don't materialize large filesystem fixtures (1000+ files, plus `git add .`) just to cross a production threshold** — on Windows CI runners that filesystem work is slow enough to exceed the per-test timeout, killing the staging subprocess mid-run (`git add . failed (exit 143)`, where 143 = 128 + SIGTERM) and producing a flaky failure that does not reproduce locally. Inject a small threshold instead (see Do's above). This was the root cause of a recurring Windows smoke-test flake on the `resolveScopedFiles` warning test. +- **Don't let tests write real user-scope state** (`~/.config/Code/User/settings.json`, `%APPDATA%`, `~/.cursor/`, `~/.config/opencode/`, etc.) — cross-file pollution combined with Bun's non-deterministic test-file execution order produces order-dependent flakes that pass on a PR run and fail after merge with identical code. This was the root cause of the v0.44.0 release failure: `init-project.test.ts` exercised the real `configureVscodeSettings` with mocked credentials and wrote the runner's real `~/.config/Code/User/settings.json`; `vscode-settings.test.ts` then read the polluted file whenever the file execution order flipped (its `HOME` env override was defeated by Bun's `homedir()` caching). The same writes also silently polluted developers' real VS Code settings on local runs. Either spy out the writer at the unit boundary (e.g., `spyOn(vscodeSettings, "configureVscodeSettings")`) or mock `os.homedir()` (see Do's above) so every write lands in a `mkdtemp` directory. ## Implementation Pattern diff --git a/.claude/agent-memory/archgate-developer/MEMORY.md b/.claude/agent-memory/archgate-developer/MEMORY.md index 8de1cc9a..f4aae448 100644 --- a/.claude/agent-memory/archgate-developer/MEMORY.md +++ b/.claude/agent-memory/archgate-developer/MEMORY.md @@ -20,6 +20,10 @@ Skipping steps 2 or 3 is a workflow violation. The user should NEVER have to inv - [Always commit with --signoff](feedback_git_signoff.md) — DCO CI check rejects commits without `Signed-off-by` +## Approach Guidance + +- [No prod changes for testability](feedback_no_prod_changes_for_tests.md) — mock implementations in tests (spyOn os.homedir works cross-module); never alter prod semantics for test isolation + ## Known Bugs - _(none currently)_ diff --git a/.claude/agent-memory/archgate-developer/feedback_no_prod_changes_for_tests.md b/.claude/agent-memory/archgate-developer/feedback_no_prod_changes_for_tests.md new file mode 100644 index 00000000..4c70f8ce --- /dev/null +++ b/.claude/agent-memory/archgate-developer/feedback_no_prod_changes_for_tests.md @@ -0,0 +1,12 @@ +--- +name: no-prod-changes-for-tests +description: Never change production code semantics (e.g., env-first home resolution) just to make tests isolatable — mock the implementation in tests instead +metadata: + type: feedback +--- + +Do not modify production code behavior solely to enable test isolation. Mock the function implementation in the tests instead. + +**Why:** When fixing the flaky vscode-settings test failures (Bun caches `os.homedir()` on Linux, defeating per-test `HOME` overrides), I changed `getVscodeUserSettingsPath()` to resolve home from `Bun.env.HOME` at call time. The user rejected this: "overriding the caching mechanism only for testing is wrong. the user will never change their home dir." Real users never change HOME mid-process, so the env-first indirection added complexity that only tests needed. + +**How to apply:** When a test needs to redirect user-scope paths (home dir, APPDATA, etc.), use `import * as os from "node:os"` + `spyOn(os, "homedir").mockReturnValue(tempDir)` — verified that Bun's spyOn on a builtin module namespace intercepts named imports in other modules (same live-binding mechanism as the first-party spyOn pattern in ARCH-005). Restore with `mockRestore()` in `afterEach`. Env-var overrides only work for code that reads `Bun.env.*` directly at call time (e.g., the APPDATA branch); they do NOT reach `os.homedir()` on Linux. diff --git a/tests/helpers/init-project.test.ts b/tests/helpers/init-project.test.ts index 53d41b87..234ca1b7 100644 --- a/tests/helpers/init-project.test.ts +++ b/tests/helpers/init-project.test.ts @@ -8,6 +8,7 @@ import { join } from "node:path"; import * as credentialStore from "../../src/helpers/credential-store"; import { initProject } from "../../src/helpers/init-project"; import * as pluginInstall from "../../src/helpers/plugin-install"; +import * as vscodeSettings from "../../src/helpers/vscode-settings"; import { safeRmSync } from "../test-utils"; describe("initProject", () => { @@ -291,12 +292,29 @@ describe("tryInstallPlugin via initProject", () => { test("vscode returns auto-installed", async () => { credSpy.mockResolvedValue({ token: "tok", github_user: "user" }); - const result = await initProject(tempDir, { - installPlugin: true, - editor: "vscode", - }); - expect(result.plugin!.installed).toBe(true); - expect(result.plugin!.autoInstalled).toBe(true); + // With credentials present, configureEditorSettings (vscode branch) writes + // the REAL user-level VS Code settings.json — spy it out so the test never + // touches user state. (This previously polluted ~/.config/Code/User/ + // settings.json on CI runners and dev machines, causing order-dependent + // failures in vscode-settings.test.ts.) + const settingsSpy = spyOn( + vscodeSettings, + "configureVscodeSettings" + ).mockResolvedValue(join(tempDir, ".vscode")); + try { + const result = await initProject(tempDir, { + installPlugin: true, + editor: "vscode", + }); + expect(result.plugin!.installed).toBe(true); + expect(result.plugin!.autoInstalled).toBe(true); + expect(settingsSpy).toHaveBeenCalledWith( + tempDir, + expect.stringContaining("vscode.git") + ); + } finally { + settingsSpy.mockRestore(); + } }); test("claude with CLI available auto-installs", async () => { diff --git a/tests/helpers/plugin-install-cleanup.test.ts b/tests/helpers/plugin-install-cleanup.test.ts index 83805139..e85e0c11 100644 --- a/tests/helpers/plugin-install-cleanup.test.ts +++ b/tests/helpers/plugin-install-cleanup.test.ts @@ -19,42 +19,11 @@ import { } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; - -// --------------------------------------------------------------------------- -// Module mocks — declared before imports that use them. -// --------------------------------------------------------------------------- - -const mockResolveCommand = mock<(name: string) => Promise>(() => - Promise.resolve(null) -); -mock.module("../../src/helpers/platform", () => ({ - resolveCommand: mockResolveCommand, -})); - -// Mock paths so cursorUserDir can be redirected without modifying -// Bun.env.HOME — env changes leak to parallel test files because Bun -// runs all tests in a single process sharing Bun.env. -let cursorDirOverride: string | undefined; -mock.module("../../src/helpers/paths", () => ({ - cursorUserDir: () => - cursorDirOverride ?? - join(Bun.env.HOME ?? Bun.env.USERPROFILE ?? "", ".cursor"), - internalPath: (...parts: string[]) => - join(Bun.env.HOME ?? Bun.env.USERPROFILE ?? "", ".archgate", ...parts), - opencodeConfigDir: () => { - const xdg = Bun.env.XDG_CONFIG_HOME; - const base = - xdg && xdg !== "undefined" - ? xdg - : join(Bun.env.HOME ?? Bun.env.USERPROFILE ?? "", ".config"); - return join(base, "opencode"); - }, -})); - // --------------------------------------------------------------------------- -// Imports under test — loaded AFTER mocks are registered. +// Imports under test // --------------------------------------------------------------------------- +import * as platform from "../../src/helpers/platform"; import { installCursorPlugin, installOpencodePlugin, @@ -106,24 +75,41 @@ let tempDir: string; let savedHome: string | undefined; let savedXdg: string | undefined; +/** + * Per-test spy on resolveCommand so CLI availability checks are deterministic. + * spyOn (not mock.module) — mock.module on a first-party module is + * process-global, replaces the WHOLE module for every other test file, and is + * not undone by mock.restore() (ARCH-005). + */ +let mockResolveCommand: ReturnType; + beforeEach(() => { originalFetch = globalThis.fetch; - mockResolveCommand.mockReset(); - mockResolveCommand.mockImplementation(() => Promise.resolve(null)); + mockResolveCommand = spyOn(platform, "resolveCommand").mockImplementation( + () => Promise.resolve(null) + ); spawnSpy = spyOn(Bun, "spawn").mockImplementation(() => fakeSpawnResult(0)); + // Redirect user-scope paths into a temp dir. cursorUserDir(), + // opencodeConfigDir(), and internalPath() all read Bun.env.HOME / + // XDG_CONFIG_HOME at call time, so an env override (saved and restored + // per-test) isolates the real paths.ts resolvers — no module mock needed. tempDir = realpathSync(mkdtempSync(join(tmpdir(), "archgate-plugin-test-"))); savedHome = Bun.env.HOME; savedXdg = Bun.env.XDG_CONFIG_HOME; + Bun.env.HOME = tempDir; Bun.env.XDG_CONFIG_HOME = tempDir; }); afterEach(() => { globalThis.fetch = originalFetch; spawnSpy.mockRestore(); + mockResolveCommand.mockRestore(); mock.restore(); - Bun.env.HOME = savedHome; - Bun.env.XDG_CONFIG_HOME = savedXdg; + if (savedHome === undefined) delete Bun.env.HOME; + else Bun.env.HOME = savedHome; + if (savedXdg === undefined) delete Bun.env.XDG_CONFIG_HOME; + else Bun.env.XDG_CONFIG_HOME = savedXdg; try { rmSync(tempDir, { recursive: true, force: true }); } catch { @@ -208,15 +194,8 @@ describe("plugin install — stale file cleanup", () => { }); describe("cursor", () => { - // Redirect cursorUserDir() via mock — NOT via Bun.env.HOME, because - // env changes leak to parallel test files in Bun's single-process runner. - beforeEach(() => { - cursorDirOverride = join(tempDir, ".cursor"); - }); - afterEach(() => { - cursorDirOverride = undefined; - }); - + // cursorUserDir() resolves to tempDir/.cursor via the Bun.env.HOME + // override installed in the file-level beforeEach. test("removes archgate-* agents and skills before extraction", async () => { const agentsDir = join(tempDir, ".cursor", "agents"); const skillsDir = join(tempDir, ".cursor", "skills"); diff --git a/tests/helpers/plugin-install.test.ts b/tests/helpers/plugin-install.test.ts index 2c791119..d91f8183 100644 --- a/tests/helpers/plugin-install.test.ts +++ b/tests/helpers/plugin-install.test.ts @@ -9,23 +9,14 @@ import { spyOn, test, } from "bun:test"; - -// --------------------------------------------------------------------------- -// Module mocks — declared before imports that use them. -// --------------------------------------------------------------------------- - -/** Mock resolveCommand so CLI availability checks are deterministic. */ -const mockResolveCommand = mock<(name: string) => Promise>(() => - Promise.resolve(null) -); -mock.module("../../src/helpers/platform", () => ({ - resolveCommand: mockResolveCommand, -})); - +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; // --------------------------------------------------------------------------- -// Imports under test — loaded AFTER mocks are registered. +// Imports under test // --------------------------------------------------------------------------- +import * as platform from "../../src/helpers/platform"; import { buildCursorMarketplaceUrl, buildMarketplaceUrl, @@ -91,18 +82,49 @@ function mockFetch(status: number, body: ArrayBuffer | null = null): void { // --------------------------------------------------------------------------- let spawnSpy: ReturnType; +let tempHome: string; +let savedHome: string | undefined; +let savedXdg: string | undefined; + +/** + * Per-test spy on resolveCommand so CLI availability checks are deterministic. + * spyOn (not mock.module) — mock.module on a first-party module is + * process-global, replaces the WHOLE module for every other test file, and is + * not undone by mock.restore() (ARCH-005). + */ +let mockResolveCommand: ReturnType; beforeEach(() => { originalFetch = globalThis.fetch; - mockResolveCommand.mockReset(); - mockResolveCommand.mockImplementation(() => Promise.resolve(null)); + mockResolveCommand = spyOn(platform, "resolveCommand").mockImplementation( + () => Promise.resolve(null) + ); spawnSpy = spyOn(Bun, "spawn").mockImplementation(() => fakeSpawnResult(0)); + + // Redirect user-scope paths into a temp dir. The install functions create + // directories AND delete stale archgate-* files under cursorUserDir() / + // opencodeConfigDir() / internalPath() before the (mocked) tar extraction — + // without this override they destroy the developer's real installed plugin + // files in ~/.cursor and ~/.config/opencode. All three resolvers read + // Bun.env.HOME / XDG_CONFIG_HOME at call time, so an env override works. + tempHome = mkdtempSync(join(tmpdir(), "archgate-plugin-install-")); + savedHome = Bun.env.HOME; + savedXdg = Bun.env.XDG_CONFIG_HOME; + Bun.env.HOME = tempHome; + delete Bun.env.XDG_CONFIG_HOME; }); afterEach(() => { globalThis.fetch = originalFetch; spawnSpy.mockRestore(); + mockResolveCommand.mockRestore(); mock.restore(); + + if (savedHome === undefined) delete Bun.env.HOME; + else Bun.env.HOME = savedHome; + if (savedXdg === undefined) delete Bun.env.XDG_CONFIG_HOME; + else Bun.env.XDG_CONFIG_HOME = savedXdg; + rmSync(tempHome, { recursive: true, force: true }); }); // --------------------------------------------------------------------------- diff --git a/tests/helpers/session-context-copilot.test.ts b/tests/helpers/session-context-copilot.test.ts index 4cefa479..301f45c0 100644 --- a/tests/helpers/session-context-copilot.test.ts +++ b/tests/helpers/session-context-copilot.test.ts @@ -1,8 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync, rmSync, writeFileSync } from "node:fs"; -import { homedir } from "node:os"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { readCopilotSession } from "../../src/helpers/session-context-copilot"; @@ -13,24 +13,28 @@ describe("readCopilotSession", () => { const uniqueId = `test-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; // Use a fake project root that we'll put in workspace.yaml cwd const projectRoot = resolve(`/__archgate_copilot_test_${uniqueId}`); + let tempHome: string; + let savedHome: string | undefined; let stateDir: string; beforeEach(() => { - stateDir = join(homedir(), ".copilot", "session-state"); + // Redirect copilotSessionStateDir() into a temp dir so these tests never + // touch the real ~/.copilot/session-state. The resolver reads Bun.env.HOME + // at call time (paths.ts), so an env override works here — unlike + // os.homedir() consumers, which require mocking (ARCH-005). + tempHome = mkdtempSync(join(tmpdir(), "archgate-copilot-session-")); + savedHome = Bun.env.HOME; + Bun.env.HOME = tempHome; + stateDir = join(tempHome, ".copilot", "session-state"); mkdirSync(stateDir, { recursive: true }); }); afterEach(() => { - // Clean up only the session dirs we created (identifiable by uniqueId in events) - // We use a tracking array instead - for (const dir of createdDirs) { - rmSync(dir, { recursive: true, force: true }); - } - createdDirs.length = 0; + if (savedHome === undefined) delete Bun.env.HOME; + else Bun.env.HOME = savedHome; + rmSync(tempHome, { recursive: true, force: true }); }); - const createdDirs: string[] = []; - function makeSession( sessionId: string, cwd: string, @@ -38,7 +42,6 @@ describe("readCopilotSession", () => { ): void { const sessionDir = join(stateDir, sessionId); mkdirSync(sessionDir, { recursive: true }); - createdDirs.push(sessionDir); // Write workspace.yaml — use JSON.stringify to escape backslashes // in Windows paths (YAML double-quoted strings use the same escaping) @@ -186,7 +189,6 @@ describe("readCopilotSession", () => { const sessionId = `copilot-${uniqueId}-bad`; const sessionDir = join(stateDir, sessionId); mkdirSync(sessionDir, { recursive: true }); - createdDirs.push(sessionDir); writeFileSync( join(sessionDir, "workspace.yaml"), `cwd: ${JSON.stringify(projectRoot)}\nid: ${JSON.stringify(sessionId)}\n` diff --git a/tests/helpers/session-context-cursor.test.ts b/tests/helpers/session-context-cursor.test.ts index eefd1fed..77e07c13 100644 --- a/tests/helpers/session-context-cursor.test.ts +++ b/tests/helpers/session-context-cursor.test.ts @@ -1,8 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync, rmSync, writeFileSync } from "node:fs"; -import { homedir } from "node:os"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import * as os from "node:os"; import { join } from "node:path"; import { readCursorSession } from "../../src/helpers/session-context"; @@ -11,20 +11,24 @@ import { readCursorSession } from "../../src/helpers/session-context"; // Error cases for readCursorSession live in session-context.test.ts. describe("readCursorSession", () => { - // Use a unique encoded project name under the *real* homedir so that - // homedir() caching on Linux doesn't break the tests. - const uniqueId = `test-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; - const projectRoot = `/__archgate_cursor_test_${uniqueId}`; + // Redirect homedir() into a temp dir so these tests never touch the real + // ~/.cursor/projects. A HOME env override does NOT work here — Bun caches + // homedir() on Linux — so the implementation is mocked instead (ARCH-005). + const projectRoot = "/__archgate_cursor_test_project"; const encodedProject = projectRoot .replaceAll("/", "-") .replaceAll("\\", "-") .replaceAll(":", "") .replaceAll(".", "-"); + let tempHome: string; + let homedirSpy: ReturnType; let transcriptsDir: string; beforeEach(() => { + tempHome = mkdtempSync(join(os.tmpdir(), "archgate-cursor-session-")); + homedirSpy = spyOn(os, "homedir").mockReturnValue(tempHome); transcriptsDir = join( - homedir(), + tempHome, ".cursor", "projects", encodedProject, @@ -34,9 +38,8 @@ describe("readCursorSession", () => { }); afterEach(() => { - // Clean up from .cursor/projects/ level - const projectDir = join(homedir(), ".cursor", "projects", encodedProject); - rmSync(projectDir, { recursive: true, force: true }); + homedirSpy.mockRestore(); + rmSync(tempHome, { recursive: true, force: true }); }); function makeSession(sessionId: string, lines: string[]): void { diff --git a/tests/helpers/session-context.test.ts b/tests/helpers/session-context.test.ts index d0099c7c..70ec25e9 100644 --- a/tests/helpers/session-context.test.ts +++ b/tests/helpers/session-context.test.ts @@ -1,8 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync, rmSync, writeFileSync } from "node:fs"; -import { homedir } from "node:os"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import * as os from "node:os"; import { join } from "node:path"; import { @@ -102,24 +102,29 @@ describe("readClaudeCodeSession", () => { }); describe("happy path", () => { - // Use a unique encoded project name under the *real* homedir so that - // homedir() caching on Linux doesn't break the tests. - const uniqueId = `test-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; - const projectRoot = `/__archgate_test_${uniqueId}`; + // Redirect homedir() into a temp dir so these tests never touch the real + // ~/.claude/projects. A HOME env override does NOT work here — Bun caches + // homedir() on Linux — so the implementation is mocked instead (ARCH-005). + const projectRoot = "/__archgate_test_project"; const encodedProject = projectRoot .replaceAll("/", "-") .replaceAll("\\", "-") .replaceAll(":", "-") .replaceAll(".", "-"); + let tempHome: string; + let homedirSpy: ReturnType; let projectsDir: string; beforeEach(() => { - projectsDir = join(homedir(), ".claude", "projects", encodedProject); + tempHome = mkdtempSync(join(os.tmpdir(), "archgate-claude-session-")); + homedirSpy = spyOn(os, "homedir").mockReturnValue(tempHome); + projectsDir = join(tempHome, ".claude", "projects", encodedProject); mkdirSync(projectsDir, { recursive: true }); }); afterEach(() => { - rmSync(projectsDir, { recursive: true, force: true }); + homedirSpy.mockRestore(); + rmSync(tempHome, { recursive: true, force: true }); }); function writeSession(entries: object[]): void { diff --git a/tests/helpers/vscode-settings.test.ts b/tests/helpers/vscode-settings.test.ts index 6bb4758b..bf995336 100644 --- a/tests/helpers/vscode-settings.test.ts +++ b/tests/helpers/vscode-settings.test.ts @@ -1,9 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate -import { describe, expect, test, beforeEach, afterEach } from "bun:test"; +import { describe, expect, test, beforeEach, afterEach, spyOn } from "bun:test"; import { mkdtempSync, rmSync, existsSync, mkdirSync } from "node:fs"; -import { homedir } from "node:os"; -import { tmpdir } from "node:os"; +import * as os from "node:os"; import { join } from "node:path"; import { @@ -19,6 +18,21 @@ import { getVscodeUserSettingsPath, } from "../../src/helpers/vscode-settings"; +/** + * Restore saved env vars, deleting keys that were originally unset. + * `Object.assign(process.env, saved)` would stringify `undefined` into the + * literal "undefined", corrupting the env for subsequent test files. + */ +function restoreEnv(saved: Record): void { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } +} + describe("mergeMarketplaceUrl", () => { const URL = "https://user:token@plugins.archgate.dev/archgate.git"; @@ -66,16 +80,23 @@ describe("mergeMarketplaceUrl", () => { describe("configureVscodeSettings", () => { let tempDir: string; let savedEnv: Record; + let homedirSpy: ReturnType; beforeEach(() => { - tempDir = mkdtempSync(join(tmpdir(), "archgate-vscode-settings-test-")); - savedEnv = { APPDATA: process.env.APPDATA, HOME: process.env.HOME }; + tempDir = mkdtempSync(join(os.tmpdir(), "archgate-vscode-settings-test-")); + // Redirect user-scope path resolution into tempDir so these tests NEVER + // touch the real VS Code settings.json: + // - Windows branch reads Bun.env.APPDATA → env override works + // - macOS/Linux branches call os.homedir() → must be mocked, because Bun + // caches homedir() on Linux and ignores runtime HOME env overrides + savedEnv = { APPDATA: process.env.APPDATA }; process.env.APPDATA = tempDir; - process.env.HOME = tempDir; + homedirSpy = spyOn(os, "homedir").mockReturnValue(tempDir); }); afterEach(() => { - Object.assign(process.env, savedEnv); + homedirSpy.mockRestore(); + restoreEnv(savedEnv); rmSync(tempDir, { recursive: true, force: true }); }); @@ -124,17 +145,20 @@ describe("configureVscodeSettings", () => { describe("addMarketplaceToUserSettings", () => { let tempDir: string; let savedEnv: Record; + let homedirSpy: ReturnType; beforeEach(() => { - tempDir = mkdtempSync(join(tmpdir(), "archgate-user-settings-test-")); - // Save and override env so getVscodeUserSettingsPath() resolves into tempDir - savedEnv = { APPDATA: process.env.APPDATA, HOME: process.env.HOME }; + tempDir = mkdtempSync(join(os.tmpdir(), "archgate-user-settings-test-")); + // Redirect user-scope path resolution into tempDir so these tests NEVER + // touch the real VS Code settings.json (see configureVscodeSettings above). + savedEnv = { APPDATA: process.env.APPDATA }; process.env.APPDATA = tempDir; // Windows - process.env.HOME = tempDir; // macOS/Linux (homedir()) + homedirSpy = spyOn(os, "homedir").mockReturnValue(tempDir); // macOS/Linux }); afterEach(() => { - Object.assign(process.env, savedEnv); + homedirSpy.mockRestore(); + restoreEnv(savedEnv); rmSync(tempDir, { recursive: true, force: true }); }); @@ -199,7 +223,7 @@ describe("addMarketplaceToUserSettings", () => { // Use a deeply nested subdir that definitely doesn't exist yet const deepHome = join(tempDir, "non", "existent", "deep"); process.env.APPDATA = deepHome; // Windows - process.env.HOME = deepHome; // macOS/Linux + homedirSpy.mockReturnValue(deepHome); // macOS/Linux // addMarketplaceToUserSettings should create the entire directory tree await addMarketplaceToUserSettings(URL); @@ -259,7 +283,7 @@ describe("getVscodeUserSettingsPath", () => { if (isWindows()) { // Windows: %APPDATA%/Code/User/settings.json const appData = ( - process.env.APPDATA ?? join(homedir(), "AppData", "Roaming") + process.env.APPDATA ?? join(os.homedir(), "AppData", "Roaming") ).replaceAll("\\", "/"); expect(normalized.startsWith(appData.replaceAll("\\", "/"))).toBe(true); } else if (isMacOS()) { @@ -269,7 +293,7 @@ describe("getVscodeUserSettingsPath", () => { ); } else if (!isWSL()) { // Linux (non-WSL): ~/.config/Code/User/settings.json - const home = homedir().replaceAll("\\", "/"); + const home = os.homedir().replaceAll("\\", "/"); expect(normalized.startsWith(home)).toBe(true); expect(normalized).toContain(".config/Code/User/settings.json"); }