diff --git a/tools/pr_overlap.py b/tools/pr_overlap.py index 582f6bb07..4a1a4b472 100644 --- a/tools/pr_overlap.py +++ b/tools/pr_overlap.py @@ -45,7 +45,24 @@ def open_prs(base): query = urlencode({"state": "open", "base": base, "per_page": 100}) out = gh("api", "--paginate", f"repos/{REPO}/pulls?{query}") prs = {} - for pr in json.loads(out): + decoder = json.JSONDecoder() + offset = 0 + page_count = 0 + pull_requests = [] + while offset < len(out): + while offset < len(out) and out[offset].isspace(): + offset += 1 + if offset == len(out): + break + page, offset = decoder.raw_decode(out, offset) + if not isinstance(page, list): + raise ValueError("gh api --paginate returned a non-array page") + page_count += 1 + pull_requests.extend(page) + if page_count == 0: + raise ValueError("gh api --paginate returned no JSON pages") + + for pr in pull_requests: prs[pr["number"]] = { "ref": f"refs/pr/{pr['number']}", "sha": pr["head"]["sha"], @@ -127,7 +144,7 @@ def clashes(a, b, path): def conflicts(a, b): if not (a["tree"] and b["tree"] and a["files"] & b["files"]): return {} - files = merge("--merge-base=refs/pr/base", a["tree"], b["tree"])[1] + files = merge(a["tree"], b["tree"])[1] return {path: [] if path in a["added"] else clashes(a, b, path) for path in files} diff --git a/tools/tests/test_pr_overlap.py b/tools/tests/test_pr_overlap.py index f250ca15d..ed4f4e4b2 100644 --- a/tools/tests/test_pr_overlap.py +++ b/tools/tests/test_pr_overlap.py @@ -54,6 +54,28 @@ class OpenPullRequestsTests(unittest.TestCase): 7: {"ref": "refs/pr/7", "sha": "head-sha", "author": "contributor", "depends": set()}, }) + def test_collects_concatenated_json_pages_from_paginated_gh_output(self): + pull_requests = [ + {"number": number, "user": {"login": f"author{number}"}, + "head": {"sha": f"sha{number}"}, "body": None} + for number in range(1, 4) + ] + pages = [pull_requests[:2], pull_requests[2:]] + output = "\n".join(json.dumps(page) for page in pages) + + with patch.object(pr_overlap, "REPO", "owner/project", create=True), \ + patch.object(pr_overlap, "gh", return_value=output): + found = pr_overlap.open_prs("main") + + self.assertEqual(set(found), {1, 2, 3}) + self.assertEqual(found[3]["sha"], "sha3") + + def test_rejects_empty_paginated_output(self): + with patch.object(pr_overlap, "REPO", "owner/project", create=True), \ + patch.object(pr_overlap, "gh", return_value=" \n"): + with self.assertRaisesRegex(ValueError, "returned no JSON pages"): + pr_overlap.open_prs("main") + class FetchTests(unittest.TestCase): def test_fetches_each_head_commit_the_api_reported(self): @@ -86,5 +108,14 @@ class FetchTests(unittest.TestCase): self.assertEqual(git.call_args_list[-1].args[-1], "+00000000000000000000000000000000000000f9:refs/pr/249") +class ConflictTests(unittest.TestCase): + def test_uses_the_shared_base_parent_without_newer_merge_tree_option(self): + first = {"tree": "first-tree", "files": {"shared.cpp"}, "added": set()} + second = {"tree": "second-tree", "files": {"shared.cpp"}, "added": set()} + with patch.object(pr_overlap, "merge", return_value=("merged-tree", set())) as merge: + self.assertEqual(pr_overlap.conflicts(first, second), {}) + merge.assert_called_once_with("first-tree", "second-tree") + + if __name__ == "__main__": unittest.main()