Skip to content

refactor(test): extract createTempDir shared test helper#814

Open
kelsonpw wants to merge 6 commits into
mainfrom
refactor/extract-temp-dir-helper
Open

refactor(test): extract createTempDir shared test helper#814
kelsonpw wants to merge 6 commits into
mainfrom
refactor/extract-temp-dir-helper

Conversation

@kelsonpw

@kelsonpw kelsonpw commented May 17, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds a single shared createTempDir(prefix?) helper at src/utils/__tests__/helpers/temp-dir.ts that consolidates ~75 inlined fs.mkdtempSync(path.join(os.tmpdir(), 'prefix-')) + fs.rmSync(dir, { recursive: true, force: true }) blocks scattered across src/**/__tests__/.
  • Migrates 27 test files (utils, frameworks, lib/session-checkpoint, lib/agent-state) to use the helper. Each call site preserves its prior beforeEach/afterEach semantics and any test-specific env-var stubbing — the helper only owns the temp directory itself.
  • Helper has its own 5-test suite (temp-dir.test.ts) covering uniqueness, prefix normalisation, cleanup idempotency, and default-prefix behaviour.

Helper shape

export interface TempDirHandle {
  readonly dir: string;
  readonly cleanup: () => void;
}

export function createTempDir(prefix: string = 'wizard-test-'): TempDirHandle {
  const normalisedPrefix = prefix.endsWith('-') ? prefix : `${prefix}-`;
  const dir = fs.mkdtempSync(path.join(os.tmpdir(), normalisedPrefix));
  return {
    dir,
    cleanup: () => fs.rmSync(dir, { recursive: true, force: true }),
  };
}

I chose the { dir, cleanup } shape over a useTempDir flavour that auto-registers vitest hooks because (a) it maps 1:1 to the existing inline patterns — same number of lines, same scoping — and (b) several call sites need the dir inside an it() block with their own try/finally, which the auto-hook flavour wouldn't support cleanly.

Files migrated (27)

  • utils (16): api-key-store, apply-lock, atomic-write, cleanup-shell-rc, debug, file-utils, install-id, last-used-selection, orchestrator-context, package-json-light, package-manager, port-detection, project-marker, setup-utils, storage-migration, storage-migration-exdev, storage-paths, update-notifier
  • frameworks (10): django/utils, fastapi/utils, flask/utils, javascript-web/utils, javascript-web-wizard-agent, nextjs/utils, python/utils, python/detect-ignores-node-modules, python/preflight, vue/vue-wizard-agent
  • lib (3): agent-state, agent-state-hydrate, session-checkpoint

Follow-up

~48 more test files still inline mkdtempSync. They're a mechanical migration to this same helper but stayed out of this PR to keep it reviewable; opening a follow-up issue/PR is straightforward.

Patterns NOT touched

  • src/utils/wizard-abort.ts — out of scope per spec.
  • Non-test source code — untouched.
  • Non-mkdtempSync patterns (mock setup, env-var stubbing, snapshot teardown) — kept inline.
  • The handful of mkdtempSync call sites that use ad-hoc prefixes inside it() blocks were migrated to inline createTempDir(...) + try/finally cleanup() calls, matching the file's existing style.

Test plan

  • pnpm test — 4474/4474 pass (same count as baseline f49da82d).
  • pnpm exec tsc --noEmit — clean.
  • pnpm exec eslint on changed files — clean.
  • Spot-checked pnpm exec vitest run --pool=forks --maxWorkers=1 for each migrated file as I went.
  • wizard-abort.ts untouched (verified via git diff).
  • Worktree at /tmp/test-helpers to be removed after merge.

Note

Low Risk
Low risk because changes are confined to test code and primarily refactor temp-dir setup/teardown, with a small new helper covered by dedicated unit tests.

Overview
Introduces a shared createTempDir(prefix?) test helper returning { dir, cleanup }, standardizing temp-directory creation and idempotent cleanup under os.tmpdir().

Refactors a large set of existing tests across CLI/commands/frameworks/lib/ui/utils to replace inline mkdtempSync/rmSync patterns with the helper (including cases with multiple temp dirs and try/finally cleanup), and adds a focused test suite validating prefix normalization, uniqueness, and cleanup behavior.

Reviewed by Cursor Bugbot for commit 424da78. Bugbot is set up for automated code reviews on this repo. Configure here.

