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
8 changes: 5 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ repos:
- id: trailing-whitespace
- id: name-tests-test
- id: requirements-txt-fixer
- repo: https://github.com/psf/black-pre-commit-mirror
rev: 26.10.0
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.17.4
hooks:
- id: black
- id: ruff-check
args: [ --fix ]
- id: ruff-format
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v2.4.0
hooks:
Expand Down
31 changes: 13 additions & 18 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ def get_pr_event() -> dict[str, Any]:
with open(event_path, "r", encoding="utf-8") as f:
event = json.load(f)
return event.get("pull_request") or {}
except Exception as e:
except Exception as e: # noqa: BLE001 - a bad payload warns, never fails the step
print(f"::warning::Failed to read the PR from the event: {e}", file=sys.stderr)
return {}

Expand All @@ -302,8 +302,7 @@ def _rev_resolves(rev: str) -> bool:
try:
result = subprocess.run(
["git", "rev-parse", "--verify", "--quiet", f"{rev}^{{commit}}"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
capture_output=True,
encoding="utf-8",
check=False,
)
Expand All @@ -317,8 +316,7 @@ def _fetch_commit(sha: str) -> bool:
try:
result = subprocess.run(
["git", "fetch", "--no-tags", "--quiet", "--depth=1", "origin", sha],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
capture_output=True,
encoding="utf-8",
check=False,
)
Expand Down Expand Up @@ -392,7 +390,7 @@ def get_pr_title() -> str | None:
with open(event_path, "r", encoding="utf-8") as f:
event = json.load(f)
return event.get("pull_request", {}).get("title")
except Exception as e:
except Exception as e: # noqa: BLE001 - a bad payload warns, never fails the step
print(f"::warning::Failed to read PR title from event: {e}", file=sys.stderr)
return None

Expand Down Expand Up @@ -427,8 +425,7 @@ def _messages_in_range(revision_range: str) -> list[Commit]:
"""Commits in ``revision_range`` as ``(sha, message)``, oldest first, or ``[]``."""
result = subprocess.run(
["git", "log", COMMIT_LOG_FORMAT, "--reverse", revision_range],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
capture_output=True,
encoding="utf-8",
check=False,
)
Expand All @@ -446,8 +443,7 @@ def head_sha() -> str:
try:
result = subprocess.run(
["git", "rev-parse", "HEAD"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
capture_output=True,
encoding="utf-8",
check=False,
)
Expand Down Expand Up @@ -555,7 +551,7 @@ def get_pr_commit_messages() -> list[Commit]:
"the others were not checked",
)
return messages
except Exception as e:
except Exception as e: # noqa: BLE001 - warn, then check what we have
print(
f"::warning::Failed to retrieve PR commit messages: {e}",
file=sys.stderr,
Expand Down Expand Up @@ -602,8 +598,7 @@ def run_check_json(
result = subprocess.run(
command,
input=input_text,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
capture_output=True,
text=True,
encoding="utf-8",
check=False,
Expand Down Expand Up @@ -1414,7 +1409,7 @@ def _commit_check_version() -> str:
from importlib.metadata import version

return version("commit-check")
except Exception:
except Exception: # noqa: BLE001 - the version is decoration, never fail on it
return ""


Expand Down Expand Up @@ -1564,7 +1559,7 @@ def is_fork_pr() -> bool:
return bool(
head_full_name and base_full_name and head_full_name != base_full_name
)
except Exception:
except Exception: # noqa: BLE001 - an unreadable payload is not a fork PR
return False


Expand Down Expand Up @@ -1600,7 +1595,7 @@ def get_pr_number() -> int:
)
if number:
return int(number)
except Exception:
except Exception: # noqa: BLE001, S110 - fall through to the error below
pass
raise ValueError(
"Unable to determine PR number from GITHUB_REF or GITHUB_EVENT_PATH"
Expand All @@ -1611,7 +1606,7 @@ def _is_bot(comment: Any) -> bool:
"""Whether a comment was posted by a bot account rather than a person."""
try:
return comment.user.type == "Bot"
except Exception:
except Exception: # noqa: BLE001 - an unknown author is not a bot
return False


Expand Down Expand Up @@ -1737,7 +1732,7 @@ def add_pr_comments(results: list[ScopeResult]) -> int:
# comment is absent, and nothing says why.
print(f"::warning::Unable to post PR comment: {e}", file=sys.stderr)
return 0
except Exception as e:
except Exception as e: # noqa: BLE001 - the comment is best-effort
print(f"::warning::Unable to post PR comment: {e}", file=sys.stderr)
return 0

Expand Down
56 changes: 31 additions & 25 deletions main_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

os.environ.setdefault("GITHUB_STEP_SUMMARY", "/tmp/step_summary.txt")

import main # noqa: E402
import main

#: The report footer names the installed commit-check version, which differs
#: between a contributor's machine and CI. Golden tests pin it so they assert on
Expand Down Expand Up @@ -313,7 +313,7 @@ def test_invalid_json_returns_none(self):
class TestRunCheckJson(unittest.TestCase):
def test_parses_json_output(self):
mock_result = MagicMock(returncode=0, stdout=json_output(make_check("branch")))
with patch("main.subprocess.run", return_value=mock_result) as mock_run:
with patch("main.subprocess.run", return_value=mock_result):
rc, data, raw = main.run_check_json(["--branch"])
self.assertEqual(rc, 0)
self.assertEqual(data["status"], "pass")
Expand Down Expand Up @@ -757,7 +757,7 @@ def test_pr_title_check_runs_when_enabled(self):
) as mock_scope,
patch("main.run_other_checks", return_value=[]),
):
rc, results = main.run_commit_check()
rc, _results = main.run_commit_check()
self.assertEqual(rc, 0)
mock_scope.assert_called_once_with(
"PR title", ["--message"], input_text="feat: a feature"
Expand All @@ -775,7 +775,7 @@ def test_pr_title_failure_propagates(self):
patch("main.check_scope", return_value=fail_scope("PR title")),
patch("main.run_other_checks", return_value=[]),
):
rc, results = main.run_commit_check()
rc, _results = main.run_commit_check()
self.assertEqual(rc, 1)

def test_pr_title_skipped_outside_pr_context(self):
Expand All @@ -789,7 +789,7 @@ def test_pr_title_skipped_outside_pr_context(self):
patch("main.get_pr_title") as mock_title,
patch("main.run_other_checks", return_value=[]),
):
rc, results = main.run_commit_check()
rc, _results = main.run_commit_check()
self.assertEqual(rc, 0)
mock_title.assert_not_called()

Expand Down Expand Up @@ -825,7 +825,7 @@ def test_non_pr_message_check_uses_commit_message_scope(self):
) as mock_scope,
patch("main.run_other_checks", return_value=[]),
):
rc, results = main.run_commit_check()
rc, _results = main.run_commit_check()
self.assertEqual(rc, 0)
mock_pr.assert_not_called()
# HEAD is the commit that was checked, so the scope names it too.
Expand Down Expand Up @@ -882,7 +882,7 @@ def test_pr_without_enumerable_commits_warns_and_checks_head(self):
) as mock_scope,
patch("main.run_other_checks", return_value=[]),
):
rc, results, output = self._run_capturing_stdout()
rc, _results, output = self._run_capturing_stdout()
self.assertEqual(rc, 0)
self.assertIn(self.SHALLOW_PR_WARNING, output)
mock_scope.assert_called_once_with("Commit message", ["--message"], sha="")
Expand Down Expand Up @@ -1162,9 +1162,7 @@ def _run(self, total, api_commits, head="b2"):
for sha, msg in api_commits
]
github_module = MagicMock()
github_module.Github.return_value.get_repo.return_value.get_pull.return_value = (
pull
)
github_module.Github.return_value.get_repo.return_value.get_pull.return_value = pull
env = {
"GITHUB_TOKEN": "token",
"GITHUB_REPOSITORY": "owner/repo",
Expand Down Expand Up @@ -1554,7 +1552,7 @@ def test_annotation_payload_is_escaped(self):
],
)
output = self._run([scope])
annotation = [ln for ln in output.splitlines() if ln.startswith("::error")][0]
annotation = next(ln for ln in output.splitlines() if ln.startswith("::error"))
self.assertNotIn("\n", annotation.removeprefix("::error "))
self.assertIn("first line", annotation)

