test(globals): cover parse_args, the thread/RAM scheduler, and the print_* formatters (#182) - #202
Merged
Merged
Conversation
…int_* formatters (#182) Adds 96 cases to tests/unit/src/test_globals.cpp, the last major gap in the unit suite. No source file is modified. parse_args (83 cases): the argc < 4 short-circuit group, distinguished by output rather than exit code alone; the three mandatory positionals; the verbosity pre-pass; every optional flag's happy, missing-value, bad-numeric, and bound-violation cases; the main-loop -h/--version/-ci paths that do not exit; unrecognized options; and the four post-loop cross-field checks. set_thread_ram_steps (7 cases) is driven directly rather than through an argv array, since the constructor and parse_args both call it. print_version, print_usage and print_citation (6 cases) assert on captured stdout, including that the usage text interpolates live settings and omits the four flags whose usage lines are commented out. Three harness pieces, all local to the test file: - ArgsFixture writes an openable query and truth VCF and points the reference at the committed tiny.fasta, so a case states only the flags it varies. - parse() copies the arguments into mutable buffers, since parse_args takes char**, and closes the reference FASTA that parse_args opens and never closes. - parse_showing_stdout() redirects stdout onto stderr inside the death-test child, because a death-test matcher only sees stderr while print_usage, print_version and print_citation write to stdout. Without it the argc < 4 group could assert only an exit code, making -v, -h and -ci indistinguishable. Two cases document current behavior rather than enforcing it, each marked in-file with the issue that will change it: a trailing comma in --filter appends an empty filter name (#199), and -b's catch block reports an invalid filename for what is actually invalid file contents (#200). A third pins that the missing-value error for --verbosity is unreachable, since the pre-pass loop stops at argc-2. Closes #182
This was referenced Aug 5, 2026
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.
Note
Authorship: the content below was drafted by Claude Opus 5 (an AI coding agent) and
filed via
ghunder @TimD1-bot, a bot account operated by @TimD1. It reflects theagent's analysis, not a statement authored by @TimD1.
Adds 96 cases to
tests/unit/src/test_globals.cpp, takingGlobals::parse_argsfrom zero unitcoverage to the whole CLI contract. No source file is modified. The suite goes from 561 to
657 passing cases.
What's covered
parse_args— 83 casesargc < 4short-circuit-h,--help,-v,--version,-ci, and an unrecognized argument — allexit(0)-b,-p,-f,-l,-sv,-q,-mq,-n,-x,-o,-e,-i,-s,-t,-ct,-r— a happy case each, plus missing-value, bad-numeric, and bound-violation wherever the parser enforces one-h,--version,-ci— none of these exit, unlike theirargc < 4counterparts--boguserrors;-v <n>is consumed without effect and the flag after it still parses--max-qualbelow--min-qual;--largest-variantbelow--sv-threshold;--max-supercluster-sizebelow--largest-variant + 2;--largest-variant 1set_thread_ram_steps— 7 cases, driven directly rather than through an argv array, sinceboth the constructor and the end of
parse_argscall it: the 64-thread ladder, a single thread,integer halving on a non-power-of-two (10 →
{10, 5, 2, 1}), the RAM-per-thread division, thatrepeated calls replace rather than append,
thread_nstepsagreeing with both vector sizes, andthat an out-of-range thread count yields no steps rather than dividing by zero.
print_version,print_usage,print_citation— 6 cases on captured stdout, includingthat the usage text interpolates the live settings rather than compiled-in literals, and that it
omits the four flags whose usage lines are commented out at
src/globals.cpp:492-499.Harness notes
Three pieces, all local to the test file:
ArgsFixturewrites an openable query and truth VCF into aTempDirand points thereference at the committed
tiny.fasta, so each case states only the flags it varies.parse_argsopens argv[1] and argv[2] withbcf_openand argv[3] withfopen, so every casereaching the optional-argument loop needs three files that exist.
parse()copies each argument into its own mutable buffer, sinceparse_argstakeschar **rather thanconst char **. It also closes the reference FASTA, whichparse_argsopens and never closes — 96 cases would otherwise leak one descriptor each.
parse_showing_stdout()dup2s stdout onto stderr inside the death-test child. Adeath-test matcher only sees the child's stderr, but
print_usage,print_versionandprint_citationwrite to stdout; without this theargc < 4group could assert only an exitcode, which makes
-v,-hand-ciindistinguishable. I confirmed the matchers actuallybite by temporarily patching one to a sentinel string and watching the case fail.
Defects found — pinned, not fixed
Three cases document current behavior rather than enforcing it. Each is marked as such in-file
and names the issue that will change it, so the amendment is a
grepaway:--filterappends an empty filter name, which then warns onceper callset from
src/variant.cpp:790-793. Pinned byParseArgs.FilterTrailingComma.-b'scatchblock is unreachable for an unopenable BED (thebedDataconstructor exits first), and for its one reachable trigger it reports an invalid filename
when the contents were invalid. Both routes are pinned, by
ParseArgs.BedBadFileErrorsandParseArgs.BedMalformedLineErrors.--verbosityatsrc/globals.cpp:92isunreachable: the pre-pass loop is
for (int i = 0; i+1 < argc; i++), soinever exceedsargc-2andi == argccannot hold. A trailing-vis never examined, the main loop thenadvances past the end, and the parse succeeds with verbosity unchanged instead of reporting the
missing value. Pinned by
ParseArgs.VerbosityMissingNotReached.-i/--max-iterationsand-s/--max-supercluster-sizecarry in-file comments naming #46 and#47, since those issues remove and rename them respectively.
Verification
cd tests/unit/build && make— clean; the only warning is the pre-existing unused-parameterone at
src/dist.cpp:1063../test_vcfdist— 657 passed, 74 suites.cd tests && pytest— 65 passed.Closes #182