Repository navigation
fix(tests): a fixture one directory down was graded by nothing - #208
Conversation
`test_action_receipt_fixtures` ran `conformance/*.json` and `test_vector_completeness` graded `conformance/*.json`, both flat. A fixture in a subdirectory was run by neither, and nothing said so. Demonstrated rather than argued. Copy `04-signature-key-mismatch.json` into `conformance/cand/`, change only its expected failure code to one no registered rule can emit, and the suite reports 729 passed: the same count as without the file. That fixture holds an assertion that cannot fail, in the corpus whose job is to hold assertions that can, and `test_no_fixture_expects_a_code_the_registry_cannot_emit` exists to catch exactly it and never sees it. With this change the same experiment turns three tests red, including the fixture actually being executed. Discovery is now one shared function used by both readers, recursive and bounded. `test_fixture_set_is_complete` still names every file, so a nested fixture stays an explicit decision rather than an accident. Three guards, and two of them exist because this change was wrong twice first: - `test_discovery_reaches_a_nested_fixture` builds its own tree under `tmp_path`. A guard reading only the committed corpus would pass under either glob, since nothing is nested today: it would pass in exactly the state it exists to detect. - `test_discovery_skips_machine_written_directories` covers the bound. Recursing without one sweeps in `__pycache__`, which was the first version. Bounding it on the absolute path discards every fixture whenever the checkout sits under a dot-directory, which was the second. The skip is judged below `root`. - `test_both_corpus_readers_go_through_the_shared_discovery` parses each module and checks what the binding is assigned to. It first compared the two file lists, which is vacuous for the same reason as the first guard: while nothing is nested, a flat glob and a recursive one return identical files, and mutating each reader back to its own glob caught nothing. Its second version matched the assignment as a line of text and failed on `BINDING = (` with the call underneath, and on an annotated assignment. Both are legal, both are shapes this repository already contains, and neither is the defect being guarded against. Five mutations, each red in the test that names it: discovery back to a flat glob, the skip judged on the absolute path, the skip disabled, and either reader binding its own glob. Applied one at a time with the tree restored between. 733 passed, 1 skipped; ruff and mypy clean. Related, not fixed here. `test_canonicalization_boundary`, `test_build_provenance_depth_vectors`, `test_delegation_vectors` and the loader inside `test_adequacy_all_sets` all discover flat, and none of those directories has a subdirectory today, so none is currently hiding anything. `test_acta_fixtures` is a different shape: it selects with `0*.json`, a filename rather than a shape, which excludes `expected.json` as presumably intended and would also drop a tenth vector without mentioning it. Signed-off-by: lywinged <louie.lunz@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
❔ Contributor Check: UNKNOWN
Automated check by AgenTrust Contributor Check. |
imran-siddique
left a comment
There was a problem hiding this comment.
Verified the defect at pinned main 11c340e8: test_action_receipt_fixtures.py:292 is FIXTURE_DIR.glob("*.json") and test_vector_completeness.py:73 is the same. Both flat, so a nested fixture is read by neither, exactly as you describe.
The demonstration is what makes this easy to take. A fixture holding an assertion that cannot fail, sitting in the corpus whose job is to hold assertions that can, and test_no_fixture_expects_a_code_the_registry_cannot_emit never seeing it. Identical 729 passed, 1 skipped with and without the file is the whole argument in one line.
Two things I want to name because they are the difference between a guard and a decoration:
test_discovery_reaches_a_nested_fixture builds its own tree under tmp_path instead of reading the committed corpus. A guard over the committed corpus would pass under either glob, since nothing is nested today, which means it would pass in precisely the state it exists to detect. Getting that right unprompted is the part I would have flagged if it were missing.
And bounding the recursion below root rather than on the absolute path. The failure mode you name is real: a checkout under a dot-directory would otherwise discard every fixture, and that reads as an empty corpus rather than an error.
Keeping test_fixture_set_is_complete naming every file is the right call too. Recursive discovery plus an explicit manifest means a nested fixture stays a decision rather than becoming an accident in the other direction.
Approving and merging.
A flat `glob("*.json")` saw thirty of forty-eight vectors: everything in
`conformance/proposal-117/` was invisible. `receipt_gap_disclosed` is held by
exactly two vectors and both are down there, so the report called the obligation
unheld and exited non-zero, while `pair_mutation` and `triple_mutation`, which
import FIXTURES from this module, drew conclusions over a corpus missing a fifth
of itself and exited zero.
Discovery now comes from `discover_fixtures` in the verifier module, the same
function upstream added in agentrust-io#208 for its two readers, so a third reader cannot
drift away from them again.
Measured before and after on the same tree: 30 vectors and exit 1 with
`receipt_gap_disclosed` reported as held by nothing, against 48 vectors and exit
0 with that obligation at margin 2 and attributed to a vector that names it.
test_action_receipt_fixturesrunsconformance/*.jsonandtest_vector_completenessgrades
conformance/*.json. Both flat. A fixture in a subdirectory is run by neither,and nothing says so.
Demonstration
Copy
04-signature-key-mismatch.jsonintoconformance/cand/, change only its expectedfailure code to one no registered rule can emit, and run the suite:
The same count either way. That file holds an assertion that cannot fail, in the corpus
whose job is to hold assertions that can, and
test_no_fixture_expects_a_code_the_registry_cannot_emitexists to catch exactly it andnever sees it.
With this change, the same experiment:
The middle line is the one to look at: the fixture is now actually executed.
The change
Discovery becomes one shared function, recursive and bounded, used by both readers.
test_fixture_set_is_completestill names every file, so a nested fixture stays anexplicit decision rather than an accident.
Three guards, and two of them exist because this was wrong first
test_discovery_reaches_a_nested_fixturebuilds its own tree undertmp_path. Aguard that read only the committed corpus would pass under either glob, because nothing
is nested today: it would pass in exactly the state it exists to detect.
test_discovery_skips_machine_written_directoriescovers the bound, and both halvesof it were wrong in turn. Recursing without a bound sweeps in
__pycache__. Bounding iton the absolute path instead discards every fixture whenever the checkout sits under a
dot-directory, which is a plausible place to keep one. The skip is judged below
root.test_both_corpus_readers_go_through_the_shared_discoveryparses each module andchecks what the binding is assigned to. It went through two worse versions. Comparing the
two file lists is vacuous for the same reason as the first guard: while nothing is
nested, a flat glob and a recursive one return identical files, and mutating each reader
back to its own glob caught nothing. Matching the assignment as a line of text then
failed on
BINDING = (with the call underneath, and on an annotated assignment; bothare legal, both are shapes this repository already contains, and neither is the defect.
...skips_machine_written_directoriestest_vector_completenessbinds its own glob...shared_discovery[test_vector_completeness.py-FIXTURES]test_action_receipt_fixturesbinds its own glob...shared_discovery[test_action_receipt_fixtures.py-FIXTURE_PATHS]Applied one at a time, restored from the commit object between rather than from a copy,
after a copy silently reverted an earlier version of this change mid-run.
Three legal spellings were checked for false positives and none fails: an annotated
assignment,
BINDING = (with the call on the next line, and the call wrapped acrosslines.
733 passed, 1 skipped;ruff check src tests scriptsclean;mypyclean; parses underfeature_version=(3, 11), the declared floor. Branched offmainat4785fa7.Related, and deliberately not in this PR
test_canonicalization_boundary,test_build_provenance_depth_vectors,test_delegation_vectorsand the loader insidetest_adequacy_all_setsall discoverflat. None of those directories has a subdirectory today, so none is currently hiding
anything, and each is a separate judgement about whether nesting belongs there.
test_generators_reproduce_fixturesalso globs flat and is not in that list: itsscope is one generator's own directory, which its comment states as the intent, so flat
is correct there.
test_acta_fixturesis a different shape. It selects with0*.json, a filename ratherthan a shape, which excludes
expected.jsonas presumably intended and would also drop atenth vector without mentioning it.