story-015: A test that cannot fail must not count as validation - #13
Merged
Conversation
Implemented by the l5 harness story workflow.
The check flagged a dishonest git baseline only when the call stated it ran against the repository root, through -C <root> or cwd=<root>. A call stating no target was skipped entirely - but subprocess with no cwd inherits the parent's, and pytest runs this suite from the repository root. So deleting one keyword from `git diff HEAD` made it invisible to the check while changing nothing about what it did. The target is now three-valued: stated and naming this repository, stated and naming something else, or not stated at all. The third case is not an inference about the author's intent - it is what subprocess does. The scan still evaluates nothing and resolves no paths; it asks only whether a target was stated and, if so, whether it is written as one of the two names standing for this repository. A second and stricter rule sits beside it: no git call in tests/ may leave its target unstated, whatever the call asks for. That removes the ambiguity rather than answering it per subcommand, so it cannot return through a different one. No module is exempt - the baseline exemption exists because comparing the working tree against HEAD is correct in one place, and there is nowhere leaving the target unsaid is correct. Cost measured before the change: three git calls in the suite stated no target, all `git clone <source> <dest>`, none carrying a baseline. Each now names its target. Verified by injecting the evasion into a real module - both rules go red, and green again when it is removed. 688 passed on Python 3.14 and 3.10. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The scan matched only a literal `subprocess.run` or `subprocess.check_output`, so `import subprocess as sp` and `from subprocess import run` both walked past it. An import statement should not be able to hide a call from a check about what that call does. A qualified call now matches on the attribute alone, whatever qualifies it, and Popen/call/check_call join run/check_output. What identifies these calls is the literal "git" heading their argument list, which _git_argument_list already insists on, so matching the tail loses nothing. A bare call matches only when the module imported that name from subprocess, and the asymmetry is deliberate. Matching every bare `run(...)` flagged tests/test_story_016_validation.py's `run = functools.partial(subprocess.run, cwd=root)`, where the target is declared one line above the call. Reading it would mean following an assignment; this check reads what the source states rather than tracking values. Imports are stated, a local rebinding is not, and the latter is left uncovered rather than guessed at. Verified against the evasion that found this: injecting an aliased-import `git diff HEAD` into a real module turns both rules red and green again on removal. 695 passed on Python 3.14 and 3.10. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
Five stories shipped "X is unchanged" tests that stopped checking anything the moment their own story merged. Each asserted
git diff HEAD -- <paths>is empty — and on a committed tree that diff is empty for every path, so the assertion held no matter what the story did.Demonstrated rather than argued: story-009's
test_the_definitions_this_story_injects_are_unchangedassertedschemas/was unchanged, and passed on a branch that addedschemas/manifest.json.The idiom had been accumulating since story-007 and was written five times by five separate tester runs — once after the rule against it was recorded in
.harness/docs/ARCHITECTURE.mdand injected into every stage. Prose guidance was already tried here and did not hold. This story therefore ships a mechanical check as well as guidance, and repairs the four instances already onmain.Changes
tests/conftest.py— one shared baseline resolution,story_commit_range(validation_file, repo), resolving a story's run commit as the commit that added its validation file and the baseline as that commit's parent. Bounded at both ends, so it survives a rebase or a squash, stays correct as later commits stack on top, and degrades to the working tree while a story is still in flight. It raises rather than returning a range that cannot differ.tests/test_baseline_honesty.py(new) — an AST scan over every module intests/, flagging a git invocation that targets the repository root and carries aHEAD-derived revision or a working-tree status query. Exactly one module is exempt, by name, and it is the one holding the shared resolution.tests/test_story_007_validation.py,…008…,…009…,…010…— the four merged instances repointed at the shared resolution. Each keeps the subject and strictness it was written with, with one authorized exception: the twotest_no_committed_story_artifact_was_editedassertions are narrowed to modifications and deletions, because each story's own artifact was added in its own run commit and an addition was never an edit.prompts/tester.md,prompts/verifier.md— the general rule the mechanical check cannot cover: an assertion claiming an absence needs a negative control, because it "passes when the property holds and it passes just as happily when the test is looking in the wrong place." Positive assertions fail loudly on their own and need nothing extra. The verifier gets the corresponding requirement..harness/docs/ARCHITECTURE.md— the resolution, the check, and what the check does and does not cover.Testing
685 passedon Python 3.14 and on Python 3.10.The verifier did not accept the repairs on description. Its decisive check was to clone the repository, commit all nine changed files as a single run commit, and re-verify there — since the story's entire subject is assertions that change behavior once the story commits:
06eaa477(11 files changed), story-008 →14f47eac(7 files), story-009 →ff808499(26 files). The ranges report real changes; the guarded paths are empty because those stories genuinely left them alone, not because the comparison is degenerate..harness/runs-archive/.subprocess.run(['git', '-C', str(REPO_ROOT), 'diff', 'HEAD'])totests/test_story_011_validation.pyturnedtest_no_module_in_the_suite_resolves_a_dishonest_baselinered; restoring the file turned it green.Notes for review
clean-clone-result.jsonrecordsexit_code: 0,685 passed, onpython_version: 3.10.20— the suite run in a fresh clone with the story committed into it, on the oldest Python CI tests. I had predicted this story might trip it, since it is repairing exactly the class of defect the check exists to catch. It did not, and the verifier's independent post-commit clone agrees.-C <root>orcwd=<root>, which is what the acceptance criterion enumerates. A git call written with neither —subprocess.run(['git', 'diff', 'HEAD', ...])— inherits the process working directory and would not be flagged. Worth knowing before relying on it as a complete guard.tests/test_baseline_honesty.pyunder the story'sstage_exceptionsgrant — the check is this story's deliverable, not validation of it. Independence is preserved: the tester wrotetests/test_story_015_validation.pyseparately and validates the check, the shared resolution, and the four repairs.🤖 Generated with Claude Code