gh-109276: regrtest: add WORKER_FAILED state (#110148)

Rename WORKER_ERROR to WORKER_BUG. Add WORKER_FAILED state: it does
not stop the manager, whereas WORKER_BUG does.

Change also TestResults.display_result() order: display failed tests
at the end, the important important information.

WorkerThread now tries to get the signal name for negative exit code.

5 files changed, 83 insertions, 28 deletions

diff --git a/Lib/test/libregrtest/result.py b/Lib/test/libregrtest/result.py
index bf88526..d6b0d5a 100644
--- a/Lib/test/libregrtest/result.py
+++ b/Lib/test/libregrtest/result.py
@@ -19,7 +19,8 @@ class State:
     ENV_CHANGED = "ENV_CHANGED"
     RESOURCE_DENIED = "RESOURCE_DENIED"
     INTERRUPTED = "INTERRUPTED"
-    MULTIPROCESSING_ERROR = "MULTIPROCESSING_ERROR"
+    WORKER_FAILED = "WORKER_FAILED"   # non-zero worker process exit code
+    WORKER_BUG = "WORKER_BUG"         # exception when running a worker
     DID_NOT_RUN = "DID_NOT_RUN"
     TIMEOUT = "TIMEOUT"
 
@@ -29,7 +30,8 @@ class State:
             State.FAILED,
             State.UNCAUGHT_EXC,
             State.REFLEAK,
-            State.MULTIPROCESSING_ERROR,
+            State.WORKER_FAILED,
+            State.WORKER_BUG,
             State.TIMEOUT}
 
     @staticmethod
@@ -42,14 +44,16 @@ class State:
             State.SKIPPED,
             State.RESOURCE_DENIED,
             State.INTERRUPTED,
-            State.MULTIPROCESSING_ERROR,
+            State.WORKER_FAILED,
+            State.WORKER_BUG,
             State.DID_NOT_RUN}
 
     @staticmethod
     def must_stop(state):
         return state in {
             State.INTERRUPTED,
-            State.MULTIPROCESSING_ERROR}
+            State.WORKER_BUG,
+        }
 
 
 @dataclasses.dataclass(slots=True)
@@ -108,8 +112,10 @@ class TestResult:
                 return f"{self.test_name} skipped (resource denied)"
             case State.INTERRUPTED:
                 return f"{self.test_name} interrupted"
-            case State.MULTIPROCESSING_ERROR:
-                return f"{self.test_name} process crashed"
+            case State.WORKER_FAILED:
+                return f"{self.test_name} worker non-zero exit code"
+            case State.WORKER_BUG:
+                return f"{self.test_name} worker bug"
             case State.DID_NOT_RUN:
                 return f"{self.test_name} ran no tests"
             case State.TIMEOUT:
diff --git a/Lib/test/libregrtest/results.py b/Lib/test/libregrtest/results.py
index 35df50d..3708078 100644
--- a/Lib/test/libregrtest/results.py
+++ b/Lib/test/libregrtest/results.py
@@ -30,6 +30,7 @@ class TestResults:
         self.rerun_results: list[TestResult] = []
 
         self.interrupted: bool = False
+        self.worker_bug: bool = False
         self.test_times: list[tuple[float, TestName]] = []
         self.stats = TestStats()
         # used by --junit-xml
