Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 50 additions & 6 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,19 @@ def status(self) -> str:
return "skip"
return "pass"

@property
def skip_reason(self) -> str:
"""Why a skipped scope did not run, or ``""`` when nobody said.

Newer commit-check releases name the reason on each skipped check (e.g.
"author dependabot[bot] is in [commit].ignore_authors"). A bare
"skipped" left the reader unable to tell a deliberate bypass from a
broken setup.
"""
if self.status != "skip":
return ""
return next((c["reason"] for c in self.checks if c.get("reason")), "")

@property
def failures(self) -> list[dict[str, str]]:
"""The checks that failed in this scope."""
Expand Down Expand Up @@ -362,6 +375,7 @@ def skipped_author_scope(flag: str) -> ScopeResult:
"error": "",
"suggest": "",
"docs_url": "",
"reason": "the pull request's head commit could not be resolved",
}
],
)
Expand Down Expand Up @@ -843,7 +857,8 @@ def _render_scopes(scopes: list[ScopeResult], include_docs: bool) -> list[str]:
if scope.status == "skip":
# Deliberately not a ✔. Nothing was validated here, and a tick
# claiming otherwise is what made a bypassed policy look enforced.
lines.append(f" ⊘ {label} (skipped)")
reason = scope.skip_reason
lines.append(f" ⊘ {label} (skipped{f': {reason}' if reason else ''})")
continue
if scope.status == "pass":
value = _scope_value(scope)
Expand Down Expand Up @@ -988,6 +1003,8 @@ def render_step_log(results: list[ScopeResult]) -> None:
)
if total and skipped == total:
print("\u2298 commit-check: all checks skipped, nothing was validated")
if _skip_reasons(results):
print(_skip_reasons(results))
elif warned or skipped:
passed = total - skipped - warned
tail = []
Expand All @@ -998,6 +1015,8 @@ def render_step_log(results: list[ScopeResult]) -> None:
print(
f"\u2714 commit-check: {passed} of {total} checks passed, {', '.join(tail)}"
)
if _skip_reasons(results):
print(_skip_reasons(results))
else:
print("\u2714 commit-check: all checks passed")

Expand Down Expand Up @@ -1031,6 +1050,22 @@ def _skip_count(results: list[ScopeResult]) -> int:
return sum(1 for scope in results if scope.status == "skip")


def _skip_reasons(results: list[ScopeResult]) -> str:
"""One sentence naming why checks were skipped, or ``""`` if unknown.

The verdict line is all most readers see, so it stays short: the config
section is dropped ("[commit].ignore_authors" and "[branch].ignore_authors"
both read "ignore_authors"), which folds the usual bot case into one
reason. The per-scope lines in the collapsed block keep the section.
"""
reasons = list(
dict.fromkeys(
re.sub(r"\[\w+\]\.", "", s.skip_reason) for s in results if s.skip_reason
)
)
return f"Skipped because {'; '.join(reasons)}." if reasons else ""