@kelsonpw kelsonpw requested a review from a team as a code owner May 17, 2026 07:12
@kelsonpw kelsonpw force-pushed the refactor/extract-temp-dir-helper branch from 18a9eb5 to 7b8c420 Compare May 18, 2026 03:51
@kelsonpw kelsonpw removed the request for review from a team May 18, 2026 18:04
kelsonpw added a commit that referenced this pull request May 22, 2026
…ollow-up) (#819)

* test(helpers): introduce createTempDir shared test helper

Adds src/utils/__tests__/helpers/temp-dir.ts. ~75 test files inline the
same mkdtempSync + rmSync boilerplate, and the helper consolidates that
into one returning { dir, cleanup } so migrations are a pure refactor
with identical setup/teardown semantics.

* test(utils): migrate batch 1 to createTempDir helper

Migrates 6 src/utils/__tests__ files to use the shared createTempDir
helper instead of inlined mkdtempSync + rmSync blocks. Identical setup
and teardown semantics; only the temp-directory boilerplate moves.

* test(utils): migrate batch 2 to createTempDir helper

Continues the temp-dir helper migration across src/utils/__tests__:
file-utils, last-used-selection, orchestrator-context, package-json-light,
package-manager, port-detection, project-marker, setup-utils,
update-notifier, storage-paths, storage-migration, and
storage-migration-exdev. No behaviour change.

* test(frameworks): migrate framework tests to createTempDir helper

Migrates 10 framework test files (django/fastapi/flask/javascript-web/
nextjs/python utils plus python preflight/detect-ignores, javascript-web
agent, vue agent) to import createTempDir from the shared helpers
module. No behaviour change.

* test(lib): migrate session-checkpoint and agent-state tests to createTempDir

Continues the temp-dir helper migration across src/lib/__tests__ for the
session-checkpoint, agent-state, and agent-state-hydrate suites. Identical
beforeEach/afterEach semantics.

* test(lib): migrate single-call lib tests to createTempDir (batch 1)

Migrates 9 src/lib/__tests__/ files from inlined `mkdtempSync` +
`rmSync` patterns to the shared `createTempDir` helper. Preserves
existing `beforeEach`/`afterEach` semantics — only the temp directory
ownership moves to the helper.

Files migrated:
- agent-interface.test.ts
- agent-ops.test.ts
- claude-settings-scope.test.ts
- dashboard-plan.test.ts (drops local makeTmpDir/cleanup wrappers)
- detect-amplitude.test.ts
- diagnostics-collector.test.ts (drops local wrappers; keeps os for homedir())
- event-plan-parser.test.ts
- integration-skill-resolve.test.ts
- package-manager-detection.test.ts (drops local wrappers)

* test(lib): migrate multi-call lib tests to createTempDir (batch 2)

Migrates 7 src/lib/__tests__/ files that contain 2-4 mkdtempSync sites
each. The wizard-tools.test.ts file keeps its makeTmpDir/cleanup
wrappers (called from 30+ scattered describe blocks) and delegates them
to createTempDir to avoid touching every callsite.

Files migrated:
- wizard-tools.test.ts (wrappers delegate to helper)
- wizard-mcp-server.test.ts
- file-change-ledger.integration.test.ts
- agent-plans.test.ts
- pre-stage-skills.test.ts
- session-checkpoint-events.test.ts
- workspace-analysis.test.ts

* test(lib): migrate remaining lib/__tests__ files to createTempDir (batch 3)

Migrates the rest of src/lib/__tests__/ files using mkdtempSync. These
hold 1-4 sites each, with patterns including beforeEach/afterEach blocks,
nested it()-level temp dirs (try/finally), and best-effort try/catch
cleanup.

Files migrated:
- download-skill.test.ts (4 sites, all in it() blocks)
- ampli-config.test.ts (4 sites, distinct describes)
- file-change-ledger.test.ts (2 sites, one in nested it())
- self-heal.test.ts (3 sites; preserves os.homedir() use)
- agent-runner.test.ts (3 sites, distinct describes)

* test(lib): migrate lib subdirectory tests to createTempDir (batch 4)

Migrates mkdtempSync sites in src/lib/{agent,middleware,observability,
orchestration,wizard-tools}/__tests__/ to the shared createTempDir
helper.

Files migrated:
- agent/preflight-context.test.ts (3 sites: 2 beforeEach + 1 in-it loop)
- agent/project-size.test.ts (drops local makeTmpDir wrapper)
- middleware/config.test.ts (2 sites, dual cleanup vars)
- observability/logger.test.ts (keeps rmSync for mid-test simulation)
- wizard-tools/bundled-skills-index.test.ts (1 site in try/finally)
- orchestration/store.test.ts (drops manual setup/teardown helpers)
- orchestration/last-stopping-point.test.ts

* test(ui): migrate TUI screen tests to createTempDir (batch 5)

Migrates 8 src/ui/tui/screens/__tests__/ files to the shared
createTempDir helper. Each test file's cleanup pattern (try/catch
best-effort or simple rmSync) is preserved.

