Repository navigation
gh-158895: Teach libregrtest to output github job summaries #158896
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3eeb4db
8fcfa5d
e6e09bd
54d123f
3468742
ff7b75a
946c801
14394da
6359876
041e577
ba449b4
59b1608
75c44b0
1222c34
287df00
53024ed
c2d1d79
c3cca61
b125f6f
3f320dd
9b1ce48
b561060
e506d7d
2ca705b
fe08263
b5247ed
9a94bf1
70cae99
8f231a6
f2ba9db
42d7918
e4d41bf
b0e9c5e
01ad16c
475549a
dc83a3c
589e6a7
24e7820
6d2f432
f8189f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,137 @@ | ||||||
| import html | ||||||
| import os | ||||||
| import pathlib | ||||||
| import re | ||||||
| from _colorize import decolor # type: ignore[import-not-found] | ||||||
| from typing import TextIO | ||||||
|
|
||||||
| from .findtests import findtestdir | ||||||
| from .result import TestResult | ||||||
| from .results import TestResults | ||||||
| from .runtests import RunTests | ||||||
| from .utils import count | ||||||
|
|
||||||
|
|
||||||
| class Reporter: | ||||||
| """Report test failures, for example to a CI service. A no-op here.""" | ||||||
|
|
||||||
| def run_started(self, runtests: RunTests) -> None: | ||||||
| """Called at the start of each run, and in the test process.""" | ||||||
|
|
||||||
| def test_case_failed(self, test_case: str, traceback: str, | ||||||
| stream: TextIO) -> None: | ||||||
| """Called in the test process after a test case failed.""" | ||||||
|
|
||||||
| def test_result(self, result: TestResult) -> None: | ||||||
| """Called in the main process after a test file completed.""" | ||||||
|
|
||||||
| def summary(self, results: TestResults, state: str) -> None: | ||||||
| """Called in the main process after all runs.""" | ||||||
|
|
||||||
| @staticmethod | ||||||
| def parse_traceback(traceback: str) -> tuple[str | None, str | None, str]: | ||||||
| """Return (filename, line, exception) of a failure.""" | ||||||
| # A frame is its "File" line and its source lines | ||||||
| frames = list(re.finditer( | ||||||
| r'^ *File "(.*)", line (\d+).*\n(?: .*\n)*', traceback, | ||||||
| re.MULTILINE)) | ||||||
| filename = line = None | ||||||
| exception = traceback | ||||||
| if frames: | ||||||
| filename, line = frames[0].groups() | ||||||
| exception = traceback[frames[-1].end():] | ||||||
| # Prefer the last frame in a test file, the assertion | ||||||
| for frame in frames: | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe I'm wrong but will this get executed only when the above comparison is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct.In general this tries to find a filename from the traceback but if we fail that's okay (it shouldn't block the test run from finishing). The The loop then finds the last frame with a name that starts with "test_" and uses that for the annotation. Definitely not a perfect metric but seems to work reasonably for getting back to the test filename. |
||||||
| if pathlib.Path(frame[1]).name.startswith("test_"): | ||||||
| filename, line = frame.groups() | ||||||
| return filename, line, exception.strip() | ||||||
|
|
||||||
|
|
||||||
| def get_reporter() -> Reporter: | ||||||
| if GitHubActionsReporter.enabled(): | ||||||
| return GitHubActionsReporter() | ||||||
| return Reporter() | ||||||
|
|
||||||
|
|
||||||
| class GitHubActionsReporter(Reporter): | ||||||
| """Annotate failures in the job log and list them in the job summary.""" | ||||||
|
|
||||||
| ESCAPE_DATA = str.maketrans({"%": "%25", "\r": "%0D", "\n": "%0A"}) | ||||||
| ESCAPE_PROPERTY = ESCAPE_DATA | str.maketrans({":": "%3A", ",": "%2C"}) | ||||||
|
|
||||||
| @staticmethod | ||||||
| def enabled() -> bool: | ||||||
| return "GITHUB_STEP_SUMMARY" in os.environ | ||||||
|
|
||||||
| def run_started(self, runtests: RunTests) -> None: | ||||||
| self.runtests = runtests | ||||||
|
|
||||||
| def test_case_failed(self, test_case: str, traceback: str, | ||||||
| stream: TextIO) -> None: | ||||||
| if not self.runtests.final_run: | ||||||
| return | ||||||
| filename, line, exception = self.parse_traceback(decolor(traceback)) | ||||||
| self.annotate(test_case, exception, filename, line, stream) | ||||||
|
|
||||||
| def test_result(self, result: TestResult) -> None: | ||||||
| if not self.runtests.final_run: | ||||||
| return | ||||||
| if not result.is_failed(self.runtests.fail_env_changed): | ||||||
| return | ||||||
| # Already annotated by test_case_failed() | ||||||
| if result.failed_test_cases: | ||||||
| return | ||||||
| # "test.test_x.test_y" is test_x/test_y.py or test_x/test_y/__init__.py | ||||||
| name = result.test_name.removeprefix("test.") | ||||||
| path = pathlib.Path(findtestdir(self.runtests.test_dir), | ||||||
| *name.split(".")) | ||||||
| filename = None | ||||||
| for candidate in (path.with_suffix(".py"), path / "__init__.py"): | ||||||
| if candidate.exists(): | ||||||
| filename = str(candidate) | ||||||
| message = "\n".join([str(result), *(result.env_changed_reasons or ())]) | ||||||
| self.annotate(result.test_name, decolor(message), filename) | ||||||
|
|
||||||
| def annotate(self, title: str, message: str, filename: str | None, | ||||||
| line: str | None = None, | ||||||
| stream: TextIO | None = None) -> None: | ||||||
| if filename: | ||||||
| # Make installed stdlib paths relative to the checkout | ||||||
| path = pathlib.Path(filename) | ||||||
| stdlib_dir = pathlib.Path(os.__file__).parent | ||||||
| if path.is_relative_to(stdlib_dir): | ||||||
| filename = ("Lib" / path.relative_to(stdlib_dir)).as_posix() | ||||||
| # GitHub needs a line to show the annotation on the file | ||||||
| line = line or "1" | ||||||
| props = {"file": filename, "line": line, "title": title} | ||||||
| props_text = ",".join(f"{key}={value.translate(self.ESCAPE_PROPERTY)}" | ||||||
| for key, value in props.items() if value) | ||||||
| print(f"::error {props_text}::{message.translate(self.ESCAPE_DATA)}", | ||||||
| file=stream, flush=True) | ||||||
|
|
||||||
| def summary(self, results: TestResults, state: str) -> None: | ||||||
| failed = [result for result in results.rerun_results | ||||||
| if result.is_failed(self.runtests.fail_env_changed)] | ||||||
| if not failed: | ||||||
| return | ||||||
| ncases = sum(len(result.failed_test_cases) for result in failed) | ||||||
| blocks = [f"## {decolor(state)}: " | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Maybe it makes it a little easier to parse vs just markdown text?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we output success as well as failure definitely, this only outputs failures to github summaries. I tried outputting both success and failures but we run 30+ test jobs and that just made a lot of summaries that needed to be browsed past and weren't particularly useful. |
||||||
| f"{count(len(failed), 'test file')} and " | ||||||
| f"{count(ncases, 'test case')} failed"] | ||||||
| for result in failed: | ||||||
| blocks.append(f"### {decolor(str(result))}") | ||||||
| if result.env_changed_reasons: | ||||||
| blocks.append("\n".join( | ||||||
| f"- {reason}" for reason in result.env_changed_reasons)) | ||||||
| for name, traceback in result.failed_test_cases: | ||||||
| # Expand short tracebacks if there are only a few | ||||||
| if ncases <= 5 and traceback.count("\n") < 30: | ||||||
| details = "<details open>" | ||||||
| else: | ||||||
| details = "<details>" | ||||||
| blocks += [f"{details}<summary>{html.escape(name)}</summary>", | ||||||
| f"```pytb\n{decolor(traceback).rstrip()}\n```", | ||||||
| "</details>"] | ||||||
| with open(os.environ["GITHUB_STEP_SUMMARY"], "a", | ||||||
| encoding="utf-8") as fp: | ||||||
| fp.write("".join(f"{block}\n\n" for block in blocks)) | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, for parity with the other methods?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "run" concept is different than a "test" or "test suite" hence the different name. Parallels RunTests vs. a TestResult