diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 73aaa4f..95e7940 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -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: diff --git a/main.py b/main.py index 73c6b29..2843a76 100755 --- a/main.py +++ b/main.py @@ -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 {} @@ -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, ) @@ -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, ) @@ -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 @@ -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, ) @@ -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, ) @@ -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, @@ -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, @@ -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 "" @@ -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 @@ -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" @@ -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 @@ -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 diff --git a/main_test.py b/main_test.py index 8a5c507..8ab2f76 100644 --- a/main_test.py +++ b/main_test.py @@ -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 @@ -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") @@ -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" @@ -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): @@ -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() @@ -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. @@ -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="") @@ -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", @@ -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) @@ -3391,7 +3389,7 @@ 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( @@ -3399,9 +3397,11 @@ def test_a_notice_does_not_break_the_json(self): 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") @@ -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, "") @@ -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. @@ -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)