diff --git a/main.py b/main.py index 6fdc056..205ed22 100755 --- a/main.py +++ b/main.py @@ -2027,7 +2027,25 @@ def _report_footer() -> str: ) -def render_report(results: list[ScopeResult]) -> str: +def _run_url() -> str: + """Link to this workflow run, whose page holds the job summary.""" + repository = os.getenv("GITHUB_REPOSITORY", "") + run_id = os.getenv("GITHUB_RUN_ID", "") + if not (repository and run_id): + return "" + server = os.getenv("GITHUB_SERVER_URL") or "https://github.com" + return f"{server.rstrip('/')}/{repository}/actions/runs/{run_id}" + + +def _elsewhere(what: str) -> str: + """Point to where the parts a shortened comment left out can be read.""" + where = "job summary" if _summary_written() else "step log" + url = _run_url() + place = f"[{where}]({url})" if url else where + return f"{what} are in the {place} of this run." + + +def render_report(results: list[ScopeResult], detail: str = "full") -> str: """Render the Markdown report shared by the job summary and PR comment. Opens with the hidden marker and a heading that is the verdict — the @@ -2036,6 +2054,10 @@ def render_report(results: list[ScopeResult]) -> str: listed any rule under ``warn``, the folded "How to fix", and the folded listing of every scope. + ``detail`` is for a PR comment that would not fit (``render_pr_comment``): + ``"short"`` leaves out the listing, ``"headline"`` everything below the + group lines, and either says where to read what it left out. + A warned scope counts toward "passed", never toward "failed": it ran, found something, and is reported in full, but the config asked for it to be surfaced rather than enforced. @@ -2063,6 +2085,10 @@ def render_report(results: list[ScopeResult]) -> str: lines.extend([_group_lines(results), ""]) if skipped and _skip_reasons(results): lines.extend([_skip_reasons(results), ""]) + if detail == "headline": + what = "The findings and how to fix them" if failed or warned else "All checks" + lines.extend([_elsewhere(what), "", _report_footer()]) + return "\n".join(lines) if failed: lines.extend([*_diff_block(results), ""]) if warned: @@ -2070,7 +2096,11 @@ def render_report(results: list[ScopeResult]) -> str: if failed: how = _how_to_fix(results) lines.extend([*how, ""] if how else []) - lines.extend([*_all_checks(results), "", _report_footer()]) + if detail == "short": + lines.extend([_elsewhere(f"All {_plural(total, 'check')}"), ""]) + else: + lines.extend([*_all_checks(results), ""]) + lines.append(_report_footer()) return "\n".join(lines) @@ -2079,9 +2109,31 @@ def render_job_summary(results: list[ScopeResult]) -> str: return render_report(results) +#: The longest comment GitHub accepts, in UTF-16 code units. +COMMENT_LIMIT = 65536 + + +def _comment_length(body: str) -> int: + """Length of a comment as GitHub counts it: in UTF-16 code units.""" + return len(body.encode("utf-16-le")) // 2 + + def render_pr_comment(results: list[ScopeResult]) -> str: - """Create the Markdown body for the PR comment (same report as summary).""" - return render_report(results) + """Create the Markdown body for the PR comment (same report as summary). + + The report grows with the pull request: every failing commit's message + is quoted in the diff, the prompt and the listing. A few dozen failing + commits ran past GitHub's limit, the API refused the comment, and an + earlier one, perhaps saying "passed", stayed on the pull request. So a + report that does not fit drops the listing, then everything under the + group lines, and points to the run for the rest. The job summary, with + its far higher limit, keeps the full report. + """ + for detail in ("full", "short"): + body = render_report(results, detail) + if _comment_length(body) <= COMMENT_LIMIT: + return body + return render_report(results, "headline") # --------------------------------------------------------------------------- diff --git a/main_test.py b/main_test.py index 2779b5f..06f2ddd 100644 --- a/main_test.py +++ b/main_test.py @@ -2705,6 +2705,95 @@ def test_failure_matches_job_summary(self): self.assertIn("```diff\n@@ Commit 1/1 @@\n", comment) +class TestPrCommentSizeLimit(unittest.TestCase): + """A report too long for a comment gives up its longest parts first.""" + + RUN_URL = "https://github.com/o/r/actions/runs/42" + + def setUp(self): + self.RUN = { + "GITHUB_REPOSITORY": "o/r", + "GITHUB_RUN_ID": "42", + "GITHUB_SERVER_URL": "https://github.com", + } + + def _failing_commits(self, count: int) -> list[main.ScopeResult]: + body = "\n".join( + f"Line {i} of a body that explains the change." for i in range(8) + ) + return [ + main.ScopeResult( + label=f"Commit {i}/{count}", + sha=f"{i:040x}", + checks=[ + make_check( + "require_signed_off_by", + "fail", + "CC012", + value=f"Update module {i}\n\n{body}", + error="Signed-off-by trailer not found in the commit message", + fix=f"Update module {i}\n\n{body}\n\nSigned-off-by: A ", + ) + ], + ) + for i in range(1, count + 1) + ] + + def test_a_report_that_fits_is_the_job_summary(self): + results = self._failing_commits(3) + self.assertEqual( + main.render_pr_comment(results), main.render_job_summary(results) + ) + + def test_dozens_of_failing_commits_still_fit(self): + results = self._failing_commits(60) + self.assertGreater(len(main.render_job_summary(results)), main.COMMENT_LIMIT) + with ( + patch.dict(os.environ, self.RUN), + patch("main.JOB_SUMMARY_ENABLED", True), + patch("main.GITHUB_STEP_SUMMARY", "/tmp/summary.md"), + ): + comment = main.render_pr_comment(results) + self.assertLessEqual(main._comment_length(comment), main.COMMENT_LIMIT) + self.assertTrue(comment.startswith(main.COMMENT_MARKER)) + self.assertIn("Commit Check failed", comment) + self.assertNotIn("All 60 checks", comment) + self.assertIn( + f"All 60 checks are in the [job summary]({self.RUN_URL}) of this run.", + comment, + ) + + def test_the_diff_goes_too_when_the_listing_was_not_enough(self): + results = self._failing_commits(2) + with ( + patch.dict(os.environ, self.RUN), + patch("main.JOB_SUMMARY_ENABLED", False), + ): + short = main.render_report(results, "short") + with patch("main.COMMENT_LIMIT", len(short) - 1): + comment = main.render_pr_comment(results) + self.assertNotIn("```diff", comment) + self.assertNotIn("How to fix", comment) + self.assertIn( + "The findings and how to fix them are in the " + f"[step log]({self.RUN_URL}) of this run.", + comment, + ) + self.assertIn("Commits: 2 of 2 failed", comment) + + def test_a_passing_headline_names_no_findings(self): + env = {k: v for k, v in os.environ.items() if k != "GITHUB_RUN_ID"} + with ( + patch.dict(os.environ, env, clear=True), + patch("main.JOB_SUMMARY_ENABLED", False), + ): + body = main.render_report([pass_scope("Branch")], "headline") + self.assertIn("All checks are in the step log of this run.", body) + + def test_length_is_counted_as_github_counts_it(self): + self.assertEqual(main._comment_length("a🤖"), 3) + + class TestUnparsableScopeInTheReport(unittest.TestCase): """A CLI response that is not JSON is a failure with no rule to link."""