Draft: add validation of generically generated invalid files #420
Conversation
|
@mpsijm @RagnarGrootKoerkamp more input here is welcome :) |
| action = ( | ||
| "Invalidation" | ||
| if mode == validate.Mode.INVALID | ||
| else ( | ||
| f"Collecting {mode} constraints" if constraints else f"{mode} validation" | ||
| ).capitalize() | ||
| ) |
There was a problem hiding this comment.
Why .capitalize() the action when both branches return something that already starts with a capital? 🤔
Oh wait, the mode is not capitalized. Maybe this can be simplified to only mode.capitalize() then 🙂
| if test_path: | ||
| test_case = test_path.relative_to(p.path / "data").with_suffix("") | ||
| log(f"Generating invalid testcases based on: {test_case}") | ||
| debug(f"writing generated invalid testcases to: {base_path}") |
There was a problem hiding this comment.
This debug log is visible, even without -v, I guess that's intended ('cause the log is "debug" and not "verbose")?
| from typing import Optional, TypeVar | ||
|
|
||
| _GENERATOR_NAMES: set[str] = set() | ||
| GENERATORS: list[tuple[str, str | Callable[[str], Optional[str]], bool]] = [] |
There was a problem hiding this comment.
Maybe add documentation comments about what going on here? From what I'm gathering, these generators are created once validator_tests is imported, and these generators are then used to generate invalid test cases in problem.py.
One note about creating these generators in top-level code: this does mean that this is always executed, also in the many instances where a command different than bt validate is called, and also when performing tab-completion in your shell. Since these generators are only used inside Problem.validate_invalid_extra_data, perhaps we should wrap this in a function anyway 😛 I think Python will still us nest the decorator magic inside that function 🙂
There was a problem hiding this comment.
this is just a global var with all the generators, i.e. the same as GENERATORS = [(), (), ()...] i.e. trivial to construct. I think its just clearer this way and easier to extend.
|
|
||
| testcases.append(testcase.Testcase(p, full_path, short_path=short_path)) | ||
|
|
||
| return p._validate_data(validate.Mode.INVALID, None, "Generic Invalidation", testcases) |
There was a problem hiding this comment.
Currently, doc/validation.md only talks about Input/Answer/Output (in)validation, I think it would be good to add a section about this new "Generic Invalidation" as well 🙂
Dynamically generates (likely) invalid testcases to challenge validators
related: #352