Expand Down Expand Up @@ -3391,17 +3389,19 @@ def test_the_two_streams_are_read_separately(self):
mock_result = MagicMock(returncode=0, stdout="{}", stderr="")
with patch("main.subprocess.run", return_value=mock_result) as mock_run:
main.run_check_json(["--branch"])
self.assertIs(mock_run.call_args[1]["stderr"], main.subprocess.PIPE)
self.assertIs(mock_run.call_args[1]["capture_output"], True)

def test_a_notice_does_not_break_the_json(self):
mock_result = MagicMock(
returncode=0,
stdout=json_output(make_check("branch")),
stderr=self.NOTICE,
)
with patch("main.subprocess.run", return_value=mock_result):
with patch.object(sys, "stderr", io.StringIO()):
rc, data, raw = main.run_check_json(["--branch"])
with (
patch("main.subprocess.run", return_value=mock_result),
patch.object(sys, "stderr", io.StringIO()),
):
rc, data, _raw = main.run_check_json(["--branch"])
self.assertEqual(rc, 0)
self.assertIsNotNone(data)
self.assertEqual(data["status"], "pass")
Expand All @@ -3410,9 +3410,11 @@ def test_a_notice_does_not_break_the_json(self):

def test_a_notice_alone_does_not_fail_the_scope(self):
mock_result = MagicMock(returncode=0, stdout=json_output(), stderr=self.NOTICE)
with patch("main.subprocess.run", return_value=mock_result):
with patch.object(sys, "stderr", io.StringIO()):
scope = main.check_scope("Branch", ["--branch"])
with (
patch("main.subprocess.run", return_value=mock_result),
patch.object(sys, "stderr", io.StringIO()),
):
scope = main.check_scope("Branch", ["--branch"])
self.assertEqual(scope.status, "pass")
self.assertEqual(scope.raw_text, "")

