From b15dfd789a77e59e92655e33288054dedb0ff2bc Mon Sep 17 00:00:00 2001 From: shenxianpeng Date: Fri, 29 May 2026 09:08:36 +0300 Subject: [PATCH] feat: add structured compliance policy report to job summary Replace the bare pass/fail job summary with a structured 'Commit Check Policy Report' that shows per-check results as a Markdown table with pass/fail indicators. The report leverages commit-check's --format json output to determine per-check pass/fail status without changing the existing validation flow or PR comment behavior. Key changes: - Add CheckResult dataclass and report-building functions - On pass: shortcuts to all-pass results (no extra work) - On failure: runs per-check JSON lookups to identify which exact checks failed - Raw output preserved in a collapsible
section - Backward compatible: run_commit_check() and add_pr_comments() are unchanged --- main.py | 151 +++++++++++++++++++++++++++++++++- main_test.py | 225 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 370 insertions(+), 6 deletions(-) diff --git a/main.py b/main.py index 7f0f1e2..99ebf47 100755 --- a/main.py +++ b/main.py @@ -4,6 +4,7 @@ import re import subprocess import sys +from dataclasses import dataclass from typing import TextIO # Constants for message titles @@ -12,6 +13,14 @@ COMMIT_MESSAGE_DELIMITER = "\x00" COMMIT_SECTION_SEPARATOR = "\n---\n" +# Map CLI flags to human-readable check labels for the policy report +CHECK_LABEL_MAP = { + "--message": "Commit message", + "--branch": "Branch naming", + "--author-name": "Author name", + "--author-email": "Author email", +} + # Environment variables MESSAGE = os.getenv("MESSAGE", "false") BRANCH = os.getenv("BRANCH", "false") @@ -232,17 +241,155 @@ def build_result_body(result_text: str | None) -> str: return f"{FAILURE_TITLE}\n```\n{result_text}\n```" +@dataclass +class CheckResult: + """Per-check result for the policy report.""" + + label: str + passed: bool + + +def run_check_json(flag: str, input_text: str | None = None) -> tuple[int, dict]: + """Run commit-check with --format json and return (returncode, parsed_json).""" + command = ["commit-check", flag, "--format", "json", "--no-banner"] + result = subprocess.run( + command, + input=input_text, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + check=False, + ) + try: + data = ( + json.loads(result.stdout) + if result.stdout.strip() + else {"status": "pass", "checks": []} + ) + except json.JSONDecodeError: + data = {"status": "fail", "checks": []} + return result.returncode, data + + +def get_policy_file() -> str: + """Detect which commit-check config file exists in the repository.""" + for filename in ["commit-check.toml", "cchk.toml", ".commit-check.yml"]: + if os.path.exists(filename): + return filename + return "commit-check.toml (default)" + + +def _run_all_message_checks(pr_messages: list[str]) -> bool: + """Check all PR commit messages individually via JSON. + + Returns True if all messages pass, False if any fails. + """ + for msg in pr_messages: + rc, _ = run_check_json("--message", input_text=msg) + if rc != 0: + return False + return True + + +def determine_check_results() -> list[CheckResult]: + """Run each enabled check with --format json to determine per-check pass/fail.""" + results: list[CheckResult] = [] + + if MESSAGE_ENABLED: + pr_messages = get_pr_commit_messages() + if pr_messages: + passed = _run_all_message_checks(pr_messages) + else: + rc, _ = run_check_json("--message") + passed = rc == 0 + results.append(CheckResult("Commit message", passed)) + + if BRANCH_ENABLED: + rc, _ = run_check_json("--branch") + results.append(CheckResult("Branch naming", rc == 0)) + + if AUTHOR_NAME_ENABLED: + rc, _ = run_check_json("--author-name") + results.append(CheckResult("Author name", rc == 0)) + + if AUTHOR_EMAIL_ENABLED: + rc, _ = run_check_json("--author-email") + results.append(CheckResult("Author email", rc == 0)) + + return results + + +def build_pass_results() -> list[CheckResult]: + """Build all-pass results for enabled checks (shortcut for success case).""" + results: list[CheckResult] = [] + if MESSAGE_ENABLED: + results.append(CheckResult("Commit message", True)) + if BRANCH_ENABLED: + results.append(CheckResult("Branch naming", True)) + if AUTHOR_NAME_ENABLED: + results.append(CheckResult("Author name", True)) + if AUTHOR_EMAIL_ENABLED: + results.append(CheckResult("Author email", True)) + return results + + +def build_policy_report(results: list[CheckResult]) -> str: + """Build a structured compliance report in Markdown format.""" + repo = os.getenv("GITHUB_REPOSITORY", "unknown") + policy_file = get_policy_file() + event = os.getenv("GITHUB_EVENT_NAME", "unknown") + + lines = [ + "# Commit Check Policy Report", + "", + f"**Repository**: `{repo}` ", + f"**Policy file**: `{policy_file}` ", + f"**Trigger**: `{event}` ", + "", + "| Check | Result |", + "|---|---|", + ] + + all_passed = True + for r in results: + status = "✅ Pass" if r.passed else "❌ Fail" + if not r.passed: + all_passed = False + lines.append(f"| {r.label} | {status} |") + + lines.append("") + if all_passed: + lines.append("### ✅ All checks passed") + else: + lines.append("### ❌ Some checks failed") + + return "\n".join(lines) + + def add_job_summary() -> int: """Adds the commit check result to the GitHub job summary.""" if not JOB_SUMMARY_ENABLED: return 0 result_text = read_result_file() + overall_pass = result_text is None + + if overall_pass: + report_results = build_pass_results() + else: + report_results = determine_check_results() + + report = build_policy_report(report_results) with open(GITHUB_STEP_SUMMARY, "a") as summary_file: - summary_file.write(build_result_body(result_text)) + summary_file.write(report) + if result_text: + summary_file.write( + "\n\n
\nDetails\n\n```\n" + f"{result_text}\n```\n
\n" + ) - return 0 if result_text is None else 1 + return 0 if overall_pass else 1 def is_fork_pr() -> bool: diff --git a/main_test.py b/main_test.py index 3a5d04c..e876dab 100644 --- a/main_test.py +++ b/main_test.py @@ -439,34 +439,251 @@ def test_false_skips(self): rc = main.add_job_summary() self.assertEqual(rc, 0) - def test_success_writes_success_title(self): + def test_success_writes_policy_report(self): summary_path = os.path.join(self._tmpdir, "summary.txt") with ( patch("main.JOB_SUMMARY_ENABLED", True), patch("main.GITHUB_STEP_SUMMARY", summary_path), patch("main.read_result_file", return_value=None), + patch("main.MESSAGE_ENABLED", True), + patch("main.BRANCH_ENABLED", True), ): rc = main.add_job_summary() self.assertEqual(rc, 0) with open(summary_path, encoding="utf-8") as file_obj: content = file_obj.read() - self.assertIn(main.SUCCESS_TITLE, content) + self.assertIn("# Commit Check Policy Report", content) + self.assertIn("Commit message | ✅ Pass", content) + self.assertIn("Branch naming | ✅ Pass", content) + self.assertIn("### ✅ All checks passed", content) - def test_failure_writes_failure_title(self): + def test_failure_writes_policy_report(self): summary_path = os.path.join(self._tmpdir, "summary.txt") with ( patch("main.JOB_SUMMARY_ENABLED", True), patch("main.GITHUB_STEP_SUMMARY", summary_path), patch("main.read_result_file", return_value="bad commit message"), + patch("main.MESSAGE_ENABLED", True), + patch("main.BRANCH_ENABLED", True), + patch("main.determine_check_results") as mock_determine, ): + from dataclasses import dataclass + + mock_determine.return_value = [ + main.CheckResult("Commit message", False), + main.CheckResult("Branch naming", True), + ] rc = main.add_job_summary() self.assertEqual(rc, 1) with open(summary_path, encoding="utf-8") as file_obj: content = file_obj.read() - self.assertIn(main.FAILURE_TITLE, content) + self.assertIn("# Commit Check Policy Report", content) + self.assertIn("Commit message | ❌ Fail", content) + self.assertIn("Branch naming | ✅ Pass", content) + self.assertIn("### ❌ Some checks failed", content) self.assertIn("bad commit message", content) +class TestRunCheckJson(unittest.TestCase): + def test_pass_returns_empty_checks(self): + mock_result = MagicMock(returncode=0, stdout='{"status": "pass", "checks": []}') + with patch("main.subprocess.run", return_value=mock_result) as mock_run: + rc, data = main.run_check_json("--message") + self.assertEqual(rc, 0) + self.assertEqual(data["status"], "pass") + call_args = mock_run.call_args[0][0] + self.assertIn("--format", call_args) + self.assertIn("json", call_args) + self.assertIn("--no-banner", call_args) + + def test_fail_returns_failed_checks(self): + mock_result = MagicMock( + returncode=1, + stdout='{"status": "fail", "checks": [{"check": "message", "status": "fail", "value": "bad", "error": "Bad msg", "suggest": "Fix it"}]}', + ) + with patch("main.subprocess.run", return_value=mock_result): + rc, data = main.run_check_json("--message") + self.assertEqual(rc, 1) + self.assertEqual(data["status"], "fail") + self.assertEqual(len(data["checks"]), 1) + + def test_passes_input_text_to_stdin(self): + mock_result = MagicMock(returncode=0, stdout="{}") + with patch("main.subprocess.run", return_value=mock_result) as mock_run: + main.run_check_json("--message", input_text="fix: test") + self.assertEqual(mock_run.call_args[1]["input"], "fix: test") + + def test_empty_stdout_treated_as_pass(self): + mock_result = MagicMock(returncode=0, stdout="") + with patch("main.subprocess.run", return_value=mock_result): + rc, data = main.run_check_json("--branch") + self.assertEqual(rc, 0) + self.assertEqual(data["status"], "pass") + + def test_invalid_json_treated_as_fail(self): + mock_result = MagicMock(returncode=1, stdout="not json") + with patch("main.subprocess.run", return_value=mock_result): + rc, data = main.run_check_json("--branch") + self.assertEqual(rc, 1) + self.assertEqual(data["status"], "fail") + + +class TestGetPolicyFile(unittest.TestCase): + def setUp(self): + import tempfile + + self._orig_dir = os.getcwd() + self._tmpdir = tempfile.mkdtemp() + os.chdir(self._tmpdir) + + def tearDown(self): + os.chdir(self._orig_dir) + + def test_finds_commit_check_toml(self): + with open("commit-check.toml", "w") as f: + f.write("[commit]\n") + self.assertEqual(main.get_policy_file(), "commit-check.toml") + + def test_finds_cchk_toml(self): + with open("cchk.toml", "w") as f: + f.write("[commit]\n") + self.assertEqual(main.get_policy_file(), "cchk.toml") + + def test_prefers_commit_check_toml_over_cchk(self): + with open("commit-check.toml", "w") as f: + f.write("[commit]\n") + with open("cchk.toml", "w") as f: + f.write("[commit]\n") + self.assertEqual(main.get_policy_file(), "commit-check.toml") + + def test_returns_default_when_no_config(self): + self.assertEqual(main.get_policy_file(), "commit-check.toml (default)") + + +class TestBuildPolicyReport(unittest.TestCase): + def test_all_pass_report(self): + results = [ + main.CheckResult("Commit message", True), + main.CheckResult("Branch naming", True), + ] + report = main.build_policy_report(results) + self.assertIn("# Commit Check Policy Report", report) + self.assertIn("Commit message | ✅ Pass", report) + self.assertIn("Branch naming | ✅ Pass", report) + self.assertIn("### ✅ All checks passed", report) + self.assertNotIn("❌", report) + + def test_mixed_result_report(self): + results = [ + main.CheckResult("Commit message", False), + main.CheckResult("Branch naming", True), + ] + report = main.build_policy_report(results) + self.assertIn("Commit message | ❌ Fail", report) + self.assertIn("Branch naming | ✅ Pass", report) + self.assertIn("### ❌ Some checks failed", report) + + def test_includes_repository_and_policy_file(self): + with patch.dict( + os.environ, + {"GITHUB_REPOSITORY": "owner/repo", "GITHUB_EVENT_NAME": "push"}, + ): + results = [main.CheckResult("Commit message", True)] + report = main.build_policy_report(results) + self.assertIn("`owner/repo`", report) + self.assertIn("`push`", report) + + def test_empty_results_shows_all_pass(self): + report = main.build_policy_report([]) + self.assertIn("### ✅ All checks passed", report) + + +class TestBuildPassResults(unittest.TestCase): + def test_all_disabled_returns_empty(self): + with ( + patch("main.MESSAGE_ENABLED", False), + patch("main.BRANCH_ENABLED", False), + patch("main.AUTHOR_NAME_ENABLED", False), + patch("main.AUTHOR_EMAIL_ENABLED", False), + ): + results = main.build_pass_results() + self.assertEqual(results, []) + + def test_message_and_branch_enabled(self): + with ( + patch("main.MESSAGE_ENABLED", True), + patch("main.BRANCH_ENABLED", True), + patch("main.AUTHOR_NAME_ENABLED", False), + patch("main.AUTHOR_EMAIL_ENABLED", False), + ): + results = main.build_pass_results() + self.assertEqual(len(results), 2) + self.assertTrue(all(r.passed for r in results)) + labels = [r.label for r in results] + self.assertIn("Commit message", labels) + self.assertIn("Branch naming", labels) + + +class TestDetermineCheckResults(unittest.TestCase): + def test_all_checks_pass(self): + with ( + patch("main.MESSAGE_ENABLED", True), + patch("main.BRANCH_ENABLED", True), + patch("main.AUTHOR_NAME_ENABLED", False), + patch("main.AUTHOR_EMAIL_ENABLED", False), + patch("main.get_pr_commit_messages", return_value=[]), + patch("main.run_check_json", return_value=(0, {"status": "pass"})), + ): + results = main.determine_check_results() + self.assertEqual(len(results), 2) + self.assertTrue(all(r.passed for r in results)) + + def test_message_fails_branch_passes(self): + call_count = [0] + + def mock_run_json(flag, input_text=None): + call_count[0] += 1 + if flag == "--message": + return 1, {"status": "fail"} + return 0, {"status": "pass"} + + with ( + patch("main.MESSAGE_ENABLED", True), + patch("main.BRANCH_ENABLED", True), + patch("main.AUTHOR_NAME_ENABLED", False), + patch("main.AUTHOR_EMAIL_ENABLED", False), + patch("main.get_pr_commit_messages", return_value=[]), + patch("main.run_check_json", side_effect=mock_run_json), + ): + results = main.determine_check_results() + self.assertEqual(len(results), 2) + self.assertFalse(results[0].passed) # message + self.assertTrue(results[1].passed) # branch + + def test_pr_context_checks_all_messages(self): + with ( + patch("main.MESSAGE_ENABLED", True), + patch("main.BRANCH_ENABLED", False), + patch("main.AUTHOR_NAME_ENABLED", False), + patch("main.AUTHOR_EMAIL_ENABLED", False), + patch( + "main.get_pr_commit_messages", + return_value=["fix: one", "bad two", "feat: three"], + ), + patch("main.run_check_json") as mock_run, + ): + # Second message fails + mock_run.side_effect = [ + (0, {"status": "pass"}), # fix: one + (1, {"status": "fail"}), # bad two <-- fails here, stops + ] + results = main.determine_check_results() + # Should have stopped after first failure + self.assertEqual(len(results), 1) + self.assertFalse(results[0].passed) + self.assertEqual(mock_run.call_count, 2) + + class TestIsForkPr(unittest.TestCase): def test_no_event_path(self): with patch.dict(os.environ, {}, clear=True):