Skip to content

Commit e4d41bf

Browse files
committed
Self review fixes
1 parent 42d7918 commit e4d41bf

4 files changed

Lines changed: 20 additions & 32 deletions

File tree

‎Lib/test/libregrtest/main.py‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ def write_github_summary(self) -> None:
492492
self.fail_rerun)
493493
if not self.github_summary or not failed or not exitcode:
494494
return
495-
ncases = sum(len(result.test_cases) for result in failed)
495+
ncases = sum(len(result.failed_test_cases) for result in failed)
496496
with open(self.github_summary, "a", encoding="utf-8") as fp:
497497
fp.write(f"## {decolor(self.get_state())}: "
498498
f"{count(len(failed), 'test file')} and "
@@ -538,8 +538,8 @@ def create_run_tests(self, tests: TestTuple) -> RunTests:
538538
hunt_refleak=self.hunt_refleak,
539539
test_dir=self.test_dir,
540540
use_junit=(self.junit_filename is not None),
541-
# Only annotate failures of the last run: not the first run if
542-
# failed tests will be re-run (with --python, they are not)
541+
# Only annotate the failure that results in job failure.
542+
# Ex: When reruning failed tests, only annotate the the final run.
543543
github_annotations=(bool(self.github_summary) and
544544
not (self.want_rerun and not self.python_cmd)),
545545
coverage=self.coverage,
@@ -789,8 +789,6 @@ def _init(self):
789789

790790
strip_py_suffix(self.cmdline_args)
791791

792-
# Pass GitHub Actions annotations to workers in RunTests, not to
793-
# regrtest run by tests
794792
self.github_summary = os.environ.pop("GITHUB_STEP_SUMMARY", None)
795793

796794
self._tmp_dir = get_temp_dir(self._tmp_dir)

‎Lib/test/libregrtest/result.py‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,7 @@ def has_meaningful_duration(self):
184184
return State.has_meaningful_duration(self.state)
185185

186186
@property
187-
def test_cases(self) -> list[tuple[str, str]]:
188-
"""Failed test cases: list of (name, traceback)."""
187+
def failed_test_cases(self) -> list[tuple[str, str]]:
189188
return (self.errors or []) + (self.failures or [])
190189

191190
def format_github_summary(self, expand: bool) -> str:
@@ -194,37 +193,34 @@ def format_github_summary(self, expand: bool) -> str:
194193
if self.env_changed_reasons:
195194
blocks.append("\n".join(f"- {reason}"
196195
for reason in self.env_changed_reasons))
197-
for name, traceback in self.test_cases:
198-
# Expand the tracebacks if they fit on a screen: a traceback of
199-
# less than 30 lines
196+
for name, traceback in self.failed_test_cases:
197+
# Only default open short tracebacks.
200198
is_open = expand and traceback.count("\n") < 30
201199
blocks += [f"<details{' open' if is_open else ''}>"
202200
f"<summary>{html.escape(name)}</summary>",
203201
f"```pytb\n{decolor(traceback).rstrip()}\n```",
204202
"</details>"]
203+
205204
# Separate Markdown blocks with an empty line
206205
return "".join(f"{block}\n\n" for block in blocks)
207206

208207
def print_github_annotation(self, runtests: RunTests) -> None:
209-
"""Annotate each failed test case, or else the failed test file.
210-
211-
For example, the test file for a crash, timeout or env changed.
212-
"""
208+
"""Print github error annotation line for each failure."""
213209
if not (runtests.github_annotations
214210
and self.is_failed(runtests.fail_env_changed)):
215211
return
216-
# Test file: "test_x" is test_x.py or test_x/__init__.py,
217-
# "test.test_x.test_y" is test_x/test_y.py
212+
# Convert python import test name to relative file path.
213+
# Ex: "test.test_x.test_y" is test_x/test_y.py
218214
name = self.test_name.removeprefix("test.")
219215
path = pathlib.Path(findtestdir(runtests.test_dir), *name.split("."))
220216
filename = path.with_suffix(".py")
221217
if not filename.exists():
222218
filename = path / "__init__.py"
223-
test_cases = self.test_cases
224-
if not test_cases:
219+
annotations = self.failed_test_cases
220+
if not annotations:
225221
message = "\n".join([str(self), *(self.env_changed_reasons or ())])
226-
test_cases = [(self.test_name, message)]
227-
for title, message in test_cases:
222+
annotations = [(self.test_name, message)]
223+
for title, message in annotations:
228224
print_github_annotation(title, message, str(filename))
229225

230226
def set_env_changed(self, *reasons):

‎Lib/test/libregrtest/run_workers.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,6 @@ def _process_result(self, item: QueueOutput) -> TestResult:
624624
stdout = mp_result.worker_stdout
625625
if stdout:
626626
print(stdout, flush=True)
627-
# Annotate after the output: env changed warnings, crash traceback
628627
result.print_github_annotation(self.runtests)
629628

630629
return result

‎Lib/test/libregrtest/utils.py‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -149,25 +149,20 @@ def print_warning(msg: str) -> None:
149149

150150

151151
def print_github_annotation(title: str, message: str, filename: str) -> None:
152-
"""Print a GitHub Actions error annotation.
153-
154-
message is a traceback or a failure description. Only keep the exception
155-
which ends the traceback: the job log has the full traceback. Locate the
156-
annotation at the last traceback frame in a test file, or else at the
157-
first frame (the test), or else in filename (the test file).
158-
"""
152+
"""Print a GitHub Actions error annotation for the failure."""
159153
if not filename:
160-
raise ValueError(f"missing test file of annotation {title!r}")
154+
raise ValueError(f"missing filename to annotate {title!r}")
161155
message = decolor(message)
156+
157+
# Try to get a line number from backtrace.
162158
line = None
163-
# Traceback frames (' File "filename", line 123') and failed doctest
164-
# examples (same, but not indented)
165159
if frames := re.findall(r'^ *File "(.*)", line (\d+)', message,
166160
re.MULTILINE):
167161
test_frames = [frame for frame in frames
168162
if pathlib.Path(frame[0]).name.startswith('test_')]
169163
filename, line = test_frames[-1] if test_frames else frames[0]
170-
# In the job, the stdlib can be outside of the repository checkout
164+
165+
# Normalize stdlib path to be relative inside the repository checkout.
171166
path = pathlib.Path(filename)
172167
stdlib_dir = pathlib.Path(os.__file__).parent
173168
if path.is_relative_to(stdlib_dir):

0 commit comments

Comments
 (0)