test(cli): drop vestigial --open/--closed boolean-flag tests#558
Open
FileSystemGuy wants to merge 1 commit into
Open
test(cli): drop vestigial --open/--closed boolean-flag tests#558FileSystemGuy wants to merge 1 commit into
FileSystemGuy wants to merge 1 commit into
Conversation
PR #412 reshaped the CLI from boolean `--open`/`--closed` flags into a positional `mode` argument (closed|open|whatif). Argparse no longer defines either flag — `grep -rn 'add_argument.*--open|--closed' mlpstorage_py/` returns zero hits — so the four `TestOpenClosedCLIFlags` tests now exercise nonexistent parser behavior, and the six `TestVerifyBenchmarkOpenFlag` tests build pre-#412 args.Namespace shapes that `verify_benchmark` no longer consumes (it reads args.mode now per the post-#412 dispatch contract). All 10 tests were failing with predictable AttributeError / SystemExit-from-unknown-argument errors. They aren't asserting any contract worth re-affirming on the current codebase. The post-#412 dispatch tests (`TestVerifyBenchmarkPost412ModeDispatch`) already cover the modern contract — that's the file's value going forward. Kept those plus the kvcache helper-predicate test. Updated the module docstring to reflect the new scope and the history of why the file exists (links the original #349 / #412 / #352 thread for archaeology). Result: 5 passed + 10 failed → 5 passed.
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
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
1 participant
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
PR #412 reshaped the CLI from boolean `--open`/`--closed` flags into a positional `mode` argument (`closed`|`open`|`whatif`). Argparse no longer defines either flag — `grep -rn 'add_argument.*--open|--closed' mlpstorage_py/` returns zero hits — so:
All 10 tests aren't asserting any contract worth re-affirming on the current codebase. The post-#412 dispatch tests (`TestVerifyBenchmarkPost412ModeDispatch`) already cover the modern contract — that's the file's value going forward. Kept those plus the kvcache helper-predicate test.
Refreshed the module docstring to reflect the new scope and the history of why the file exists (links the original #349 → #412 → #352 thread for future archaeology).
Test plan