Skip to content

fix(test): recreate tmpDir per test in onboard-session.test.ts (#1284)#1562

Merged
cv merged 2 commits intoNVIDIA:mainfrom
ColinM-sys:fix/1284-onboard-session-tmpdir-isolation
Apr 9, 2026
Merged

fix(test): recreate tmpDir per test in onboard-session.test.ts (#1284)#1562
cv merged 2 commits intoNVIDIA:mainfrom
ColinM-sys:fix/1284-onboard-session-tmpdir-isolation

Conversation

@ColinM-sys
Copy link
Copy Markdown
Contributor

@ColinM-sys ColinM-sys commented Apr 7, 2026

Summary

Closes #1284.

The lock-related tests in src/lib/onboard-session.test.ts share a single tmpDir created once at module load. releaseOnboardLock() (and any future test that crashes mid-flight) can leave lock artifacts behind that the existing beforeEach does not clean up — it only clears the module cache and session file. That makes lock cases order-dependent.

Fix

Recreate tmpDir in beforeEach and fs.rmSync it in afterEach, so every test starts from a guaranteed-clean directory.

No production code changes; this is purely test isolation.

Test plan

  • vitest run src/lib/onboard-session.test.ts — 13 lock/session tests still pass.
  • One pre-existing failure (mode & 0o777 === 0o600) is a Windows-only POSIX-permissions limitation that exists on main with or without this change. Confirmed by stashing the patch and re-running on the same checkout.

Summary by CodeRabbit

  • Tests
    • Improved test isolation and cleanup procedures to ensure each test runs with a fresh environment and all temporary artifacts are properly removed.

The previous setup created tmpDir once at module load and shared it
across all tests in the file. Lock files left behind by
releaseOnboardLock() (or any future test that crashes mid-flight) leak
into the next test, making lock-related cases order-dependent.

Recreate tmpDir in beforeEach and remove it in afterEach so every test
starts from a guaranteed-clean directory.

Closes NVIDIA#1284
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 7, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 669bd0ff-c2c0-482b-a015-f1c8abc895ec

📥 Commits

Reviewing files that changed from the base of the PR and between bbec268 and beba464.

📒 Files selected for processing (1)
  • src/lib/onboard-session.test.ts

📝 Walkthrough

Walkthrough

The test file now creates and destroys a fresh temporary directory for each test case in beforeEach and afterEach hooks respectively, replacing the module-level tmpDir pattern. Module cache clearing occurs after directory creation, and cleanup now includes recursive removal of the temporary directory to eliminate leftover lock artifacts between tests.

Changes

Cohort / File(s) Summary
Test Isolation Fix
src/lib/onboard-session.test.ts
Moved tmpDir creation from module-level into beforeEach and added recursive directory cleanup in afterEach. Cache clearing now occurs after directory setup. Resolves test order dependency caused by shared lock files persisting across test runs.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

A rabbit hops through test files with care,
Creating fresh tunnels, wiping lock hairs,
Each test starts clean, no stragglers left behind,
Order dependence? Not this time we'll find! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: fixing a test isolation issue by recreating tmpDir per test in a specific test file, with issue reference.
Linked Issues check ✅ Passed The PR directly addresses issue #1284 by recreating tmpDir in beforeEach and removing it in afterEach to eliminate test order dependence caused by shared lock artifacts.
Out of Scope Changes check ✅ Passed All changes are scoped to test isolation improvements in src/lib/onboard-session.test.ts with no production code modifications, fully aligned with issue #1284 objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@wscurran wscurran added enhancement: testing Use this label to identify requests to improve NemoClaw test coverage. fix labels Apr 8, 2026
@wscurran
Copy link
Copy Markdown
Contributor

wscurran commented Apr 8, 2026

✨ Thanks for submitting this fix, which proposes a way to improve test reliability by ensuring a clean temporary directory for each test case.

@cv cv merged commit 887408f into NVIDIA:main Apr 9, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement: testing Use this label to identify requests to improve NemoClaw test coverage. fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(test): onboard-session lock tests share tmpDir causing order dependence

3 participants