From 3ca7ea4228a0b17a181d8342b976b5c877c90204 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Fri, 12 Jun 2026 06:04:38 +0000 Subject: [PATCH] test(merge-queue): regression tests for non-main base skip (#2615) Adds coverage for molecule-core#2548: - Extract _early_skip_reason helper so the early-skip decision is pure and unit-testable while preserving the observable-vs-silent distinction. - Assert non-main base PRs are skipped with an observable reason and no PR-comment noise. - Assert main-targeted PRs are not early-skipped and proceed to the merge readiness evaluation. - Cover fork, closed, draft, and opt-out skip cases. Refs molecule-core#2615. Co-Authored-By: Claude --- .gitea/scripts/gitea-merge-queue.py | 60 +++++++---- .../scripts/tests/test_gitea_merge_queue.py | 102 ++++++++++++++++++ 2 files changed, 139 insertions(+), 23 deletions(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index 186596f7..1231ec8e 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -1147,6 +1147,36 @@ def process_once(*, dry_run: bool = False) -> int: return 0 +def _early_skip_reason( + pr: dict, + *, + watch_branch: str, +) -> tuple[str | None, str | None]: + """Return an observable skip reason for PRs that are not merge-eligible. + + Returns (reason, pr_comment). When reason is non-None the PR should be + skipped by the queue. pr_comment is the body to post on the PR; None means + the skip is silent w.r.t. PR comments (the reason is still emitted as a + workflow notice by the caller). This keeps the early-skip decision pure and + unit-testable while preserving the observable-vs-silent distinction. + """ + if pr.get("state") != "open": + return "not open", None + if OPT_OUT_LABELS & label_names(pr): + return "opt-out label", None + if pr.get("draft") is True: + return "draft", None + if pr.get("base", {}).get("ref") != watch_branch: + # Silent w.r.t. PR comments: stacked PRs whose base is not main are + # re-evaluated every conductor tick; posting a comment each time floods + # the PR. The caller still emits a workflow notice so the skip is + # observable in logs. + return f"base is not `{watch_branch}`", None + if pr.get("head", {}).get("repo_id") != pr.get("base", {}).get("repo_id"): + return "fork PR", "merge-queue: skipped; fork PRs are not supported by the serialized queue." + return None, None + + def _evaluate_candidate( issue: dict, *, @@ -1171,29 +1201,13 @@ def _evaluate_candidate( pr_number = int(issue["number"]) ctx = {"pr_number": pr_number} pr = get_pull(pr_number) - if pr.get("state") != "open": - print(f"::notice::PR #{pr_number} is not open; skipping") - return None, ctx - # Defensive opt-out/draft re-check on the authoritative pull payload: the - # /issues listing's label/draft view can lag, but the merge bar must respect - # the live pull state. (choose_candidate_issues already filtered on the - # listing; this guards against a stale listing racing a just-added opt-out.) - if OPT_OUT_LABELS & label_names(pr): - print(f"::notice::PR #{pr_number} carries an opt-out label; skipping") - return None, ctx - if pr.get("draft") is True: - print(f"::notice::PR #{pr_number} is a draft; skipping") - return None, ctx - if pr.get("base", {}).get("ref") != WATCH_BRANCH: - # SILENT skip (matches the draft/opt-out skips above): a stacked PR - # whose base is not main is re-evaluated EVERY conductor tick; posting - # a comment each time floods the PR (2026-06-10: ~74 stacked PRs -> - # ~28k skip comments + measurable operator load). A no-op observation - # must not write. The PR is auto-considered once its base becomes main. - print(f"::notice::PR #{pr_number} base is not `{WATCH_BRANCH}`; skipping (silent)") - return None, ctx - if pr.get("head", {}).get("repo_id") != pr.get("base", {}).get("repo_id"): - post_comment(pr_number, "merge-queue: skipped; fork PRs are not supported by the serialized queue.", dry_run=dry_run) + reason, pr_comment = _early_skip_reason(pr, watch_branch=WATCH_BRANCH) + if reason is not None: + # Observable in workflow logs (LOUD relative to silent disappearance), + # but avoid PR-comment noise for the high-volume skip classes. + print(f"::notice::PR #{pr_number} {reason}; skipping") + if pr_comment is not None: + post_comment(pr_number, pr_comment, dry_run=dry_run) return None, ctx head_sha = pr.get("head", {}).get("sha") diff --git a/.gitea/scripts/tests/test_gitea_merge_queue.py b/.gitea/scripts/tests/test_gitea_merge_queue.py index 3a0ba2d9..e5b5d345 100644 --- a/.gitea/scripts/tests/test_gitea_merge_queue.py +++ b/.gitea/scripts/tests/test_gitea_merge_queue.py @@ -1916,3 +1916,105 @@ def test_load_conductor_snapshot_ignores_stale_snapshot(monkeypatch): assert mq.load_conductor_snapshot() is None finally: os.unlink(path) + + +# -------------------------------------------------------------------------- +# Fix 3: non-main base PRs are skipped loudly/observably, not silently. +# core#2548 stopped the per-tick PR-comment flood; the skip must still be +# observable in workflow logs and must not affect legitimate main-targeted PRs. +# -------------------------------------------------------------------------- + +def _make_pr(**overrides) -> dict: + base = { + "state": "open", + "draft": False, + "labels": [], + "base": {"ref": "main", "repo_id": 1}, + "head": {"repo_id": 1, "sha": "a" * 40}, + } + base.update(overrides) + return base + + +def test_early_skip_reason_skips_non_main_base_observably(): + """A stacked PR whose base is not main is skipped with an observable reason + and NO PR comment (the observable path is the workflow notice).""" + pr = _make_pr(base={"ref": "feature/stacked", "repo_id": 1}) + reason, comment = mq._early_skip_reason(pr, watch_branch="main") + assert reason == "base is not `main`" + assert comment is None + + +def test_early_skip_reason_does_not_skip_main_targeted_pr(): + """A legitimate main-targeted PR is not skipped for base reasons.""" + pr = _make_pr() + reason, comment = mq._early_skip_reason(pr, watch_branch="main") + assert reason is None + assert comment is None + + +def test_early_skip_reason_skips_fork_pr_with_comment(): + """Fork PRs are skipped AND receive a PR comment (not silent).""" + pr = _make_pr(head={"repo_id": 2, "sha": "a" * 40}) + reason, comment = mq._early_skip_reason(pr, watch_branch="main") + assert reason == "fork PR" + assert comment is not None + assert "fork PRs are not supported" in comment + + +def test_early_skip_reason_skips_closed_draft_and_opt_out(): + """Other early-skip classes produce observable reasons and no comment.""" + assert mq._early_skip_reason(_make_pr(state="closed"), watch_branch="main") == ("not open", None) + assert mq._early_skip_reason(_make_pr(draft=True), watch_branch="main") == ("draft", None) + assert mq._early_skip_reason( + _make_pr(labels=[{"name": "do-not-auto-merge"}]), watch_branch="main" + ) == ("opt-out label", None) + + +def test_evaluate_candidate_non_main_base_skips_without_comment(capsys, monkeypatch): + """End-to-end: _evaluate_candidate skips a non-main base PR, emits a workflow + notice, and does NOT post a PR comment.""" + pr = _make_pr(number=42, base={"ref": "feature/stacked", "repo_id": 1}) + monkeypatch.setattr(mq, "get_pull", lambda _n: pr) + + comments_posted = [] + monkeypatch.setattr(mq, "post_comment", lambda n, b, *, dry_run: comments_posted.append((n, b, dry_run))) + + decision, ctx = mq._evaluate_candidate( + {"number": 42}, + main_sha="m" * 40, + main_status={"state": "success", "statuses": []}, + required_contexts=[], + required_approvals=2, + dry_run=False, + ) + + assert decision is None + assert ctx["pr_number"] == 42 + assert comments_posted == [] + captured = capsys.readouterr() + assert "base is not `main`" in captured.out + assert "skipping" in captured.out + + +def test_evaluate_candidate_main_base_proceeds_to_merge_bar(monkeypatch): + """A main-targeted PR is not early-skipped; it reaches the merge-readiness + evaluation (which, with no statuses/approvals in this minimal mock, waits).""" + pr = _make_pr(number=99, base={"ref": "main", "repo_id": 1}) + monkeypatch.setattr(mq, "get_pull", lambda _n: pr) + monkeypatch.setattr(mq, "get_pull_commits", lambda _n: [{"sha": "m" * 40}]) + monkeypatch.setattr(mq, "get_combined_status", lambda _sha: {"state": "success", "statuses": []}) + monkeypatch.setattr(mq, "get_pull_reviews", lambda _n: []) + + decision, ctx = mq._evaluate_candidate( + {"number": 99}, + main_sha="m" * 40, + main_status={"state": "success", "statuses": []}, + required_contexts=[], + required_approvals=2, + dry_run=False, + ) + + assert decision is not None + assert ctx["pr_number"] == 99 + assert decision.ready is False -- 2.52.0