Merge pull request #4955 from Ondra97/codex/fix-high-refresh-skip-policy

fix(tools): accept skips on displays without 120 Hz

tools/check_skips.py

@@ -7,9 +7,22 @@ DEVICE_LIMITATION = re.compile(r"skipped,.*(subgroup|the device (has no|reports|
 
 NO_DISPLAY = re.compile(r"skipped,.*no display", re.IGNORECASE)
 
+HIGH_REFRESH_LIMITATION = re.compile(r"current display does not provide 120 Hz", re.IGNORECASE)
+
 WINDOWS_PREFIX = "agc"
 
 
+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 reason:
+            reasons.append(reason)
+    return bool(reasons) and all(HIGH_REFRESH_LIMITATION.fullmatch(reason) for reason in reasons)
+
+
 def skips(path):
     for case in ET.parse(path).getroot().iter("testcase"):
         if case.get("status") == "disabled":
@@ -41,11 +54,11 @@ if __name__ == "__main__":
             print(f"{name}: {reason}")
         raise SystemExit
     if args.job == "windows":
-        unexpected = [name for name, output in found if not name.startswith(WINDOWS_PREFIX) and not NO_DISPLAY.search(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"
+        unexpected = [name for name, output in found if not name.startswith(WINDOWS_PREFIX) and not NO_DISPLAY.search(output) 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)]
-        expected = "any test that reports the device limitation it needs, such as narrow subgroups, a missing extension or no display for a Vulkan window"
+        unexpected = [name for name, output in found if not DEVICE_LIMITATION.search(output) 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")
     if unexpected:

tools/tests/test_check_skips.py

@@ -34,6 +34,76 @@ OTHER = [
 
 
 class CheckSkipsTests(unittest.TestCase):
+    def test_missing_high_refresh_display_is_expected_in_both_jobs(self):
+        reason = "current display does not provide 120 Hz"
+        reports = (
+            f'<skipped/><system-out>skipped, {reason}</system-out>',
+            f'<skipped message="{reason}"/>',
+            f'<skipped>{reason}</skipped>',
+            f'<skipped/><system-err>skipped, {reason}</system-err>',
+            f'<skipped message="{reason}">\n\n{reason}\n</skipped>'
+            f'<system-out>\n\nskipped, {reason}\n\n</system-out>'
+            f'<system-err>\nskipped, {reason}\n</system-err>',
+            f'<skipped>\n\n{reason}\n\n{reason}\n</skipped>',
+        )
+        with tempfile.TemporaryDirectory() as directory:
+            junit = Path(directory) / "junit.xml"
+            for report in reports:
+                junit.write_text(f'<testsuite><testcase name="guest_video_out_high_frame_rate">{report}</testcase></testsuite>', encoding="utf-8")
+                for job in ("linux", "windows"):
+                    with self.subTest(job=job, report=report):
+                        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("1 test(s) skipped, all expected", result.stdout)
+
+    def test_high_refresh_reason_cannot_hide_additional_diagnostics(self):
+        reason = "current display does not provide 120 Hz"
+        known = f"skipped, {reason}"
+        extra = "unexpected failure detail"
+        reports = (
+            f'<skipped/><system-out>{known}\n{extra}</system-out>',
+            f'<skipped/><system-out>{extra}\n{known}</system-out>',
+            f'<skipped/><system-err>{known}\n{extra}</system-err>',
+            f'<skipped/><system-out>{known}</system-out><system-err>{extra}</system-err>',
+            f'<skipped/><system-out>{extra}</system-out><system-err>{known}</system-err>',
+            f'<skipped message="{reason}"/><system-err>{extra}</system-err>',
+            f'<skipped>{reason}</skipped><system-out>{extra}</system-out>',
+            f'<skipped message="{reason}">{extra}</skipped>',
+            f'<skipped message="{extra}"/><system-out>{known}</system-out>',
+            f'<skipped>{reason}\n{extra}</skipped>',
+            f'<skipped/><system-out>{known}\n{known}\n{extra}</system-out>',
+        )
+        with tempfile.TemporaryDirectory() as directory:
+            junit = Path(directory) / "junit.xml"
+            for report in reports:
+                junit.write_text(f'<testsuite><testcase name="guest_video_out_high_frame_rate">{report}</testcase></testsuite>', encoding="utf-8")
+                for job in ("linux", "windows"):
+                    with self.subTest(job=job, report=report):
+                        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_video_out_high_frame_rate skipped", result.stdout)
+
+    def test_unknown_display_reasons_stay_unexpected_in_both_jobs(self):
+        reasons = (
+            "current display is broken",
+            "current display does not provide 120 Hz because the test is flaky",
+        )
+        with tempfile.TemporaryDirectory() as directory:
+            junit = Path(directory) / "junit.xml"
+            for reason in reasons:
+                junit.write_text(f'<testsuite><testcase name="guest_video_out_high_frame_rate"><skipped/><system-out>skipped, {reason}</system-out></testcase></testsuite>', encoding="utf-8")
+                for job in ("linux", "windows"):
+                    with self.subTest(job=job, reason=reason):
+                        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_video_out_high_frame_rate skipped", result.stdout)
+
     def test_disabled_tests_fail_in_both_jobs_including_agc(self):
         xml = '<testsuite><testcase name="agc_disabled" status="disabled"/></testsuite>'
         with tempfile.TemporaryDirectory() as directory: