Skip to content

Commit 7995ca8

Browse files
committed
chore: replace black with ruff in pre-commit
Use ruff-check (with --fix) and ruff-format, the same hooks the commit-check CLI repo runs, and make the code pass ruff's default rules: - subprocess.run uses capture_output=True instead of stdout/stderr PIPE - _commit_check_version catches only PackageNotFoundError - deliberate best-effort `except Exception` blocks say why with noqa - tests: merge nested with blocks, drop unused names and an unused noqa
1 parent 194cdd3 commit 7995ca8

3 files changed

Lines changed: 51 additions & 48 deletions

File tree

‎.pre-commit-config.yaml‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@ repos:
1515
- id: trailing-whitespace
1616
- id: name-tests-test
1717
- id: requirements-txt-fixer
18-
- repo: https://github.com/psf/black-pre-commit-mirror
19-
rev: 26.10.0
18+
- repo: https://github.com/astral-sh/ruff-pre-commit
19+
rev: v0.17.4
2020
hooks:
21-
- id: black
21+
- id: ruff-check
22+
args: [ --fix ]
23+
- id: ruff-format
2224
- repo: https://github.com/pre-commit/mirrors-mypy
2325
rev: v2.4.0
2426
hooks:

‎main.py‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ def get_pr_event() -> dict[str, Any]:
282282
with open(event_path, "r", encoding="utf-8") as f:
283283
event = json.load(f)
284284
return event.get("pull_request") or {}
285-
except Exception as e:
285+
except Exception as e: # noqa: BLE001 - a bad payload warns, never fails the step
286286
print(f"::warning::Failed to read the PR from the event: {e}", file=sys.stderr)
287287
return {}
288288

@@ -302,8 +302,7 @@ def _rev_resolves(rev: str) -> bool:
302302
try:
303303
result = subprocess.run(
304304
["git", "rev-parse", "--verify", "--quiet", f"{rev}^{{commit}}"],
305-
stdout=subprocess.PIPE,
306-
stderr=subprocess.PIPE,
305+
capture_output=True,
307306
encoding="utf-8",
308307
check=False,
309308
)
@@ -317,8 +316,7 @@ def _fetch_commit(sha: str) -> bool:
317316
try:
318317
result = subprocess.run(
319318
["git", "fetch", "--no-tags", "--quiet", "--depth=1", "origin", sha],
320-
stdout=subprocess.PIPE,
321-
stderr=subprocess.PIPE,
319+
capture_output=True,
322320
encoding="utf-8",
323321
check=False,
324322
)
@@ -392,7 +390,7 @@ def get_pr_title() -> str | None:
392390
with open(event_path, "r", encoding="utf-8") as f:
393391
event = json.load(f)
394392
return event.get("pull_request", {}).get("title")
395-
except Exception as e:
393+
except Exception as e: # noqa: BLE001 - a bad payload warns, never fails the step
396394
print(f"::warning::Failed to read PR title from event: {e}", file=sys.stderr)
397395
return None
398396

