Skip to content

Commit

Permalink
pythongh-109276: regrtest: add WORKER_FAILED state (python#110148)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vstinner committed Sep 30, 2023
1 parent c62b49e commit 2c23419
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 28 deletions.
18 changes: 12 additions & 6 deletions Lib/test/libregrtest/result.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -29,7 +30,8 @@ def is_failed(state):
State.FAILED,
State.UNCAUGHT_EXC,
State.REFLEAK,
State.MULTIPROCESSING_ERROR,
State.WORKER_FAILED,
State.WORKER_BUG,
State.TIMEOUT}

@staticmethod
Expand All @@ -42,14 +44,16 @@ def has_meaningful_duration(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)
Expand Down Expand Up @@ -108,8 +112,10 @@ def __str__(self) -> str:
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:
Expand Down
32 changes: 20 additions & 12 deletions Lib/test/libregrtest/results.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def __init__(self):
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
Expand All @@ -38,7 +39,8 @@ def __init__(self):
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)
Expand All @@ -60,6 +62,8 @@ def get_state(self, fail_env_changed):

if self.interrupted:
state.append("INTERRUPTED")
if self.worker_bug:
state.append("WORKER BUG")
if not state:
state.append("SUCCESS")

Expand All @@ -77,6 +81,8 @@ def get_exitcode(self, fail_env_changed, fail_rerun):
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):
Expand Down Expand Up @@ -105,6 +111,9 @@ def accumulate_result(self, result: TestResult, runtests: RunTests):
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:
Expand Down Expand Up @@ -173,29 +182,28 @@ def write_junit(self, filename: StrPath):
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()
print("10 slowest tests:")
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()
Expand Down
29 changes: 19 additions & 10 deletions Lib/test/libregrtest/run_workers.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
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:
Expand Down Expand Up @@ -92,7 +92,7 @@ def __init__(self,
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__()
Expand Down Expand Up @@ -298,7 +298,9 @@ def read_stdout(self, stdout_file: TextIO) -> str:
# 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]:
Expand All @@ -317,10 +319,11 @@ def read_json(self, json_file: JsonFile, json_tmpfile: TextIO | None,
# 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)
Expand All @@ -329,7 +332,7 @@ def read_json(self, json_file: JsonFile, json_tmpfile: TextIO | None,
# 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)

Expand All @@ -345,9 +348,15 @@ def _runtest(self, test_name: TestName) -> MultiprocessResult:
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)

Expand Down Expand Up @@ -527,7 +536,7 @@ def display_result(self, mp_result: MultiprocessResult) -> None:

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)
Expand All @@ -543,7 +552,7 @@ def _process_result(self, item: QueueOutput) -> TestResult:
# 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

Expand Down
22 changes: 22 additions & 0 deletions Lib/test/libregrtest/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os.path
import platform
import random
import signal
import sys
import sysconfig
import tempfile
Expand Down Expand Up @@ -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
10 changes: 10 additions & 0 deletions Lib/test/test_regrtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import random
import re
import shlex
import signal
import subprocess
import sys
import sysconfig
Expand Down Expand Up @@ -2066,6 +2067,15 @@ def test_normalize_test_name(self):
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()

0 comments on commit 2c23419

Please sign in to comment.