Force the use of output_validator_args in testdata.yaml, instead of validator_flags in problem.yaml (#428)

* Rename {in,out}put_validator_{flags => args}, only allow it in testdata.yaml

(no longer in problem.yaml)

* [generate] Add "version" to KNOWN_ROOT_KEYS

* support/schemas/generators_yaml_schema.json: Grammar: space between "test{case,data,group}"

* [testcase] Remove some unused `args` parameters in validation methods

* [skel] Move validator_flags from problem.yaml to output_validator_flags in testdata.yaml in generators.yaml

* [problem] Make sure that only .get_testdata_yaml is used, instead of .validator_flags

* [interactive][problem] Fix typos

* [export] Set validator_flags for legacy DOMjudge zip format

* [test] Replace {in,out}put_validator_{flags => args}

* Remove outdated generators_yaml_schema_0_9.json

It's not referenced anywhere, and if we ever want to support version-switching to outdated schemata, we can get it back from the Git history.

* [test] Remove unnecessary quotes from generators.yamls

* [export] Fix backslash in f-string

Author: mpsijm

bin/export.py

@@ -9,6 +9,7 @@
 from typing import Optional
 
 from contest import *
+from problem import Problem
 
 
 # Replace \problemname{...} by the value of `name:` in problems.yaml in all .tex files.
@@ -113,7 +114,7 @@ def build_samples_zip(problems, output, statement_language):
     print("Wrote zip to samples.zip", file=sys.stderr)
 
 
-def build_problem_zip(problem, output):
+def build_problem_zip(problem: Problem, output: Path):
     """Make DOMjudge ZIP file for specified problem."""
 
     # Add problem PDF for only one language to the zip file (note that Kattis export does not include PDF)
@@ -157,10 +158,19 @@ def build_problem_zip(problem, output):
 
     print("Preparing to make ZIP file for problem dir %s" % problem.path, file=sys.stderr)
 
-    # DOMjudge does not support 'type' in problem.yaml yet.
+    # DOMjudge does not support 'type' in problem.yaml nor 'output_validator_args' in testdata.yaml yet.
     # TODO: Remove this once it does.
     problem_yaml_str = (problem.path / "problem.yaml").read_text()
     if not config.args.kattis and not problem.settings.is_legacy():
+        validator_flags = " ".join(
+            problem.get_testdata_yaml(
+                problem.path / "data",
+                "output_validator_args",
+                PrintBar("Getting validator_flags for legacy DOMjudge export"),
+            )
+        )
+        if validator_flags:
+            validator_flags = "validator_flags: " + validator_flags + "\n"
         write_file_strs.append(
             (
                 "problem.yaml",
@@ -172,7 +182,7 @@ def build_problem_zip(problem, output):
                     else "custom"
                     if problem.custom_output
                     else "default"
-                }\n""",
+                }\n{validator_flags}""",
             )
         )
     else:

bin/generate.py

@@ -246,6 +246,7 @@ def run_interaction(self, bar, cwd, t):
             return True
 
         testcase = Testcase(self.problem, in_path, short_path=(t.path.parent / (t.name + ".in")))
+        assert isinstance(self.program, run.Submission)
         r = run.Run(self.problem, self.program, testcase)
 
         # No {name}/{seed} substitution is done since all IO should be via stdin/stdout.
@@ -341,7 +342,7 @@ def __init__(self, generator_config):
     "retries",
 ]
 RESERVED_DIRECTORY_KEYS: Final[Sequence[str]] = ["command"]
-KNOWN_ROOT_KEYS: Final[Sequence[str]] = ["generators", "parallel"]
+KNOWN_ROOT_KEYS: Final[Sequence[str]] = ["generators", "parallel", "version"]
 DEPRECATED_ROOT_KEYS: Final[Sequence[str]] = ["gitignore_generated"]
 
 
