fix(main): restore check_skips.py and its tests that #14881 overwrote

Culprit #14881 (0a0daa5f8e): its base was stale, so it dropped --require-tests, the ctest-status handling and the duplicate/notrun checks, and test_check_skips_ctest failed on windows (run 41360379322: unrecognized arguments: --require-tests). The large-file test prints 'skipped, the host ...', which the restored DEVICE_LIMITATION already accepts, so no new pattern is needed.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

tools/check_skips.py

@@ -4,46 +4,60 @@ import re
 import xml.etree.ElementTree as ET
 from pathlib import Path
 
-DEVICE_LIMITATION = re.compile(r"skipped,.*(subgroup|the device (has no|reports|lacks|does not|imports no|flushes|binds fewer)|no display|no usable Vulkan device|host imports are unavailable)", re.IGNORECASE)
-
-HOST_LIMITATION = re.compile(r"^skipping: (not enough free disk space for a file|cannot allocate a buffer) of more than 2 GiB", re.IGNORECASE | re.MULTILINE)
+DEVICE_LIMITATION = re.compile(r"skipped,.*(subgroup|the device (has no|reports|lacks|does not|imports no|flushes|binds fewer)|the host (has not enough free disk space|cannot allocate a buffer)|no display|no usable Vulkan device|host imports are unavailable|not enough free disk space)", re.IGNORECASE)
 
 NO_DISPLAY = re.compile(r"skipped,.*no display", re.IGNORECASE)
 
 NO_VULKAN_DEVICE = re.compile(r"skipped,.*no usable Vulkan device", re.IGNORECASE)
 
 HIGH_REFRESH_LIMITATION = re.compile(r"current display does not provide 120 Hz", re.IGNORECASE)
+CTEST_SKIP_STATUS = re.compile(r"SKIP_RETURN_CODE=\d+|SKIP_REGULAR_EXPRESSION_MATCHED", re.IGNORECASE)
 
 WINDOWS_PREFIX = "agc"
 
 
+def device_limitations(output, patterns):
+    reasons = [match.group(0) for line in output.splitlines() if (match := re.search(r"skipped,.*", line, re.IGNORECASE))]
+    return bool(reasons) and all(any(pattern.search(reason) for pattern in patterns) for reason in reasons)
+
+
 def high_refresh_limitation(output):
     reasons = []
     for line in output.splitlines():
         reason = line.strip()
-        if reason.lower().startswith("skipped,"):
-            reason = reason[len("skipped,"):].strip()
+        if match := re.search(r"skipped,", reason, re.IGNORECASE):
+            reason = reason[match.end():].strip()
         if reason:
             reasons.append(reason)
     return bool(reasons) and all(HIGH_REFRESH_LIMITATION.fullmatch(reason) for reason in reasons)
 
 
-def skips(path):
+def skips(path, require_tests=False):
     root = ET.parse(path).getroot()
     if root.tag not in ("testsuite", "testsuites"):
         raise ValueError(f"{path}: expected a JUnit testsuite or testsuites root, got {root.tag!r}")
+    found = False
+    seen = set()
     for case in root.iter("testcase"):
-        if case.get("status") == "disabled":
-            raise ValueError(f"{path}: testcase {case.get('name')!r} is disabled and was not run")
-        skipped = case.find("skipped")
-        if skipped is None:
-            continue
+        found = True
         name = case.get("name")
         if not name:
             raise ValueError(f"{path}: skipped testcase has no name")
-        output = ["skipped, " + reason.strip() for reason in (skipped.get("message", ""), skipped.text or "") if reason.strip()]
+        if name in seen:
+            raise ValueError(f"{path}: testcase {name!r} is listed more than once")
+        seen.add(name)
+        skipped = case.find("skipped")
+        ctest_skip = skipped is not None and CTEST_SKIP_STATUS.fullmatch(skipped.get("message", "").strip())
+        if case.get("status") == "disabled" or (case.get("status") == "notrun" and not ctest_skip):
+            raise ValueError(f"{path}: testcase {name!r} is {case.get('status')} and was not run")
+        if skipped is None:
+            continue
+        output = ["skipped, " + reason.strip() for reason in (skipped.get("message", ""), skipped.text or "")
+                  if reason.strip() and not CTEST_SKIP_STATUS.fullmatch(reason.strip())]
         output += [case.findtext(channel) or "" for channel in ("system-out", "system-err")]
         yield name, "\n".join(output)