Files migrated:
- OutroScreen.overdraw.test.tsx (2 it()-block sites; no cleanup pre-existing)
- IntroScreen.snap.test.tsx (1 try/finally + 1 beforeEach)
- OutroScreen.preserveFiles.test.tsx
- OutroScreen.cancelFileState.test.tsx
- OutroScreen.error.test.tsx (preserves tmpdir() use in module-scope literal)
- OutroScreen.eventList.test.tsx
- RunScreen.eventsTab.test.tsx
- DataIngestionCheckScreen.tracking-plan.test.tsx

* test: migrate remaining mkdtempSync sites to createTempDir (batch 6)

Final batch — migrates the last 10 test files holding inlined
mkdtempSync calls (TUI primitives/components/utils, root commands,
steps, top-level cli test). After this batch, the only remaining
mkdtempSync call in src/**/__tests__/ is inside the helper's own test.

Files migrated:
- src/__tests__/cli.test.ts (5 sites; mix of beforeEach + in-test dirs)
- src/commands/__tests__/orchestration.test.ts
- src/commands/__tests__/apply-lock-held.test.ts
- src/steps/__tests__/create-dashboard.test.ts
- src/ui/tui/__tests__/store.test.ts (createStore() helper)
- src/ui/tui/components/__tests__/PathInput.test.ts (keeps homedir())
- src/ui/tui/primitives/__tests__/LogViewer.snap.test.tsx
- src/ui/tui/primitives/__tests__/LogViewer.collapse.test.tsx
- src/ui/tui/primitives/__tests__/ReportViewer.overdraw.test.tsx
- src/ui/tui/utils/__tests__/welcome-back-context.test.ts
@kelsonpw kelsonpw enabled auto-merge (squash) May 22, 2026 17:58
kelsonpw added 6 commits May 22, 2026 11:04
Adds src/utils/__tests__/helpers/temp-dir.ts. ~75 test files inline the
same mkdtempSync + rmSync boilerplate, and the helper consolidates that
into one returning { dir, cleanup } so migrations are a pure refactor
with identical setup/teardown semantics.
Migrates 6 src/utils/__tests__ files to use the shared createTempDir
helper instead of inlined mkdtempSync + rmSync blocks. Identical setup
and teardown semantics; only the temp-directory boilerplate moves.
Continues the temp-dir helper migration across src/utils/__tests__:
file-utils, last-used-selection, orchestrator-context, package-json-light,
package-manager, port-detection, project-marker, setup-utils,
update-notifier, storage-paths, storage-migration, and
storage-migration-exdev. No behaviour change.
Migrates 10 framework test files (django/fastapi/flask/javascript-web/
nextjs/python utils plus python preflight/detect-ignores, javascript-web
agent, vue agent) to import createTempDir from the shared helpers
module. No behaviour change.
…TempDir