@@ -445,7 +446,7 @@ def __init__(self, problem, generator_config, key, name: str, yaml, parent, coun
         self.hardcoded = {}
 
         # Hash of testcase for caching.
-        self.hash = None
+        self.hash: str
 
         # Yaml of rule
         self.rule = dict[str, str | int]()
@@ -654,12 +655,12 @@ def link(t, problem, generator_config, bar, dst):
                 # both source and target do not exist
                 pass
 
-    def validate_in(t, problem, testcase, meta_yaml, bar):
+    def validate_in(t, problem: Problem, testcase: Testcase, meta_yaml: dict, bar: ProgressBar):
         infile = problem.tmpdir / "data" / t.hash / "testcase.in"
         assert infile.is_file()
 
         input_validator_hashes = testcase.validator_hashes(validate.InputValidator, bar)
-        if all(h in meta_yaml.get("input_validator_hashes") for h in input_validator_hashes):
+        if all(h in meta_yaml["input_validator_hashes"] for h in input_validator_hashes):
             return True
 
         if not testcase.validate_format(
@@ -696,7 +697,7 @@ def validate_in(t, problem, testcase, meta_yaml, bar):
             )
         return True
 
-    def validate_ans(t, problem, testcase, meta_yaml, bar):
+    def validate_ans(t, problem: Problem, testcase: Testcase, meta_yaml: dict, bar: ProgressBar):
         infile = problem.tmpdir / "data" / t.hash / "testcase.in"
         assert infile.is_file()
 
@@ -725,7 +726,7 @@ def validate_ans(t, problem, testcase, meta_yaml, bar):
                 **testcase.validator_hashes(validate.AnswerValidator, bar),
                 **testcase.validator_hashes(validate.OutputValidator, bar),
             }
-            if all(h in meta_yaml.get("answer_validator_hashes") for h in answer_validator_hashes):
+            if all(h in meta_yaml["answer_validator_hashes"] for h in answer_validator_hashes):
                 return True
 
             if not testcase.validate_format(
@@ -1176,7 +1177,7 @@ def __init__(
                     )
                 if key in DEPRECATED_ROOT_KEYS:
                     message(
-                        f"Dreprecated root level key: {key}, ignored",
+                        f"Deprecated root level key: {key}, ignored",
                         "generators.yaml",
                         self.path,
                         color_type=MessageType.WARN,

bin/interactive.py

@@ -29,6 +29,7 @@ def run_interactive_testcase(
     # else: path
     interaction: Optional[bool | Path] = False,
     submission_args: Optional[list[str]] = None,
+    bar: Optional[ProgressBar] = None,
 ):
     output_validators = run.problem.validators(validate.OutputValidator)
     if len(output_validators) != 1:
@@ -53,9 +54,13 @@ def get_validator_command():
                 run.testcase.ans_path.resolve(),
                 run.feedbackdir.resolve(),
             ]
-            + run.problem.settings.validator_flags
+            + run.testcase.testdata_yaml_validator_args(
+                output_validator,
+                bar or PrintBar("Run interactive test case"),
+            )
         )
 
+    assert run.submission.run_command, "Submission must be built"
     submission_command = run.submission.run_command
     if submission_args:
         submission_command += submission_args
@@ -64,7 +69,7 @@ def get_validator_command():
     validator_dir = output_validator.tmpdir
     submission_dir = run.submission.tmpdir
 
-    nextpass = run.feedbackdir / "nextpass.in" if run.problem.multi_pass else False
+    nextpass = run.feedbackdir / "nextpass.in" if run.problem.multi_pass else None
 
     if config.args.verbose >= 2:
         print("Validator:  ", *get_validator_command(), file=sys.stderr)

bin/problem.py

