Cover the remaining audit gaps in the test suite - #25
Conversation
The audit in #16 listed eleven untested behaviours. Six were covered by the fix PRs that closed the related issues; this adds the five that were not: parsed-ref on an undeclared name and parsed-flag? on unset, non-boolean and undeclared values; a missing positional binding #f at both levels, which the README's or-default idiom depends on; the empty attached value giving ""; the three-argument option form with no default, absent, given, and in help; and the shape of the result after --help short-circuits the parse. The suite used to end by dumping both generated help pages to stdout, so a non-verbose run still printed two pages. Those dumps are now exact row assertions on the help layout, which also pins column alignment and section order the way the README samples rely on. Closes #16 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Baiju Muthukadan <baiju.m.mail@gmail.com>
baijum
left a comment
baijum
left a comment
There was a problem hiding this comment.
Verified locally on the branch: the suite passes 238/0 (exit 0), procedure coverage of (kaappi cli) stays 14/14, and the suite's stdout is now nothing but PASS: lines and section headers — the cosmetic ask in #16 is done. I also checked each new assertion against the library and the README contract ("No default → string" matches the --config=42 test, and the exact-row padding matches pad's 28-column target). The gap table in the description checks out against #16, and the commit carries the DCO sign-off.
Three non-blocking comments inline.
| (check "help short" #t (parsed-ref r "help"))) | ||
|
|
||
| ;; help short-circuits the parse: what came before it is not returned | ||
| (let ((r (run-cli-parse app '("data.csv" "-n" "3" "--help")))) |
There was a problem hiding this comment.
"help short-circuits the parse: what came before it is not returned" slightly overclaims. The short-circuit keeps option values parsed before --help: lib/kaappi/cli.sld:287 builds the help result from the accumulated opts, so myapp data.csv -n 3 -v --help yields count = 3 and verbose = #t while parsed-args is '(). Only positionals/command/sub/errors are dropped.
Since this block exists to pin the result shape, consider asserting the kept half too:
(check "help keeps options parsed before it" 3 (parsed-ref r "count"))so a future change to either half can't land silently. If not, the comment could still say "positionals and the command are not returned; options parsed before it are kept".
There was a problem hiding this comment.
Both halves pinned in 456470a: the argv now includes -v as well, and two checks assert count = 3 and verbose = #t are kept while positionals, command, sub and errors are dropped. The comment says exactly that.
| (parsed-args (parsed-sub r)))) | ||
|
|
||
| ;; option with no default: #f when absent, an uncoerced string when given | ||
| (define dapp |
There was a problem hiding this comment.
These four checks cover the no-default option form (item 7 of #16) rather than accessors. Since the === ... === headers are the suite's only navigation, this block might deserve its own header (e.g. === No-default options ===) before === Commands ===.
There was a problem hiding this comment.
Split out in 456470a under its own === No-default options === header, before Commands.
| (check "app help usage line" #t | ||
| (string-contains? out "Usage: myapp [options] <command> <input>\n\nOptions:\n")) | ||
| (check "app help flag row" #t | ||
| (string-contains? out " -v, --verbose Verbose output\n")) |
There was a problem hiding this comment.
These exact rows deliberately hardcode pad's 28-column target as literal space runs — fine, since the alignment stays readable in the source — just noting the maintenance cost: if the target ever changes, all ~12 strings need respacing by hand, and the tempting shortcut is pasting actual output over expected. If that ever bites, a small helper computing the gap (make-string (- 28 n) #\space) would turn a deliberate alignment change into a one-line edit. Not asking for it now.
There was a problem hiding this comment.
Agreed on the trade-off, leaving the literal rows as they are for now. If the 28-column target ever moves, a (row short long desc) helper built on make-string is the change to make in the same PR, and this thread is the pointer.
Review follow-ups. The help short-circuit block only asserted what is dropped, and its comment claimed nothing before --help is returned, which overstates it: option values and flags parsed before --help are kept. Both halves are asserted now so neither can change silently. The no-default option checks were filed under the accessor header; they get their own, since the headers are the suite's only navigation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Baiju Muthukadan <baiju.m.mail@gmail.com>
|
Review addressed in 456470a:
Suite: 240 passed, 0 failed. |
1 participant
Closes #16.
The audit listed eleven untested behaviours. Six were covered by the PRs that fixed the related issues (#18, #20, #22, #23, #24); this PR adds the rest, plus the cosmetic fix the issue asked for.
Gaps, by the issue's numbering
--flag=valueon a flagparsed-refon an unknown name;parsed-flag?on unset and non-#tvalues(("input" . #f))(or ... "World")idiom--count=abccovered in #18;--count=and--output=giving""addedoptionform (no default)#fwhen absent, uncoerced string when given, no(default: ...)in helprun-clidispatch error pathsexitcannot run in-process)generate-helpwith an unknown subcommand--help--helpare dropped, no command, no sub result, no errorsHelp dumps
The suite ended by printing both generated help pages to stdout. Those are replaced by exact row assertions on the app and
buildpages: title line, usage line, each option row with its padding and default, the built-in help row, and the Arguments and Commands sections in order. That pins the column alignment and section order the README samples rely on, which no test did before. Every remaining line of suite output is aPASS:line or a section header.Suite: 238 passed, 0 failed on Kaappi v0.27.1. Procedure coverage of
(kaappi cli)was already 14/14.No library or doc changes.
🤖 Generated with Claude Code