test(runtime): cover subprocess execution failures - #6854
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new test uses a non-existent errors.AsType helper and will not compile without switching to errors.As.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds regression coverage in the runtime command runner tests to ensure subprocess startup/exit failures preserve both combined output (stdout+stderr) and typed error causes, preventing regressions where stderr or exit information is lost.
Changes:
- Add test coverage for non-zero exit handling (including preserving stdout/stderr output).
- Add test coverage for command start failures (missing executable / missing working directory) and verify error context.
File summaries
| File | Description |
|---|---|
| server/core/runtime/models/shell_command_runner_test.go | Adds tests for subprocess exit/start failure cases to validate output capture and error typing/context. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code Coverage OverviewLanguages: Go Go / code-coverage/goThe overall line coverage in commit 3a876bd in the Show a line coverage summary of the most covered files.
Updated |
0d824d1 to
e593ce4
Compare
Verify both command runners retain stdout, stderr, and typed exit errors. Cover missing executables and working directories with real subprocess boundaries and temporary paths. Assisted-by: OpenAI Signed-off-by: Rui Chen <rui@chenrui.dev>
e593ce4 to
3a876bd
Compare
What
Cover command output and typed errors on subprocess execution and startup failures.
Why
A regression could discard stderr or the exit cause while existing happy-path tests still passed.
Testing
Focused subprocess cases and the full models package pass with -race and coverage.