test(capture): cover launch-env helpers and add EnvironmentModification mock

[BANANASJIM]

Summary

Follow-up to #254. Adds unit coverage for the Windows launch-env helpers introduced there
(_build_launch_env / _make_env_mod in src/rdc/capture_core.py) and the mock symbols they
require. No production code is changed.

• tests/mocks/mock_renderdoc.py: add EnvMod / EnvSep enums and the EnvironmentModification
  dataclass (canonical RenderDoc surface) so the Windows path is exercisable under the mock.
• tests/unit/test_capture_core.py: cover _make_env_mod (name/value/mod/sep) and all four
  _build_launch_env branches — non-win32, missing __file__, missing sibling renderdoc.json,
  and the happy path (asserts both ENABLE_VULKAN_RENDERDOC_CAPTURE=1 and
  VK_IMPLICIT_LAYER_PATH equal to the module directory). Adds one execute_and_capture wiring
  test that the env list reaches ExecuteAndInject on win32 and is empty on other platforms.

Why

#254 shipped with manual Windows smoke only; on Linux CI the helpers were almost entirely
uncovered (only the early non-win32 return). These tests close that gap.

Verification

• pixi run lint: pass
• pixi run test: 2999 passed, 6 skipped, 92.94% coverage

Note

The GPU-gated mock-sync test (tests/integration/test_mock_api_sync.py) is intentionally left
unchanged. Registering EnvMod / EnvSep / EnvironmentModification in its curated lists should
be verified against a real renderdoc module and can be a separate change.

Summary by CodeRabbit

• Tests
  • Enhanced test infrastructure to validate environment variable modification handling during capture execution.
  • Added comprehensive test coverage for environment setup across different platforms and configurations.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds EnvMod, EnvSep enums and an EnvironmentModification dataclass to mock_renderdoc.py. Updates the ExecuteAndInject mock to accept and record an env_list argument. Adds unit tests for _make_env_mod, _build_launch_env, and execute_and_capture environment list wiring on Windows and non-Windows paths.

Changes

Environment Modification Mock and Tests

Layer / File(s)	Summary
EnvMod/EnvSep enums and EnvironmentModification dataclass tests/mocks/mock_renderdoc.py	Adds EnvMod (Set, Add), EnvSep (Platform, SemiColon, Colon, NoSep) enums and EnvironmentModification dataclass with name, value, mod, and sep fields. The mock RenderDoc namespace is also extended to expose these three new types.
ExecuteAndInject mock update tests/unit/test_capture_core.py	_make_mock_rd's fake ExecuteAndInject now accepts env_list and records it into the calls dict for assertion use.
Unit tests: env-mod construction and wiring tests/unit/test_capture_core.py	Adds TestMakeEnvMod (verifies name/value, EnvMod.Set, EnvSep.NoSep), TestBuildLaunchEnv (verifies empty list on non-Windows/missing __file__/missing renderdoc.json, and two mods including VK_IMPLICIT_LAYER_PATH when renderdoc.json is present), and TestExecuteAndCaptureEnvWiring (verifies non-empty env_list on Windows with renderdoc.json, empty on non-Windows).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• BANANASJIM/rdc-cli#106: Also expanded the ExecuteAndInject mock signature and the mock RenderDoc API surface in the same mock_renderdoc.py file.
• BANANASJIM/rdc-cli#254: Directly related — introduces the production-side EnvironmentModification/EnvMod/EnvSep usage and execute_and_capture Windows env-list logic that these tests exercise.

Poem

🐰 A bunny named Mod hopped through the code,
With EnvSep.NoSep lighting the road.
She mocked ExecuteAndInject with care,
And built launch envs beyond compare.
"On Windows I'll pass the right list!" she cried,
Then skipped non-Windows with [] inside. 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and concisely summarizes the main change: adding unit test coverage for launch-environment helpers and the EnvironmentModification mock.
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 test/capture-launch-env-coverage

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

tests/unit/test_capture_core.py (1)

551-572: 💤 Low value

Optional: Consider asserting the value of ENABLE_VULKAN_RENDERDOC_CAPTURE.

The test validates the presence and value of VK_IMPLICIT_LAYER_PATH, but does not check that ENABLE_VULKAN_RENDERDOC_CAPTURE has the value "1". Adding this assertion would provide slightly more thorough coverage.

✨ Optional enhancement

 vk_mod = next(m for m in mods if m.name == "VK_IMPLICIT_LAYER_PATH")
 assert vk_mod.value == str(tmp_path.resolve())
+enable_mod = next(m for m in mods if m.name == "ENABLE_VULKAN_RENDERDOC_CAPTURE")
+assert enable_mod.value == "1"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_capture_core.py` around lines 551 - 572, In the
test_win32_with_renderdoc_json_returns_two_mods test method, add an assertion to
validate the value of the ENABLE_VULKAN_RENDERDOC_CAPTURE environment
modification. After the existing assertion for the vk_mod variable (which checks
VK_IMPLICIT_LAYER_PATH), use a similar pattern to retrieve the modification with
name ENABLE_VULKAN_RENDERDOC_CAPTURE from the mods list and assert that its
value equals "1" to ensure complete coverage of the modifications returned by
_build_launch_env.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/unit/test_capture_core.py`:
- Around line 551-572: In the test_win32_with_renderdoc_json_returns_two_mods
test method, add an assertion to validate the value of the
ENABLE_VULKAN_RENDERDOC_CAPTURE environment modification. After the existing
assertion for the vk_mod variable (which checks VK_IMPLICIT_LAYER_PATH), use a
similar pattern to retrieve the modification with name
ENABLE_VULKAN_RENDERDOC_CAPTURE from the mods list and assert that its value
equals "1" to ensure complete coverage of the modifications returned by
_build_launch_env.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 79c14726-499e-43bb-9be2-cf393d35cbce

📥 Commits

Reviewing files that changed from the base of the PR and between 805efa1 and e9818d7.

📒 Files selected for processing (2)

• tests/mocks/mock_renderdoc.py
• tests/unit/test_capture_core.py