@@ -427,8 +425,7 @@ def _messages_in_range(revision_range: str) -> list[Commit]:
427425
"""Commits in ``revision_range`` as ``(sha, message)``, oldest first, or ``[]``."""
428426
result = subprocess.run(
429427
["git", "log", COMMIT_LOG_FORMAT, "--reverse", revision_range],
430-
stdout=subprocess.PIPE,
431-
stderr=subprocess.PIPE,
428+
capture_output=True,
432429
encoding="utf-8",
433430
check=False,
434431
)
@@ -446,8 +443,7 @@ def head_sha() -> str:
446443
try:
447444
result = subprocess.run(
448445
["git", "rev-parse", "HEAD"],
449-
stdout=subprocess.PIPE,
450-
stderr=subprocess.PIPE,
446+
capture_output=True,
451447
encoding="utf-8",
452448
check=False,
453449
)
@@ -555,7 +551,7 @@ def get_pr_commit_messages() -> list[Commit]:
555551
"the others were not checked",
556552
)
557553
return messages
558-
except Exception as e:
554+
except Exception as e: # noqa: BLE001 - warn, then check what we have
559555
print(
560556
f"::warning::Failed to retrieve PR commit messages: {e}",
561557
file=sys.stderr,
@@ -602,8 +598,7 @@ def run_check_json(
602598
result = subprocess.run(
603599
command,
604600
input=input_text,
605-
stdout=subprocess.PIPE,
606-
stderr=subprocess.PIPE,
601+
capture_output=True,
607602
text=True,
608603
encoding="utf-8",
609604
check=False,
@@ -1410,11 +1405,11 @@ def _scope_value(scope: ScopeResult, max_len: int = 60) -> str:
14101405

14111406
def _commit_check_version() -> str:
14121407
"""Version of the commit-check CLI that produced these results."""
1413-
try:
1414-
from importlib.metadata import version
1408+
from importlib.metadata import PackageNotFoundError, version
14151409

1410+
try:
14161411
return version("commit-check")
1417-
except Exception:
1412+
except PackageNotFoundError:
14181413
return ""
14191414

14201415

@@ -1564,7 +1559,7 @@ def is_fork_pr() -> bool:
15641559
return bool(
15651560
head_full_name and base_full_name and head_full_name != base_full_name
15661561
)
1567-
except Exception:
1562+
except Exception: # noqa: BLE001 - an unreadable payload is not a fork PR
15681563
return False
15691564

15701565

@@ -1600,7 +1595,7 @@ def get_pr_number() -> int:
16001595
)
16011596
if number:
16021597
return int(number)
1603-
except Exception:
1598+
except Exception: # noqa: BLE001, S110 - fall through to the error below
16041599
pass
16051600
raise ValueError(
16061601
"Unable to determine PR number from GITHUB_REF or GITHUB_EVENT_PATH"
@@ -1611,7 +1606,7 @@ def _is_bot(comment: Any) -> bool:
16111606
"""Whether a comment was posted by a bot account rather than a person."""
16121607
try:
16131608
return comment.user.type == "Bot"
1614-
except Exception:
1609+
except Exception: # noqa: BLE001 - an unknown author is not a bot
16151610
return False
16161611

16171612

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

‎main_test.py‎

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

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

17-
import main # noqa: E402
17+
import main
1818

1919
#: The report footer names the installed commit-check version, which differs
2020
#: between a contributor's machine and CI. Golden tests pin it so they assert on
@@ -313,7 +313,7 @@ def test_invalid_json_returns_none(self):
313313
class TestRunCheckJson(unittest.TestCase):
314314
def test_parses_json_output(self):
315315
mock_result = MagicMock(returncode=0, stdout=json_output(make_check("branch")))
316-
with patch("main.subprocess.run", return_value=mock_result) as mock_run:
316+
with patch("main.subprocess.run", return_value=mock_result):
317317
rc, data, raw = main.run_check_json(["--branch"])
318318
self.assertEqual(rc, 0)
319319
self.assertEqual(data["status"], "pass")
@@ -757,7 +757,7 @@ def test_pr_title_check_runs_when_enabled(self):
757757
) as mock_scope,
758758
patch("main.run_other_checks", return_value=[]),
759759
):
760-
rc, results = main.run_commit_check()
760+
rc, _results = main.run_commit_check()
761761
self.assertEqual(rc, 0)
762762
mock_scope.assert_called_once_with(
763763
"PR title", ["--message"], input_text="feat: a feature"
@@ -775,7 +775,7 @@ def test_pr_title_failure_propagates(self):
775775
patch("main.check_scope", return_value=fail_scope("PR title")),
776776
patch("main.run_other_checks", return_value=[]),
777777
):
778-
rc, results = main.run_commit_check()
778+
rc, _results = main.run_commit_check()
779779
self.assertEqual(rc, 1)
780780

781781
def test_pr_title_skipped_outside_pr_context(self):
@@ -789,7 +789,7 @@ def test_pr_title_skipped_outside_pr_context(self):
789789
patch("main.get_pr_title") as mock_title,
790790
patch("main.run_other_checks", return_value=[]),
791791
):
792-
rc, results = main.run_commit_check()
792+
rc, _results = main.run_commit_check()
793793
self.assertEqual(rc, 0)
794794
mock_title.assert_not_called()
795795