Expand All @@ -3421,10 +3423,12 @@ def test_notices_reach_the_job_log_once(self):
returncode=0, stdout=json_output(make_check("branch")), stderr=self.NOTICE
)
fake_err = io.StringIO()
with patch("main.subprocess.run", return_value=mock_result):
with patch.object(sys, "stderr", fake_err):
for _ in range(3):
main.run_check_json(["--branch"])
with (
patch("main.subprocess.run", return_value=mock_result),
patch.object(sys, "stderr", fake_err),
):
for _ in range(3):
main.run_check_json(["--branch"])
printed = fake_err.getvalue()
self.assertIn("--branch requested but no branch rules are configured", printed)
# Once, not once per scope and per commit in the pull request.
Expand All @@ -3434,9 +3438,11 @@ def test_unparsable_output_keeps_both_streams(self):
mock_result = MagicMock(
returncode=2, stdout="", stderr="Error: cchk.toml: Expected ']'\n"
)
with patch("main.subprocess.run", return_value=mock_result):
with patch.object(sys, "stderr", io.StringIO()):
scope = main.check_scope("Branch", ["--branch"])
with (
patch("main.subprocess.run", return_value=mock_result),
patch.object(sys, "stderr", io.StringIO()),
):
scope = main.check_scope("Branch", ["--branch"])
self.assertEqual(scope.status, "fail")
self.assertIn("Expected ']'", scope.raw_text)

Expand Down
Loading