Reject a repeated assertion flag instead of dropping the earlier one - #73
Merged
Conversation
…rlier one
Repeating an assertion did one of two opposite things depending on how the
flag happened to be declared. The header flags accumulate, so a second
occurrence adds an assertion. Everything else kept the last value and
discarded the rest, without a word:
$ http-assert --assert-status 500 --assert-status 200 http://…/ok
[+] PASSED <- the 500 was never checked
A test tool reporting success for a check it never ran is the worst failure
mode available to it, and the caller has no way to detect it: the exit code
is 0 and the output says PASSED.
Seven flags were affected, not the one reported. The booleans lose
information too -- `--assert-ok --assert-ok=false` silently inverts the
assertion, and `--assert-body-empty --assert-body-empty=false` erases it,
after which the tool answers "no assertions defined" to somebody who named
one twice.
pflag records whether a flag was set but never how often, so the count is
gathered by wrapping each single-valued assertion flag's Value. Which flags
get wrapped is derived from their own type rather than listed, because a
list is precisely how this went wrong: --assert-header was made repeatable,
the others were not, and nothing connected the two decisions. An assertion
flag added later is covered the day it is added.
The check runs before applyEnv so that a value arriving from the
environment can never be counted as a second occurrence on the command line.
The end-to-end guard enumerates the assertion flags from --help and holds
each to the rule its type implies, reusing the config matrix for a valid
invocation of each. Both of its failure modes -- a flag escaping the
wrapping, and the enumeration silently matching nothing -- were confirmed by
mutation before committing.
Closes #35
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019EMMhgmTkbzAsmeNy97PrP
Exit 71 was documented as "a flag or environment value failed to parse", which a repeated flag is not -- the value parsed fine, there were simply two of them. The wording now covers both, in the three places it appears: the package comment, --help and the README. The asymmetry itself is also worth stating rather than leaving to be discovered: three flags repeat to add assertions and the rest do not, and nothing in the flag names distinguishes them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019EMMhgmTkbzAsmeNy97PrP
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.
Problem
Repeating an assertion silently threw one away, and the run went green anyway.
Which of the two things happens depends on how the flag was declared. The three header flags accumulate, so repeating them adds assertions. Every other assertion flag keeps the last value and discards the rest, without a word. Nothing in the flag names distinguishes the two groups.
For a tool whose entire output is an exit code, reporting success for a check it never ran is the worst failure mode available — and the caller cannot detect it.
Seven flags were affected, not the one reported. The booleans lose information too:
That last message is the bug in one line: the user named an assertion twice and was told there are none.
Solution
Repeating a single-valued assertion now exits
71and says so:pflag records whether a flag was set but never how often, so the count is gathered by wrapping each affected flag's
Value. Which flags get wrapped is derived from the flag's own type rather than a hardcoded list — a list is precisely how this went wrong, since--assert-headerwas made repeatable, the others were not, and nothing connected the two decisions. An assertion flag added later is covered the day it is added.The check runs before
applyEnv, so a value arriving from the environment can never be counted as a second occurrence on the command line.The end-to-end guard enumerates the assertion flags from
--helpand holds each to the rule its type implies, reusing the existing config matrix for a valid invocation of each. Both of its failure modes — a flag escaping the wrapping, and the enumeration silently matching nothing — were confirmed by mutation before commit. Coverage stays at 100.0%.Other Changes
Exit
71was documented as "a flag or environment value failed to parse", which a repeated flag is not — the value parsed fine, there were simply two of them. Reworded in all three places it appears, and the repeatable-vs-single distinction is now stated rather than left to be discovered.Notes for the reviewer
This is a breaking change. An invocation that exits 0 today will exit 71 after this. It only breaks invocations that were silently checking less than they asked for, which is the point, but it belongs in the release notes alongside #71.
71is a slight stretch — a repeated flag is a malformed command line, nearer to103's territory than to a bad value. It was chosen deliberately over103; the wording change above is what makes it honest.The alternative considered and rejected was making every assertion flag collecting, so repetition accumulates uniformly. That is less code, but it turns
--assert-status 200 --assert-status 201into something that always fails rather than something that is refused, and it changes what a repeated flag means rather than refusing to guess.Closes #35
🤖 Generated with Claude Code