test: pin expected exception contracts - #46
Merged
Conversation
…h types Stack A / PR-A2. Each narrowed type was observed from the running code via the synced test env, not guessed: - Frozen dataclasses -> dataclasses.FrozenInstanceError: ImmutableScope + ContextBinding (test_immutability.py x5), Artifact (test_emit_artifact.py), RunRef (test_kernel_re_exports.py). - Frozen pydantic models -> pydantic.ValidationError: KVStoreContext (test_kvstore.py), ValidationResult + ToolCallRejected (test_capability_hooks.py x2). - test_programmatic_device_execution.py: the lifecycle wraps the device's SandboxExecutionError and re-raises TaskExecutionError (shepherd_core.errors), so the assertion now pins TaskExecutionError (the propagated type), not the mock's inner cause. - test_mcp_stdio_lifecycle.py: spawning a missing binary -> FileNotFoundError. - test_static_fragment_corpus.py: the core-0 trace validators raise TraceValidationError; verified across all 18 parametrized corpus cases. Left intentionally broad (documented, not a miss): test_lane_c_run.py:84 asserts the raised error is NOT the removed LC-3 fence (a negative-match on the message), so the type is polymorphic by design. Tested-by: uv run ruff check (clean, 8 files); pytest on each affected file/test (immutability 10, kvstore 54, capability_hooks 22, emit_artifact 13, kernel_re_exports 8, device-cleanup 1, mcp invalid-command 1, corpus 18) — all pass.
Deploying shepherd-agents-docs with
|
| Latest commit: |
f3e398a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4e97a22e.shepherd-agents-docs.pages.dev |
| Branch Preview URL: | https://test-pin-exception-contracts.shepherd-agents-docs.pages.dev |
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
pytest.raises(Exception)assertions with the concrete exception contracts emitted by the code under test.Motivation / Context
Broad exception assertions can pass when code fails for the wrong reason. These tests now pin the actual failure contracts observed in the synced test environment, making regressions easier to diagnose without changing runtime behavior.
Delta Breakdown
FrozenInstanceError; frozen Pydantic model mutations now expectValidationError. Reviewers should verify each model family uses its native exception type.TaskExecutionError, while a missing MCP executable expectsFileNotFoundError. Reviewers should verify the tests assert at the correct abstraction boundary.TraceValidationErroracross the parametrized corpus. Reviewers should verify this remains the public validator contract.Entry Point
test_programmatic_device_execution.py→ immutability/model tests → MCP lifecycle test → static fragment corpus test.The executable entrypoints are the existing pytest tests; this PR introduces no runtime or API entrypoint.
Review Guide
The main review question is whether each narrowed type represents the intended boundary rather than an implementation detail. In particular, the device lifecycle deliberately asserts the wrapper
TaskExecutionError, not the inner sandbox exception. The corpus assertion covers 18 parametrized invalid traces with the sharedTraceValidationErrorcontract.Validation
uv lock --checkpassed.make baselinecompleted 27 tests with 5 skips, then hit its existing Linux/macOS mismatch because the test config selects the APFS-onlyclonefilebackend.Documentation / Changelog / Decisions
No documentation, changelog, or decision record is needed because this PR changes only test precision and has no user-visible behavior.
Risk / Rollback / Migration
Risk is low and limited to exposing tests whose exception contracts change in the future. Rollback is a normal revert of this commit. No migration or compatibility action is required.
Follow-ups
None.