From 5e2c7ebcb19447629a3bda6e76af13566e44f72c Mon Sep 17 00:00:00 2001 From: Ilya Priven Date: Tue, 6 Jun 2023 20:43:14 -0400 Subject: [PATCH 1/2] Fix testcase parsing gotchas --- mypy/test/data.py | 58 +++++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/mypy/test/data.py b/mypy/test/data.py index daf815dbdbdc..0d822868c846 100644 --- a/mypy/test/data.py +++ b/mypy/test/data.py @@ -189,10 +189,15 @@ def parse_test_case(case: DataDrivenTestCase) -> None: elif item.id == "triggered" and item.arg is None: triggered = item.data else: - raise ValueError(f"Invalid section header {item.id} in {case.file}:{item.line}") + section = item.id + (f" {item.arg}" if item.arg else "") + raise ValueError( + f"{case.file}:{case.line + item.line - 2}: Invalid section header [{section}] in case {case.name!r}" + ) if out_section_missing: - raise ValueError(f"{case.file}, line {first_item.line}: Required output section not found") + raise ValueError( + f"{case.file}:{case.line}: Required output section not found in case {case.name!r}" + ) for passnum in stale_modules.keys(): if passnum not in rechecked_modules: @@ -220,7 +225,7 @@ def parse_test_case(case: DataDrivenTestCase) -> None: for file, _ in files: if file in seen_files: raise ValueError( - f"{case.file}, line {first_item.line}: Duplicated filename {file}. Did you include" + f"{case.file}:{case.line}: Duplicated filename {file}. Did you include" " it multiple times?" ) @@ -634,6 +639,16 @@ def pytest_pycollect_makeitem(collector: Any, name: str, obj: object) -> Any | N return None +_case_name_pattern = re.compile( + r"(?P[a-zA-Z_0-9]+)" + r"(?P-writescache)?" + r"(?P-only_when_cache|-only_when_nocache)?" + r"(-(?Pposix|windows))?" + r"(?P-skip)?" + r"(?P-xfail)?" +) + + def split_test_cases( parent: DataFileCollector, suite: DataSuite, file: str ) -> Iterator[DataDrivenTestCase]: @@ -644,40 +659,33 @@ def split_test_cases( """ with open(file, encoding="utf-8") as f: data = f.read() - # number of groups in the below regex - NUM_GROUPS = 7 - cases = re.split( - r"^\[case ([a-zA-Z_0-9]+)" - r"(-writescache)?" - r"(-only_when_cache|-only_when_nocache)?" - r"(-posix|-windows)?" - r"(-skip)?" - r"(-xfail)?" - r"\][ \t]*$\n", - data, - flags=re.DOTALL | re.MULTILINE, - ) - line_no = cases[0].count("\n") + 1 + cases = re.split(r"^\[case ([^]+)]+)\][ \t]*$\n", data, flags=re.DOTALL | re.MULTILINE) + cases_iter = iter(cases) + line_no = next(cases_iter).count("\n") + 1 test_names = set() - for i in range(1, len(cases), NUM_GROUPS): - name, writescache, only_when, platform_flag, skip, xfail, data = cases[i : i + NUM_GROUPS] + for case_id in cases_iter: + data = next(cases_iter) + + m = _case_name_pattern.match(case_id) + if not m: + raise RuntimeError(f"Invalid testcase id {case_id!r}") + name = m.group("name") if name in test_names: raise RuntimeError( 'Found a duplicate test name "{}" in {} on line {}'.format( name, parent.name, line_no ) ) - platform = platform_flag[1:] if platform_flag else None yield DataDrivenTestCase.from_parent( parent=parent, suite=suite, file=file, name=add_test_name_suffix(name, suite.test_name_suffix), - writescache=bool(writescache), - only_when=only_when, - platform=platform, - skip=bool(skip), - xfail=bool(xfail), + writescache=bool(m.group("writescache")), + only_when=m.group("only_when"), + platform=m.group("platform"), + skip=bool(m.group("skip")), + xfail=bool(m.group("xfail")), data=data, line=line_no, ) From b39cd0f79494d0f4e9510b1cf34f3c5a8004a15c Mon Sep 17 00:00:00 2001 From: Ilya Priven Date: Wed, 7 Jun 2023 10:03:58 -0400 Subject: [PATCH 2/2] more love --- mypy/test/data.py | 71 ++++++++----------- mypy/test/meta/__init__.py | 0 mypy/test/meta/test_parse_data.py | 66 +++++++++++++++++ .../test_update_data.py} | 7 +- 4 files changed, 102 insertions(+), 42 deletions(-) create mode 100644 mypy/test/meta/__init__.py create mode 100644 mypy/test/meta/test_parse_data.py rename mypy/test/{testupdatedata.py => meta/test_update_data.py} (94%) diff --git a/mypy/test/data.py b/mypy/test/data.py index 0d822868c846..86193e303d9c 100644 --- a/mypy/test/data.py +++ b/mypy/test/data.py @@ -12,7 +12,7 @@ from abc import abstractmethod from dataclasses import dataclass from pathlib import Path -from typing import Any, Iterator, NamedTuple, Pattern, Union +from typing import Any, Iterator, NamedTuple, NoReturn, Pattern, Union from typing_extensions import Final, TypeAlias as _TypeAlias import pytest @@ -77,11 +77,19 @@ def parse_test_case(case: DataDrivenTestCase) -> None: targets: dict[int, list[str]] = {} # Fine-grained targets (per fine-grained update) test_modules: list[str] = [] # Modules which are deemed "test" (vs "fixture") + def _case_fail(msg: str) -> NoReturn: + pytest.fail(f"{case.file}:{case.line}: {msg}", pytrace=False) + # Process the parsed items. Each item has a header of form [id args], # optionally followed by lines of text. item = first_item = test_items[0] test_modules.append("__main__") for item in test_items[1:]: + + def _item_fail(msg: str) -> NoReturn: + item_abs_line = case.line + item.line - 2 + pytest.fail(f"{case.file}:{item_abs_line}: {msg}", pytrace=False) + if item.id in {"file", "fixture", "outfile", "outfile-re"}: # Record an extra file needed for the test case. assert item.arg is not None @@ -132,9 +140,11 @@ def parse_test_case(case: DataDrivenTestCase) -> None: # File/directory to delete during a multi-step test case assert item.arg is not None m = re.match(r"(.*)\.([0-9]+)$", item.arg) - assert m, f"Invalid delete section: {item.arg}" + if m is None: + _item_fail(f"Invalid delete section {item.arg!r}") num = int(m.group(2)) - assert num >= 2, f"Can't delete during step {num}" + if num < 2: + _item_fail(f"Can't delete during step {num}") full = join(base_path, m.group(1)) deleted_paths.setdefault(num, set()).add(full) elif re.match(r"out[0-9]*$", item.id): @@ -150,29 +160,18 @@ def parse_test_case(case: DataDrivenTestCase) -> None: if arg.startswith("version"): compare_op = arg[7:9] if compare_op not in {">=", "=="}: - raise ValueError( - "{}, line {}: Only >= and == version checks are currently supported".format( - case.file, item.line - ) - ) + _item_fail("Only >= and == version checks are currently supported") version_str = arg[9:] try: version = tuple(int(x) for x in version_str.split(".")) except ValueError: - raise ValueError( - '{}, line {}: "{}" is not a valid python version'.format( - case.file, item.line, version_str - ) - ) + _item_fail(f"{version_str!r} is not a valid python version") if compare_op == ">=": version_check = sys.version_info >= version elif compare_op == "==": if not 1 < len(version) < 4: - raise ValueError( - "{}, line {}: Only minor or patch version checks " - 'are currently supported with "==": "{}"'.format( - case.file, item.line, version_str - ) + _item_fail( + f'Only minor or patch version checks are currently supported with "==": {version_str!r}' ) version_check = sys.version_info[: len(version)] == version if version_check: @@ -189,15 +188,11 @@ def parse_test_case(case: DataDrivenTestCase) -> None: elif item.id == "triggered" and item.arg is None: triggered = item.data else: - section = item.id + (f" {item.arg}" if item.arg else "") - raise ValueError( - f"{case.file}:{case.line + item.line - 2}: Invalid section header [{section}] in case {case.name!r}" - ) + section_str = item.id + (f" {item.arg}" if item.arg else "") + _item_fail(f"Invalid section header [{section_str}] in case {case.name!r}") if out_section_missing: - raise ValueError( - f"{case.file}:{case.line}: Required output section not found in case {case.name!r}" - ) + _case_fail(f"Required output section not found in case {case.name!r}") for passnum in stale_modules.keys(): if passnum not in rechecked_modules: @@ -209,11 +204,7 @@ def parse_test_case(case: DataDrivenTestCase) -> None: and passnum in rechecked_modules and not stale_modules[passnum].issubset(rechecked_modules[passnum]) ): - raise ValueError( - ( - "Stale modules after pass {} must be a subset of rechecked modules ({}:{})" - ).format(passnum, case.file, first_item.line) - ) + _case_fail(f"Stale modules after pass {passnum} must be a subset of rechecked modules") output_inline_start = len(output) input = first_item.data @@ -224,10 +215,7 @@ def parse_test_case(case: DataDrivenTestCase) -> None: seen_files = set() for file, _ in files: if file in seen_files: - raise ValueError( - f"{case.file}:{case.line}: Duplicated filename {file}. Did you include" - " it multiple times?" - ) + _case_fail(f"Duplicated filename {file}. Did you include it multiple times?") seen_files.add(file) @@ -372,12 +360,13 @@ def setup(self) -> None: self.steps = [steps.get(num, []) for num in range(2, max_step + 1)] def teardown(self) -> None: - assert self.old_cwd is not None and self.tmpdir is not None, "test was not properly set up" - os.chdir(self.old_cwd) - try: - self.tmpdir.cleanup() - except OSError: - pass + if self.old_cwd is not None: + os.chdir(self.old_cwd) + if self.tmpdir is not None: + try: + self.tmpdir.cleanup() + except OSError: + pass self.old_cwd = None self.tmpdir = None @@ -666,7 +655,7 @@ def split_test_cases( for case_id in cases_iter: data = next(cases_iter) - m = _case_name_pattern.match(case_id) + m = _case_name_pattern.fullmatch(case_id) if not m: raise RuntimeError(f"Invalid testcase id {case_id!r}") name = m.group("name") diff --git a/mypy/test/meta/__init__.py b/mypy/test/meta/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/mypy/test/meta/test_parse_data.py b/mypy/test/meta/test_parse_data.py new file mode 100644 index 000000000000..6d9b32b96c7e --- /dev/null +++ b/mypy/test/meta/test_parse_data.py @@ -0,0 +1,66 @@ +""" +A "meta test" which tests the parsing of .test files. This is not meant to become exhaustive +but to ensure we maintain a basic level of ergonomics for mypy contributors. +""" +import subprocess +import sys +import textwrap +from pathlib import Path + +from mypy.test.config import test_data_prefix +from mypy.test.helpers import Suite + + +class ParseTestDataSuite(Suite): + def _dedent(self, s: str) -> str: + return textwrap.dedent(s).lstrip() + + def _run_pytest(self, data_suite: str) -> str: + p = Path(test_data_prefix) / "check-__fixture__.test" + assert not p.exists() + try: + p.write_text(data_suite) + test_nodeid = f"mypy/test/testcheck.py::TypeCheckSuite::{p.name}" + args = [sys.executable, "-m", "pytest", "-n", "0", "-s", test_nodeid] + proc = subprocess.run(args, capture_output=True, check=False) + return proc.stdout.decode() + finally: + p.unlink() + + def test_parse_invalid_case(self) -> None: + # Arrange + data = self._dedent( + """ + [case abc] + s: str + [case foo-XFAIL] + s: str + """ + ) + + # Act + actual = self._run_pytest(data) + + # Assert + assert "Invalid testcase id 'foo-XFAIL'" in actual + + def test_parse_invalid_section(self) -> None: + # Arrange + data = self._dedent( + """ + [case abc] + s: str + [unknownsection] + abc + """ + ) + + # Act + actual = self._run_pytest(data) + + # Assert + expected_lineno = data.splitlines().index("[unknownsection]") + 1 + expected = ( + f".test:{expected_lineno}: Invalid section header [unknownsection] in case 'abc'" + ) + assert expected in actual diff --git a/mypy/test/testupdatedata.py b/mypy/test/meta/test_update_data.py similarity index 94% rename from mypy/test/testupdatedata.py rename to mypy/test/meta/test_update_data.py index 2a6d2462f10c..879e7e4a85a8 100644 --- a/mypy/test/testupdatedata.py +++ b/mypy/test/meta/test_update_data.py @@ -1,3 +1,8 @@ +""" +A "meta test" which tests the `--update-data` feature for updating .test files. +Updating the expected output, especially when it's in the form of inline (comment) assertions, +can be brittle, which is why we're "meta-testing" here. +""" import shlex import subprocess import sys @@ -14,7 +19,7 @@ def _run_pytest_update_data(self, data_suite: str, *, max_attempts: int) -> str: Runs a suite of data test cases through 'pytest --update-data' until either tests pass or until a maximum number of attempts (needed for incremental tests). """ - p = Path(test_data_prefix) / "check-update-data.test" + p = Path(test_data_prefix) / "check-__fixture__.test" assert not p.exists() try: p.write_text(textwrap.dedent(data_suite).lstrip())