fix(test): skip broken opencode shims - #561
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughOpenCode live smoke tests now use a shared availability helper. The helper requires a successful ChangesOpenCode live gate
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: 🔵 Low · up to The PR makes unavailable OpenCode shims skip cleanly, but merge should retain owner awareness for the test helper’s real CLI invocation and for proving that negative cases detect the intended failure modes rather than merely returning false. Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/test_opencode_http.py (2)
26-26: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid importing the live-test module for this helper.
Importing
test_opencode_liveexecutes its module-level availability probe before these tests run. A targetedpytest tests/test_opencode_http.pyrun can therefore launch a real probe and wait up to ten seconds. Move_opencode_runsto a side-effect-free test-support module and import it from both test modules.🤖 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/test_opencode_http.py` at line 26, Move the _opencode_runs helper out of test_opencode_live into a side-effect-free shared test-support module, then update both test_opencode_http.py and test_opencode_live.py to import it from that module. Ensure importing the helper does not execute the live-test module’s availability probe.
55-58: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAssert the version-probe invocation contract.
The subprocess fakes accept all arguments and discard them. These tests pass if the implementation removes
--version, removestimeout=10, or changescheck=False. Record the call and assert the command, timeout,capture_output=True, andcheck=False.As per coding guidelines: “Add new behavior tests at the lowest layer capable of catching the regression: pure-core unit test, then seam test, then sandbox E2E.”
Also applies to: 70-73, 80-82
🤖 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/test_opencode_http.py` around lines 55 - 58, Update the subprocess fakes in the affected version-probe tests to record invocation arguments instead of discarding them, then assert the command includes the expected version probe and that timeout=10, capture_output=True, and check=False are passed. Apply this to each referenced test while preserving their existing result assertions.Source: Coding guidelines
tests/test_opencode_live.py (1)
33-37: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse the executable path returned by
shutil.which.The code checks one PATH resolution, then resolves bare
opencodeagain. IfPATHchanges between these calls, the probe can execute a different binary than the one checked. Store the result and pass it asargv[0].Proposed fix
- if sys.platform == "win32" or shutil.which("opencode") is None: + if sys.platform == "win32": return False + binary = shutil.which("opencode") + if binary is None: + return False try: probe = subprocess.run( - ["opencode", "--version"], capture_output=True, timeout=10, check=False + [binary, "--version"], capture_output=True, timeout=10, check=False )Verify the returned path form on supported POSIX environments.
🤖 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/test_opencode_live.py` around lines 33 - 37, Update the opencode availability check to store the path returned by shutil.which, use that stored path for the subprocess.run probe’s executable argument, and pass it as argv[0] rather than resolving the bare “opencode” name again. Preserve the Windows skip behavior and verify the resolved path form on supported POSIX environments.Source: Linters/SAST tools
🤖 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/test_opencode_http.py`:
- Line 26: Move the _opencode_runs helper out of test_opencode_live into a
side-effect-free shared test-support module, then update both
test_opencode_http.py and test_opencode_live.py to import it from that module.
Ensure importing the helper does not execute the live-test module’s availability
probe.
- Around line 55-58: Update the subprocess fakes in the affected version-probe
tests to record invocation arguments instead of discarding them, then assert the
command includes the expected version probe and that timeout=10,
capture_output=True, and check=False are passed. Apply this to each referenced
test while preserving their existing result assertions.
In `@tests/test_opencode_live.py`:
- Around line 33-37: Update the opencode availability check to store the path
returned by shutil.which, use that stored path for the subprocess.run probe’s
executable argument, and pass it as argv[0] rather than resolving the bare
“opencode” name again. Preserve the Windows skip behavior and verify the
resolved path form on supported POSIX environments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e76beba0-5c72-4983-8c20-70240a3c7365
📒 Files selected for processing (3)
CHANGELOG.mdtests/test_opencode_http.pytests/test_opencode_live.py
26385b4 to
ac9d7d4
Compare
1 participant
What
Harden the zero-token OpenCode live-test gate so stale or broken shims skip cleanly instead of failing during server startup.
Why
shutil.which()only proves that a path resolves. A dead WSL/npm shim still passed the gate and produced one failure plus four startup errors. Fixes #294.How
opencode --versionwith a 10-second timeout before enabling the live module.Testing
uv run pytest tests/test_opencode_http.py -q— 89 passeduv run pytest tests/test_opencode_live.py -q— 5 skipped without OpenCodeuv run pytest -q -n auto— 5,048 passed, 78 skipped, 5 xfailed after deselecting three existing macOS undecodable-filename cases that fail unchanged onmaintrunk check— passedSummary by CodeRabbit
Bug Fixes
Tests
Documentation