Continues the temp-dir helper migration across src/lib/__tests__ for the
session-checkpoint, agent-state, and agent-state-hydrate suites. Identical
beforeEach/afterEach semantics.
…ollow-up) (#819)

* test(helpers): introduce createTempDir shared test helper

Adds src/utils/__tests__/helpers/temp-dir.ts. ~75 test files inline the
same mkdtempSync + rmSync boilerplate, and the helper consolidates that
into one returning { dir, cleanup } so migrations are a pure refactor
with identical setup/teardown semantics.

* test(utils): migrate batch 1 to createTempDir helper

Migrates 6 src/utils/__tests__ files to use the shared createTempDir
helper instead of inlined mkdtempSync + rmSync blocks. Identical setup
and teardown semantics; only the temp-directory boilerplate moves.

* test(utils): migrate batch 2 to createTempDir helper

Continues the temp-dir helper migration across src/utils/__tests__:
file-utils, last-used-selection, orchestrator-context, package-json-light,
package-manager, port-detection, project-marker, setup-utils,
update-notifier, storage-paths, storage-migration, and
storage-migration-exdev. No behaviour change.

* test(frameworks): migrate framework tests to createTempDir helper

Migrates 10 framework test files (django/fastapi/flask/javascript-web/
nextjs/python utils plus python preflight/detect-ignores, javascript-web
agent, vue agent) to import createTempDir from the shared helpers
module. No behaviour change.

* test(lib): migrate session-checkpoint and agent-state tests to createTempDir

Continues the temp-dir helper migration across src/lib/__tests__ for the
session-checkpoint, agent-state, and agent-state-hydrate suites. Identical
beforeEach/afterEach semantics.

* test(lib): migrate single-call lib tests to createTempDir (batch 1)

Migrates 9 src/lib/__tests__/ files from inlined `mkdtempSync` +
`rmSync` patterns to the shared `createTempDir` helper. Preserves
existing `beforeEach`/`afterEach` semantics — only the temp directory
ownership moves to the helper.

Files migrated:
- agent-interface.test.ts
- agent-ops.test.ts
- claude-settings-scope.test.ts
- dashboard-plan.test.ts (drops local makeTmpDir/cleanup wrappers)
- detect-amplitude.test.ts
- diagnostics-collector.test.ts (drops local wrappers; keeps os for homedir())
- event-plan-parser.test.ts
- integration-skill-resolve.test.ts
- package-manager-detection.test.ts (drops local wrappers)

* test(lib): migrate multi-call lib tests to createTempDir (batch 2)

Migrates 7 src/lib/__tests__/ files that contain 2-4 mkdtempSync sites
each. The wizard-tools.test.ts file keeps its makeTmpDir/cleanup
wrappers (called from 30+ scattered describe blocks) and delegates them
to createTempDir to avoid touching every callsite.

Files migrated:
- wizard-tools.test.ts (wrappers delegate to helper)
- wizard-mcp-server.test.ts
- file-change-ledger.integration.test.ts
- agent-plans.test.ts
- pre-stage-skills.test.ts
- session-checkpoint-events.test.ts
- workspace-analysis.test.ts

* test(lib): migrate remaining lib/__tests__ files to createTempDir (batch 3)

Migrates the rest of src/lib/__tests__/ files using mkdtempSync. These
hold 1-4 sites each, with patterns including beforeEach/afterEach blocks,
nested it()-level temp dirs (try/finally), and best-effort try/catch
cleanup.

Files migrated:
- download-skill.test.ts (4 sites, all in it() blocks)
- ampli-config.test.ts (4 sites, distinct describes)
- file-change-ledger.test.ts (2 sites, one in nested it())
- self-heal.test.ts (3 sites; preserves os.homedir() use)
- agent-runner.test.ts (3 sites, distinct describes)

* test(lib): migrate lib subdirectory tests to createTempDir (batch 4)

Migrates mkdtempSync sites in src/lib/{agent,middleware,observability,
orchestration,wizard-tools}/__tests__/ to the shared createTempDir
helper.

Files migrated:
- agent/preflight-context.test.ts (3 sites: 2 beforeEach + 1 in-it loop)
- agent/project-size.test.ts (drops local makeTmpDir wrapper)
- middleware/config.test.ts (2 sites, dual cleanup vars)
- observability/logger.test.ts (keeps rmSync for mid-test simulation)
- wizard-tools/bundled-skills-index.test.ts (1 site in try/finally)
- orchestration/store.test.ts (drops manual setup/teardown helpers)
- orchestration/last-stopping-point.test.ts

* test(ui): migrate TUI screen tests to createTempDir (batch 5)

Migrates 8 src/ui/tui/screens/__tests__/ files to the shared
createTempDir helper. Each test file's cleanup pattern (try/catch
best-effort or simple rmSync) is preserved.

Files migrated:
- OutroScreen.overdraw.test.tsx (2 it()-block sites; no cleanup pre-existing)
- IntroScreen.snap.test.tsx (1 try/finally + 1 beforeEach)
- OutroScreen.preserveFiles.test.tsx
- OutroScreen.cancelFileState.test.tsx
- OutroScreen.error.test.tsx (preserves tmpdir() use in module-scope literal)
- OutroScreen.eventList.test.tsx
- RunScreen.eventsTab.test.tsx
- DataIngestionCheckScreen.tracking-plan.test.tsx

* test: migrate remaining mkdtempSync sites to createTempDir (batch 6)

Final batch — migrates the last 10 test files holding inlined
mkdtempSync calls (TUI primitives/components/utils, root commands,
steps, top-level cli test). After this batch, the only remaining
mkdtempSync call in src/**/__tests__/ is inside the helper's own test.

Files migrated:
- src/__tests__/cli.test.ts (5 sites; mix of beforeEach + in-test dirs)
- src/commands/__tests__/orchestration.test.ts
- src/commands/__tests__/apply-lock-held.test.ts
- src/steps/__tests__/create-dashboard.test.ts
- src/ui/tui/__tests__/store.test.ts (createStore() helper)
- src/ui/tui/components/__tests__/PathInput.test.ts (keeps homedir())
- src/ui/tui/primitives/__tests__/LogViewer.snap.test.tsx
- src/ui/tui/primitives/__tests__/LogViewer.collapse.test.tsx
- src/ui/tui/primitives/__tests__/ReportViewer.overdraw.test.tsx
- src/ui/tui/utils/__tests__/welcome-back-context.test.ts
@kelsonpw kelsonpw force-pushed the refactor/extract-temp-dir-helper branch from ef1d220 to 424da78 Compare May 22, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant