Skip to content

test(ci): keep WSL gitconfig fixture suite scoped#4141

Merged
cv merged 1 commit into
mainfrom
fix/wsl-gitconfig-cleanup
May 23, 2026
Merged

test(ci): keep WSL gitconfig fixture suite scoped#4141
cv merged 1 commit into
mainfrom
fix/wsl-gitconfig-cleanup

Conversation

@cv
Copy link
Copy Markdown
Collaborator

@cv cv commented May 23, 2026

Summary

Follow-up to PR #4137 review feedback: keep the temporary git config fixture alive for the full sandbox-base-image test suite. This prevents per-test cleanup from deleting the suite-wide GIT_CONFIG_GLOBAL file after the first test.

Changes

  • Stop registering the suite-wide temporary git config directory in the per-test tmpRoots cleanup list.
  • Remove the full temporary git config directory in afterAll once the suite is finished.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Tests
    • Improved test cleanup logic for sandbox base image tests.

Review Change Stack

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this May 23, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 23, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a2235fbd-dd61-4b14-818d-9b287632dd86

📥 Commits

Reviewing files that changed from the base of the PR and between de3b9ba and 9a3651c.

📒 Files selected for processing (1)
  • src/lib/sandbox-base-image.test.ts

📝 Walkthrough

Walkthrough

This PR refactors temporary test directory cleanup in a sandbox base image test. The emptyGitConfigDir is removed from the per-test cleanup list and instead is explicitly deleted in the final test suite teardown with recursive and forceful options.

Changes

Test Cleanup Refactoring

Layer / File(s) Summary
Git config directory cleanup refactoring
src/lib/sandbox-base-image.test.ts
emptyGitConfigDir is removed from per-test afterEach cleanup and instead explicitly deleted in afterAll with recursive and forceful options.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#4137: Both PRs modify src/lib/sandbox-base-image.test.ts test teardown around the temporary global git config (emptyGitConfig/emptyGitConfigDir) in the afterAll cleanup lifecycle, with this PR's refined emptyGitConfigDir deletion directly building on that prior PR's git-config cleanup changes.

Poem

🐰 A test cleanup dance, swift and true,
From per-test care to final adieu,
Git config dirs swept away with force,
Leaving the test suite clean, of course! ✨

🚥 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 accurately describes the main change: adjusting test fixture cleanup logic to keep the WSL gitconfig fixture suite-scoped rather than per-test scoped.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/wsl-gitconfig-cleanup

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

@github-actions
Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. No E2E is recommended because this PR changes only a unit test file and cannot affect runtime behavior or user-facing flows.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions
Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • None. No scenario workflow, scenario metadata, scenario runtime, or validation-suite files changed.

Optional scenario E2E

  • None.

Relevant changed files

  • None.

@github-actions
Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@cv cv merged commit 42ac98d into main May 23, 2026
22 checks passed
@cv cv deleted the fix/wsl-gitconfig-cleanup branch May 27, 2026 21:16
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.

2 participants