fix/improve argparsing tests

Author: olupton

.github/triage/jax_toolbox_triage/args.py

@@ -10,9 +10,9 @@ def parse_commit_argument(s):
     ret = {}
     for part in s.split(","):
         sw, commit = part.split(":", 1)
-        assert sw in {"jax", "xla"}, sw
         assert sw not in ret, ret
         ret[sw] = commit
+    assert ret.keys() == {"jax", "xla"}, ret.keys()
     return ret
 
 
@@ -175,26 +175,19 @@ def parse_args(args=None):
     )
     args = parser.parse_args(args=args)
     assert args.container_runtime in {"docker", "pyxis"}, args.container_runtime
-    num_explicit_containers = (args.passing_container is not None) + (
-        args.failing_container is not None
-    )
-    if num_explicit_containers == 2:
-        # Explicit mode, --container, --start-date and --end-date are all ignored
-        # TODO: --passing-commits --failing-commits
-        if args.container:
-            raise Exception(
-                "--container must not be passed if --passing-container and --failing-container are"
-            )
-        if args.start_date:
-            raise Exception(
-                "--start-date must not be passed if --passing-container and --failing-container are"
-            )
-        if args.end_date:
-            raise Exception(
-                "--end-date must not be passed if --passing-container and --failing-container are"
-            )
-    elif num_explicit_containers == 0 and args.container is None:
-        raise Exception(
-            "--container must be passed if --passing-container and --failing-container are not"
-        )
+    passing_commits_known = (args.passing_container is not None) or (args.passing_commits is not None)
+    failing_commits_known = (args.failing_container is not None) or (args.failing_commits is not None)
+    sets_of_known_commits = passing_commits_known + failing_commits_known
+    if sets_of_known_commits == 2:
+        # If the container-level search is being skipped, because a valid combination
+        # of --{passing,failing}-{commits,container} is passed, then no container-level
+        # search options should be passed.
+        assert args.container is None and args.start_date is None and args.end_date is None, "No container-level search options should be passed if the passing/failing containers/commits have been passed explicitly."
+        assert args.passing_container is not None or args.failing_container is not None, ""
+    elif sets_of_known_commits == 1:
+        raise Exception("If --passing-{commits OR container} is passed then --failing-{commits OR container} should be too")
+    else:
+        # None of --{passing,failing}-{commits,container} were passed, make sure the
+        # compulsory arguments for the container-level search were passed
+        assert args.container is not None, "--container must be passed for the container-level search"
     return args

.github/triage/tests/test_arg_parsing.py

@@ -8,6 +8,10 @@
     "--failing-container",
     "failing-url",
 ]
+valid_container_and_commits = [
+    ["--passing-container", "passing-url", "--failing-commits", "jax:0123456789,xla:fedcba9876543210"],
+    ["--failing-container", "failing-url", "--passing-commits", "xla:fedcba9876543210,jax:0123456789"],
+]
 valid_start_end_date_args = [
     ["--container", "jax"],
     ["--container", "jax", "--start-date", "2024-10-02"],
@@ -17,7 +21,7 @@
 
 
 @pytest.mark.parametrize(
-    "good_args", [valid_start_end_container] + valid_start_end_date_args
+    "good_args", [valid_start_end_container] + valid_start_end_date_args + valid_container_and_commits
 )
 def test_good_container_args(good_args):
     args = parse_args(good_args + test_command)
@@ -39,9 +43,20 @@ def test_bad_container_arg_combinations_across_groups(date_args):
         ["--start-date", "2024-10-01"],
         ["--end-date", "2024-10-02"],
         ["--start-date", "2024-10-01", "--end-date", "2024-10-02"],
-        # Need both if either is passed
+        # If --passing-container (or --passing-commits) is passed then
+        # --failing-container (or --failing-commits) must be too
         ["--passing-container", "passing-url"],
         ["--failing-container", "failing-url"],
+        # Need at least one container
+        ["--passing-commits", "jax:0123456789,xla:fedcba9876543210", "--failing-commits", "xla:fedcba9876543210,jax:0123456789"],
+        # Cannot combine --passing-container with --passing-commits etc.
+        ["--passing-container", "passing-url", "--passing-commits", "jax:123,xla:456"],
+        ["--failing-container", "failing-url", "--failing-commits", "jax:123,xla:456"],
+        # --{passing,failing}-commits must be formatted correctly
+        ["--passing-container", "passing-url", "--failing-commits", "jax:123"],
+        ["--passing-container", "passing-url", "--failing-commits", "jax:123,jax:456"],
+        ["--passing-container", "passing-url", "--failing-commits", "xla:123"],
+        ["--passing-container", "passing-url", "--failing-commits", "bob:123"],
     ],
 )
 def test_bad_container_arg_combinations_within_groups(container_args):
@@ -60,3 +75,7 @@ def test_bad_container_arg_combinations_within_groups(container_args):
 def test_unparsable_container_args(container_args):
     with pytest.raises(SystemExit):
         parse_args(container_args + test_command)
+
+def test_invalid_container_runtime():
+    with pytest.raises(Exception):
+        parse_args(["--container-runtime=magic-beans"] + test_command)