@@ -268,9 +268,6 @@ def __init__(
         if "name" in yaml_data and isinstance(yaml_data["name"], str):
             yaml_data["name"] = {"en": yaml_data["name"]}
 
-        if "validator_flags" in yaml_data and isinstance(yaml_data["validator_flags"], str):
-            yaml_data["validator_flags"] = shlex.split(yaml_data["validator_flags"])
-
         # Known keys:
         # (defaults from https://icpc.io/problem-package-format/spec/2023-07-draft.html#problem-metadata)
         self.problem_format_version: str = parse_setting(
@@ -328,8 +325,21 @@ def __init__(
         # Not implemented in BAPCtools. Should be a date, but we don't do anything with this anyway.
         self.embargo_until: str = parse_setting(yaml_data, "embargo-until", "")
         self.limits = ProblemLimits(parse_setting(yaml_data, "limits", {}), problem, self)
-        # TODO: move to testdata.yaml
-        self.validator_flags: list[str] = parse_setting(yaml_data, "validator_flags", [])
+
+        # If problem.yaml uses 2023-07-draft, disallow `validator_flags`.
+        if self.is_legacy():
+            if "validator_flags" in yaml_data and isinstance(yaml_data["validator_flags"], str):
+                yaml_data["validator_flags"] = shlex.split(yaml_data["validator_flags"])
+            # This field should not be used anywhere except the default result of Problem.get_testdata_yaml().
+            self._validator_flags: list[str] = parse_setting(yaml_data, "validator_flags", [])
+        else:
+            self._validator_flags = []
+            if "validator_flags" in yaml_data:
+                warn(
+                    "problem.yaml: 'validator_flags' is removed in 2023-07-draft, please use 'output_validator_args' in 'testdata.yaml' instead. SKIPPED."
+                )
+                yaml_data.pop("validator_flags")
+
         self.keywords: str = parse_setting(yaml_data, "keywords", "")
         # Not implemented in BAPCtools. We always test all languges in langauges.yaml.
         self.languages: list[str] = parse_optional_list_setting(yaml_data, "languages", str)
@@ -481,6 +491,7 @@ def _read_settings(self):
         self.multi_pass: bool = self.settings.multi_pass
         self.custom_output: bool = self.settings.custom_output
 
+    # TODO #102 move to TestData class
     def _parse_testdata_yaml(p, path, bar):
         assert path.is_relative_to(p.path / "data")
         for dir in [path] + list(path.parents):
@@ -495,20 +506,36 @@ def _parse_testdata_yaml(p, path, bar):
                 if f not in p._testdata_yamls:
                     p._testdata_yamls[f] = flags = read_yaml(f, plain=True)
 
-                    # verify testdata.yaml
+                    if p.settings.is_legacy():
+                        # For legacy problems, support both _flags and _args, but move to _args.
+                        if (
+                            "output_validator_flags" in flags
+                            and "output_validator_args" not in flags
+                        ):
+                            flags["output_validator_args"] = flags.pop("output_validator_flags")
+                        if "input_validator_flags" in flags and "input_validator_args" not in flags:
+                            flags["input_validator_args"] = flags.pop("input_validator_flags")
+                    else:
+                        # For 2023-07-draft problems, skip the old name and warn to use the new one.
+                        if "input_validator_flags" in flags:
+                            bar.warn(
+                                "input_validator_flags is removed in 2023-07-draft, use ..._args instead. SKIPPED."
+                            )
+                        if "output_validator_flags" in flags:
+                            bar.warn(
+                                "output_validator_flags is removed in 2023-07-draft, use ..._args instead. SKIPPED."
+                            )
+
+                    # Verify testdata.yaml
                     for k in flags:
                         match k:
-                            case "output_validator_flags":
+                            case "output_validator_args":
                                 if not isinstance(flags[k], str):
-                                    bar.error(
-                                        "ouput_validator_flags must be string",
-                                        resume=True,
-                                        print_item=False,
-                                    )
-                            case "input_validator_flags":
+                                    bar.error(f"{k} must be string", resume=True, print_item=False)
+                            case "input_validator_args":
                                 if not isinstance(flags[k], (str, dict)):
                                     bar.error(
-                                        "input_validator_flags must be string or map",
+                                        f"{k} must be string or map",
                                         resume=True,
                                         print_item=False,
                                     )
@@ -521,16 +548,32 @@ def _parse_testdata_yaml(p, path, bar):
                                             f"Unknown input validator {name}; expected {input_validator_names}",
                                             print_item=False,
                                         )
-                            case "grading" | "run_samples":
-                                bar.warn(f"{k} not implemented in BAPCtools", print_item=False)
+                            case (
+                                "args"
+                                | "description"
+                                | "full_feedback"
+                                | "hint"
+                                | "scoring"
+                                | "static_validation"
+                            ):
+                                bar.warn(
+                                    f"{k} in testdata.yaml not implemented in BAPCtools",
+                                    print_item=False,
+                                )
                             case _:
                                 path = f.relative_to(p.path / "data")
                                 bar.warn(f'Unknown key "{k}" in {path}', print_item=False)
             # Do not go above the data directory.
             if dir == p.path / "data":
                 break
 
-    def get_testdata_yaml(p, path, key, bar, name=None) -> str | None:
+    def get_testdata_yaml(
+        p,
+        path: Path,
+        key: Literal["input_validator_args"] | Literal["output_validator_args"],
+        bar: ProgressBar | PrintBar,
+        name: Optional[str] = None,
+    ) -> list[str]:
         """
         Find the testdata flags applying at the given path for the given key.
         If necessary, walk up from `path` looking for the first testdata.yaml file that applies,
@@ -540,50 +583,55 @@ def get_testdata_yaml(p, path, key, bar, name=None) -> str | None:
         Arguments
         ---------
         path: absolute path (a file or a directory)
-        key: The testdata.yaml key to look for, either of 'input_validator_flags', 'output_validator_flags', or 'grading'.
-            'grading' is not implemented
-        name: If key == 'input_validator_flags', optionally the name of the input validator
+        key: The testdata.yaml key to look for, either of 'input_validator_args', 'output_validator_args', or 'grading'.
+            TODO: 'grading' is not yet implemented.
+        name: If key == 'input_validator_args', optionally the name of the input validator.
 
         Returns:
         --------
-        string or None if no testdata.yaml is found.
+        A list of string arguments, which is empty if no testdata.yaml is found.
         TODO: when 'grading' is supported, it also can return dict
         """
-        if key not in ["input_validator_flags", "output_validator_flags"]:
+        if key not in ["input_validator_args", "output_validator_args"]:
             raise NotImplementedError(key)
-        if key != "input_validator_flags" and name is not None:
+        if key != "input_validator_args" and name is not None:
             raise ValueError(
                 f"Only input validators support flags by validator name, got {key} and {name}"
             )
 
         # parse and cache testdata.yaml
         p._parse_testdata_yaml(path, bar)
 
+        # For legacy problems, default to validator_flags from problem.yaml
+        default_result = []
+        if p.settings.is_legacy() and p.settings._validator_flags:
+            default_result = p.settings._validator_flags
+
         # extract the flags
         for dir in [path] + list(path.parents):
             # Do not go above the data directory.
             if dir == p.path:
-                return None
+                return default_result
 
             f = dir / "testdata.yaml"
             if f not in p._testdata_yamls:
                 continue
             flags = p._testdata_yamls[f]
             if key in flags:
-                if key == "output_validator_flags":
+                if key == "output_validator_args":
                     if not isinstance(flags[key], str):
-                        bar.error("ouput_validator_flags must be string")
-                    return flags[key]
+                        bar.error("ouput_validator_args must be string")
+                    return flags[key].split()
 
-                if key == "input_validator_flags":
+                if key == "input_validator_args":
                     if not isinstance(flags[key], (str, dict)):
-                        bar.error("input_validator_flags must be string or map")
+                        bar.error("input_validator_args must be string or map")
                     if isinstance(flags[key], str):
-                        return flags[key]
+                        return flags[key].split()
                     elif name in flags[key]:
-                        return flags[key][name]
+                        return flags[key][name].split()
 
-        return None
+        return default_result
 
     def testcases(
         p,
@@ -1210,14 +1258,11 @@ def validate_valid_extra_data(p) -> bool:
         if not p.validators(validate.OutputValidator, strict=True, print_warn=False):
             return True
 
-        args = (
-            p.get_testdata_yaml(
-                p.path / "data" / "valid_output",
-                "output_validator_flags",
-                PrintBar("Generic Output Validation"),
-            )
-            or ""
-        ).split()
+        args = p.get_testdata_yaml(
+            p.path / "data" / "valid_output",
+            "output_validator_args",
+            PrintBar("Generic Output Validation"),
+        )
         is_space_sensitive = "space_change_sensitive" in args
         is_case_sensitive = "case_sensitive" in args
 
@@ -1313,7 +1358,7 @@ def _validate_data(
         # validate the testcases
         bar = ProgressBar(action, items=[t.name for t in testcases])
 
-        def process_testcase(testcase):
+        def process_testcase(testcase: testcase.Testcase):
             nonlocal success
 
             localbar = bar.start(testcase.name)