diff --git a/tools/check_skips.py b/tools/check_skips.py index afe85d4bb..0afad4f27 100644 --- a/tools/check_skips.py +++ b/tools/check_skips.py @@ -12,13 +12,15 @@ WINDOWS_PREFIX = "agc" def skips(path): for case in ET.parse(path).getroot().iter("testcase"): - if case.find("skipped") is None: + skipped = case.find("skipped") + if skipped is None: continue name = case.get("name") if not name: - continue - output = case.find("system-out") - yield name, (output.text or "") if output is not None else "" + raise ValueError(f"{path}: skipped testcase has no name") + output = ["skipped, " + reason for reason in (skipped.get("message", ""), skipped.text or "") if reason] + output += [case.findtext(channel) or "" for channel in ("system-out", "system-err")] + yield name, "\n".join(output) if __name__ == "__main__": @@ -27,7 +29,10 @@ if __name__ == "__main__": parser.add_argument("--job", choices=("linux", "windows"), default="linux") parser.add_argument("--list", action="store_true", help="print the skipped tests and why the file says they skipped") args = parser.parse_args() - found = sorted(skips(args.junit)) + try: + found = sorted(skips(args.junit)) + except (OSError, ET.ParseError, ValueError) as error: + parser.error(str(error)) if args.list: for name, output in found: reason = next((line.strip() for line in output.splitlines() if line.startswith("skipped")), "") diff --git a/tools/check_test_registration.py b/tools/check_test_registration.py index 991cdb59c..83311cb20 100644 --- a/tools/check_test_registration.py +++ b/tools/check_test_registration.py @@ -4,6 +4,8 @@ import subprocess from pathlib import Path TEST_OUTPUT = r"tests[\\/]" +COMMAND = re.compile(r"^\s*(add_test|set_tests_properties)\s*\(", re.M | re.I) +ARGUMENT = re.compile(r'"(?:\\.|[^"\\])*"|\[(?P=*)\[.*?\](?P=equals)\]|[^\s()]+', re.S) KNOWN_UNRUN = set() @@ -16,10 +18,10 @@ def built(build): path, _, rule = line.partition(": ") if "EXECUTABLE" not in rule: continue - found = re.match(rf"^{TEST_OUTPUT}([A-Za-z0-9_.]+)$", path) + found = re.match(rf"^{TEST_OUTPUT}(?:.*[\\/])?([^\\/]+)$", path) if not found: continue - name = found.group(1).rstrip(".") + name = found.group(1) if name.endswith(".exe"): name = name[:-4] if name: @@ -27,16 +29,58 @@ def built(build): return names +def commands(content): + parsed_until = 0 + for command in COMMAND.finditer(content): + if command.start() < parsed_until: + continue + position, arguments = command.end(), [] + while position < len(content): + if content[position].isspace(): + position += 1 + continue + if content[position] == ")": + parsed_until = position + 1 + yield command.group(1).lower(), arguments + break + if content[position] == "#": + position = content.find("\n", position) + if position < 0: + break + continue + token = ARGUMENT.match(content, position) + if token is None: + raise ValueError(f"invalid CTest argument at position {position}") + argument = token.group(0) + if argument.startswith('"'): + argument = re.sub(r"\\(.)", lambda match: {"n": "\n", "r": "\r", "t": "\t"}.get(match[1], match[1]), argument[1:-1], flags=re.S) + elif token.group("equals") is not None: + width = len(token.group("equals")) + 2 + argument = argument[width:-width] + arguments.append(argument) + position = token.end() + + def registered(build): names = set() for file in Path(build).rglob("CTestTestfile.cmake"): - for line in file.read_text(errors="replace").splitlines(): - found = re.match(r"\s*add_test\s*\((.*)\)\s*$", line) - if not found: + tests, disabled = {}, {} + for command, arguments in commands(file.read_text(encoding="utf-8", errors="replace")): + if command == "add_test" and arguments: + tests[arguments[0]] = arguments[1:] + elif command == "set_tests_properties" and "PROPERTIES" in arguments: + properties = arguments.index("PROPERTIES") + values = arguments[properties + 1:] + for key, value in zip(values[::2], values[1::2]): + if key == "DISABLED": + for name in arguments[:properties]: + disabled[name] = value.upper() in ("1", "ON", "YES", "TRUE", "Y") + for test, arguments in tests.items(): + if disabled.get(test, False): continue - tokens = re.findall(r'"([^"]*)"|(\S+)', found.group(1)) - for token in tokens[1:]: - argument = token[0] or token[1] + for index, argument in enumerate(arguments): + if index and (argument.startswith("-") or re.match(r"^[A-Za-z_]\w*=", argument)): + continue base = re.split(r"[\\/]", argument)[-1] if base.endswith(".exe"): base = base[:-4] diff --git a/tools/tests/test_check_skips.py b/tools/tests/test_check_skips.py index c668ca75d..02ab77f59 100644 --- a/tools/tests/test_check_skips.py +++ b/tools/tests/test_check_skips.py @@ -59,5 +59,41 @@ class CheckSkipsTests(unittest.TestCase): self.assertNotIn("error: geometry skipped", result.stdout) + + +class SkipReportTests(unittest.TestCase): + def skipped(self, content): + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "ctest.xml" + path.write_text(content, encoding="utf-8") + return list(check_skips.skips(path)) + + def test_skip_message_preserves_the_device_reason(self): + found = self.skipped('') + self.assertEqual(len(found), 1) + self.assertRegex(found[0][1], check_skips.DEVICE_LIMITATION) + + def test_stderr_preserves_the_device_reason(self): + found = self.skipped('skipped, no display') + self.assertEqual(len(found), 1) + self.assertRegex(found[0][1], check_skips.DEVICE_LIMITATION) + + def test_unnamed_skipped_test_is_an_error(self): + with self.assertRaises(ValueError): + self.skipped('') + + def test_unexpected_reason_stays_unexpected(self): + found = self.skipped('') + self.assertIsNone(check_skips.DEVICE_LIMITATION.search(found[0][1])) + + def test_ctest_stdout_is_retained(self): + found = self.skipped('skipped, the device reports too few descriptors') + self.assertRegex(found[0][1], check_skips.DEVICE_LIMITATION) + + def test_message_and_element_text_are_both_retained(self): + found = self.skipped('the device has no required extension') + self.assertRegex(found[0][1], check_skips.DEVICE_LIMITATION) + + if __name__ == "__main__": unittest.main() diff --git a/tools/tests/test_test_registration.py b/tools/tests/test_test_registration.py index 4010ca6bd..5a4649281 100644 --- a/tools/tests/test_test_registration.py +++ b/tools/tests/test_test_registration.py @@ -2,6 +2,8 @@ from pathlib import Path import sys import tempfile import unittest +from types import SimpleNamespace +from unittest.mock import patch sys.path.insert(0, str(Path(__file__).resolve().parents[1])) import check_test_registration @@ -32,6 +34,52 @@ class RegistrationTests(unittest.TestCase): self.assertIn("buffer_bounds_tests", names) self.assertNotIn("missing_tests", names) + def test_hyphenated_executable_is_discovered(self): + output = "tests/buffer-bounds_tests: CXX_EXECUTABLE_LINKER\n" + with patch.object(check_test_registration.subprocess, "run", return_value=SimpleNamespace(stdout=output)): + self.assertEqual(check_test_registration.built("build"), {"buffer-bounds_tests"}) + + def test_nested_test_output_is_discovered(self): + output = "tests/gpu/buffer_bounds_tests: CXX_EXECUTABLE_LINKER\n" + with patch.object(check_test_registration.subprocess, "run", return_value=SimpleNamespace(stdout=output)): + self.assertEqual(check_test_registration.built("build"), {"buffer_bounds_tests"}) + + def test_quoted_cmake_path_is_unescaped(self): + names = self.registered(r'add_test("bounds" "/build/tests/a\"b_tests")' + '\n') + self.assertIn('a"b_tests', names) + + def test_bracket_quoted_command_is_recognized(self): + names = self.registered('add_test([=[bounds]=] [=[/build/tests/buffer_bounds_tests]=])\n') + self.assertIn("buffer_bounds_tests", names) + + def test_multiline_test_command_is_recognized(self): + names = self.registered('add_test("bounds"\n "/build/tests/buffer_bounds_tests"\n)\n') + self.assertIn("buffer_bounds_tests", names) + + def test_disabled_test_is_not_counted_as_run(self): + names = self.registered('add_test("bounds" "/build/tests/buffer_bounds_tests")\nset_tests_properties("bounds" PROPERTIES DISABLED TRUE)\n') + self.assertNotIn("buffer_bounds_tests", names) + + def test_option_value_does_not_register_executable(self): + names = self.registered('add_test("bounds" "/usr/bin/python3" "runner.py" "--report=/build/tests/missing_tests")\n') + self.assertNotIn("missing_tests", names) + + def test_reenabled_test_is_counted_as_run(self): + names = self.registered('add_test("bounds" "/build/tests/buffer_bounds_tests")\nset_tests_properties("bounds" PROPERTIES DISABLED TRUE)\nset_tests_properties("bounds" PROPERTIES DISABLED OFF)\n') + self.assertIn("buffer_bounds_tests", names) + + def test_disabled_wrapper_does_not_register_its_argument(self): + names = self.registered('add_test("bounds" "/usr/bin/python3" "runner.py" "/build/tests/buffer_bounds_tests")\nset_tests_properties([=[bounds]=] PROPERTIES DISABLED "ON")\n') + self.assertNotIn("buffer_bounds_tests", names) + + def test_command_text_inside_an_argument_is_not_another_test(self): + names = self.registered('add_test("bounds" "/usr/bin/python3" [=[\nadd_test("fake" "/build/tests/missing_tests")\n]=])\n') + self.assertNotIn("missing_tests", names) + + def test_windows_escaped_path_is_decoded(self): + names = self.registered(r'add_test("bounds" "C:\\build\\tests\\buffer_bounds_tests.exe")' + '\n') + self.assertIn("buffer_bounds_tests", names) + if __name__ == "__main__": unittest.main()