Skip to content

Commit eb88506

Browse files
committed
fix: keep the PR comment under GitHub's size limit
A pull request with a few dozen failing commits made a report longer than the 65,536 characters GitHub accepts in a comment. The API refused it, and an earlier comment, perhaps one that said passed, stayed up. A report that does not fit now leaves out the listing of all checks, then everything under the group lines, and links to the run for the rest. The job summary keeps the full report.
1 parent d68e05b commit eb88506

2 files changed

Lines changed: 145 additions & 4 deletions

File tree

‎main.py‎

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2027,7 +2027,25 @@ def _report_footer() -> str:
20272027
)
20282028

20292029

2030-
def render_report(results: list[ScopeResult]) -> str:
2030+
def _run_url() -> str:
2031+
"""Link to this workflow run, whose page holds the job summary."""
2032+
repository = os.getenv("GITHUB_REPOSITORY", "")
2033+
run_id = os.getenv("GITHUB_RUN_ID", "")
2034+
if not (repository and run_id):
2035+
return ""
2036+
server = os.getenv("GITHUB_SERVER_URL") or "https://github.com"
2037+
return f"{server.rstrip('/')}/{repository}/actions/runs/{run_id}"
2038+
2039+
2040+
def _elsewhere(what: str) -> str:
2041+
"""Point to where the parts a shortened comment left out can be read."""
2042+
where = "job summary" if _summary_written() else "step log"
2043+
url = _run_url()
2044+
place = f"[{where}]({url})" if url else where
2045+
return f"{what} are in the {place} of this run."
2046+
2047+
2048+
def render_report(results: list[ScopeResult], detail: str = "full") -> str:
20312049
"""Render the Markdown report shared by the job summary and PR comment.
20322050
20332051
Opens with the hidden marker and a heading that is the verdict — the
@@ -2036,6 +2054,10 @@ def render_report(results: list[ScopeResult]) -> str:
20362054
listed any rule under ``warn``, the folded "How to fix", and the folded
20372055
listing of every scope.
20382056
2057+
``detail`` is for a PR comment that would not fit (``render_pr_comment``):
2058+
``"short"`` leaves out the listing, ``"headline"`` everything below the
2059+
group lines, and either says where to read what it left out.
2060+
20392061
A warned scope counts toward "passed", never toward "failed": it ran,
20402062
found something, and is reported in full, but the config asked for it to
20412063
be surfaced rather than enforced.
@@ -2063,14 +2085,22 @@ def render_report(results: list[ScopeResult]) -> str:
20632085
lines.extend([_group_lines(results), ""])
20642086
if skipped and _skip_reasons(results):
20652087
lines.extend([_skip_reasons(results), ""])
2088+
if detail == "headline":
2089+
what = "The findings and how to fix them" if failed or warned else "All checks"
2090+
lines.extend([_elsewhere(what), "", _report_footer()])
2091+
return "\n".join(lines)
20662092
if failed:
20672093
lines.extend([*_diff_block(results), ""])
20682094
if warned:
20692095
lines.extend(["**Warnings**", "", *_diff_block(results, "warn"), ""])
20702096
if failed:
20712097
how = _how_to_fix(results)
20722098
lines.extend([*how, ""] if how else [])
2073-
lines.extend([*_all_checks(results), "", _report_footer()])
2099+
if detail == "short":
2100+
lines.extend([_elsewhere(f"All {_plural(total, 'check')}"), ""])
2101+
else:
2102+
lines.extend([*_all_checks(results), ""])
2103+
lines.append(_report_footer())
20742104
return "\n".join(lines)
20752105

20762106

@@ -2079,9 +2109,31 @@ def render_job_summary(results: list[ScopeResult]) -> str:
20792109
return render_report(results)
20802110

20812111

2112+
#: The longest comment GitHub accepts, in UTF-16 code units.
2113+
COMMENT_LIMIT = 65536
2114+
2115+
2116+
def _comment_length(body: str) -> int:
2117+
"""Length of a comment as GitHub counts it: in UTF-16 code units."""
2118+
return len(body.encode("utf-16-le")) // 2
2119+
2120+
20822121
def render_pr_comment(results: list[ScopeResult]) -> str:
2083-
"""Create the Markdown body for the PR comment (same report as summary)."""
2084-
return render_report(results)
2122+
"""Create the Markdown body for the PR comment (same report as summary).
2123+
2124+
The report grows with the pull request: every failing commit's message
2125+
is quoted in the diff, the prompt and the listing. A few dozen failing
2126+
commits ran past GitHub's limit, the API refused the comment, and an
2127+
earlier one, perhaps saying "passed", stayed on the pull request. So a
2128+
report that does not fit drops the listing, then everything under the
2129+
group lines, and points to the run for the rest. The job summary, with
2130+
its far higher limit, keeps the full report.
2131+
"""
2132+
for detail in ("full", "short"):
2133+
body = render_report(results, detail)
2134+
if _comment_length(body) <= COMMENT_LIMIT:
2135+
return body
2136+
return render_report(results, "headline")
20852137

20862138

20872139
# ---------------------------------------------------------------------------

‎main_test.py‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2705,6 +2705,95 @@ def test_failure_matches_job_summary(self):
27052705
self.assertIn("```diff\n@@ Commit 1/1 @@\n", comment)
27062706

27072707

2708+
class TestPrCommentSizeLimit(unittest.TestCase):
2709+
"""A report too long for a comment gives up its longest parts first."""
2710+
2711+
RUN_URL = "https://github.com/o/r/actions/runs/42"
2712+
2713+
def setUp(self):
2714+
self.RUN = {
2715+
"GITHUB_REPOSITORY": "o/r",
2716+
"GITHUB_RUN_ID": "42",
2717+
"GITHUB_SERVER_URL": "https://github.com",
2718+
}
2719+
2720+
def _failing_commits(self, count: int) -> list[main.ScopeResult]:
2721+
body = "\n".join(
2722+
f"Line {i} of a body that explains the change." for i in range(8)
2723+
)
2724+
return [
2725+
main.ScopeResult(
2726+
label=f"Commit {i}/{count}",
2727+
sha=f"{i:040x}",
2728+
checks=[
2729+
make_check(
2730+
"require_signed_off_by",
2731+
"fail",
2732+
"CC012",
2733+
value=f"Update module {i}\n\n{body}",
2734+
error="Signed-off-by trailer not found in the commit message",
2735+
fix=f"Update module {i}\n\n{body}\n\nSigned-off-by: A <a@b.c>",
2736+
)
2737+
],
2738+
)
2739+
for i in range(1, count + 1)
2740+
]
2741+
2742+
def test_a_report_that_fits_is_the_job_summary(self):
2743+
results = self._failing_commits(3)
2744+
self.assertEqual(
2745+
main.render_pr_comment(results), main.render_job_summary(results)
2746+
)
2747+
2748+
def test_dozens_of_failing_commits_still_fit(self):
2749+
results = self._failing_commits(60)
2750+
self.assertGreater(len(main.render_job_summary(results)), main.COMMENT_LIMIT)
2751+
with (
2752+
patch.dict(os.environ, self.RUN),
2753+
patch("main.JOB_SUMMARY_ENABLED", True),
2754+
patch("main.GITHUB_STEP_SUMMARY", "/tmp/summary.md"),
2755+
):
2756+
comment = main.render_pr_comment(results)
2757+
self.assertLessEqual(main._comment_length(comment), main.COMMENT_LIMIT)
2758+
self.assertTrue(comment.startswith(main.COMMENT_MARKER))
2759+
self.assertIn("Commit Check failed", comment)
2760+
self.assertNotIn("<summary>All 60 checks</summary>", comment)
2761+
self.assertIn(
2762+
f"All 60 checks are in the [job summary]({self.RUN_URL}) of this run.",
2763+
comment,
2764+
)
2765+
2766+
def test_the_diff_goes_too_when_the_listing_was_not_enough(self):
2767+
results = self._failing_commits(2)
2768+
short = main.render_report(results, "short")
2769+
with (
2770+
patch("main.COMMENT_LIMIT", len(short) - 1),
2771+
patch.dict(os.environ, self.RUN),
2772+
patch("main.JOB_SUMMARY_ENABLED", False),
2773+
):
2774+
comment = main.render_pr_comment(results)
2775+
self.assertNotIn("```diff", comment)
2776+
self.assertNotIn("How to fix</b>", comment)
2777+
self.assertIn(
2778+
"The findings and how to fix them are in the "
2779+
f"[step log]({self.RUN_URL}) of this run.",
2780+
comment,
2781+
)
2782+
self.assertIn("Commits: 2 of 2 failed", comment)
2783+
2784+
def test_a_passing_headline_names_no_findings(self):
2785+
env = {k: v for k, v in os.environ.items() if k != "GITHUB_RUN_ID"}
2786+
with (
2787+
patch.dict(os.environ, env, clear=True),
2788+
patch("main.JOB_SUMMARY_ENABLED", False),
2789+
):
2790+
body = main.render_report([pass_scope("Branch")], "headline")
2791+
self.assertIn("All checks are in the step log of this run.", body)
2792+
2793+
def test_length_is_counted_as_github_counts_it(self):
2794+
self.assertEqual(main._comment_length("a🤖"), 3)
2795+
2796+
27082797
class TestUnparsableScopeInTheReport(unittest.TestCase):
27092798
"""A CLI response that is not JSON is a failure with no rule to link."""
27102799

0 commit comments

Comments
 (0)