fix(tests): catch 3 contract-drift failures on main#23
Merged
Conversation
Three tests on main were red, blocking unrelated PRs from getting clean
CI signal:
1. tests/e2e/test_family_and_asset.py: asserts body["families"], but
GET /assets/{id} now serializes the field as "family_ids" after a
recent rename. Updated assertion.
2. tests/contract/test_conduct_procedure_endpoint.py: expects 200 with
a lifecycle-failure body when conducting an unregistered procedure,
but the conductor re-raises ProcedureNotFoundError which the BC's
exception handler maps to 404. Updated to expect 404 with the
procedure_id in detail. Per project_conduct_procedure_test_contract_drift
memory, this drift was documented but not fixed.
3. tests/contract/test_conduct_procedure_mcp_tool.py: same drift on the
MCP path. Expected structuredContent on a path that now raises;
FastMCP surfaces the error as isError=true with the message in
content. Updated assertion shape to match the standard MCP error
contract used by every other test in the file.
No code changes; tests catch up to shipped behavior.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Main moved forward while CI was running on the prior commit: - `refactor(access): rename Actor.is_active to Actor.active` (4ea068e) broke 4 test sites that read the old field name: * tests/contract/test_get_actor_endpoint.py (×2 assertions) * tests/contract/test_get_actor_mcp_tool.py * tests/e2e/test_actor_lifecycle.py Updated to `active`. - `tests/integration/test_add_run_to_campaign_postgres.py::test_concurrent_add_runs_to_same_campaign_settle_atomically` was a known-fragile happy-race test (the docstring itself warned that "if a future change introduces true parallelism, one call would ConcurrencyError and require operator-side retry"). CI hit that case. Rewrote each gather call to retry on ConcurrencyError (max 3 attempts), pinning the documented OCC retry contract rather than the lucky no-conflict shape. The forced-conflict sibling test still pins the immediate-failure path. No production code changes; tests catch up to shipped behavior. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
e896492 to
71dd14f
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.
Summary
Three tests on main were red; this PR catches each one up to shipped behavior. No code changes.
test_family_and_asset.py— assertedbody[\"families\"]but GET /assets/{id} now serializes the field asfamily_idsafter a recent rename.test_conduct_procedure_endpoint.py— expected200+ lifecycle-failure body on an unregistered procedure, but the conductor re-raises `ProcedureNotFoundError` which the BC's central exception handler maps to404. Updated to expect404with the procedure_id in the detail. Per the `project_conduct_procedure_test_contract_drift` memory, this drift was documented but never fixed.test_conduct_procedure_mcp_tool.py— same drift on the MCP path. Updated to expectisError: truewith the message incontent(standard MCP error contract used by every other tool test).Why now
Both PR #21 and PR #22 were inheriting these failures into their CI runs despite touching unrelated code. Clearing main unblocks both.
Test plan
make lintcleanmake typecheck0 errors🤖 Generated with Claude Code