Add coverage for executor timeouts, env snapshot, and cleanup retention#58
Merged
Conversation
Closes gaps in the test suite around behavior that previously had no
coverage:
- run_subprocess timeout enforcement (SPEC section 7): a command that
outruns COMMAND_TIMEOUT is killed and reported as exit code 124.
- run_subprocess applies the env snapshot to the child process.
- handle_command marks a command failed (exit 1) when the binary does
not exist, instead of crashing the worker.
- cleanup_once deletes only old, completed ('done') commands while
preserving recent commands and old non-completed history.
Also strengthen the fork_session test to assert the environment snapshot
is copied, not just the working directory.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BixPb34oe2Ae5cXuz9WxbP
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
2 participants
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.
Summary
Adds tests for executor and cleanup behavior that previously had no coverage.
Follow-up to #57 (now merged), which wired the Postgres service into CI so the
database-backed test here actually runs instead of skipping.
New / strengthened tests
run_subprocess) — SPEC §7 requires timeoutsto be enforced, but nothing tested it. New test runs
sleep 5with a 1sCOMMAND_TIMEOUTand asserts the process is killed and reported as exit code124with aTimed out after 1smessage.run_subprocess) — asserts a variable from thecommand's
env_snapshotreaches the child process.handle_command) — asserts a non-existent commandis recorded as
failed/ exit1rather than crashing the worker loop.cleanup_once) — asserts only old + completed (done)commands are deleted, while recent commands and old non-completed history are
preserved.
fork_session— strengthened the existing test to assert the environmentenvsnapshot is copied, not just the working directory.Verification
executor tests run without a DB; the new cleanup test skips gracefully.
🤖 Generated with Claude Code
Generated by Claude Code