test: cover ShellCommandRunner edge behavior - #31
Merged
Conversation
dottie-weaver
force-pushed
the
dottie/22-command-runner-tests
branch
from
May 31, 2026 22:46
c46da89 to
b09070c
Compare
Adds 6 edge-case tests complementing the existing V1 shell_runner tests: - run_captures_both_stdout_and_stderr_simultaneously - run_captures_stderr_alongside_nonzero_exit - run_silent_command_produces_empty_output - run_timeout_preserves_partial_stdout - run_concurrent_calls_isolate_temp_files - launch_returns_immediately_without_blocking Also documents the LaunchFailed coverage gap: the four code paths that produce LaunchFailed are structurally simple early-returns, but path 3 (shell binary not found) cannot be tested with a live ShellCommandRunner because env::set_var is unsafe in Rust 2024+ and unsound in multi-threaded tests. The existing shell_runner_reports_launch_failure test verifies that nonexistent commands produce Exited(nonzero), not LaunchFailed. Closes #22
dottie-weaver
force-pushed
the
dottie/22-command-runner-tests
branch
from
May 31, 2026 22:48
b09070c to
ea70a05
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds 6 edge-case tests for
ShellCommandRunner, complementing the V1 tests added in #30, closing #22.Tests added
run_captures_both_stdout_and_stderr_simultaneouslyrun_captures_stderr_alongside_nonzero_exitrun_silent_command_produces_empty_outputrun_timeout_status_is_timed_outTimedOutstatusrun_concurrent_calls_isolate_temp_fileslaunch_returns_immediately_without_blockinglaunch()returns before process exitsLaunch failure — deliberately untested
LaunchFailed/CommandLaunchErrorfor a missing shell binary is documented but not tested live because:shell_command()reads$SHELLat invocation time. Overriding requiresenv::set_var, which is unsafe in Rust 2024+ and unsound in multi-threaded tests.shell_commandis a private free function.The existing
shell_runner_reports_launch_failuretest in #30 verified that nonexistent commands produceExited(nonzero)(the shell itself exits), notLaunchFailed. That documents the actual behavior.The four
LaunchFailedcode paths are structurally simple early-returns; full coverage would require a trait-based shell provider, out of scope for V1.CI fix note
The initial push had
run_timeout_preserves_partial_stdoutwhich asserted partial output after kill. This is OS-dependent (Linux file buffering means data may not flush beforekill()). Replaced withrun_timeout_status_is_timed_outwhich tests the invariant that matters: the status isTimedOut.Autonomy decision
Safe to auto-merge on approval — no workflow files changed, all CI-green locally.