[nv-jakob]

Description

Follow-up to the terminate-race fix in test_background.py. Now that the stand-in service really sleeps for 30 s, a failure before terminate() succeeds left it running; kill it in a finally, as the sibling spawn tests already do. On success the child is an unreaped zombie by then, so the kill is a no-op.

Also prepend the stand-in's directory to PYTHONPATH instead of replacing it, so the child keeps the inherited path.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Testing

Ran the test

Checklist

• I have read and understood the contribution guidelines
• I have run the linter and formatter with SKIP=check-copyright-year pre-commit run --all-files
• I have made corresponding changes to the documentation
• I have added tests that prove my fix/feature works (or explained why not)
• I have signed off all my commits (git commit -s) per the DCO

Summary by CodeRabbit

• Tests
  • Improved test isolation by preserving existing Python path settings while adding temporary test resources.
  • Ensured spawned test processes are cleaned up even when assertions fail.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: NVIDIA/IsaacCapture
• Branch: main
• File: .coderabbit.yaml
• Commit: ba15610
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 41bf16a4-0a25-4a32-b884-dde02e847912

📥 Commits

Reviewing files that changed from the base of the PR and between ba15610 and e3febd2.

📒 Files selected for processing: 1

• tests/python/core/cloudxr/test_background.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

The stand_in_service fixture now prepends its temporary directory to PYTHONPATH instead of replacing the existing value. The termination test now attempts to kill the spawned child in a finally block, including when an assertion fails.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Possibly related PRs

• NVIDIA/IsaacCapture#1327: Adds the stand_in_service fixture and the termination test that this PR updates.

Merge Risk: ⚪ Minimal · up to e3feb

No actionable merge-blocking risk is established.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the change accurately. The PR wraps the stand-in cleanup in a finally block so the spawned child is killed when the test fails, which matches the title's focus on cleaning up the stand…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the change accurately. The PR wraps the stand-in cleanup in a finally block so the spawned child is killed when the test fails, which matches the title's focus on cleaning up the stand-in on failure. The title uses the test(cloudxr) scope and is concise. The summary also mentions the PYTHONPATH change, which the title does not cover, but that is a secondary change and the title does not need to cover every detail.

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}