def _warn_count(results: list[ScopeResult]) -> int:
"""Number of scopes that carry at least one warning.

Expand Down Expand Up @@ -1211,17 +1246,22 @@ def _scope_value(scope: ScopeResult, max_len: int = 60) -> str:
#
# ⊘ **All 5 checks skipped** — nothing was validated
#
# Skipped because author dependabot[bot] is in ignore_authors.
#
# ```text
# Commit message
# ⊘ PR title (skipped)
# ⊘ Commit 1/1 (skipped)
# ⊘ PR title (skipped: author dependabot[bot] is in [commit].ignore_authors)
# ⊘ Commit 1/1 (skipped: author dependabot[bot] is in [commit].ignore_authors)
# Branch
# ⊘ Branch (skipped)
# ⊘ Branch (skipped: author dependabot[bot] is in [branch].ignore_authors)
# Author
# ⊘ Author name (skipped)
# ⊘ Author email (skipped)
# ⊘ Author name (skipped: author dependabot[bot] is in [commit].ignore_authors)
# ⊘ Author email (skipped: author dependabot[bot] is in [commit].ignore_authors)
# ```
#
# The reason comes from commit-check; an older CLI sends none, and the line
# reads "(skipped)" with no "Skipped because" sentence.
#
# A skipped scope deliberately carries no ✔ and no checked value: nothing was
# examined, so there is no value to report and no pass to claim. When only some
# scopes skip, the verdict reads "✅ **3 of 5 checks passed**, 2 skipped".
Expand Down Expand Up @@ -1387,6 +1427,8 @@ def render_report(results: list[ScopeResult]) -> str:
# checks passed" here is the defect this branch exists to prevent.
lines.append(f"⊘ **All {total} {unit} skipped** — nothing was validated")
lines.append("")
if _skip_reasons(results):
lines.extend([_skip_reasons(results), ""])
elif warned or skipped:
passed = total - skipped - warned
tail = []
Expand All @@ -1396,6 +1438,8 @@ def render_report(results: list[ScopeResult]) -> str:
tail.append(f"{skipped} skipped")
lines.append(f"✅ **{passed} of {total} {unit} passed**, {', '.join(tail)}")
lines.append("")
if _skip_reasons(results):
lines.extend([_skip_reasons(results), ""])
if warned:
lines.extend([_markdown_table(results, "warn", "Warnings"), ""])
else:
Expand Down
67 changes: 61 additions & 6 deletions main_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1011,6 +1011,7 @@ def test_pr_author_checks_are_skipped_on_a_shallow_clone(self):
"error": "",
"suggest": "",
"docs_url": "",
"reason": "the pull request's head commit could not be resolved",
}
],
)
Expand Down Expand Up @@ -2693,12 +2694,12 @@ def test_a_comment_without_an_author_is_not_a_bot(self):
self.assertEqual(main._find_own_comments([orphan]), (None, []))


def skip_scope(label: str = "PR title") -> main.ScopeResult:
def skip_scope(label: str = "PR title", reason: str = "") -> main.ScopeResult:
"""A scope whose every rule declined to run (e.g. author in ignore_authors)."""
return main.ScopeResult(
label=label,
checks=[make_check("message", status="skip", rule_id="CC001", value="")],
)
check = make_check("message", status="skip", rule_id="CC001", value="")
if reason:
check["reason"] = reason
return main.ScopeResult(label=label, checks=[check])


class TestSkippedScopes(unittest.TestCase):
Expand Down Expand Up @@ -2765,6 +2766,57 @@ def test_skipped_scope_reports_no_checked_value(self):
body = main.render_report([skip_scope("PR title")])
self.assertIn(" ⊘ PR title (skipped)", body)

def test_skip_reason_is_shown_on_the_scope_and_under_the_verdict(self):
"""A skip says why, so a deliberate bypass reads as one."""
commit = "author dependabot[bot] is in [commit].ignore_authors"
branch = "author dependabot[bot] is in [branch].ignore_authors"
results = [
skip_scope("PR title", reason=commit),
skip_scope("Commit 1/1", reason=commit),
skip_scope("Branch", reason=branch),
]
body = main.render_report(results)
self.assertIn(
"⊘ **All 3 checks skipped** — nothing was validated\n\n"
"Skipped because author dependabot[bot] is in ignore_authors.\n",
body,
)
self.assertIn(f" ⊘ PR title (skipped: {commit})", body)
self.assertIn(f" ⊘ Branch (skipped: {branch})", body)

def test_skip_reason_follows_a_partial_skip_verdict(self):
reason = "author dependabot[bot] is in [branch].ignore_authors"
results = [pass_scope("PR title"), skip_scope("Branch", reason=reason)]
body = main.render_report(results)
self.assertIn(
"✅ **1 of 2 checks passed**, 1 skipped\n\n"
"Skipped because author dependabot[bot] is in ignore_authors.\n",
body,
)

def test_skip_reason_reaches_the_step_log(self):
reason = "author dependabot[bot] is in [commit].ignore_authors"
with patch("sys.stdout", new_callable=io.StringIO) as out:
main.render_step_log([skip_scope("PR title", reason=reason)])
self.assertIn(f" ⊘ PR title (skipped: {reason})", out.getvalue())
self.assertIn(
"Skipped because author dependabot[bot] is in ignore_authors.",
out.getvalue(),
)

def test_skip_reason_follows_a_partial_skip_in_the_step_log(self):
reason = "author dependabot[bot] is in [branch].ignore_authors"
results = [pass_scope("PR title"), skip_scope("Branch", reason=reason)]
with patch("sys.stdout", new_callable=io.StringIO) as out:
main.render_step_log(results)
self.assertIn(
"Skipped because author dependabot[bot] is in ignore_authors.",
out.getvalue(),
)

def test_a_passing_scope_has_no_skip_reason(self):
self.assertEqual(pass_scope("Branch", value="main").skip_reason, "")

def test_older_engine_without_skip_is_unaffected(self):
"""Back-compat: engines that only emit pass/fail render as before."""
results = [pass_scope("Branch", value="main")]
Expand Down Expand Up @@ -3171,6 +3223,8 @@ def test_step_log_all_skipped_claims_no_pass(self):
"fix",
"docs_url",
}
#: Keys newer CLIs add on top: ``reason`` says why a check was skipped.
OPTIONAL_CHECK_KEYS = {"reason"}
STATUSES = {"pass", "fail", "warn", "skip"}


Expand Down Expand Up @@ -3233,7 +3287,8 @@ def _run(self, message: str) -> tuple[int, dict]:
self.assertIn("checks", data)
self.assertTrue(data["checks"], "CLI reported no checks")
for check in data["checks"]:
self.assertEqual(set(check), CHECK_KEYS, check)
self.assertLessEqual(CHECK_KEYS, set(check), check)
self.assertLessEqual(set(check), CHECK_KEYS | OPTIONAL_CHECK_KEYS, check)
self.assertIn(check["status"], STATUSES, check)
self.assertRegex(check["rule_id"], r"^CC\d{3}$")
self.assertTrue(
Expand Down
Loading