From 040a81ba6f95c2ca7924ced1a98ec643a2907670 Mon Sep 17 00:00:00 2001 From: Mehdi ABAAKOUK Date: Wed, 8 Oct 2025 07:51:35 +0200 Subject: [PATCH] chore(scopes): split cli.py Change-Id: Ie5407966b13820fa4982004e2ecc983c0e5f5a83 --- mergify_cli/ci/scopes/base_detector.py | 99 ++++++++++++ mergify_cli/ci/scopes/changed_files.py | 23 +++ mergify_cli/ci/scopes/cli.py | 117 +------------- .../tests/ci/scopes/test_base_detector.py | 108 +++++++++++++ .../tests/ci/scopes/test_changed_files.py | 38 +++++ mergify_cli/tests/ci/scopes/test_cli.py | 147 +----------------- 6 files changed, 279 insertions(+), 253 deletions(-) create mode 100644 mergify_cli/ci/scopes/base_detector.py create mode 100644 mergify_cli/ci/scopes/changed_files.py create mode 100644 mergify_cli/tests/ci/scopes/test_base_detector.py create mode 100644 mergify_cli/tests/ci/scopes/test_changed_files.py diff --git a/mergify_cli/ci/scopes/base_detector.py b/mergify_cli/ci/scopes/base_detector.py new file mode 100644 index 00000000..d9bb58a9 --- /dev/null +++ b/mergify_cli/ci/scopes/base_detector.py @@ -0,0 +1,99 @@ +from __future__ import annotations + +import json +import os +import pathlib +import typing + +import click +import yaml + + +class MergeQueuePullRequest(typing.TypedDict): + number: int + + +class MergeQueueBatchFailed(typing.TypedDict): + draft_pr_number: int + checked_pull_request: list[int] + + +class MergeQueueMetadata(typing.TypedDict): + checking_base_sha: str + pull_requests: list[MergeQueuePullRequest] + previous_failed_batches: list[MergeQueueBatchFailed] + + +def _yaml_docs_from_fenced_blocks(body: str) -> MergeQueueMetadata | None: + lines = [] + found = False + for line in body.splitlines(): + if line.startswith("```yaml"): + found = True + elif found: + if line.startswith("```"): + break + lines.append(line) + if lines: + return typing.cast("MergeQueueMetadata", yaml.safe_load("\n".join(lines))) + return None + + +def _detect_base_from_merge_queue_payload(ev: dict[str, typing.Any]) -> str | None: + pr = ev.get("pull_request") + if not isinstance(pr, dict): + return None + title = pr.get("title") or "" + if not isinstance(title, str): + return None + if not title.startswith("merge-queue: "): + return None + body = pr.get("body") or "" + content = _yaml_docs_from_fenced_blocks(body) + if content: + return content["checking_base_sha"] + return None + + +def _detect_base_from_event(ev: dict[str, typing.Any]) -> str | None: + pr = ev.get("pull_request") + if isinstance(pr, dict): + sha = pr.get("base", {}).get("sha") + if isinstance(sha, str) and sha: + return sha + return None + + +def detect() -> str: + event_path = os.environ.get("GITHUB_EVENT_PATH") + event: dict[str, typing.Any] | None = None + if event_path and pathlib.Path(event_path).is_file(): + try: + with pathlib.Path(event_path).open("r", encoding="utf-8") as f: + event = json.load(f) + except FileNotFoundError: + event = None + + if event is not None: + # 0) merge-queue PR override + mq_sha = _detect_base_from_merge_queue_payload(event) + if mq_sha: + return mq_sha + + # 1) standard event payload + event_sha = _detect_base_from_event(event) + if event_sha: + return event_sha + + # 2) base ref (e.g., PR target branch) + base_ref = os.environ.get("GITHUB_BASE_REF") + if base_ref: + return base_ref + + msg = ( + "Could not detect base SHA. Ensure checkout has sufficient history " + "(e.g., actions/checkout with fetch-depth: 0) or provide GITHUB_EVENT_PATH / GITHUB_BASE_REF." + ) + raise click.ClickException( + msg, + ) diff --git a/mergify_cli/ci/scopes/changed_files.py b/mergify_cli/ci/scopes/changed_files.py new file mode 100644 index 00000000..b4a2e56d --- /dev/null +++ b/mergify_cli/ci/scopes/changed_files.py @@ -0,0 +1,23 @@ +from __future__ import annotations + +import subprocess + +import click + + +def _run(cmd: list[str]) -> str: + try: + return subprocess.check_output(cmd, text=True, encoding="utf-8").strip() + except subprocess.CalledProcessError as e: + msg = f"Command failed: {' '.join(cmd)}\n{e}" + raise click.ClickException(msg) from e + + +def git_changed_files(base: str) -> list[str]: + # Committed changes only between base_sha and HEAD. + # Includes: Added (A), Copied (C), Modified (M), Renamed (R), Type-changed (T), Deleted (D) + # Excludes: Unmerged (U), Unknown (X), Broken (B) + out = _run( + ["git", "diff", "--name-only", "--diff-filter=ACMRTD", f"{base}...HEAD"], + ) + return [line for line in out.splitlines() if line] diff --git a/mergify_cli/ci/scopes/cli.py b/mergify_cli/ci/scopes/cli.py index 5a4ab622..a5e1bfa5 100644 --- a/mergify_cli/ci/scopes/cli.py +++ b/mergify_cli/ci/scopes/cli.py @@ -1,15 +1,16 @@ from __future__ import annotations -import json import os import pathlib -import subprocess import typing import click import pydantic import yaml +from mergify_cli.ci.scopes import base_detector +from mergify_cli.ci.scopes import changed_files + if typing.TYPE_CHECKING: from collections import abc @@ -55,114 +56,6 @@ def from_yaml(cls, path: str) -> Config: return cls.from_dict(data) -def _run(cmd: list[str]) -> str: - try: - return subprocess.check_output(cmd, text=True, encoding="utf-8").strip() - except subprocess.CalledProcessError as e: - msg = f"Command failed: {' '.join(cmd)}\n{e}" - raise click.ClickException(msg) from e - - -class MergeQueuePullRequest(typing.TypedDict): - number: int - - -class MergeQueueBatchFailed(typing.TypedDict): - draft_pr_number: int - checked_pull_request: list[int] - - -class MergeQueueMetadata(typing.TypedDict): - checking_base_sha: str - pull_requests: list[MergeQueuePullRequest] - previous_failed_batches: list[MergeQueueBatchFailed] - - -def _yaml_docs_from_fenced_blocks(body: str) -> MergeQueueMetadata | None: - lines = [] - found = False - for line in body.splitlines(): - if line.startswith("```yaml"): - found = True - elif found: - if line.startswith("```"): - break - lines.append(line) - if lines: - return typing.cast("MergeQueueMetadata", yaml.safe_load("\n".join(lines))) - return None - - -def _detect_base_from_merge_queue_payload(ev: dict[str, typing.Any]) -> str | None: - pr = ev.get("pull_request") - if not isinstance(pr, dict): - return None - title = pr.get("title") or "" - if not isinstance(title, str): - return None - if not title.startswith("merge-queue: "): - return None - body = pr.get("body") or "" - content = _yaml_docs_from_fenced_blocks(body) - if content: - return content["checking_base_sha"] - return None - - -def _detect_base_from_event(ev: dict[str, typing.Any]) -> str | None: - pr = ev.get("pull_request") - if isinstance(pr, dict): - sha = pr.get("base", {}).get("sha") - if isinstance(sha, str) and sha: - return sha - return None - - -def detect_base() -> str: - event_path = os.environ.get("GITHUB_EVENT_PATH") - event: dict[str, typing.Any] | None = None - if event_path and pathlib.Path(event_path).is_file(): - try: - with pathlib.Path(event_path).open("r", encoding="utf-8") as f: - event = json.load(f) - except FileNotFoundError: - event = None - - if event is not None: - # 0) merge-queue PR override - mq_sha = _detect_base_from_merge_queue_payload(event) - if mq_sha: - return mq_sha - - # 1) standard event payload - event_sha = _detect_base_from_event(event) - if event_sha: - return event_sha - - # 2) base ref (e.g., PR target branch) - base_ref = os.environ.get("GITHUB_BASE_REF") - if base_ref: - return base_ref - - msg = ( - "Could not detect base SHA. Ensure checkout has sufficient history " - "(e.g., actions/checkout with fetch-depth: 0) or provide GITHUB_EVENT_PATH / GITHUB_BASE_REF." - ) - raise click.ClickException( - msg, - ) - - -def git_changed_files(base: str) -> list[str]: - # Committed changes only between base_sha and HEAD. - # Includes: Added (A), Copied (C), Modified (M), Renamed (R), Type-changed (T), Deleted (D) - # Excludes: Unmerged (U), Unknown (X), Broken (B) - out = _run( - ["git", "diff", "--name-only", "--diff-filter=ACMRTD", f"{base}...HEAD"], - ) - return [line for line in out.splitlines() if line] - - def match_scopes( config: Config, files: abc.Iterable[str], @@ -210,8 +103,8 @@ def maybe_write_github_outputs( def detect(config_path: str) -> None: cfg = Config.from_yaml(config_path) - base = detect_base() - changed = git_changed_files(base) + base = base_detector.detect() + changed = changed_files.git_changed_files(base) scopes_hit, per_scope = match_scopes(cfg, changed) click.echo(f"Base: {base}") diff --git a/mergify_cli/tests/ci/scopes/test_base_detector.py b/mergify_cli/tests/ci/scopes/test_base_detector.py new file mode 100644 index 00000000..40ad7d9b --- /dev/null +++ b/mergify_cli/tests/ci/scopes/test_base_detector.py @@ -0,0 +1,108 @@ +import json +import pathlib + +import click +import pytest + +from mergify_cli.ci.scopes import base_detector + + +def test_detect_base_github_base_ref( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("GITHUB_BASE_REF", "main") + monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False) + + result = base_detector.detect() + + assert result == "main" + + +def test_detect_base_from_event_path( + monkeypatch: pytest.MonkeyPatch, + tmp_path: pathlib.Path, +) -> None: + event_data = { + "pull_request": { + "base": {"sha": "abc123"}, + }, + } + event_file = tmp_path / "event.json" + event_file.write_text(json.dumps(event_data)) + + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) + monkeypatch.delenv("GITHUB_BASE_REF", raising=False) + + result = base_detector.detect() + + assert result == "abc123" + + +def test_detect_base_merge_queue_override( + monkeypatch: pytest.MonkeyPatch, + tmp_path: pathlib.Path, +) -> None: + event_data = { + "pull_request": { + "title": "merge-queue: Merge group", + "body": "```yaml\nchecking_base_sha: xyz789\n```", + "base": {"sha": "abc123"}, + }, + } + event_file = tmp_path / "event.json" + event_file.write_text(json.dumps(event_data)) + + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) + + result = base_detector.detect() + + assert result == "xyz789" + + +def test_detect_base_no_info(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False) + monkeypatch.delenv("GITHUB_BASE_REF", raising=False) + + with pytest.raises(click.ClickException, match="Could not detect base SHA"): + base_detector.detect() + + +def test_yaml_docs_from_fenced_blocks_valid() -> None: + body = """Some text +```yaml +--- +checking_base_sha: xyz789 +pull_requests: [{"number": 1}] +previous_failed_batches: [] +... +``` +More text""" + + result = base_detector._yaml_docs_from_fenced_blocks(body) + + assert result == base_detector.MergeQueueMetadata( + { + "checking_base_sha": "xyz789", + "pull_requests": [{"number": 1}], + "previous_failed_batches": [], + }, + ) + + +def test_yaml_docs_from_fenced_blocks_no_yaml() -> None: + body = "No yaml here" + + result = base_detector._yaml_docs_from_fenced_blocks(body) + + assert result is None + + +def test_yaml_docs_from_fenced_blocks_empty_yaml() -> None: + body = """Some text +```yaml +``` +More text""" + + result = base_detector._yaml_docs_from_fenced_blocks(body) + + assert result is None diff --git a/mergify_cli/tests/ci/scopes/test_changed_files.py b/mergify_cli/tests/ci/scopes/test_changed_files.py new file mode 100644 index 00000000..59317dca --- /dev/null +++ b/mergify_cli/tests/ci/scopes/test_changed_files.py @@ -0,0 +1,38 @@ +import subprocess +from unittest import mock + +import click +import pytest + +from mergify_cli.ci.scopes import changed_files + + +@mock.patch("mergify_cli.ci.scopes.changed_files.subprocess.check_output") +def test_git_changed_files(mock_subprocess: mock.Mock) -> None: + mock_subprocess.return_value = "file1.py\nfile2.js\n" + + result = changed_files.git_changed_files("main") + + mock_subprocess.assert_called_once_with( + ["git", "diff", "--name-only", "--diff-filter=ACMRTD", "main...HEAD"], + text=True, + encoding="utf-8", + ) + assert result == ["file1.py", "file2.js"] + + +@mock.patch("mergify_cli.ci.scopes.changed_files.subprocess.check_output") +def test_git_changed_files_empty(mock_subprocess: mock.Mock) -> None: + mock_subprocess.return_value = "" + + result = changed_files.git_changed_files("main") + + assert result == [] + + +@mock.patch("mergify_cli.ci.scopes.changed_files.subprocess.check_output") +def test_run_command_failure(mock_subprocess: mock.Mock) -> None: + mock_subprocess.side_effect = subprocess.CalledProcessError(1, ["git", "diff"]) + + with pytest.raises(click.ClickException, match="Command failed"): + changed_files._run(["git", "diff"]) diff --git a/mergify_cli/tests/ci/scopes/test_cli.py b/mergify_cli/tests/ci/scopes/test_cli.py index c955b088..1180a832 100644 --- a/mergify_cli/tests/ci/scopes/test_cli.py +++ b/mergify_cli/tests/ci/scopes/test_cli.py @@ -1,9 +1,6 @@ -import json import pathlib -import subprocess from unittest import mock -import click import pytest import yaml @@ -43,138 +40,6 @@ def test_from_yaml_invalid_config(tmp_path: pathlib.Path) -> None: cli.Config.from_yaml(str(config_file)) -@mock.patch("mergify_cli.ci.scopes.cli.subprocess.check_output") -def test_git_changed_files(mock_subprocess: mock.Mock) -> None: - mock_subprocess.return_value = "file1.py\nfile2.js\n" - - result = cli.git_changed_files("main") - - mock_subprocess.assert_called_once_with( - ["git", "diff", "--name-only", "--diff-filter=ACMRTD", "main...HEAD"], - text=True, - encoding="utf-8", - ) - assert result == ["file1.py", "file2.js"] - - -@mock.patch("mergify_cli.ci.scopes.cli.subprocess.check_output") -def test_git_changed_files_empty(mock_subprocess: mock.Mock) -> None: - mock_subprocess.return_value = "" - - result = cli.git_changed_files("main") - - assert result == [] - - -@mock.patch("mergify_cli.ci.scopes.cli.subprocess.check_output") -def test_run_command_failure(mock_subprocess: mock.Mock) -> None: - mock_subprocess.side_effect = subprocess.CalledProcessError(1, ["git", "diff"]) - - with pytest.raises(click.ClickException, match="Command failed"): - cli._run(["git", "diff"]) - - -def test_detect_base_github_base_ref( - monkeypatch: pytest.MonkeyPatch, -) -> None: - monkeypatch.setenv("GITHUB_BASE_REF", "main") - monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False) - - result = cli.detect_base() - - assert result == "main" - - -def test_detect_base_from_event_path( - monkeypatch: pytest.MonkeyPatch, - tmp_path: pathlib.Path, -) -> None: - event_data = { - "pull_request": { - "base": {"sha": "abc123"}, - }, - } - event_file = tmp_path / "event.json" - event_file.write_text(json.dumps(event_data)) - - monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) - monkeypatch.delenv("GITHUB_BASE_REF", raising=False) - - result = cli.detect_base() - - assert result == "abc123" - - -def test_detect_base_merge_queue_override( - monkeypatch: pytest.MonkeyPatch, - tmp_path: pathlib.Path, -) -> None: - event_data = { - "pull_request": { - "title": "merge-queue: Merge group", - "body": "```yaml\nchecking_base_sha: xyz789\n```", - "base": {"sha": "abc123"}, - }, - } - event_file = tmp_path / "event.json" - event_file.write_text(json.dumps(event_data)) - - monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) - - result = cli.detect_base() - - assert result == "xyz789" - - -def test_detect_base_no_info(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False) - monkeypatch.delenv("GITHUB_BASE_REF", raising=False) - - with pytest.raises(click.ClickException, match="Could not detect base SHA"): - cli.detect_base() - - -def test_yaml_docs_from_fenced_blocks_valid() -> None: - body = """Some text -```yaml ---- -checking_base_sha: xyz789 -pull_requests: [{"number": 1}] -previous_failed_batches: [] -... -``` -More text""" - - result = cli._yaml_docs_from_fenced_blocks(body) - - assert result == cli.MergeQueueMetadata( - { - "checking_base_sha": "xyz789", - "pull_requests": [{"number": 1}], - "previous_failed_batches": [], - }, - ) - - -def test_yaml_docs_from_fenced_blocks_no_yaml() -> None: - body = "No yaml here" - - result = cli._yaml_docs_from_fenced_blocks(body) - - assert result is None - - -def test_yaml_docs_from_fenced_blocks_empty_yaml() -> None: - body = """Some text -```yaml -``` -More text""" - - result = cli._yaml_docs_from_fenced_blocks(body) - - assert result is None - - def test_match_scopes_basic() -> None: config = cli.Config.from_dict( { @@ -338,8 +203,8 @@ def test_maybe_write_github_outputs_no_env( cli.maybe_write_github_outputs(["backend"], {"backend"}) -@mock.patch("mergify_cli.ci.scopes.cli.detect_base") -@mock.patch("mergify_cli.ci.scopes.cli.git_changed_files") +@mock.patch("mergify_cli.ci.scopes.cli.base_detector.detect") +@mock.patch("mergify_cli.ci.scopes.changed_files.git_changed_files") @mock.patch("mergify_cli.ci.scopes.cli.maybe_write_github_outputs") def test_detect_with_matches( mock_github_outputs: mock.Mock, @@ -377,8 +242,8 @@ def test_detect_with_matches( assert "- backend" in calls -@mock.patch("mergify_cli.ci.scopes.cli.detect_base") -@mock.patch("mergify_cli.ci.scopes.cli.git_changed_files") +@mock.patch("mergify_cli.ci.scopes.cli.base_detector.detect") +@mock.patch("mergify_cli.ci.scopes.changed_files.git_changed_files") @mock.patch("mergify_cli.ci.scopes.cli.maybe_write_github_outputs") def test_detect_no_matches( _: mock.Mock, @@ -405,8 +270,8 @@ def test_detect_no_matches( assert "No scopes matched." in calls -@mock.patch("mergify_cli.ci.scopes.cli.detect_base") -@mock.patch("mergify_cli.ci.scopes.cli.git_changed_files") +@mock.patch("mergify_cli.ci.scopes.cli.base_detector.detect") +@mock.patch("mergify_cli.ci.scopes.changed_files.git_changed_files") def test_detect_debug_output( mock_git_changed: mock.Mock, mock_detect_base: mock.Mock,