diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 299adbcdb8..88af11b491 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -31,6 +31,7 @@ jobs: force_label 'unblocks title' 0e8a16 'Names a title ID on the Unblocks line' force_label perf fef2c0 'Performance change' force_label 'perf numbers missing' e99695 'perf pull request without a before and after summary line' + force_label 'review queue' 1d76db 'Next in the maintainer review queue: checks pass, no open change request, no conflict' label relinker 5319e7 'core/relinker' label libc fbca04 'libc' diff --git a/.github/workflows/pr-ready.yml b/.github/workflows/pr-ready.yml new file mode 100644 index 0000000000..3344931f57 --- /dev/null +++ b/.github/workflows/pr-ready.yml @@ -0,0 +1,28 @@ +name: PR review queue +on: + schedule: + - cron: '50 * * * *' + workflow_dispatch: +permissions: {} +concurrency: + group: pr-review-queue +jobs: + queue: + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + pull-requests: write + issues: write + steps: + - uses: actions/checkout@v7 + - env: + GH_TOKEN: ${{ github.token }} + GITHUB_REPOSITORY: ${{ github.repository }} + QUEUE_ISSUE: ${{ vars.REVIEW_QUEUE_ISSUE }} + run: | + if [ -n "$QUEUE_ISSUE" ]; then + python3 -I tools/pr_ready.py --issue "$QUEUE_ISSUE" + else + python3 -I tools/pr_ready.py + fi diff --git a/CMakeLists.txt b/CMakeLists.txt index 8fb58bddc8..1e039e57b8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -220,6 +220,8 @@ if(BUILD_TESTING) ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_pr_overlap.py) add_test(NAME pr_labels COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_pr_labels.py) + add_test(NAME pr_ready COMMAND ${Python3_EXECUTABLE} + ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_pr_ready.py) add_test(NAME progress_history COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_progress_history.py ProgressHistoryTests) add_test(NAME progress_history_output COMMAND ${Python3_EXECUTABLE} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a680127731..67d9cddc62 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,6 +62,7 @@ The [pull request template](.github/pull_request_template.md) is the checklist f - Keep the branch up to date with `main` and resolve conflicts yourself; rebasing and force-pushing is fine. - An AGC test executable and its tests are registered in `core/libs/prx/libSceAgcDriver/tests/cmake/.cmake`; the tests of an execution test in `tests/execution/` go in the fragment of its `agc_driver__tests` executable. `CMakeLists.txt` includes the fragments in sorted order, so a fragment uses nothing another fragment defines. A pull request whose change to `libSceAgcDriver/CMakeLists.txt` conflicts after the split rebases and runs `python3 tools/agc_test_split.py resolve` during the stopped rebase, which moves its registrations to their fragments and leaves conflict markers where `main` changed the same lines. - A pull request that conflicts with `main` and has no activity for 7 days is labeled `stale` with a reminder. It is not closed. +- An hourly job gives the `review queue` label to the oldest 30 pull requests that need no more work from their author, `unblocks title` ones first, and lists them in the review queue issue. A pull request needs more work while it is a draft, conflicts with `main`, has a `perf numbers missing` or `open PR limit` label, has a failing or unfinished check other than Intake, Labels, Eligibility and the progress report, or has a change request whose reviewer was not asked again; pushing alone does not answer a change request. `python3 tools/pr_ready.py --explain ` prints what blocks a pull request. - If a pull request needs another one first, say so on the checklist line (`Depends on: #N`; `none` otherwise). - If a pull request fixes a reported issue, write `Fixes #N` on its checklist line (`none` otherwise). GitHub closes the issue on merge, and the `fix error` label is added within the hour. - If a pull request fixes a stop or a wrong result seen on a title, name it on the checklist line with the title ID and its [Game bring-up](https://github.com/boykopovar/AnyPS5/discussions/categories/game-bring-up) discussion (`Unblocks: PPSA01325 #3232`; `none` otherwise). Reviewers review these first. Such a pull request gets the `unblocks title` label. diff --git a/tools/pr_ready.py b/tools/pr_ready.py new file mode 100644 index 0000000000..2c3acb1fc2 --- /dev/null +++ b/tools/pr_ready.py @@ -0,0 +1,246 @@ +import argparse +import json +import subprocess +import sys +import time +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import pr_labels + +QUEUE_LABEL = "review queue" +UNBLOCKS_LABEL = pr_labels.UNBLOCKS_LABEL +QUEUE_SIZE = 30 +BLOCKING_LABELS = {"needs rebase", "perf numbers missing", "open PR limit"} +CONVENTIONS_CHECK = "conventions" +BOT_CHECKS = {"intake", "labels", "eligibility", "report"} +BUILD_CHECKS = {"linux", "windows", "relinker", "relinker-asan", "register-context", "write-watch"} +FAILED = {"failure", "cancelled", "timed_out", "action_required", "startup_failure", "stale", "error"} +PASSED = {"success", "neutral"} +MARKER = "" +WRITE_PAUSE = 1.0 +PAGE_SIZE = 25 + +QUERY = """ +query($owner: String!, $name: String!, $after: String, $size: Int!) { + repository(owner: $owner, name: $name) { + pullRequests(states: OPEN, baseRefName: "main", first: $size, after: $after) { + pageInfo { hasNextPage endCursor } + nodes { + number title isDraft mergeable headRefOid createdAt + author { login } + labels(first: 50) { nodes { name } } + reviewRequests(first: 50) { nodes { requestedReviewer { ... on User { login } } } } + latestOpinionatedReviews(first: 50) { nodes { state author { login } } } + commits(last: 1) { nodes { commit { statusCheckRollup { contexts(first: 100) { + pageInfo { hasNextPage } + nodes { + __typename + ... on CheckRun { databaseId name status conclusion } + ... on StatusContext { context state } + } + } } } } } + } + } + } +} +""" + +CLOSED_QUERY = """ +query($owner: String!, $name: String!, $label: String!) { + repository(owner: $owner, name: $name) { + pullRequests(states: [CLOSED, MERGED], labels: [$label], first: 100) { nodes { number } } + } +} +""" + + +def listed(connection): + if not isinstance(connection, dict) or not isinstance(connection.get("nodes"), list): + raise ValueError("expected a connection with nodes") + return connection["nodes"] + + +def check_of(node): + if not isinstance(node, dict): + raise ValueError("check is not an object") + if node.get("__typename") == "StatusContext": + state = str(node.get("state") or "").lower() + done = state not in {"pending", "expected"} + return {"id": 0, "name": node.get("context"), "done": done, + "conclusion": {"success": "success"}.get(state, "failure") if done else None} + return {"id": node.get("databaseId") or 0, "name": node.get("name"), + "done": str(node.get("status") or "").lower() == "completed", + "conclusion": str(node.get("conclusion") or "").lower() or None} + + +def pull(node): + if not isinstance(node, dict) or not isinstance(node.get("number"), int): + raise ValueError("pull request needs a number") + if not isinstance(node.get("isDraft"), bool): + raise ValueError(f"#{node['number']}: draft flag must be a boolean") + commits = listed(node.get("commits")) + rollup = commits[0]["commit"].get("statusCheckRollup") if commits else None + contexts = (rollup or {}).get("contexts") or {} + author = (node.get("author") or {}).get("login") or "ghost" + reviews = [(item.get("author") or {}).get("login") or "ghost" for item in listed(node.get("latestOpinionatedReviews")) + if item.get("state") == "CHANGES_REQUESTED"] + requested = {(item.get("requestedReviewer") or {}).get("login") for item in listed(node.get("reviewRequests"))} + return { + "number": node["number"], + "title": node.get("title") or "", + "author": author, + "draft": node["isDraft"], + "mergeable": node.get("mergeable"), + "created": node.get("createdAt") or "", + "labels": {item["name"] for item in listed(node.get("labels"))}, + "changes": sorted(set(reviews) - requested), + "checks": [check_of(item) for item in listed(contexts)] if contexts else [], + "checks_incomplete": bool((contexts.get("pageInfo") or {}).get("hasNextPage")), + } + + +def latest_checks(checks): + latest = {} + for check in checks: + name = check["name"] + if name not in latest or check["id"] > latest[name]["id"]: + latest[name] = check + return [check for name, check in latest.items() if name not in BOT_CHECKS] + + +def blockers(pr): + reasons = [] + if pr["draft"]: + reasons.append("draft") + if pr["mergeable"] == "CONFLICTING" or "needs rebase" in pr["labels"]: + reasons.append("conflicts with main") + reasons += [f"label: {label}" for label in sorted(BLOCKING_LABELS - {"needs rebase"} & pr["labels"])] + checks = latest_checks(pr["checks"]) + by_name = {check["name"]: check for check in checks} + if pr["checks_incomplete"]: + reasons.append("too many checks to read") + if CONVENTIONS_CHECK not in by_name: + reasons.append("conventions has not run") + elif by_name[CONVENTIONS_CHECK]["done"] and by_name[CONVENTIONS_CHECK]["conclusion"] not in FAILED | {"success"}: + reasons.append("conventions did not pass") + failing = sorted(check["name"] for check in checks if check["done"] and check["conclusion"] in FAILED) + running = sorted(check["name"] for check in checks if not check["done"]) + if failing: + reasons.append("failing: " + ", ".join(failing)) + if running: + reasons.append("still running: " + ", ".join(running)) + if pr["changes"]: + reasons.append("change request not answered by a re-request: " + ", ".join(pr["changes"])) + return reasons + + +def built(pr): + return any(check["conclusion"] in PASSED for check in latest_checks(pr["checks"]) + if str(check["name"]).split(" (")[0] in BUILD_CHECKS) + + +def ranked(prs): + ready = [pr for pr in prs if not blockers(pr)] + return sorted(ready, key=lambda pr: (UNBLOCKS_LABEL not in pr["labels"], pr["created"], pr["number"])) + + +def clean(text): + text = "".join(ch for ch in text if ch.isprintable()).replace("@", "@​") + for char in "[]|`<>*_~": + text = text.replace(char, "\\" + char) + return text[:100] + + +def queue_body(queue, total, ready): + lines = [MARKER, f"Ready for a maintainer first look: the oldest {len(queue)} of {ready} pull requests that pass checks " + f"({total} open). Pull requests with an `{UNBLOCKS_LABEL}` label come first.", ""] + for index, pr in enumerate(queue, 1): + flag = (" (unblocks a title)" if UNBLOCKS_LABEL in pr["labels"] else "") + ("" if built(pr) else " (no build ran)") + lines.append(f"{index}. #{pr['number']} {clean(pr['title'])} by {clean(pr['author'])}{flag}") + return "\n".join(lines) + "\n" + + +def fetch(repo): + owner, name = repo.split("/", 1) + prs, after = [], None + while True: + args = ["api", "graphql", "-f", f"query={QUERY}", "-F", f"owner={owner}", "-F", f"name={name}", "-F", f"size={PAGE_SIZE}"] + if after: + args += ["-f", f"after={after}"] + page = json.loads(pr_labels.gh(*args)) + if page.get("errors"): + raise ValueError(str(page["errors"])[:300]) + connection = page["data"]["repository"]["pullRequests"] + prs += [pull(node) for node in listed(connection)] + if not connection["pageInfo"]["hasNextPage"]: + return prs + after = connection["pageInfo"]["endCursor"] + + +def closed_with_label(repo): + owner, name = repo.split("/", 1) + page = json.loads(pr_labels.gh("api", "graphql", "-f", f"query={CLOSED_QUERY}", "-F", f"owner={owner}", + "-F", f"name={name}", "-f", f"label={QUEUE_LABEL}")) + if page.get("errors"): + raise ValueError(str(page["errors"])[:300]) + return [node["number"] for node in listed(page["data"]["repository"]["pullRequests"])] + + +def apply(repo, prs, queue, dry_run, closed=()): + chosen = {pr["number"] for pr in queue} + changes = [(number, "--remove-label") for number in closed] + for pr in prs: + if pr["number"] in chosen and QUEUE_LABEL not in pr["labels"]: + changes.append((pr["number"], "--add-label")) + elif pr["number"] not in chosen and QUEUE_LABEL in pr["labels"]: + changes.append((pr["number"], "--remove-label")) + for number, flag in changes: + print(f"{'would change' if dry_run else 'changed'} #{number}: {'+' if flag == '--add-label' else '-'}{QUEUE_LABEL}") + if not dry_run: + pr_labels.gh("pr", "edit", str(number), "-R", repo, flag, QUEUE_LABEL) + time.sleep(WRITE_PAUSE) + return changes + + +def main(argv=None): + if hasattr(sys.stdout, "reconfigure"): + sys.stdout.reconfigure(encoding="utf-8", errors="backslashreplace") + if hasattr(sys.stderr, "reconfigure"): + sys.stderr.reconfigure(encoding="utf-8", errors="backslashreplace") + parser = argparse.ArgumentParser(description="Label the pull requests that are next in the review queue.") + parser.add_argument("--dry-run", action="store_true", help="print the planned changes and write nothing") + parser.add_argument("--explain", type=int, metavar="PR", help="print what blocks one pull request and write nothing") + parser.add_argument("--issue", type=int, help="issue whose body lists the queue") + args = parser.parse_args(argv) + try: + repo = pr_labels.repository() + prs = fetch(repo) + closed = closed_with_label(repo) if args.explain is None else [] + except subprocess.CalledProcessError as error: + parser.error((error.stderr or str(error)).strip()) + except (ValueError, KeyError, TypeError, RecursionError) as error: + parser.error(f"cannot read pull requests: {error}") + if args.explain is not None: + match = [pr for pr in prs if pr["number"] == args.explain] + if not match: + parser.error(f"#{args.explain} is not an open pull request to main") + print("\n".join(blockers(match[0])) or "nothing blocks it") + return 0 + ready = ranked(prs) + queue = ready[:QUEUE_SIZE] + try: + apply(repo, prs, queue, args.dry_run, closed) + if args.issue is not None and not args.dry_run: + body = queue_body(queue, len(prs), len(ready)) + current = pr_labels.gh("issue", "view", str(args.issue), "-R", repo, "--json", "body", "--jq", ".body") + if current.strip() != body.strip(): + pr_labels.gh("issue", "edit", str(args.issue), "-R", repo, "--body", body) + except subprocess.CalledProcessError as error: + sys.exit(f"cannot update the queue: {(error.stderr or '').strip()}") + print(f"{len(ready)} of {len(prs)} open pull requests have no blocker; {len(queue)} queued") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/tests/test_pr_ready.py b/tools/tests/test_pr_ready.py new file mode 100644 index 0000000000..b76da8c4df --- /dev/null +++ b/tools/tests/test_pr_ready.py @@ -0,0 +1,293 @@ +import contextlib +import io +import json +import sys +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +import pr_ready + +PASSING = [("conventions", "completed", "success"), ("relinker", "completed", "success")] + + +def run(name, status="completed", conclusion="success", id=1): + return {"__typename": "CheckRun", "databaseId": id, "name": name, "status": status.upper(), + "conclusion": conclusion.upper() if conclusion else None} + + +def node(number=7, labels=(), draft=False, mergeable="MERGEABLE", checks=None, changes=(), requested=(), + created="2026-01-01T00:00:00Z", title="fix: x", author="alice", more=False): + contexts = [run(name, status, conclusion) for name, status, conclusion in (PASSING if checks is None else checks)] + return { + "number": number, "title": title, "isDraft": draft, "mergeable": mergeable, "createdAt": created, + "author": {"login": author} if author else None, + "labels": {"nodes": [{"name": name} for name in labels]}, + "reviewRequests": {"nodes": [{"requestedReviewer": {"login": login}} for login in requested]}, + "latestOpinionatedReviews": {"nodes": [{"state": "CHANGES_REQUESTED", "author": {"login": login} if login else None} + for login in changes]}, + "commits": {"nodes": [{"commit": {"statusCheckRollup": {"contexts": {"pageInfo": {"hasNextPage": more}, "nodes": contexts}}}}]}, + } + + +def make(**kwargs): + return pr_ready.pull(node(**kwargs)) + + +class BlockerTests(unittest.TestCase): + def test_a_pull_request_with_passing_checks_has_no_blocker(self): + self.assertEqual(pr_ready.blockers(make()), []) + + def test_a_draft_is_blocked(self): + self.assertEqual(pr_ready.blockers(make(draft=True)), ["draft"]) + + def test_conflicts_come_from_github_or_the_label(self): + self.assertEqual(pr_ready.blockers(make(mergeable="CONFLICTING")), ["conflicts with main"]) + self.assertEqual(pr_ready.blockers(make(labels=["needs rebase"])), ["conflicts with main"]) + + def test_an_unknown_mergeable_state_does_not_block(self): + self.assertEqual(pr_ready.blockers(make(mergeable="UNKNOWN")), []) + + def test_other_waiting_labels_block(self): + self.assertEqual(pr_ready.blockers(make(labels=["perf numbers missing", "open PR limit"])), + ["label: open PR limit", "label: perf numbers missing"]) + + def test_the_conventions_check_must_have_run(self): + self.assertIn("conventions has not run", pr_ready.blockers(make(checks=[("relinker", "completed", "success")]))) + + def test_a_failing_check_blocks(self): + reasons = pr_ready.blockers(make(checks=PASSING + [("windows", "completed", "failure")])) + self.assertEqual(reasons, ["failing: windows"]) + + def test_a_running_check_blocks(self): + reasons = pr_ready.blockers(make(checks=PASSING + [("linux", "in_progress", None)])) + self.assertEqual(reasons, ["still running: linux"]) + + def test_bot_jobs_are_not_counted(self): + checks = PASSING + [("intake", "completed", "failure"), ("labels", "in_progress", None), ("eligibility", "queued", None)] + self.assertEqual(pr_ready.blockers(make(checks=checks)), []) + + def test_a_failing_progress_report_does_not_block(self): + self.assertEqual(pr_ready.blockers(make(checks=PASSING + [("report", "completed", "failure")])), []) + + def test_skipped_conventions_do_not_pass(self): + checks = [("conventions", "completed", "skipped"), ("relinker", "completed", "success")] + self.assertEqual(pr_ready.blockers(make(checks=checks)), ["conventions did not pass"]) + + def test_skipped_builds_do_not_block(self): + checks = [("conventions", "completed", "success"), ("linux", "completed", "skipped")] + self.assertEqual(pr_ready.blockers(make(checks=checks)), []) + + def test_a_pull_request_counts_as_built_only_after_a_passing_build(self): + self.assertTrue(pr_ready.built(make())) + self.assertTrue(pr_ready.built(make(checks=[("conventions", "completed", "success"), + ("relinker (ubuntu-latest, g++)", "completed", "success")]))) + self.assertFalse(pr_ready.built(make(checks=[("conventions", "completed", "success"), ("linux", "completed", "skipped")]))) + + def test_the_newest_run_of_a_check_wins(self): + data = node(checks=[("conventions", "completed", "success")]) + data["commits"]["nodes"][0]["commit"]["statusCheckRollup"]["contexts"]["nodes"] += [ + run("relinker", "completed", "cancelled", id=5), run("relinker", "completed", "success", id=9)] + self.assertEqual(pr_ready.blockers(pr_ready.pull(data)), []) + data["commits"]["nodes"][0]["commit"]["statusCheckRollup"]["contexts"]["nodes"][-1]["databaseId"] = 2 + self.assertEqual(pr_ready.blockers(pr_ready.pull(data)), ["failing: relinker"]) + + def test_commit_statuses_are_read(self): + data = node() + data["commits"]["nodes"][0]["commit"]["statusCheckRollup"]["contexts"]["nodes"] += [ + {"__typename": "StatusContext", "context": "ci/other", "state": "PENDING"}] + self.assertEqual(pr_ready.blockers(pr_ready.pull(data)), ["still running: ci/other"]) + data["commits"]["nodes"][0]["commit"]["statusCheckRollup"]["contexts"]["nodes"][-1]["state"] = "FAILURE" + self.assertEqual(pr_ready.blockers(pr_ready.pull(data)), ["failing: ci/other"]) + + def test_more_checks_than_one_page_is_not_ready(self): + self.assertEqual(pr_ready.blockers(make(more=True)), ["too many checks to read"]) + + def test_no_checks_at_all_is_not_ready(self): + data = node() + data["commits"]["nodes"][0]["commit"]["statusCheckRollup"] = None + self.assertEqual(pr_ready.blockers(pr_ready.pull(data)), ["conventions has not run"]) + + def test_a_change_request_blocks_until_the_reviewer_is_asked_again(self): + self.assertEqual(pr_ready.blockers(make(changes=["bob"])), + ["change request not answered by a re-request: bob"]) + self.assertEqual(pr_ready.blockers(make(changes=["bob"], requested=["bob"])), []) + + def test_a_re_request_only_clears_that_reviewer(self): + self.assertEqual(pr_ready.blockers(make(changes=["bob", "carol"], requested=["bob"])), + ["change request not answered by a re-request: carol"]) + + def test_a_deleted_reviewer_still_blocks(self): + self.assertEqual(pr_ready.blockers(make(changes=[None])), + ["change request not answered by a re-request: ghost"]) + + def test_a_deleted_author_is_read(self): + self.assertEqual(make(author=None)["author"], "ghost") + + def test_a_requested_team_is_ignored(self): + data = node(changes=["bob"]) + data["reviewRequests"]["nodes"].append({"requestedReviewer": {}}) + self.assertEqual(pr_ready.blockers(pr_ready.pull(data)), ["change request not answered by a re-request: bob"]) + + +class PullTests(unittest.TestCase): + def test_a_missing_number_is_rejected(self): + with self.assertRaises(ValueError): + pr_ready.pull({"isDraft": False}) + + def test_a_missing_draft_flag_is_rejected(self): + data = node() + del data["isDraft"] + with self.assertRaises(ValueError): + pr_ready.pull(data) + + def test_a_connection_without_nodes_is_rejected(self): + data = node() + data["labels"] = {} + with self.assertRaises(ValueError): + pr_ready.pull(data) + + +class QueueTests(unittest.TestCase): + def test_titles_come_first_then_the_oldest(self): + prs = [make(number=1, created="2026-03-01T00:00:00Z"), make(number=2, created="2026-02-01T00:00:00Z"), + make(number=3, created="2026-04-01T00:00:00Z", labels=["unblocks title"]), make(number=4, draft=True)] + self.assertEqual([pr["number"] for pr in pr_ready.ranked(prs)], [3, 2, 1]) + + def test_the_body_does_not_mention_users_or_break_markdown(self): + body = pr_ready.queue_body([make(title="fix @bob [x] | `y`", author="eve", labels=["unblocks title"])], 10, 4) + self.assertIn("oldest 1 of 4", body) + self.assertIn("(10 open)", body) + self.assertIn("(unblocks a title)", body) + self.assertNotIn("(no build ran)", body) + self.assertNotIn("@bob", body) + self.assertNotIn("[x]", body) + self.assertTrue(body.startswith(pr_ready.MARKER)) + + def test_the_body_marks_pull_requests_without_a_build(self): + body = pr_ready.queue_body([make(checks=[("conventions", "completed", "success")])], 1, 1) + self.assertIn("by alice (no build ran)", body) + + def test_checks_that_are_not_builds_do_not_count_as_a_build(self): + checks = [("conventions", "completed", "success"), ("report", "completed", "success"), + ("library-portability", "completed", "success"), ("linux", "completed", "skipped")] + pr = make(checks=checks) + self.assertEqual(pr_ready.blockers(pr), []) + self.assertFalse(pr_ready.built(pr)) + + def test_long_titles_are_cut(self): + self.assertEqual(len(pr_ready.clean("a" * 500)), 100) + + +class MainTests(unittest.TestCase): + def run_main(self, argv, prs, calls, closed=(), current=""): + pages = [{"data": {"repository": {"pullRequests": {"pageInfo": {"hasNextPage": False, "endCursor": None}, "nodes": prs}}}}] + + def fake(*args): + if args[:2] == ("api", "graphql"): + if any("CLOSED" in arg for arg in args): + return json.dumps({"data": {"repository": {"pullRequests": {"nodes": [{"number": n} for n in closed]}}}}) + return json.dumps(pages.pop(0)) + if args[:2] == ("issue", "view"): + return current + calls.append(args) + return "" + out, err = io.StringIO(), io.StringIO() + with patch("pr_ready.pr_labels.gh", fake), patch("pr_ready.pr_labels.repository", return_value="o/r"), \ + patch("pr_ready.time.sleep"), contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + try: + code = pr_ready.main(argv) + except SystemExit as exit: + code = exit.code + return code, out.getvalue(), err.getvalue() + + def test_a_ready_pull_request_gets_the_label(self): + calls = [] + code, out, _ = self.run_main([], [node(number=7)], calls) + self.assertEqual(code, 0) + self.assertEqual(calls, [("pr", "edit", "7", "-R", "o/r", "--add-label", "review queue")]) + self.assertIn("changed #7: +review queue", out) + + def test_a_queued_pull_request_that_is_blocked_loses_the_label(self): + calls = [] + self.run_main([], [node(number=7, labels=["review queue"], draft=True)], calls) + self.assertEqual(calls, [("pr", "edit", "7", "-R", "o/r", "--remove-label", "review queue")]) + + def test_a_pull_request_that_is_already_right_is_left_alone(self): + calls = [] + self.run_main([], [node(number=7, labels=["review queue"]), node(number=8, draft=True)], calls) + self.assertEqual(calls, []) + + def test_only_the_first_pull_requests_are_queued(self): + calls = [] + prs = [node(number=n, created=f"2026-01-{n:02d}T00:00:00Z") for n in range(1, 4)] + with patch.object(pr_ready, "QUEUE_SIZE", 2): + self.run_main([], prs, calls) + self.assertEqual([call[2] for call in calls], ["1", "2"]) + + def test_a_dry_run_writes_nothing(self): + calls = [] + _, out, _ = self.run_main(["--dry-run", "--issue", "3"], [node(number=7)], calls) + self.assertEqual(calls, []) + self.assertIn("would change #7", out) + + def test_the_issue_body_is_updated(self): + calls = [] + self.run_main(["--issue", "3"], [node(number=7)], calls) + self.assertEqual(calls[-1][:6], ("issue", "edit", "3", "-R", "o/r", "--body")) + self.assertIn("#7 fix: x by alice", calls[-1][6]) + + def test_an_unchanged_issue_body_is_not_rewritten(self): + calls = [] + body = pr_ready.queue_body([make(number=7)], 1, 1) + self.run_main(["--issue", "3"], [node(number=7, labels=["review queue"])], calls, current=body) + self.assertEqual(calls, []) + + def test_closed_pull_requests_lose_the_label(self): + calls = [] + _, out, _ = self.run_main([], [], calls, closed=[5]) + self.assertEqual(calls, [("pr", "edit", "5", "-R", "o/r", "--remove-label", "review queue")]) + self.assertIn("changed #5: -review queue", out) + + def test_explain_prints_the_blockers_and_writes_nothing(self): + calls = [] + _, out, _ = self.run_main(["--explain", "7"], [node(number=7, draft=True)], calls) + self.assertEqual(out.strip(), "draft") + self.assertEqual(calls, []) + + def test_explain_an_unknown_pull_request_fails(self): + code, _, err = self.run_main(["--explain", "9"], [node(number=7)], []) + self.assertEqual(code, 2) + self.assertIn("#9 is not an open pull request", err) + + def test_a_malformed_response_is_an_error(self): + code, _, err = self.run_main([], [{"number": 7}], []) + self.assertEqual(code, 2) + self.assertIn("cannot read pull requests", err) + + def test_graphql_errors_are_reported(self): + out, err = io.StringIO(), io.StringIO() + with patch("pr_ready.pr_labels.gh", return_value=json.dumps({"errors": [{"message": "boom"}]})), \ + patch("pr_ready.pr_labels.repository", return_value="o/r"), \ + contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + with self.assertRaises(SystemExit): + pr_ready.main([]) + self.assertIn("boom", err.getvalue()) + + def test_pages_are_followed(self): + first = {"data": {"repository": {"pullRequests": {"pageInfo": {"hasNextPage": True, "endCursor": "c1"}, "nodes": [node(number=1)]}}}} + second = {"data": {"repository": {"pullRequests": {"pageInfo": {"hasNextPage": False, "endCursor": None}, "nodes": [node(number=2)]}}}} + seen = [] + + def fake(*args): + seen.append(args) + return json.dumps(first if len(seen) == 1 else second) + with patch("pr_ready.pr_labels.gh", fake): + prs = pr_ready.fetch("o/r") + self.assertEqual([pr["number"] for pr in prs], [1, 2]) + self.assertIn("after=c1", seen[1]) + + +if __name__ == "__main__": + unittest.main()