From ff6a672fd8e35150dac91bdd4a55e3a3f0ae40cc Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 26 Oct 2025 20:12:58 +0200 Subject: [PATCH 1/2] ingest_mdir: fix --list-tests handling 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 --- ingest_mdir.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/ingest_mdir.py b/ingest_mdir.py index f2e253d..f863258 100755 --- a/ingest_mdir.py +++ b/ingest_mdir.py @@ -42,11 +42,11 @@ parser = argparse.ArgumentParser() -patch_arg = parser.add_mutually_exclusive_group(required=True) +patch_arg = parser.add_mutually_exclusive_group() patch_arg.add_argument('--patch', help='path to the patch file') patch_arg.add_argument('--mdir', help='path to the directory with the patches') -parser.add_argument('--tree', required=True, help='path to the tree to test on') +parser.add_argument('--tree', help='path to the tree to test on') parser.add_argument('--tree-name', help='the tree name to expect') parser.add_argument('--result-dir', help='the directory where results will be generated') @@ -286,15 +286,23 @@ def main(): YELLOW = '' RESET = '' - args.tree = os.path.abspath(args.tree) - if args.test: config.set('tests', 'include', ','.join(args.test)) + # Handle --list-tests first, as it needs no other arguments if args.list_tests: list_tests(args, config) return + # If not listing tests, manually validate the required arguments + if not args.tree: + parser.error("the following arguments are required: --tree") + + if not args.patch and not args.mdir: + parser.error("one of the arguments --patch --mdir is required") + + args.tree = os.path.abspath(args.tree) + if args.result_dir is None: args.result_dir = tempfile.mkdtemp() print("Saving output and logs to:", args.result_dir) From accbc79299a80eb06d032e8a8760209bbf7e46c2 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 26 Oct 2025 20:57:26 +0200 Subject: [PATCH 2/2] ingest_mdir: reject invalid --test and --disable-test values 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 --- ingest_mdir.py | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/ingest_mdir.py b/ingest_mdir.py index f863258..58d8c5a 100755 --- a/ingest_mdir.py +++ b/ingest_mdir.py @@ -264,11 +264,16 @@ def load_patches(args): return series -def list_tests(args, config): - """ List all available tests and exit """ +def validate_test_list(test_list, all_test_names, parser_instance, error_description): + """Check a list of test names against the set of all available tests.""" + if test_list: + invalid_tests = [name for name in test_list if name not in all_test_names] - tester = Tester(args.result_dir, None, None, None, config=config) - print(' ', '\n '.join(tester.get_test_names())) + if invalid_tests: + invalid_str = ', '.join(invalid_tests) + msg = f"the following {error_description} are invalid: {invalid_str}\n" \ + f"Run with --list-tests to see available tests." + parser_instance.error(msg) def main(): @@ -286,15 +291,28 @@ def main(): YELLOW = '' RESET = '' - if args.test: - config.set('tests', 'include', ','.join(args.test)) + # Get all available test names for validation and --list-tests + # We can instantiate a temporary Tester just for this purpose. + tester_for_names = Tester(None, None, None, None, config=config) + all_test_names = set(tester_for_names.get_test_names()) - # Handle --list-tests first, as it needs no other arguments + # Handle --list-tests first (using the list we just fetched) if args.list_tests: - list_tests(args, config) + print(' ', '\n '.join(sorted(all_test_names))) return - # If not listing tests, manually validate the required arguments + # Validate --test and --disable-test + validate_test_list(args.test, all_test_names, parser, "tests") + validate_test_list(args.disable_test, all_test_names, parser, "disabled tests") + + # Set configs after validation + if args.test: + config.set('tests', 'include', ','.join(args.test)) + + if args.disable_test: + config.set('tests', 'exclude', ','.join(args.disable_test)) + + # If not listing tests, manually validate the other required arguments if not args.tree: parser.error("the following arguments are required: --tree")