Skip to content

Commit 589e6a7

Browse files
committed
Simplify, generlize reporter
1 parent dc83a3c commit 589e6a7

7 files changed

Lines changed: 142 additions & 92 deletions

File tree

‎Lib/test/libregrtest/main.py‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ def __init__(self, ns: Namespace, _add_python_opts: bool = False):
6969

7070
# Logger
7171
self.logger = Logger(self.results, self.quiet, self.pgo)
72+
self.reporter = get_reporter()
7273

7374
# Actions
7475
self.want_header: bool = ns.header
@@ -286,8 +287,9 @@ def _rerun_failed_tests(self, runtests: RunTests) -> RunTests:
286287
fail_fast=False,
287288
match_tests_dict=match_tests_dict,
288289
output_on_failure=False,
289-
github_summary=self.github_summary)
290+
final_run=True)
290291
self.logger.set_tests(runtests)
292+
self.reporter.run_started(runtests)
291293

292294
msg = f"Re-running {len(tests)} failed tests in verbose mode"
293295
if not self.single_process:
@@ -397,7 +399,7 @@ def run_test(
397399
result = run_single_test(test_name, runtests)
398400

399401
self.results.accumulate_result(result, runtests)
400-
get_reporter(runtests.github_summary).test_result(result, runtests)
402+
self.reporter.test_result(result)
401403

402404
return result
403405

@@ -462,7 +464,8 @@ def get_state(self) -> str:
462464

463465
def _run_tests_mp(self, runtests: RunTests, num_workers: int) -> None:
464466
from .run_workers import RunWorkers
465-
RunWorkers(num_workers, runtests, self.logger, self.results).run()
467+
RunWorkers(num_workers, runtests, self.logger, self.results,
468+
self.reporter).run()
466469

467470
def finalize_tests(self, coverage: trace.CoverageResults | None) -> None:
468471
if self.next_single_filename:
@@ -483,12 +486,7 @@ def finalize_tests(self, coverage: trace.CoverageResults | None) -> None:
483486
if self.junit_filename:
484487
self.results.write_junit(self.junit_filename)
485488

486-
# Test files which failed in the last run (the re-run, if any)
487-
failed = [result for result in self.results.rerun_results
488-
if result.is_failed(self.fail_env_changed)]
489-
if failed:
490-
get_reporter(self.github_summary).run_failed(failed,
491-
self.get_state())
489+
self.reporter.summary(self.results, self.get_state())
492490

493491
def display_summary(self) -> None:
494492
if self.first_runtests is None:
@@ -527,9 +525,7 @@ def create_run_tests(self, tests: TestTuple) -> RunTests:
527525
hunt_refleak=self.hunt_refleak,
528526
test_dir=self.test_dir,
529527
use_junit=(self.junit_filename is not None),
530-
# When failed tests are re-run, only report the re-run
531-
github_summary=(None if self.want_rerun and not self.python_cmd
532-
else self.github_summary),
528+
final_run=not (self.want_rerun and not self.python_cmd),
533529
coverage=self.coverage,
534530
memory_limit=self.memory_limit,
535531
gc_threshold=self.gc_threshold,
@@ -581,6 +577,7 @@ def _run_tests(self, selected: TestTuple, tests: TestList | None) -> int:
581577
runtests = runtests.copy(tests=case_ids, case_groups=case_groups)
582578
self.first_runtests = runtests
583579
self.logger.set_tests(runtests)
580+
self.reporter.run_started(runtests)
584581

585582
if (runtests.hunt_refleak is not None) and (not self.num_workers):
586583
# gh-109739: WindowsLoadTracker thread interferes with refleak check
@@ -777,8 +774,6 @@ def _init(self):
777774

778775
strip_py_suffix(self.cmdline_args)
779776

780-
self.github_summary = os.environ.pop("GITHUB_STEP_SUMMARY", None)
781-
782777
self._tmp_dir = get_temp_dir(self._tmp_dir)
783778

784779
@property

‎Lib/test/libregrtest/reporter.py‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
from _colorize import decolor # type: ignore[import-not-found]
66
from typing import TextIO
77

8-
from .findtests import findtestdir
98
from .result import TestResult
9+
lazy from .results import TestResults
1010
from .runtests import RunTests
11-
from .utils import StrPath, count
11+
from .utils import count
1212

1313

1414
class Reporter:
@@ -17,6 +17,14 @@ class Reporter:
1717
The default reporter does nothing.
1818
"""
1919

20+
def run_started(self, runtests: RunTests) -> None:
21+
"""Start a run of tests.
22+
23+
Called in the main process at the start of each run (the first run
24+
and the re-run), and in the process running the tests before each
25+
test file.
26+
"""
27+
2028
def test_case_failed(self, test_case: str, traceback: str,
2129
stream: TextIO) -> None:
2230
"""Report a failed test case.
@@ -25,11 +33,11 @@ def test_case_failed(self, test_case: str, traceback: str,
2533
test file in quiet mode.
2634
"""
2735

28-
def test_result(self, result: TestResult, runtests: RunTests) -> None:
36+
def test_result(self, result: TestResult) -> None:
2937
"""Report the result of a test file, in the main process."""
3038

31-
def run_failed(self, failed: list[TestResult], state: str) -> None:
32-
"""Report the test files which failed the run."""
39+
def summary(self, results: TestResults, state: str) -> None:
40+
"""Report the results of all runs, in the main process."""
3341

3442
@staticmethod
3543
def parse_traceback(traceback: str) -> tuple[str | None, str | None, str]:
@@ -49,25 +57,10 @@ def parse_traceback(traceback: str) -> tuple[str | None, str | None, str]:
4957
flags=re.MULTILINE)[-1].strip()
5058
return filename, line, exception
5159

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)
60+
61+
def get_reporter() -> Reporter:
62+
if GitHubActionsReporter.enabled():
63+
return GitHubActionsReporter()
7164
return Reporter()
7265

7366

@@ -79,23 +72,31 @@ class GitHubActionsReporter(Reporter):
7972
ESCAPE_DATA = str.maketrans({"%": "%25", "\r": "%0D", "\n": "%0A"})
8073
ESCAPE_PROPERTY = ESCAPE_DATA | str.maketrans({":": "%3A", ",": "%2C"})
8174

82-
def __init__(self, summary_filename: str) -> None:
83-
self.summary_filename = summary_filename
75+
@staticmethod
76+
def enabled() -> bool:
77+
return "GITHUB_STEP_SUMMARY" in os.environ
78+
79+
def run_started(self, runtests: RunTests) -> None:
80+
self.runtests = runtests
8481

8582
def test_case_failed(self, test_case: str, traceback: str,
8683
stream: TextIO) -> None:
84+
# Only report the failures which decide the result
85+
if not self.runtests.final_run:
86+
return
8787
# The job log has the full traceback: only keep the exception
8888
filename, line, exception = self.parse_traceback(decolor(traceback))
8989
self.annotate(test_case, exception, filename, line, stream)
9090

91-
def test_result(self, result: TestResult, runtests: RunTests) -> None:
91+
def test_result(self, result: TestResult) -> None:
9292
# Failed test cases are annotated by test_case_failed()
93-
if (not result.is_failed(runtests.fail_env_changed)
93+
if (not self.runtests.final_run
94+
or not result.is_failed(self.runtests.fail_env_changed)
9495
or result.failed_test_cases):
9596
return
9697
message = "\n".join([str(result), *(result.env_changed_reasons or ())])
9798
self.annotate(result.test_name, decolor(message),
98-
self.test_filename(result, runtests.test_dir))
99+
result.find_filename(self.runtests.test_dir))
99100

100101
def annotate(self, title: str, message: str, filename: str | None,
101102
line: str | None = None, stream: TextIO | None = None) -> None:
@@ -113,7 +114,12 @@ def annotate(self, title: str, message: str, filename: str | None,
113114
print(f"::error {props_text}::{message.translate(self.ESCAPE_DATA)}",
114115
file=stream, flush=True)
115116

116-
def run_failed(self, failed: list[TestResult], state: str) -> None:
117+
def summary(self, results: TestResults, state: str) -> None:
118+
# Test files which failed in the last run (the re-run, if any)
119+
failed = [result for result in results.rerun_results
120+
if result.is_failed(self.runtests.fail_env_changed)]
121+
if not failed:
122+
return
117123
ncases = sum(len(result.failed_test_cases) for result in failed)
118124
blocks = [f"## {decolor(state)}: "
119125
f"{count(len(failed), 'test file')} and "
@@ -131,5 +137,6 @@ def run_failed(self, failed: list[TestResult], state: str) -> None:
131137
f"```pytb\n{decolor(traceback).rstrip()}\n```",
132138
"</details>"]
133139
# Markdown blocks are separated by an empty line
134-
with open(self.summary_filename, "a", encoding="utf-8") as fp:
140+
with open(os.environ["GITHUB_STEP_SUMMARY"], "a",
141+
encoding="utf-8") as fp:
135142
fp.write("".join(f"{block}\n\n" for block in blocks))

‎Lib/test/libregrtest/result.py‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import dataclasses
22
import json
3+
import pathlib
34
from _colorize import get_colors # type: ignore[import-not-found]
45
from typing import Any
56

7+
from .findtests import findtestdir
68
from .utils import (
7-
StrJSON, TestName, FilterTuple,
9+
StrJSON, StrPath, TestName, FilterTuple,
810
format_duration, normalize_test_name, print_warning)
911

1012

@@ -185,6 +187,19 @@ def has_meaningful_duration(self):
185187
def failed_test_cases(self) -> list[tuple[str, str]]:
186188
return (self.errors or []) + (self.failures or [])
187189

190+
def find_filename(self, test_dir: StrPath | None) -> str | None:
191+
"""Get the filename of the test, or None if it is not found."""
192+
if self.filename:
193+
return self.filename
194+
# The worker crashed or timed out before importing the test:
195+
# "test.test_x.test_y" is test_x/test_y.py or test_x/test_y/__init__.py
196+
name = self.test_name.removeprefix("test.")
197+
path = pathlib.Path(findtestdir(test_dir), *name.split("."))
198+
for filename in (path.with_suffix(".py"), path / "__init__.py"):
199+
if filename.exists():
200+
return str(filename)
201+
return None
202+
188203
def set_env_changed(self, *reasons):
189204
if self.state is None or self.state == State.PASSED:
190205
self.state = State.ENV_CHANGED

‎Lib/test/libregrtest/run_workers.py‎

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

1818
from .logger import Logger
19-
from .reporter import get_reporter
19+
from .reporter import Reporter
2020
from .result import TestResult, State
2121
from .results import TestResults
2222
from .runtests import RunTests, WorkerRunTests, JsonFile, JsonFileType
@@ -500,13 +500,15 @@ def get_running(workers: list[WorkerThread]) -> str | None:
500500

501501
class RunWorkers:
502502
def __init__(self, num_workers: int, runtests: RunTests,
503-
logger: Logger, results: TestResults) -> None:
503+
logger: Logger, results: TestResults,
504+
reporter: Reporter) -> None:
504505
self.num_workers = num_workers
505506
self.runtests = runtests
506507
self.logger = logger
507508
self.log = logger.log
508509
self.display_progress = logger.display_progress
509510
self.results: TestResults = results
511+
self.reporter = reporter
510512
self.live_worker_count = 0
511513

512514
self.output: queue.Queue[QueueContent] = queue.Queue()
@@ -626,8 +628,7 @@ def _process_result(self, item: QueueOutput) -> TestResult:
626628
if stdout:
627629
print(stdout, flush=True)
628630
# Report after the output: env changed warnings, crash traceback
629-
get_reporter(self.runtests.github_summary).test_result(result,
630-
self.runtests)
631+
self.reporter.test_result(result)
631632

632633
return result
633634

‎Lib/test/libregrtest/runtests.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ class RunTests:
9393
hunt_refleak: HuntRefleak | None
9494
test_dir: StrPath | None
9595
use_junit: bool
96-
# GitHub Actions job summary file, to report the failures of this run
97-
github_summary: str | None
96+
# Failed tests are not re-run after this run
97+
final_run: bool
9898
coverage: bool
9999
memory_limit: str | None
100100
gc_threshold: int | None

‎Lib/test/libregrtest/setup.py‎

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

115115
set_match_tests(runtests.match_tests)
116116

117-
RegressionTestResult.reporter = get_reporter(runtests.github_summary)
117+
reporter = get_reporter()
118+
reporter.run_started(runtests)
119+
RegressionTestResult.reporter = reporter
118120
if runtests.use_junit:
119121
support.junit_xml_list = []
120122
RegressionTestResult.USE_XML = True

0 commit comments

Comments
 (0)