Add regression test for boolean input with a default (#779)#2288
Open
arimu1 wants to merge 1 commit into
Open
Conversation
…-language#779) Issue common-workflow-language#779 reported that a boolean input with `default: false` was treated as a required CLI argument (`error: the following arguments are required: --a_false`), while `default: true` worked fine. This was fixed in 48e8afd (2019) when `required` became `default is None and input_required`, but the behavior was never locked down by a test and the issue stayed open. Add a parametrized regression test in test_toolargparse.py covering both `default: false` and `default: true`, asserting the generated parser treats the input as optional and falls back to the declared default. Verified the test fails against the pre-fix logic with the exact error from the issue. Co-Authored-By: Claude Opus 4.8 <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
Addresses #779.
Issue #779 reported that a
booleaninput withdefault: falsewas incorrectly treated as a required CLI argument:…while
default: trueworked fine — an argparse-generation asymmetry.Status: already fixed, never locked down
This was fixed in 48e8afd (Aug 2019), when the
requiredcomputation inadd_argumentbecame:Since a
booleanwithdefault: falsehasdefault=False(notNone), it is correctly optional today. I confirmed currentmainruns the issue's repro without error. However, the behavior was never covered by a test and the issue is still open.Change
Adds a parametrized regression test in
tests/test_toolargparse.py(plus two small fixtures undertests/wf/) covering bothdefault: falseanddefault: true. It builds the parser viagenerate_parser(..., input_required=True)— mirroring the default CLI behavior — and asserts that parsing with no arguments succeeds and falls back to the declared default, rather than erroring as required.I verified the test genuinely guards the bug: against the pre-2019 logic it fails on the
default: falsecase with the exact error from the issue, while thedefault: truecase still passes — matching the original report.black,flake8, andisortare clean on the changed files. Once this lands, #779 can be closed.This change was produced with the assistance of Claude Code (model: Claude Opus). The analysis and test were reviewed by a human before submission.