+    if require_tests and not found:
+        raise ValueError(f"{path}: report contains no testcases")
 
 
 def main(argv=None):
@@ -54,21 +68,23 @@ def main(argv=None):
     parser.add_argument("junit", type=Path, help="the file written by ctest --output-junit")
     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")
+    parser.add_argument("--require-tests", action="store_true", help="fail if the report contains no testcases")
     args = parser.parse_args(argv)
     try:
-        found = sorted(skips(args.junit))
+        found = sorted(skips(args.junit, require_tests=args.require_tests))
     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.strip().lower().startswith("skipped")), "")
+            reason = next((match.group(0) for line in output.splitlines()
+                           if (match := re.search(r"skipped,.*", line, re.IGNORECASE))), "")
             print(f"{name}: {reason}")
         return 0
     if args.job == "windows":
-        unexpected = [name for name, output in found if not name.startswith(WINDOWS_PREFIX) and not NO_DISPLAY.search(output) and not NO_VULKAN_DEVICE.search(output) and not high_refresh_limitation(output)]
+        unexpected = [name for name, output in found if not name.startswith(WINDOWS_PREFIX) and not device_limitations(output, (NO_DISPLAY, NO_VULKAN_DEVICE)) and not high_refresh_limitation(output)]
         expected = f"any {WINDOWS_PREFIX}* test, which the runner has no Vulkan driver for, or a test that reports no display or Vulkan device or a display without a 120 Hz mode"
     else:
-        unexpected = [name for name, output in found if not DEVICE_LIMITATION.search(output) and not HOST_LIMITATION.search(output) and not high_refresh_limitation(output)]
+        unexpected = [name for name, output in found if not device_limitations(output, (DEVICE_LIMITATION,)) and not high_refresh_limitation(output)]
         expected = "any test that reports the device limitation it needs, such as narrow subgroups, a missing extension, no display for a Vulkan window or a display without a 120 Hz mode"
     for name in unexpected:
         print(f"error: {name} skipped")

tools/tests/test_check_skips.py

@@ -1,6 +1,7 @@
 from pathlib import Path
 import contextlib
 import io
+import shutil
 import subprocess
 import sys
 import tempfile
