Skip to content

Commit dc83a3c

Browse files
committed
Personal review fixes
1 parent 475549a commit dc83a3c

7 files changed

Lines changed: 228 additions & 108 deletions

File tree

‎Lib/test/libregrtest/main.py‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ def _rerun_failed_tests(self, runtests: RunTests) -> RunTests:
299299
self.run_tests_sequentially(runtests)
300300
return runtests
301301

302-
def rerun_failed_tests(self, runtests: RunTests) -> RunTests:
302+
def rerun_failed_tests(self, runtests: RunTests) -> None:
303303
ansi = get_colors()
304304
red, reset = ansi.BOLD_RED, ansi.RESET
305305

@@ -309,7 +309,7 @@ def rerun_failed_tests(self, runtests: RunTests) -> RunTests:
309309
"Re-running failed tests is not supported with --python "
310310
"host runner option."
311311
)
312-
return runtests
312+
return
313313

314314
self.first_state = self.get_state()
315315

@@ -324,7 +324,6 @@ def rerun_failed_tests(self, runtests: RunTests) -> RunTests:
324324
printlist(self.results.bad)
325325

326326
self.display_result(rerun_runtests)
327-
return rerun_runtests
328327

329328
def _run_bisect(self, runtests: RunTests, test: str, progress: str) -> bool:
330329
print()
@@ -398,6 +397,7 @@ def run_test(
398397
result = run_single_test(test_name, runtests)
399398

400399
self.results.accumulate_result(result, runtests)
400+
get_reporter(runtests.github_summary).test_result(result, runtests)
401401

402402
return result
403403

@@ -464,8 +464,7 @@ def _run_tests_mp(self, runtests: RunTests, num_workers: int) -> None:
464464
from .run_workers import RunWorkers
465465
RunWorkers(num_workers, runtests, self.logger, self.results).run()
466466

467-
def finalize_tests(self, coverage: trace.CoverageResults | None,
468-
runtests: RunTests) -> None:
467+
def finalize_tests(self, coverage: trace.CoverageResults | None) -> None:
469468
if self.next_single_filename:
470469
if self.next_single_test:
471470
with open(self.next_single_filename, 'w') as fp:
@@ -488,7 +487,8 @@ def finalize_tests(self, coverage: trace.CoverageResults | None,
488487
failed = [result for result in self.results.rerun_results
489488
if result.is_failed(self.fail_env_changed)]
490489
if failed:
491-
get_reporter(runtests).run_failed(failed, self.get_state())
490+
get_reporter(self.github_summary).run_failed(failed,
491+
self.get_state())
492492

493493
def display_summary(self) -> None:
494494
if self.first_runtests is None:
@@ -527,8 +527,7 @@ def create_run_tests(self, tests: TestTuple) -> RunTests:
527527
hunt_refleak=self.hunt_refleak,
528528
test_dir=self.test_dir,
529529
use_junit=(self.junit_filename is not None),
530-
# Only report the failures which fail the job: when failed tests
531-
# are re-run, only report the re-run.
530+
# When failed tests are re-run, only report the re-run
532531
github_summary=(None if self.want_rerun and not self.python_cmd
533532
else self.github_summary),
534533
coverage=self.coverage,
@@ -601,10 +600,8 @@ def _run_tests(self, selected: TestTuple, tests: TestList | None) -> int:
601600
coverage = self.results.get_coverage_results()
602601
self.display_result(runtests)
603602

604-
# The re-run, if any, is the last run
605-
last_runtests = runtests
606603
if self.want_rerun and self.results.need_rerun():
607-
last_runtests = self.rerun_failed_tests(runtests)
604+
self.rerun_failed_tests(runtests)
608605

609606
if self.want_bisect and self.results.need_rerun():
610607
self.run_bisect(runtests)
@@ -613,7 +610,7 @@ def _run_tests(self, selected: TestTuple, tests: TestList | None) -> int:
613610
self.logger.stop_load_tracker()
614611

615612
self.display_summary()
616-
self.finalize_tests(coverage, last_runtests)
613+
self.finalize_tests(coverage)
617614

618615
return self.results.get_exitcode(self.fail_env_changed,
619616
self.fail_rerun)

‎Lib/test/libregrtest/reporter.py‎

Lines changed: 52 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from .findtests import findtestdir
99
from .result import TestResult
1010
from .runtests import RunTests
11-
from .utils import count
11+
from .utils import StrPath, count
1212

1313

1414
class Reporter:
@@ -22,27 +22,58 @@ def test_case_failed(self, test_case: str, traceback: str,
2222
"""Report a failed test case.
2323
2424
Called just after unittest printed the failure, or at the end of the
25-
test file in quiet mode (failures are not printed).
25+
test file in quiet mode.
2626
"""
2727

2828
def test_result(self, result: TestResult, runtests: RunTests) -> None:
2929
"""Report the result of a test file, in the main process."""
3030

3131
def run_failed(self, failed: list[TestResult], state: str) -> None:
32-
"""Report a failed run: failed is the list of failed test files."""
32+
"""Report the test files which failed the run."""
3333

34+
@staticmethod
35+
def parse_traceback(traceback: str) -> tuple[str | None, str | None, str]:
36+
"""Parse the traceback of a failure into (filename, line, exception).
3437
35-
def get_reporter(runtests: RunTests) -> Reporter:
36-
if runtests.github_summary:
37-
return GitHubActionsReporter(runtests.github_summary)
38+
The location is the last frame in a test file, or else the first frame
39+
(the test).
40+
"""
41+
frames = re.findall(r'^ *File "(.*)", line (\d+)', traceback,
42+
re.MULTILINE)
43+
test_frames = [frame for frame in frames
44+
if pathlib.Path(frame[0]).name.startswith('test_')]
45+
filename, line = (test_frames[-1] if test_frames
46+
else frames[0] if frames
47+
else (None, None))
48+
exception = re.split(r'^ File .*\n(?: .*\n)*', traceback,
49+
flags=re.MULTILINE)[-1].strip()
50+
return filename, line, exception
51+
52+
@staticmethod
53+
def test_filename(result: TestResult,
54+
test_dir: StrPath | None) -> str | None:
55+
"""Get the filename of a test, or None if it is not found."""
56+
if result.filename:
57+
return result.filename
58+
# The worker crashed or timed out before importing the test:
59+
# "test.test_x.test_y" is test_x/test_y.py or test_x/test_y/__init__.py
60+
name = result.test_name.removeprefix("test.")
61+
path = pathlib.Path(findtestdir(test_dir), *name.split("."))
62+
for filename in (path.with_suffix(".py"), path / "__init__.py"):
63+
if filename.exists():
64+
return str(filename)
65+
return None
66+
67+
68+
def get_reporter(github_summary: str | None) -> Reporter:
69+
if github_summary:
70+
return GitHubActionsReporter(github_summary)
3871
return Reporter()
3972

4073

4174
class GitHubActionsReporter(Reporter):
42-
"""Report test failures to GitHub Actions.
43-
44-
Annotate failures in the job log and list them in the job summary.
45-
"""
75+
"""Annotate failures in the GitHub Actions job log and list them in the
76+
job summary."""
4677

4778
# Escape the data and the properties of workflow commands
4879
ESCAPE_DATA = str.maketrans({"%": "%25", "\r": "%0D", "\n": "%0A"})
@@ -53,54 +84,29 @@ def __init__(self, summary_filename: str) -> None:
5384

5485
def test_case_failed(self, test_case: str, traceback: str,
5586
stream: TextIO) -> None:
56-
traceback = decolor(traceback)
57-
# Get a file + line number from the backtrace: the last frame in a
58-
# test file, or else the first frame (the test). Without frames, the
59-
# failure is still annotated, just not located.
60-
filename = line = None
61-
if frames := re.findall(r'^ *File "(.*)", line (\d+)', traceback,
62-
re.MULTILINE):
63-
test_frames = [frame for frame in frames
64-
if pathlib.Path(frame[0]).name.startswith('test_')]
65-
filename, line = test_frames[-1] if test_frames else frames[0]
66-
# Only keep the exception: strip the traceback frames, each one a
67-
# ' File' line followed by its indented source lines
68-
message = re.split(r'^ File .*\n(?: .*\n)*', traceback,
69-
flags=re.MULTILINE)[-1].strip()
70-
self.annotate(test_case, message, filename, line, stream)
87+
# The job log has the full traceback: only keep the exception
88+
filename, line, exception = self.parse_traceback(decolor(traceback))
89+
self.annotate(test_case, exception, filename, line, stream)
7190

7291
def test_result(self, result: TestResult, runtests: RunTests) -> None:
73-
# Annotate a test file which failed without failed test cases, for
74-
# example: crash, timeout, env changed. Failed test cases are
75-
# annotated by test_case_failed().
92+
# Failed test cases are annotated by test_case_failed()
7693
if (not result.is_failed(runtests.fail_env_changed)
7794
or result.failed_test_cases):
7895
return
79-
filename = result.filename
80-
if not filename:
81-
# The worker crashed or timed out before reporting the test file:
82-
# convert python import test name to relative file path.
83-
# Ex: "test.test_x.test_y" is test_x/test_y.py
84-
name = result.test_name.removeprefix("test.")
85-
path = pathlib.Path(findtestdir(runtests.test_dir),
86-
*name.split("."))
87-
if path.with_suffix(".py").exists():
88-
filename = str(path.with_suffix(".py"))
89-
elif (path / "__init__.py").exists():
90-
filename = str(path / "__init__.py")
91-
# else not a test file (ex: "<regrtest worker>" bug): no location
9296
message = "\n".join([str(result), *(result.env_changed_reasons or ())])
93-
self.annotate(result.test_name, decolor(message), filename)
97+
self.annotate(result.test_name, decolor(message),
98+
self.test_filename(result, runtests.test_dir))
9499

95100
def annotate(self, title: str, message: str, filename: str | None,
96101
line: str | None = None, stream: TextIO | None = None) -> None:
97102
if filename:
98-
# Normalize stdlib path to be relative inside the repository
99-
# checkout.
103+
# The stdlib can be outside of the repository checkout
100104
path = pathlib.Path(filename)
101105
stdlib_dir = pathlib.Path(os.__file__).parent
102106
if path.is_relative_to(stdlib_dir):
103107
filename = ("Lib" / path.relative_to(stdlib_dir)).as_posix()
108+
# Without a line, the annotation is not shown on the file
109+
line = line or "1"
104110
props = {"file": filename, "line": line, "title": title}
105111
props_text = ",".join(f"{key}={value.translate(self.ESCAPE_PROPERTY)}"
106112
for key, value in props.items() if value)
@@ -118,13 +124,12 @@ def run_failed(self, failed: list[TestResult], state: str) -> None:
118124
blocks.append("\n".join(f"- {reason}"
119125
for reason in result.env_changed_reasons))
120126
for name, traceback in result.failed_test_cases:
121-
# Only default open short tracebacks of up to 5 test cases.
127+
# Expand short tracebacks if there are only a few
122128
is_open = ncases <= 5 and traceback.count("\n") < 30
123129
blocks += [f"<details{' open' if is_open else ''}>"
124130
f"<summary>{html.escape(name)}</summary>",
125131
f"```pytb\n{decolor(traceback).rstrip()}\n```",
126132
"</details>"]
127-
# Separate Markdown blocks with an empty line
133+
# Markdown blocks are separated by an empty line
128134
with open(self.summary_filename, "a", encoding="utf-8") as fp:
129135
fp.write("".join(f"{block}\n\n" for block in blocks))
130-

‎Lib/test/libregrtest/results.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from _colorize import get_colors # type: ignore[import-not-found]
44
lazy from xml.etree.ElementTree import Element
55

6-
from .reporter import get_reporter
76
from .runtests import RunTests
87
from .result import State, TestResult, TestStats, Location
98
from .utils import (
@@ -143,7 +142,6 @@ def accumulate_result(self, result: TestResult, runtests: RunTests) -> None:
143142
xml_data = result.xml_data
144143
if xml_data:
145144
self.add_junit(xml_data)
146-
get_reporter(runtests).test_result(result, runtests)
147145

148146
def get_coverage_results(self) -> trace.CoverageResults:
149147
counts = {loc: 1 for loc in self.covered_lines}

‎Lib/test/libregrtest/run_workers.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from test.support import os_helper, MS_WINDOWS
1717

1818
from .logger import Logger
19+
from .reporter import get_reporter
1920
from .result import TestResult, State
2021
from .results import TestResults
2122
from .runtests import RunTests, WorkerRunTests, JsonFile, JsonFileType
@@ -624,6 +625,9 @@ def _process_result(self, item: QueueOutput) -> TestResult:
624625
stdout = mp_result.worker_stdout
625626
if stdout:
626627
print(stdout, flush=True)
628+
# Report after the output: env changed warnings, crash traceback
629+
get_reporter(self.runtests.github_summary).test_result(result,
630+
self.runtests)
627631

628632
return result
629633

‎Lib/test/libregrtest/setup.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ def setup_tests(runtests: RunTests) -> None:
114114

115115
set_match_tests(runtests.match_tests)
116116

117-
RegressionTestResult.reporter = get_reporter(runtests)
117+
RegressionTestResult.reporter = get_reporter(runtests.github_summary)
118118
if runtests.use_junit:
119119
support.junit_xml_list = []
120120
RegressionTestResult.USE_XML = True

‎Lib/test/libregrtest/testresult.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,6 @@ def addUnexpectedSuccess(self, test):
135135
def printErrorList(self, flavour, errors):
136136
for test, err in errors:
137137
super().printErrorList(flavour, [(test, err)])
138-
# Report just after unittest printed the failure, so that a CI
139-
# annotation links to it in the job log
140138
self.report_failure(test, err)
141139

142140
def report_failure(self, test, err):
@@ -158,7 +156,7 @@ def __init__(self, stream, buffer=False):
158156

159157
def run(self, test):
160158
test(self.result)
161-
# Failures are not printed in quiet mode: report them at the end
159+
# Failures are not printed in quiet mode
162160
for test, err in self.result.errors + self.result.failures:
163161
self.result.report_failure(test, err)
164162
return self.result

0 commit comments

Comments
 (0)