Skip to content

Commit 3f320dd

Browse files
committed
lots of review changes
1 parent b125f6f commit 3f320dd

9 files changed

Lines changed: 253 additions & 55 deletions

File tree

‎Lib/test/libregrtest/main.py‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import html
12
import os
23
import random
34
import re
@@ -397,7 +398,7 @@ def run_test(
397398

398399
self.results.accumulate_result(result, runtests)
399400
if runtests.github_annotations:
400-
result.print_github_crash_annotation(runtests.fail_env_changed)
401+
result.print_github_annotation(runtests)
401402

402403
return result
403404

@@ -487,34 +488,35 @@ def finalize_tests(self, coverage: trace.CoverageResults | None) -> None:
487488

488489
def write_github_summary(self) -> None:
489490
filename = os.environ.get("GITHUB_STEP_SUMMARY")
491+
# Tests which failed in the last run (the re-run, if any)
492+
failed = self.results.rerun_results
490493
# Only write a summary on failure: a CI run has many test jobs
491494
exitcode = self.results.get_exitcode(self.fail_env_changed,
492495
self.fail_rerun)
493-
if not filename or not exitcode:
496+
if not filename or not failed or not exitcode:
494497
return
495-
# Tests which failed in the last run (the re-run, if any)
496-
failed = self.results.rerun_results
497498
cases = [(result.errors or []) + (result.failures or [])
498499
for result in failed]
499-
ncase = sum(map(len, cases))
500+
ncases = sum(map(len, cases))
500501
with open(filename, "a", encoding="utf-8") as fp:
501502
def write(text: str) -> None:
502503
# Separate Markdown blocks with an empty line
503504
print(text, end="\n\n", file=fp)
504505

505506
write(f"## {decolor(self.get_state())}: "
506507
f"{count(len(failed), 'test file')} and "
507-
f"{count(ncase, 'test case')} failed")
508+
f"{count(ncases, 'test case')} failed")
508509
for result, result_cases in zip(failed, cases):
509510
write(f"### {decolor(str(result))}")
510511
if result.env_changed_reasons:
511512
write("\n".join(f"- {reason}"
512513
for reason in result.env_changed_reasons))
513514
for name, traceback in result_cases:
514-
# Expand short tracebacks when there are only a few
515-
is_open = ncase <= 5 and traceback.count("\n") < 30
515+
# Expand the tracebacks if they fit on a screen: up to 5
516+
# test cases with a traceback of less than 30 lines
517+
is_open = ncases <= 5 and traceback.count("\n") < 30
516518
write(f"<details{' open' if is_open else ''}>"
517-
f"<summary>{name}</summary>")
519+
f"<summary>{html.escape(name)}</summary>")
518520
write(f"```pytb\n{decolor(traceback).rstrip()}\n```")
519521
write("</details>")
520522

@@ -538,6 +540,12 @@ def display_summary(self) -> None:
538540
print(f"Result: {state}")
539541