@@ -38,7 +39,8 @@ class TestResults:
     def is_all_good(self):
         return (not self.bad
                 and not self.skipped
-                and not self.interrupted)
+                and not self.interrupted
+                and not self.worker_bug)
 
     def get_executed(self):
         return (set(self.good) | set(self.bad) | set(self.skipped)
@@ -60,6 +62,8 @@ class TestResults:
 
         if self.interrupted:
             state.append("INTERRUPTED")
+        if self.worker_bug:
+            state.append("WORKER BUG")
         if not state:
             state.append("SUCCESS")
 
@@ -77,6 +81,8 @@ class TestResults:
             exitcode = EXITCODE_NO_TESTS_RAN
         elif fail_rerun and self.rerun:
             exitcode = EXITCODE_RERUN_FAIL
+        elif self.worker_bug:
+            exitcode = EXITCODE_BAD_TEST
         return exitcode
 
     def accumulate_result(self, result: TestResult, runtests: RunTests):
@@ -105,6 +111,9 @@ class TestResults:
                 else:
                     raise ValueError(f"invalid test state: {result.state!r}")
 
+        if result.state == State.WORKER_BUG:
+            self.worker_bug = True
+
         if result.has_meaningful_duration() and not rerun:
             self.test_times.append((result.duration, test_name))
         if result.stats is not None:
@@ -173,12 +182,6 @@ class TestResults:
                 f.write(s)
 
     def display_result(self, tests: TestTuple, quiet: bool, print_slowest: bool):
-        omitted = set(tests) - self.get_executed()
-        if omitted:
-            print()
-            print(count(len(omitted), "test"), "omitted:")
-            printlist(omitted)
-
         if print_slowest:
             self.test_times.sort(reverse=True)
             print()
@@ -186,16 +189,21 @@ class TestResults:
             for test_time, test in self.test_times[:10]:
                 print("- %s: %s" % (test, format_duration(test_time)))
 
-        all_tests = [
-            (self.bad, "test", "{} failed:"),
-            (self.env_changed, "test", "{} altered the execution environment (env changed):"),
-        ]
+        all_tests = []
+        omitted = set(tests) - self.get_executed()
+
+        # less important
+        all_tests.append((omitted, "test", "{} omitted:"))
         if not quiet:
             all_tests.append((self.skipped, "test", "{} skipped:"))
             all_tests.append((self.resource_denied, "test", "{} skipped (resource denied):"))
-        all_tests.append((self.rerun, "re-run test", "{}:"))
         all_tests.append((self.run_no_tests, "test", "{} run no tests:"))
 
+        # more important
+        all_tests.append((self.env_changed, "test", "{} altered the execution environment (env changed):"))
+        all_tests.append((self.rerun, "re-run test", "{}:"))
+        all_tests.append((self.bad, "test", "{} failed:"))
+
         for tests_list, count_text, title_format in all_tests:
             if tests_list:
                 print()
diff --git a/Lib/test/libregrtest/run_workers.py b/Lib/test/libregrtest/run_workers.py
index 41ed7b0..6eb32e5 100644
--- a/Lib/test/libregrtest/run_workers.py
+++ b/Lib/test/libregrtest/run_workers.py
@@ -22,7 +22,7 @@ from .runtests import RunTests, JsonFile, JsonFileType
 from .single import PROGRESS_MIN_TIME
 from .utils import (
     StrPath, TestName, MS_WINDOWS,
-    format_duration, print_warning, count, plural)
+    format_duration, print_warning, count, plural, get_signal_name)
 from .worker import create_worker_process, USE_PROCESS_GROUP
 
 if MS_WINDOWS:
@@ -92,7 +92,7 @@ class WorkerError(Exception):
                  test_name: TestName,
                  err_msg: str | None,
                  stdout: str | None,
-                 state: str = State.MULTIPROCESSING_ERROR):
+                 state: str):
         result = TestResult(test_name, state=state)
         self.mp_result = MultiprocessResult(result, stdout, err_msg)
         super().__init__()
@@ -298,7 +298,9 @@ class WorkerThread(threading.Thread):
             # gh-101634: Catch UnicodeDecodeError if stdout cannot be
             # decoded from encoding
             raise WorkerError(self.test_name,
-                              f"Cannot read process stdout: {exc}", None)
+                              f"Cannot read process stdout: {exc}",
+                              stdout=None,
+                              state=State.WORKER_BUG)
 
     def read_json(self, json_file: JsonFile, json_tmpfile: TextIO | None,
                   stdout: str) -> tuple[TestResult, str]:
@@ -317,10 +319,11 @@ class WorkerThread(threading.Thread):
             # decoded from encoding
             err_msg = f"Failed to read worker process JSON: {exc}"
             raise WorkerError(self.test_name, err_msg, stdout,
-                              state=State.MULTIPROCESSING_ERROR)
+                              state=State.WORKER_BUG)
 
         if not worker_json:
-            raise WorkerError(self.test_name, "empty JSON", stdout)
+            raise WorkerError(self.test_name, "empty JSON", stdout,
+                              state=State.WORKER_BUG)
 
         try:
             result = TestResult.from_json(worker_json)
