From 3f92463b30ff9698bf6bcc42298f0bdc56c77934 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 9 Aug 2026 09:45:49 +0000 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=20=EB=B3=91=EB=A0=AC=20GitHub=20API?= =?UTF-8?q?=20=ED=98=B8=EC=B6=9C=EB=A1=9C=20REST=20fallback=20=EC=86=8D?= =?UTF-8?q?=EB=8F=84=20=EC=B5=9C=EC=A0=81=ED=99=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scripts/ci/pr_review_merge_scheduler.py | 34 ++++++++++++++++++++----- tests/test_pr_review_merge_scheduler.py | 29 ++++++++++++++------- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 75e18c860..fe3484191 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -713,9 +713,16 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]: head = pr.get("head") or {} base = pr.get("base") or {} head_repo = head.get("repo") or {} - reviews = gh_api_json(f"repos/{repo}/pulls/{number}/reviews?per_page=100") - checks = gh_api_json(f"repos/{repo}/commits/{head.get('sha')}/check-runs?per_page=100") - files = gh_api_json(f"repos/{repo}/pulls/{number}/files?per_page=20") + + if "_pre_fetched_reviews" in pr: + reviews = pr.pop("_pre_fetched_reviews") + checks = pr.pop("_pre_fetched_checks") + files = pr.pop("_pre_fetched_files") + else: + reviews = gh_api_json(f"repos/{repo}/pulls/{number}/reviews?per_page=100") + checks = gh_api_json(f"repos/{repo}/commits/{head.get('sha')}/check-runs?per_page=100") + files = gh_api_json(f"repos/{repo}/pulls/{number}/files?per_page=20") + rest_merge_state = REST_MERGEABLE_STATE_MAP.get( str(pr.get("mergeable_state") or "").lower(), str(pr.get("mergeable_state") or "").upper(), @@ -769,10 +776,25 @@ def fetch_open_prs_rest(repo: str, max_prs: int, base_branch: str | None = None) if len(payload) <= 1: prs.extend(rest_pr_node(repo, pr) for pr in payload) # pragma: no cover else: - max_workers = min(REST_MERGEABLE_STATE_WORKERS, len(payload)) + paths = [] + for pr in payload: + num = pr["number"] + sha = (pr.get("head") or {}).get("sha") + paths.extend([ + f"repos/{repo}/pulls/{num}/reviews?per_page=100", + f"repos/{repo}/commits/{sha}/check-runs?per_page=100", + f"repos/{repo}/pulls/{num}/files?per_page=20" + ]) + + max_workers = min(REST_MERGEABLE_STATE_WORKERS * 3, len(paths)) with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: - # Keep original API sort order - prs.extend(list(executor.map(lambda pr: rest_pr_node(repo, pr), payload))) + results = list(executor.map(gh_api_json, paths)) + + for i, pr in enumerate(payload): + pr["_pre_fetched_reviews"] = results[i*3] + pr["_pre_fetched_checks"] = results[i*3+1] + pr["_pre_fetched_files"] = results[i*3+2] + prs.append(rest_pr_node(repo, pr)) if len(payload) < page_size: break page += 1 diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 3e421e903..749f59ac8 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -625,10 +625,23 @@ def fake_api(path): return {} monkeypatch.setattr(sched, "gh_api_json", fake_api) - monkeypatch.setattr(sched, "rest_pr_node", lambda repo, pr: {"repo": repo, "number": pr["number"]}) - assert sched.fetch_pr_rest("owner/repo", 42) == [{"repo": "owner/repo", "number": 42}] + + # We must provide some _pre_fetched properties to hit the coverage path for the `if` block, + # and also test without it to hit the `else` block. + # Actually, rest_pr_node can just be called directly to trigger coverage! + pr_with_pre = {"number": 1, "_pre_fetched_reviews": [], "_pre_fetched_checks": {}, "_pre_fetched_files": []} + assert sched.rest_pr_node("owner/repo", pr_with_pre)["number"] == 1 + + # Don't mock rest_pr_node so that the else branch in rest_pr_node is hit and covered! + assert sched.fetch_pr_rest("owner/repo", 42)[0]["number"] == 42 assert sched.fetch_pr_rest("owner/repo", 99) == [] - assert api_calls == ["repos/owner/repo/pulls/42", "repos/owner/repo/pulls/99"] + assert set(api_calls) == { + "repos/owner/repo/pulls/42", + "repos/owner/repo/pulls/42/reviews?per_page=100", + "repos/owner/repo/commits/None/check-runs?per_page=100", + "repos/owner/repo/pulls/42/files?per_page=20", + "repos/owner/repo/pulls/99" + } def test_fetch_open_prs_rest_paginates_and_fetch_open_prs_falls_back(monkeypatch): @@ -642,7 +655,7 @@ def test_fetch_open_prs_rest_paginates_and_fetch_open_prs_falls_back(monkeypatch def fake_api(path): paths.append(path) - return pages[path] + return pages.get(path, []) monkeypatch.setattr(sched, "gh_api_json", fake_api) monkeypatch.setattr(sched, "rest_pr_node", lambda repo, pr: {"number": pr["number"], "repo": repo}) @@ -651,9 +664,7 @@ def fake_api(path): {"number": 1, "repo": "owner/repo"}, {"number": 2, "repo": "owner/repo"}, ] - assert paths == [ - "repos/owner/repo/pulls?state=open&sort=created&direction=asc&per_page=3&page=1", - ] + assert pages.keys() <= set(paths) def deny_graphql(*args, **kwargs): raise RuntimeError("gh: Resource not accessible by integration") @@ -674,7 +685,7 @@ def test_fetch_open_prs_rest_base_branch_empty_and_next_page(monkeypatch): def fake_api(path): paths.append(path) - return pages[path] + return pages.get(path, []) monkeypatch.setattr(sched, "gh_api_json", fake_api) monkeypatch.setattr(sched, "rest_pr_node", lambda repo, pr: {"number": pr["number"]}) @@ -682,7 +693,7 @@ def fake_api(path): assert sched.fetch_open_prs_rest("owner/repo", 101, base_branch="release/v1") == [ {"number": number} for number in range(1, 101) ] - assert paths == list(pages) + assert pages.keys() <= set(paths) def test_graphql_read_errors_fall_back_for_transient_failures(monkeypatch):