ingest_mdir improvements for test listing and including/excluding #61
Conversation
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
There are 2 problems with the --list-tests command: - The arguments --tree and the --patch/--mdir group are marked as required=True, forcing argparse to demand them even when they aren't needed (like for --list-tests). - The main() function tries to access args.tree before checking if --list-tests was specified, which would cause a crash even if the parser requirement was removed. This problem is masked by the first one, so it makes sense to handle them together. $ ./ingest_mdir.py --list-tests # should have succeeded ingest_mdir.py: error: the following arguments are required: --tree $ ./ingest_mdir.py --tree dummy --list-tests ingest_mdir.py: error: one of the arguments --patch --mdir is required $ ./ingest_mdir.py --tree dummy --patch dummy2 --list-tests # only this variant succeeds Remove the required=True from the patch_arg group and the --tree argument, and add manual handling for them if --list-tests wasn't specified. Also, move the args.tree dereference after the list_tests check and the manual verification that the argument was provided. Signed-off-by: Vladimir Oltean <[email protected]>
Currently, if we specify an invalid test name such as "checkpatch" instead of "patch/checkpatch", the script falls through with no test run, and no indication as to what was wrong. That is admittedly not very friendly. We'd need to call list_tests regardless of whether the --list-tests argument was specified, and validate against its output. However, this makes it redundant to have that function, we can just move it into main(). Signed-off-by: Vladimir Oltean <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
--list-testscommand doesn't actually work unless you specify a lot of unnecessary arguments.--testand--disable-testunless you see the format that the script wants them in, as per--list-test. Actually reject the wrong names.