@@ -329,7 +332,7 @@ class WorkerThread(threading.Thread):
             # decoded from encoding
             err_msg = f"Failed to parse worker process JSON: {exc}"
             raise WorkerError(self.test_name, err_msg, stdout,
-                              state=State.MULTIPROCESSING_ERROR)
+                              state=State.WORKER_BUG)
 
         return (result, stdout)
 
@@ -345,9 +348,15 @@ class WorkerThread(threading.Thread):
             stdout = self.read_stdout(stdout_file)
 
             if retcode is None:
-                raise WorkerError(self.test_name, None, stdout, state=State.TIMEOUT)
+                raise WorkerError(self.test_name, stdout=stdout,
+                                  err_msg=None,
+                                  state=State.TIMEOUT)
             if retcode != 0:
-                raise WorkerError(self.test_name, f"Exit code {retcode}", stdout)
+                name = get_signal_name(retcode)
+                if name:
+                    retcode = f"{retcode} ({name})"
+                raise WorkerError(self.test_name, f"Exit code {retcode}", stdout,
+                                  state=State.WORKER_FAILED)
 
             result, stdout = self.read_json(json_file, json_tmpfile, stdout)
 
@@ -527,7 +536,7 @@ class RunWorkers:
 
         text = str(result)
         if mp_result.err_msg:
-            # MULTIPROCESSING_ERROR
+            # WORKER_BUG
             text += ' (%s)' % mp_result.err_msg
         elif (result.duration >= PROGRESS_MIN_TIME and not pgo):
             text += ' (%s)' % format_duration(result.duration)
@@ -543,7 +552,7 @@ class RunWorkers:
             # Thread got an exception
             format_exc = item[1]
             print_warning(f"regrtest worker thread failed: {format_exc}")
-            result = TestResult("<regrtest worker>", state=State.MULTIPROCESSING_ERROR)
+            result = TestResult("<regrtest worker>", state=State.WORKER_BUG)
             self.results.accumulate_result(result, self.runtests)
             return result
 
diff --git a/Lib/test/libregrtest/utils.py b/Lib/test/libregrtest/utils.py
index 4645115..dc1fa51 100644
--- a/Lib/test/libregrtest/utils.py
+++ b/Lib/test/libregrtest/utils.py
@@ -5,6 +5,7 @@ import math
 import os.path
 import platform
 import random
+import signal
 import sys
 import sysconfig
 import tempfile
@@ -581,3 +582,24 @@ def cleanup_temp_dir(tmp_dir: StrPath):
         else:
             print("Remove file: %s" % name)
             os_helper.unlink(name)
+
+WINDOWS_STATUS = {
+    0xC0000005: "STATUS_ACCESS_VIOLATION",
+    0xC00000FD: "STATUS_STACK_OVERFLOW",
+    0xC000013A: "STATUS_CONTROL_C_EXIT",
+}
+
+def get_signal_name(exitcode):
+    if exitcode < 0:
+        signum = -exitcode
+        try:
+            return signal.Signals(signum).name
+        except ValueError:
+            pass
+
+    try:
+        return WINDOWS_STATUS[exitcode]
+    except KeyError:
+        pass
+
+    return None
diff --git a/Lib/test/test_regrtest.py b/Lib/test/test_regrtest.py
index c98b05a..e940cf0 100644
--- a/Lib/test/test_regrtest.py
+++ b/Lib/test/test_regrtest.py
@@ -14,6 +14,7 @@ import platform
 import random
 import re
 import shlex
+import signal
 import subprocess
 import sys
 import sysconfig
@@ -2066,6 +2067,15 @@ class TestUtils(unittest.TestCase):
         self.assertIsNone(normalize('setUpModule (test.test_x)', is_error=True))
         self.assertIsNone(normalize('tearDownModule (test.test_module)', is_error=True))
 
+    def test_get_signal_name(self):
+        for exitcode, expected in (
+            (-int(signal.SIGINT), 'SIGINT'),
+            (-int(signal.SIGSEGV), 'SIGSEGV'),
+            (3221225477, "STATUS_ACCESS_VIOLATION"),
+            (0xC00000FD, "STATUS_STACK_OVERFLOW"),
+        ):
+            self.assertEqual(utils.get_signal_name(exitcode), expected, exitcode)
+
 
 if __name__ == '__main__':
     unittest.main()