@@ -24,14 +25,12 @@ LIMITATIONS = [
     "skipped, no usable Vulkan device: ErrorInitializationFailed",
     "skipped, unit shadows or host imports are unavailable",
     "skipped, the device has no compute queue",
+    "skipped, not enough free disk space for a file of more than 2 GiB",
     "skipped, the device reports 4 lanes",
     "skipped, subgroup size 8",
     "skipped, no display",
-]
-
-HOST_LIMITATIONS = [
-    "skipping: not enough free disk space for a file of more than 2 GiB",
-    "skipping: cannot allocate a buffer of more than 2 GiB",
+    "skipped, the host has not enough free disk space for a file of more than 2 GiB",
+    "skipped, the host cannot allocate a buffer of more than 2 GiB",
 ]
 
 OTHER = [
@@ -42,6 +41,102 @@ OTHER = [
 
 
 class CheckSkipsTests(unittest.TestCase):
+    def test_ctest_status_and_prefixed_device_reasons_are_expected(self):
+        reports = (
+            ('<skipped message="SKIP_RETURN_CODE=77"/><system-out>skipped, the device lacks geometryShader</system-out>', "linux"),
+            ('<skipped message="SKIP_RETURN_CODE=77"/><system-out>f16 native: skipped, the device has no native f16 with RTE rounding and kept denormals</system-out>', "linux"),
+            ('<skipped message="SKIP_RETURN_CODE=77"/><system-err>video: skipped, no display or Vulkan device: ErrorInitializationFailed</system-err>', "windows"),
+            ('<skipped message="SKIP_REGULAR_EXPRESSION_MATCHED"/><system-out>skipped, no usable Vulkan device: none</system-out>', "windows"),
+        )
+        with tempfile.TemporaryDirectory() as directory:
+            junit = Path(directory) / "junit.xml"
+            for report, job in reports:
+                with self.subTest(job=job, report=report):
+                    junit.write_text(f'<testsuite><testcase name="guest_probe">{report}</testcase></testsuite>', encoding="utf-8")
+                    result = subprocess.run([sys.executable, "-I", str(TOOL), str(junit), "--job", job],
+                                            capture_output=True, text=True)
+                    self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+                    self.assertIn("all expected", result.stdout)
+
+    def test_ctest_status_does_not_allow_missing_or_unexpected_reasons(self):
+        reports = (
+            '<skipped message="SKIP_RETURN_CODE=77"/>',
+            '<skipped message="SKIP_REGULAR_EXPRESSION_MATCHED"/>',
+            '<skipped message="SKIP_RETURN_CODE=77"/><system-out>skipped, flaky\nskipped, the device lacks geometryShader</system-out>',
+        )
+        with tempfile.TemporaryDirectory() as directory:
+            junit = Path(directory) / "junit.xml"
+            for report in reports:
+                for job in ("linux", "windows"):
+                    with self.subTest(job=job, report=report):
+                        junit.write_text(f'<testsuite><testcase name="guest_probe">{report}</testcase></testsuite>', encoding="utf-8")
+                        result = subprocess.run([sys.executable, "-I", str(TOOL), str(junit), "--job", job],
+                                                capture_output=True, text=True)
+                        self.assertEqual(result.returncode, 1, result.stdout + result.stderr)
+                        self.assertIn("error: guest_probe skipped", result.stdout)
+
+    def test_ctest_status_preserves_high_refresh_policy(self):
+        reason = "skipped, current display does not provide 120 Hz"
+        reports = (
+            (f'<skipped message="SKIP_RETURN_CODE=77"/><system-out>{reason}</system-out>', 0),
+            (f'<skipped message="SKIP_REGULAR_EXPRESSION_MATCHED"/><system-out>{reason}</system-out>', 0),
+            (f'<skipped message="SKIP_RETURN_CODE=77"/><system-out>{reason}\nunexpected failure detail</system-out>', 1),
+            (f'<skipped message="SKIP_RETURN_CODE=77"/><system-out>{reason}\nskipped, flaky</system-out>', 1),
+            (f'<skipped message="SKIP_RETURN_CODE=77"/><system-out>skipped, no display\n{reason}</system-out>', 1),
+        )
+        with tempfile.TemporaryDirectory() as directory:
+            junit = Path(directory) / "junit.xml"
+            for report, expected_code in reports:
+                for job in ("linux", "windows"):
+                    with self.subTest(job=job, report=report):
+                        junit.write_text(f'<testsuite><testcase name="guest_video_out_high_frame_rate">{report}</testcase></testsuite>', encoding="utf-8")
+                        result = subprocess.run([sys.executable, "-I", str(TOOL), str(junit), "--job", job],
+                                                capture_output=True, text=True)
+                        self.assertEqual(result.returncode, expected_code, result.stdout + result.stderr)
+
+    def test_list_reports_device_reason_after_ctest_status(self):
+        xml = ('<testsuite><testcase name="guest_probe"><skipped message="SKIP_RETURN_CODE=77"/>'
+               '<system-out>f16 native: skipped, the device has no native f16</system-out></testcase></testsuite>')
+        with tempfile.TemporaryDirectory() as directory:
+            junit = Path(directory) / "junit.xml"
+            junit.write_text(xml, encoding="utf-8")
+            result = subprocess.run([sys.executable, "-I", str(TOOL), str(junit), "--list"],
+                                    capture_output=True, text=True)
+            self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+            self.assertIn("guest_probe: skipped, the device has no native f16", result.stdout)
+            self.assertNotIn("SKIP_RETURN_CODE", result.stdout)
+
+    def test_unexpected_skip_reason_cannot_hide_behind_a_device_limitation(self):
+        reports = (
+            ('<skipped message="flaky"/><system-out>skipped, the device lacks geometryShader</system-out>', "linux"),
+            ('<skipped message="flaky"/><system-err>skipped, no display</system-err>', "windows"),
+            ('<skipped>flaky</skipped><system-out>skipped, no usable Vulkan device</system-out>', "windows"),
+            ('<skipped/><system-out>skipped, flaky\nskipped, the device lacks geometryShader</system-out>', "linux"),
+        )
+        with tempfile.TemporaryDirectory() as directory:
+            junit = Path(directory) / "junit.xml"
+            for report, job in reports:
+                with self.subTest(job=job, report=report):
+                    junit.write_text(f'<testsuite><testcase name="guest_probe">{report}</testcase></testsuite>', encoding="utf-8")
+                    result = subprocess.run([sys.executable, "-I", str(TOOL), str(junit), "--job", job],
+                                            capture_output=True, text=True)
+                    self.assertEqual(result.returncode, 1, result.stdout + result.stderr)
+                    self.assertIn("error: guest_probe skipped", result.stdout)
+
+    def test_multiple_allowed_skip_reasons_remain_expected(self):
+        reports = (
+            ('<skipped message="the device lacks geometryShader"/><system-out>skipped, no display</system-out>', "linux"),
+            ('<skipped message="no display"/><system-err>skipped, no usable Vulkan device</system-err>', "windows"),
+        )
+        with tempfile.TemporaryDirectory() as directory:
+            junit = Path(directory) / "junit.xml"
+            for report, job in reports:
+                with self.subTest(job=job, report=report):
+                    junit.write_text(f'<testsuite><testcase name="guest_probe">{report}</testcase></testsuite>', encoding="utf-8")
+                    result = subprocess.run([sys.executable, "-I", str(TOOL), str(junit), "--job", job],
+                                            capture_output=True, text=True)
+                    self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+
     def test_non_junit_xml_fails_in_every_cli_mode(self):
         reports = ('<config/>', '<Site><Testing><Test Status="notrun"><Name>coverage</Name></Test></Testing></Site>')
         with tempfile.TemporaryDirectory() as directory:
@@ -146,12 +241,6 @@ class CheckSkipsTests(unittest.TestCase):
             with self.subTest(message=message):
                 self.assertTrue(check_skips.DEVICE_LIMITATION.search(message))
 
-    def test_host_limitations_match(self):
-        for message in HOST_LIMITATIONS:
-            with self.subTest(message=message):
-                self.assertTrue(check_skips.HOST_LIMITATION.search("skipped, Skipped\n" + message + "\n"))
-        self.assertFalse(check_skips.HOST_LIMITATION.search("skipping: not enough free disk space for a file of 1 MiB"))
-
     def test_other_reasons_do_not_match(self):
         for message in OTHER:
             with self.subTest(message=message):
@@ -260,6 +349,40 @@ class CheckSkipsTests(unittest.TestCase):
 
 
 
+class RequiredTestsTests(unittest.TestCase):
+    def test_required_reports_without_testcases_fail_in_both_jobs(self):
+        for xml in ('<testsuite/>', '<testsuites><testsuite tests="0"/></testsuites>'):
+            with tempfile.TemporaryDirectory() as directory:
+                junit = Path(directory) / "junit.xml"
+                junit.write_text(xml, encoding="utf-8")
+                for job in ("linux", "windows"):
+                    for options in ([], ["--list"]):
+                        with self.subTest(xml=xml, job=job, options=options):
+                            result = subprocess.run(
+                                [sys.executable, "-I", str(TOOL), str(junit), "--job", job, "--require-tests", *options],
+                                capture_output=True, text=True, timeout=5)
+                            self.assertEqual(result.returncode, 2, result.stdout + result.stderr)
+                            self.assertIn("no testcases", result.stderr)
+                            result = subprocess.run(
+                                [sys.executable, "-I", str(TOOL), str(junit), "--job", job, *options],
+                                capture_output=True, text=True, timeout=5)
+                            self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+
+    def test_executed_tests_without_skips_pass_in_both_jobs(self):
+        for xml in ('<testsuite><testcase name="normal"/></testsuite>',
+                    '<testsuites><testsuite><testcase name="normal"/></testsuite></testsuites>'):
+            with tempfile.TemporaryDirectory() as directory:
+                junit = Path(directory) / "junit.xml"
+                junit.write_text(xml, encoding="utf-8")
+                for job in ("linux", "windows"):
+                    with self.subTest(xml=xml, job=job):
+                        result = subprocess.run(
+                            [sys.executable, "-I", str(TOOL), str(junit), "--job", job, "--require-tests"],
+                            capture_output=True, text=True, timeout=5)
+                        self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+                        self.assertIn("0 test(s) skipped, all expected", result.stdout)
+
+
 class SkipReportTests(unittest.TestCase):
     def skipped(self, content):
         with tempfile.TemporaryDirectory() as directory:
@@ -267,6 +390,30 @@ class SkipReportTests(unittest.TestCase):
             path.write_text(content, encoding="utf-8")
             return list(check_skips.skips(path))
 
+    def test_a_test_listed_twice_is_rejected(self):
+        reason = ('<skipped message="SKIP_RETURN_CODE=77"/>'
+                  '<system-out>skipped, the device lacks geometryShader</system-out>')
+        flaky = ('<skipped message="SKIP_RETURN_CODE=77"/>'
+                 '<system-out>skipped, flaky</system-out>')
+        for content in (
+            f'<testsuite><testcase name="t">{reason}</testcase><testcase name="t">{reason}</testcase></testsuite>',
+            f'<testsuite><testcase name="t">{flaky}</testcase><testcase name="t">{reason}</testcase></testsuite>',
+            f'<testsuite><testcase name="t">{reason}</testcase><testcase name="t">{flaky}</testcase></testsuite>',
+        ):
+            with self.subTest(content=content), self.assertRaisesRegex(ValueError, "listed more than once"):
+                self.skipped(content)
+        with tempfile.TemporaryDirectory() as directory:
+            junit = Path(directory) / "junit.xml"
+            junit.write_text(
+                f'<testsuite><testcase name="t">{reason}</testcase><testcase name="t">{reason}</testcase></testsuite>',
+                encoding="utf-8",
+            )
+            result = subprocess.run([sys.executable, "-I", str(TOOL), str(junit), "--job", "linux"],
+                                    capture_output=True, text=True)
+            self.assertEqual(result.returncode, 2, result.stdout + result.stderr)
+            self.assertIn("listed more than once", result.stderr)
+            self.assertNotIn("Traceback", result.stderr)
+
     def test_non_junit_roots_are_rejected_even_with_testcases(self):
         for content in ('<report/>', '<testcase name="passed"/>',
                         '<report><testcase name="passed"/></report>',
@@ -317,5 +464,26 @@ class SkipReportTests(unittest.TestCase):
         self.assertRegex(found[0][1], check_skips.DEVICE_LIMITATION)
 
 
+class NotRunTests(unittest.TestCase):
+    @unittest.skipUnless(shutil.which("ctest"), "CTest is required to write a real JUnit report")
+    def test_real_notrun_case_is_rejected_before_windows_agc_exemption(self):
+        with tempfile.TemporaryDirectory() as directory:
+            root = Path(directory)
+            (root / "CTestTestfile.cmake").write_text('add_test(agc_missing "' + (root / "missing-program").as_posix() + '")\n', encoding="utf-8")
+            report = root / "report.xml"
+            generated = subprocess.run(["ctest", "--test-dir", str(root), "--output-junit", str(report)],
+                                       capture_output=True, text=True, timeout=10)
+            self.assertNotEqual(generated.returncode, 0)
+            self.assertIn('status="notrun"', report.read_text(encoding="utf-8"))
+            for job in ("windows", "linux"):
+                with self.subTest(job=job):
+                    checked = subprocess.run([sys.executable, str(TOOL), str(report), "--job", job, "--require-tests"],
+                                             capture_output=True, text=True, timeout=10)
+                    self.assertEqual(checked.returncode, 2, checked.stdout + checked.stderr)
+                    self.assertIn("notrun", checked.stderr)
+                    self.assertIn("agc_missing", checked.stderr)
+                    self.assertNotIn("Traceback", checked.stderr)
+
+
 if __name__ == "__main__":
     unittest.main()