@@ -825,7 +825,7 @@ def test_non_pr_message_check_uses_commit_message_scope(self):
825825
) as mock_scope,
826826
patch("main.run_other_checks", return_value=[]),
827827
):
828-
rc, results = main.run_commit_check()
828+
rc, _results = main.run_commit_check()
829829
self.assertEqual(rc, 0)
830830
mock_pr.assert_not_called()
831831
# HEAD is the commit that was checked, so the scope names it too.
@@ -882,7 +882,7 @@ def test_pr_without_enumerable_commits_warns_and_checks_head(self):
882882
) as mock_scope,
883883
patch("main.run_other_checks", return_value=[]),
884884
):
885-
rc, results, output = self._run_capturing_stdout()
885+
rc, _results, output = self._run_capturing_stdout()
886886
self.assertEqual(rc, 0)
887887
self.assertIn(self.SHALLOW_PR_WARNING, output)
888888
mock_scope.assert_called_once_with("Commit message", ["--message"], sha="")
@@ -1162,9 +1162,7 @@ def _run(self, total, api_commits, head="b2"):
11621162
for sha, msg in api_commits
11631163
]
11641164
github_module = MagicMock()
1165-
github_module.Github.return_value.get_repo.return_value.get_pull.return_value = (
1166-
pull
1167-
)
1165+
github_module.Github.return_value.get_repo.return_value.get_pull.return_value = pull
11681166
env = {
11691167
"GITHUB_TOKEN": "token",
11701168
"GITHUB_REPOSITORY": "owner/repo",
@@ -1554,7 +1552,7 @@ def test_annotation_payload_is_escaped(self):
15541552
],
15551553
)
15561554
output = self._run([scope])
1557-
annotation = [ln for ln in output.splitlines() if ln.startswith("::error")][0]
1555+
annotation = next(ln for ln in output.splitlines() if ln.startswith("::error"))
15581556
self.assertNotIn("\n", annotation.removeprefix("::error "))
15591557
self.assertIn("first line", annotation)
15601558

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

33963394
def test_a_notice_does_not_break_the_json(self):
33973395
mock_result = MagicMock(
33983396
returncode=0,
33993397
stdout=json_output(make_check("branch")),
34003398
stderr=self.NOTICE,
34013399
)
3402-
with patch("main.subprocess.run", return_value=mock_result):
3403-
with patch.object(sys, "stderr", io.StringIO()):
3404-
rc, data, raw = main.run_check_json(["--branch"])
3400+
with (
3401+
patch("main.subprocess.run", return_value=mock_result),
3402+
patch.object(sys, "stderr", io.StringIO()),
3403+
):
3404+
rc, data, _raw = main.run_check_json(["--branch"])
34053405
self.assertEqual(rc, 0)
34063406
self.assertIsNotNone(data)
34073407
self.assertEqual(data["status"], "pass")
@@ -3410,9 +3410,11 @@ def test_a_notice_does_not_break_the_json(self):
34103410

34113411
def test_a_notice_alone_does_not_fail_the_scope(self):
34123412
mock_result = MagicMock(returncode=0, stdout=json_output(), stderr=self.NOTICE)
3413-
with patch("main.subprocess.run", return_value=mock_result):
3414-
with patch.object(sys, "stderr", io.StringIO()):
3415-
scope = main.check_scope("Branch", ["--branch"])
3413+
with (
3414+
patch("main.subprocess.run", return_value=mock_result),
3415+
patch.object(sys, "stderr", io.StringIO()),
3416+
):
3417+
scope = main.check_scope("Branch", ["--branch"])
34163418
self.assertEqual(scope.status, "pass")
34173419
self.assertEqual(scope.raw_text, "")
34183420

@@ -3421,10 +3423,12 @@ def test_notices_reach_the_job_log_once(self):
34213423
returncode=0, stdout=json_output(make_check("branch")), stderr=self.NOTICE
34223424
)
34233425
fake_err = io.StringIO()
3424-
with patch("main.subprocess.run", return_value=mock_result):
3425-
with patch.object(sys, "stderr", fake_err):
3426-
for _ in range(3):
3427-
main.run_check_json(["--branch"])
3426+
with (
3427+
patch("main.subprocess.run", return_value=mock_result),
3428+
patch.object(sys, "stderr", fake_err),
3429+
):
3430+
for _ in range(3):
3431+
main.run_check_json(["--branch"])
34283432
printed = fake_err.getvalue()
34293433
self.assertIn("--branch requested but no branch rules are configured", printed)
34303434
# Once, not once per scope and per commit in the pull request.
@@ -3434,9 +3438,11 @@ def test_unparsable_output_keeps_both_streams(self):
34343438
mock_result = MagicMock(
34353439
returncode=2, stdout="", stderr="Error: cchk.toml: Expected ']'\n"
34363440
)
3437-
with patch("main.subprocess.run", return_value=mock_result):
3438-
with patch.object(sys, "stderr", io.StringIO()):
3439-
scope = main.check_scope("Branch", ["--branch"])
3441+
with (
3442+
patch("main.subprocess.run", return_value=mock_result),
3443+
patch.object(sys, "stderr", io.StringIO()),
3444+
):
3445+
scope = main.check_scope("Branch", ["--branch"])
34403446
self.assertEqual(scope.status, "fail")
34413447
self.assertIn("Expected ']'", scope.raw_text)
34423448

0 commit comments

Comments
 (0)