540542
def create_run_tests(self, tests: TestTuple) -> RunTests:
543+
# Annotate test failures in the GitHub Actions job log of the last
544+
# run (the re-run, if any), if it reports failures
545+
will_rerun = self.want_rerun and not self.python_cmd
546+
github_annotations = (bool(os.environ.get("GITHUB_STEP_SUMMARY"))
547+
and not will_rerun
548+
and bool(self.verbose or self.output_on_failure))
541549
return RunTests(
542550
tests,
543551
fail_fast=self.fail_fast,
@@ -555,10 +563,7 @@ def create_run_tests(self, tests: TestTuple) -> RunTests:
555563
hunt_refleak=self.hunt_refleak,
556564
test_dir=self.test_dir,
557565
use_junit=(self.junit_filename is not None),
558-
# Only annotate failures of the last run: the re-run, if any
559-
github_annotations=(bool(os.environ.get("GITHUB_STEP_SUMMARY"))
560-
and not (self.want_rerun
561-
and not self.python_cmd)),
566+
github_annotations=github_annotations,
562567
coverage=self.coverage,
563568
memory_limit=self.memory_limit,
564569
gc_threshold=self.gc_threshold,

‎Lib/test/libregrtest/result.py‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import dataclasses
2+
import importlib.util
23
import json
34
from _colorize import get_colors # type: ignore[import-not-found]
45
from typing import Any
56

7+
from .runtests import RunTests
68
from .utils import (
7-
StrJSON, TestName, FilterTuple,
9+
StrJSON, TestName, FilterTuple, abs_module_name,
810
format_duration, normalize_test_name, print_warning, github_annotation)
911

1012

@@ -178,17 +180,21 @@ def __str__(self) -> str:
178180
def has_meaningful_duration(self):
179181
return State.has_meaningful_duration(self.state)
180182

181-
def print_github_crash_annotation(self, fail_env_changed: bool) -> None:
183+
def print_github_annotation(self, runtests: RunTests) -> None:
182184
"""Annotate a failed test without test case failures.
183185
184-
For example: crash, timeout, env changed. Test case failures are
185-
annotated by the test runner, where they are reported (see
186-
RegressionTestResult.printErrorList()).
186+
For example: crash, timeout, env changed. Locate the annotation in the
187+
test file. Test case failures are annotated by the test runner, where
188+
they are reported (see RegressionTestResult.printErrorList()).
187189
"""
188-
if (self.is_failed(fail_env_changed)
190+
if (self.is_failed(runtests.fail_env_changed)
189191
and not self.errors and not self.failures):
190192
message = "\n".join([str(self), *(self.env_changed_reasons or ())])
191-
print(github_annotation(self.test_name, message), flush=True)
193+
spec = importlib.util.find_spec(
194+
abs_module_name(self.test_name, runtests.test_dir))
195+
filename = spec.origin if spec is not None else None
196+
print(github_annotation(self.test_name, message, filename),
197+
flush=True)
192198

193199
def set_env_changed(self, *reasons):
194200
if self.state is None or self.state == State.PASSED:

‎Lib/test/libregrtest/run_workers.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ def _process_result(self, item: QueueOutput) -> TestResult:
614614
self.results.accumulate_result(result, self.runtests)
615615
self.display_result(mp_result)
616616
if self.runtests.github_annotations:
617-
result.print_github_crash_annotation(self.runtests.fail_env_changed)
617+
result.print_github_annotation(self.runtests)
618618

619619
# Display worker stdout
620620
if not self.runtests.output_on_failure:

‎Lib/test/libregrtest/runtests.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class RunTests:
9393
hunt_refleak: HuntRefleak | None
9494
test_dir: StrPath | None
9595
use_junit: bool
96-
# Print a GitHub Actions error annotation before each failure report
96+
# Annotate test failures in the GitHub Actions job log
9797
github_annotations: bool
9898
coverage: bool
9999
memory_limit: str | None

‎Lib/test/libregrtest/setup.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,12 @@ def setup_tests(runtests: RunTests) -> None:
112112

113113
set_match_tests(runtests.match_tests)
114114

115+
from .testresult import RegressionTestResult
115116
if runtests.use_junit:
116117
support.junit_xml_list = []
117-
from .testresult import RegressionTestResult
118118
RegressionTestResult.USE_XML = True
119119
else:
120120
support.junit_xml_list = None
121-
122-
from .testresult import RegressionTestResult
123121
RegressionTestResult.GITHUB_ANNOTATIONS = runtests.github_annotations
124122

125123
if runtests.memory_limit is not None:

‎Lib/test/libregrtest/testresult.py‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import functools
66
import io
7+
import re
78
import sys
89
import time
910
import traceback
@@ -132,19 +133,25 @@ def addUnexpectedSuccess(self, test):
132133
super().addUnexpectedSuccess(test)
133134

134135
def printErrorList(self, flavour, errors):
135-
if not self.GITHUB_ANNOTATIONS:
136-
super().printErrorList(flavour, errors)
137-
return
138136
for test, err in errors:
139-
# Precede each failure report with a GitHub Actions annotation,
140-
# so that the annotation links to the report in the job log
141-
# Locate the annotation in the test file
142-
case = getattr(test, "test_case", test) # subTest()
143-
module = sys.modules.get(type(case).__module__)
144-
self.stream.writeln(github_annotation(
145-
str(test), err, getattr(module, "__file__", None)))
137+
if self.GITHUB_ANNOTATIONS:
138+
# Write the annotation just before the failure report, so
139+
# that it links to the report in the job log
140+
self.stream.writeln(github_annotation(str(test), err,
141+
self._test_file(test)))
146142
super().printErrorList(flavour, [(test, err)])
147143

144+
@staticmethod
145+
def _test_file(test):
146+
# Test id: "module.Class.method", "module.function" (doctest), or
147+
# "setUpClass (module.Class)" (error in a class or module fixture)
148+
name = getattr(test, 'test_case', test).id() # subTest()
149+
if match := re.fullmatch(r'\w+ \((.+)\)', name):
150+
name = match[1]
151+
while name and name not in sys.modules:
152+
name = name.rpartition('.')[0]
153+
return getattr(sys.modules.get(name), '__file__', None)
154+
148155
def get_xml_element(self):
149156
if not self.USE_XML:
150157
raise ValueError("USE_XML is false")
@@ -156,16 +163,11 @@ def get_xml_element(self):
156163

157164
class QuietRegressionTestRunner:
158165
def __init__(self, stream, buffer=False):
159-
# Wrap the stream as TextTestRunner does, for printErrors()
160-
stream = unittest.runner._WritelnDecorator(stream)
161166
self.result = RegressionTestResult(stream, None, 0)
162167
self.result.buffer = buffer
163168

164169
def run(self, test):
165170
test(self.result)
166-
if self.result.GITHUB_ANNOTATIONS:
167-
# Report annotated failures, as in verbose mode
168-
self.result.printErrors()
169171
return self.result
170172

171173
def get_test_runner_class(verbosity, buffer=False):

‎Lib/test/libregrtest/utils.py‎

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,30 @@ def print_warning(msg: str) -> None:
141141
orig_unraisablehook: Callable[..., None] | None = None
142142

143143

144+
def repository_path(filename: str) -> str:
145+
"""Path of a stdlib file relative to the repository.
146+
147+
Map the stdlib directory (Lib/ in a source checkout or an installed
148+
lib/python3.X/) to Lib/. Return other paths unchanged.
149+
"""
150+
stdlib_dir = os.path.dirname(os.__file__) + os.sep
151+
if not filename.startswith(stdlib_dir):
152+
return filename
153+
return "Lib/" + filename.removeprefix(stdlib_dir).replace(os.sep, "/")
154+
155+
144156
def github_annotation(title: str, message: str,
145-
filename: str | None = None) -> str:
157+
filename: str | None) -> str:
146158
"""Format a GitHub Actions error annotation.
147159
148160
message is a traceback or a failure description. Only keep the exception
149161
which ends the traceback: the job log has the full traceback. Locate the
150-
annotation at the last frame of the traceback in filename, if any.
162+
annotation in filename, the test file, at the last frame of the traceback
163+
in this file, if any.
151164
"""
165+
if not filename:
166+
raise ValueError(f"missing test file of annotation {title!r}")
167+
152168
def escape(text: str) -> str:
153169
return (text.replace("%", "%25").replace("\r", "%0D")
154170
.replace("\n", "%0A"))
@@ -157,18 +173,15 @@ def escape_property(text: str) -> str:
157173
return escape(text).replace(":", "%3A").replace(",", "%2C")
158174

159175
message = decolor(message)
160-
props: dict[str, str] = {}
161-
# Map the stdlib directory (Lib/ in a source checkout or an installed
162-
# lib/python3.X/) to Lib/ in the repository
163-
stdlib_dir = os.path.dirname(os.__file__) + os.sep
164-
if filename and filename.startswith(stdlib_dir):
165-
lines = re.findall(rf'^ File "{re.escape(filename)}", line (\d+)',
166-
message, re.MULTILINE)
167-
if lines:
168-
relpath = filename.removeprefix(stdlib_dir).replace(os.sep, "/")
169-
props |= {"file": f"Lib/{relpath}", "line": lines[-1]}
176+
props = {"file": repository_path(filename)}
177+
# Line of the last traceback frame (' File "filename", line 123') or
178+
# failed doctest example (same, but not indented) in filename
179+
if lines := re.findall(rf'^ *File "{re.escape(filename)}", line (\d+)',
180+
message, re.MULTILINE):
181+
props["line"] = lines[-1]
170182
props["title"] = title
171-
# Strip the frames: each " File" line and its indented source lines
183+
# Only keep the exception: strip the traceback frames, each one a
184+
# ' File' line followed by its indented source lines
172185
message = re.split(r'^ File .*\n(?: .*\n)*', message,
173186
flags=re.MULTILINE)[-1].strip()
174187
props_text = ",".join(f"{key}={escape_property(value)}"

0 commit comments

Comments
 (0)