diff --git a/scripts/README.md b/scripts/README.md index 850f08f4..3060ebf9 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -199,12 +199,13 @@ uv run --no-project --script scripts/pr_review.py cleanup 452 --repo ptr727/Proj | Code | Command | Meaning | | --- | --- | --- | -| `0` | `status`, `wait` | No Copilot review covers the head yet (`status`), or the review covering it states no partial coverage and its verdict counts what it flags. On a held head, an attested local pass whose checks no longer hold the poll, which does not prove its required checks ran. | +| `0` | `status`, `wait` | No Copilot review covers the head yet (`status`), or the review covering it states no partial coverage and its verdict counts what it flags. On a held head, an attested local pass whose checks no longer hold the poll, short of `45`. | | `30` | `wait` | Still pending at the timeout, which is pending rather than failed. On a held head, `status=CHECKS_PENDING`. A round past `--min-rounds` not yet landed. | | `40` | `wait` | No Copilot review covers the head: the reviewer answered outside a formal review, the request returned success and recorded nothing, or the reviewer writes under a login no query filters on. | | `41` | `wait` | The newest round is a refusal or a quota state, its kind printed in the status line, and in `refusal=` for a refusal on this pull request. | | `42` | `status`, `wait` | The round covering the head states partial coverage. Or a non-refusal head round exists while an earlier round states partial, and one reads `UNKNOWN` or none states coverage. | | `44` | `wait` | The review loop closed, the merge reads `BLOCKED`, and a required check is in a shape no wait clears. | +| `45` | `wait` | An open attested held head that nothing holds, its merge other than `CLEAN`, `UNSTABLE`, or `HAS_HOOKS`. Its rollup was unreadable or none posted, or a `DIRTY` or `BEHIND` merge lacks a required check or has it from another app, judged on rules read and a whole rollup. | | `49` | `wait` | No Copilot round covers the head and none was requested, on a pull request into a branch other than the default with no attestation. | | `50` | `status`, `wait` | The round covering the head opens on a verdict other than the clean one while it counts no finding. Or a head round carries a part the reader cannot place, and nothing acknowledges its fingerprint. | | `60` to `63` | `reply` | No thread matched, more than one did, the reply returned no comment url, or the resolve did not report the thread resolved. | @@ -214,7 +215,7 @@ uv run --no-project --script scripts/pr_review.py cleanup 452 --repo ptr727/Proj `1` is a GraphQL read that failed and ended the run, and `2` is argparse's own usage error. -`wait` exits `30` with `status=CHECKS_PENDING` on an attested held head while its checks can still move the merge at the timeout. It reads them in this order. A pull request no longer open holds nothing, and a merge reading `UNKNOWN` holds. A `BLOCKED` merge ends the poll on the first read where a required check is stuck in a shape waiting does not clear. A check running long or gated is not such a shape. A failed one is the exception while a newer run of its workflow on the head has not finished, which the digest names `CHECK SUPERSEDED`. Next, a workflow run on the head that has not completed holds, where the read reaches it. The read reaches the head's last 50 check suites and those of its first `CHECKS_WINDOW` rollup contexts. A run's later jobs enter the rollup only as their `needs:` finish, so a green rollup proves nothing while one is open. The status line names those runs. Next, a merge reading `CLEAN`, `UNSTABLE`, or `HAS_HOOKS` ends the poll. Otherwise a required check still settling holds, one running long included, and so does any settling check while no required one has posted. A `BLOCKED` merge also holds while a check the base branch's rules require has not registered. That hold needs the base branch's rules read and a rollup that is not `CHECKS TRUNCATED`. An empty rollup holds under `BLOCKED`, `BEHIND`, or `DRAFT`. So exit `0` on a held head does not prove its required checks ran. A payload with no commit matching the head, which the digest names `CHECKS UNREADABLE`, reads no run and no check. Short of those holds, a merge with some check posted, or a `DIRTY` one with none, can release with a required check unregistered. `--min-rounds N` counts what `status` prints as `rounds=`, every round on every head, refusals included, so N is the `rounds=` a `status` read before a re-request. +`wait` exits `30` with `status=CHECKS_PENDING` on an attested held head while its checks can still move the merge at the timeout. It reads them in this order. A pull request no longer open holds nothing, and a merge reading `UNKNOWN` holds. A `BLOCKED` merge ends the poll on the first read where a required check is stuck in a shape waiting does not clear. A check running long or gated is not such a shape. A failed one is the exception while a newer run of its workflow on the head has not finished, which the digest names `CHECK SUPERSEDED`. Next, a workflow run on the head that has not completed holds, where the read reaches it. The read reaches the head's last 50 check suites and those of its first `CHECKS_WINDOW` rollup contexts. A run's later jobs enter the rollup only as their `needs:` finish, so a green rollup proves nothing while one is open. The status line names those runs. Next, a merge reading `CLEAN`, `UNSTABLE`, or `HAS_HOOKS` ends the poll. Otherwise a required check still settling holds, one running long included, and so does any settling check while no required one has posted. A `BLOCKED` merge also holds while a check the base branch's rules require has not registered. That hold needs the base branch's rules read and a rollup that is not `CHECKS TRUNCATED`. An empty rollup holds under `BLOCKED`, `BEHIND`, or `DRAFT`. An open head released short of those holds exits `45` with `status=CHECKS_MISSING` where a check it is owed is missing from the read. Each such case falls under a merge other than `CLEAN`, `UNSTABLE`, or `HAS_HOOKS`. One is a payload with no commit matching the head, which the digest names `CHECKS UNREADABLE` and which reads no run and no check. Another is a rollup with no check posted, which only a `DIRTY` merge releases, since a conflicted pull request runs no `pull_request` workflow. The last is a `DIRTY` or `BEHIND` merge whose rollup lacks a context the base branch's rules require, or carries it from another app. That one needs the rules read and a rollup that is not `CHECKS TRUNCATED`. Exit `0` can then leave a required check unregistered only where the rules were unread, the rollup was cut, or the merge reads `DRAFT`. A pull request no longer open never exits `45`. `--min-rounds N` counts what `status` prints as `rounds=`, every round on every head, refusals included, so N is the `rounds=` a `status` read before a re-request. `wait` exits `40` when no Copilot review covers the head and the reviewer has said why in another shape. A plain comment newer than its newest review carries no commit, so it satisfies no coverage check whatever it says, and its body prints whole because its wording is the only thing separating a refusal from a remark. A comment older than the newest review is spent. Every connection reads the newest `WINDOW` nodes rather than the reviewer's own, so `window_blind` guards both sides: blind on comments reads as `answered_outside_review=unknown`, and blind on reviews reports nothing and lets the wait keep polling. A request that returns success and adds neither a pending reviewer nor a review-request event also exits `40`, as `status=REQUEST_NOT_RECORDED`, as does a reviewer writing under a renamed login, `status=REVIEWER_LOGIN_DRIFT`. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 1fa20559..06cba3a3 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -116,13 +116,13 @@ Exit 0 = review present, or on a held head an attested local pass whose checks no longer hold the poll. On an open pull request whose head commit the payload carries, every workflow run the read reaches on that head has then completed. The read reaches the - head's last 50 check suites and those of its first CHECKS_WINDOW rollup contexts. It - does not prove the required checks ran. - A payload with no commit matching the head, named `CHECKS UNREADABLE` in the digest, - reads no run and no check. A required check not yet registered holds only a BLOCKED - merge whose base branch rules were read and whose rollup is not `CHECKS TRUNCATED`. An - empty rollup holds only a BLOCKED, BEHIND, or DRAFT one. Short of those holds, a merge with - some check posted, or a DIRTY one with none, can exit 0 with a required check unregistered. + head's last 50 check suites and those of its first CHECKS_WINDOW rollup contexts. A + CLEAN, UNSTABLE, or HAS_HOOKS merge owes no required check. Under any other merge, an + open attested held head nothing holds exits 45 where its rollup was unreadable or none + posted. A DIRTY or BEHIND one also exits 45 where a required check is unregistered or + from another app. Exit 0 can then leave a required check unregistered only where the + rules were unread, the rollup is `CHECKS TRUNCATED`, or the merge reads DRAFT. A pull + request no longer open never exits 45. 30 = still pending at the timeout. On a held head it is any hold `status=CHECKS_PENDING` names, a required check still settling and a workflow run still open among them. It is also a round past --min-rounds not yet landed. @@ -140,6 +140,15 @@ is in a shape no wait clears. It is queued with nothing acting on it, expected and never posted, running far past the job's cost, held by its environment, or failed. On a held head a check running long or held by its environment polls to 30 instead. + 45 = an open attested held head that nothing holds, its merge other than CLEAN, + UNSTABLE, or HAS_HOOKS. Either the payload carries no commit matching the head, named + `CHECKS UNREADABLE` in the digest, or no check has posted. Or the merge reads DIRTY or + BEHIND and a check the base branch's rules require has not registered, or came from + another app. That last needs the rules read and a rollup read whole. + `status=CHECKS_MISSING` names which. A DIRTY merge runs no pull_request workflow until + its conflict is resolved. A required check not yet registered holds a BLOCKED merge to + 30 instead, where the rules were read and the rollup read whole. An empty rollup read + off the head holds a BLOCKED, BEHIND, or DRAFT one. 49 = no Copilot round covers this head and none was requested, the pull request merging into a branch other than the default with no attestation vouching for the head. 64 = the write scope could not @@ -181,6 +190,7 @@ _unmet_note, check_lines, check_nodes, + checks_never_ran, checks_settling, checks_stuck, checks_tally, @@ -664,9 +674,11 @@ def parsed_json(proc: subprocess.CompletedProcess) -> object: return None -def required_for(pr: dict, owner: str, repo: str) -> frozenset[tuple[str, int | None]]: - """The required entries a BLOCKED merge can be short of, empty wherever none can matter.""" - if pr.get("mergeStateStatus") != "BLOCKED": +def required_for( + pr: dict, owner: str, repo: str, gated: frozenset[str] = frozenset({"BLOCKED"}) +) -> frozenset[tuple[str, int | None]]: + """The required entries a merge in `gated` can be short of, empty wherever none can matter.""" + if pr.get("state", "OPEN") != "OPEN" or pr.get("mergeStateStatus") not in gated: return frozenset() return required_contexts(owner, repo, pr.get("baseRefName") or "") or frozenset() @@ -707,7 +719,7 @@ def required_contexts(owner: str, repo: str, base: str) -> frozenset[tuple[str, ): print( f"note: the rules for {base} were unread, so which checks they require is unknown and " - "this wait cannot hold for one that has not registered yet." + "this wait cannot judge one that has not registered." ) return None return frozenset((e[0], e[1]) for e in entries) @@ -2013,7 +2025,7 @@ def held_read() -> dict: "had not completed, a check the base branch requires had not registered or came " "from another app, or the merge was still UNKNOWN, so the merge is not ready yet: " "wait again, or read the checks above" - + _unmet_note(required_for(final, owner, repo), checks) + + _unmet_note(final, required_for(final, owner, repo), checks) + (f" (not completed: {', '.join(runs)})" if runs else "") ) return 30 @@ -2030,6 +2042,17 @@ def held_read() -> dict: "the merge, because BLOCKED is also worn by a thread or a missing approval" ) return 44 + if covered and ( + unrun := checks_never_ran( + final, checks, required_for(final, owner, repo, frozenset({"DIRTY", "BEHIND"})) + ) + ): + print( + "status=CHECKS_MISSING an attested local pass covers this head and nothing holds " + f"the poll, but {unrun}. The merge reads {final.get('mergeStateStatus')}, so it is " + "not ready to merge" + ) + return 45 return 0 # Every refusal and quota state is 41, the kind named by its status line. refusal = refusing_review(final) diff --git a/scripts/pr_review_checks.py b/scripts/pr_review_checks.py index bd044afd..94f602a0 100644 --- a/scripts/pr_review_checks.py +++ b/scripts/pr_review_checks.py @@ -20,6 +20,8 @@ # SKIPPED and NEUTRAL count as passed, since the fleet aggregator skips conditional jobs. CHECK_OK = frozenset({"SUCCESS", "SKIPPED", "NEUTRAL"}) +MERGE_UNGATED = frozenset({"CLEAN", "UNSTABLE", "HAS_HOOKS"}) + WORKFLOW_KEYS_UNAVAILABLE = "workflowKeysUnavailable" # How long a queued check is simply starting, the pickup grace's five minutes. @@ -283,23 +285,39 @@ def held_checks_open( return False if runs_open(pr): return True - if merge in ("CLEAN", "UNSTABLE", "HAS_HOOKS"): + if merge in MERGE_UNGATED: return False settling = checks_settling(nodes, now, grace, stall) if any(n.get("required") for n in settling): return True if settling and not any(n.get("required") for n in nodes): return True - if ( - merge == "BLOCKED" - and not checks_unreadable(pr) - and not checks_truncated(pr) - and required_unmet(required, nodes) - ): + if merge == "BLOCKED" and required_unregistered(pr, nodes, required): return True return not nodes and merge in ("BLOCKED", "BEHIND", "DRAFT") and not checks_unreadable(pr) +def checks_never_ran( + pr: dict, nodes: list[dict], required: frozenset[tuple[str, int | None]] +) -> str: + """Why an open head's gated merge lacks a check it is owed, with its remedy, or "" for none. + + A CLEAN, UNSTABLE, or HAS_HOOKS merge is owed nothing. The caller passes `required` only for a + merge whose release judges registration, and a context is judged only on a rollup read whole. + """ + if pr.get("state", "OPEN") != "OPEN" or pr.get("mergeStateStatus") in MERGE_UNGATED: + return "" + if checks_unreadable(pr): + return "the payload carries no commit matching the head, so read the check runs on the head commit" + if not nodes: + return "no check has posted, and a DIRTY merge runs no pull_request workflow until resolved" + unmet = ", ".join(required_unregistered(pr, nodes, required)) + return unmet and ( + f"a required check has not registered or came from another app, {unmet}, so update a " + "BEHIND head, resolve a DIRTY one's conflict, or correct the check's poster or its rule" + ) + + def runs_open(pr: dict) -> list[str]: """The head's workflow runs whose suite has not completed, each named `run `, sorted. @@ -322,8 +340,12 @@ def runs_open(pr: dict) -> list[str]: return sorted(named) -def required_unmet(required: frozenset[tuple[str, int | None]], nodes: list[dict]) -> list[str]: - """The contexts among `required` that no rollup node satisfies, sorted.""" +def required_unregistered( + pr: dict, nodes: list[dict], required: frozenset[tuple[str, int | None]] +) -> list[str]: + """The contexts among `required` no rollup node satisfies, sorted, on a rollup read whole.""" + if checks_unreadable(pr) or checks_truncated(pr): + return [] seen = [n for n in nodes if not n.get("unreadable")] return sorted( { @@ -337,9 +359,9 @@ def required_unmet(required: frozenset[tuple[str, int | None]], nodes: list[dict ) -def _unmet_note(required: frozenset[tuple[str, int | None]], nodes: list[dict]) -> str: +def _unmet_note(pr: dict, required: frozenset[tuple[str, int | None]], nodes: list[dict]) -> str: """Names the required contexts still unmet, so a reader sees what the hold waits on.""" - unmet = required_unmet(required, nodes) + unmet = required_unregistered(pr, nodes, required) return f" (required, unmet: {', '.join(unmet)})" if unmet else "" diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index a31a9cdb..b18bf024 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -3571,20 +3571,28 @@ def test_a_required_check_running_at_the_timeout_outranks_an_optional_failure(se self.assertIn("status=CHECKS_PENDING", self.out.getvalue()) def test_an_empty_rollup_holds_any_merge_but_a_conflicted_one(self) -> None: - """A conflicted pull request runs no `pull_request` workflow, so none is coming.""" + """A conflicted pull request runs no `pull_request` workflow, so none is coming. + + Its release names that state rather than exiting 0 over checks that never ran. + """ for merge, code in ( ("BLOCKED", 30), ("BEHIND", 30), ("DRAFT", 30), ("UNKNOWN", 30), - ("DIRTY", 0), + ("DIRTY", 45), ): with self.subTest(merge=merge): pr = self.into(payload([review(oid=OLD)], merge=merge, checks=[]), attest=True) self.answer(pr) self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) + self.out.seek(0) + self.out.truncate() with mock.patch.object(pr_review.time, "sleep"): self.assertEqual(code, self.cli(["wait", "7", "--timeout", "0"])) + if code == 45: + self.assertIn("status=CHECKS_MISSING", self.out.getvalue()) + self.assertIn("but no check has posted", self.out.getvalue()) def test_an_optional_check_does_not_hold_once_the_required_gate_concluded(self) -> None: """A merge BLOCKED on a thread is not waiting on a check nothing requires.""" @@ -3664,6 +3672,116 @@ def test_every_required_check_registered_does_not_hold_a_blocked_head(self) -> N self.assertNotEqual(30, self.cli(["wait", "7", "--timeout", "0"])) slept.assert_not_called() + def test_a_required_check_unregistered_under_a_released_merge_exits_45(self) -> None: + """A BEHIND or DIRTY merge is not held on an unregistered required check. + + Its release names the check rather than exiting 0 as though the required checks settled. + """ + self.rules("Check pull request workflow status job", "Later workflow") + for merge in ("BEHIND", "DIRTY"): + with self.subTest(merge=merge): + self.out.seek(0) + self.out.truncate() + pr = self.into( + payload([review(oid=OLD)], merge=merge, checks=[check()]), attest=True + ) + self.answer(pr) + self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) + with mock.patch.object(pr_review.time, "sleep") as slept: + self.assertEqual(45, self.cli(["wait", "7", "--timeout", "1"])) + slept.assert_not_called() + out = self.out.getvalue() + self.assertIn("status=CHECKS_MISSING", out) + self.assertIn(f"The merge reads {merge}", out) + self.assertIn("came from another app, Later workflow, so update a BEHIND head", out) + + def test_a_released_head_exits_by_what_its_read_shows(self) -> None: + """Another app's check, an unread payload, an ungated merge, and a closed pull request.""" + name = "Check pull request workflow status job" + other_app = json.dumps({"n": 1, "c": [[name, 999]]}) + unread = payload([review(oid=OLD)], merge="DIRTY", checks=[check()], rollup_oid="d" * 40) + closed = payload([review(oid=OLD)], merge="DIRTY", checks=[check()]) + cases = [ + ( + "another app", + payload([review(oid=OLD)], merge="BEHIND", checks=[check()]), + 45, + "came from another app, " + name, + ), + ("unread payload", unread, 45, "no commit matching the head"), + ("closed", {**closed, "state": "CLOSED"}, 0, ""), + ] + [ + (merge, payload([review(oid=OLD)], merge=merge, checks=[]), 0, "") + for merge in ("CLEAN", "UNSTABLE", "HAS_HOOKS") + ] + self.enterContext( + mock.patch.object( + pr_review, + "gh_rest", + return_value=subprocess.CompletedProcess([], 0, other_app, ""), + ) + ) + for label, pr, code, reason in cases: + with self.subTest(label): + pr_review.required_contexts.cache_clear() + self.out.seek(0) + self.out.truncate() + self.answer(self.into(pr, attest=True)) + self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) + with mock.patch.object(pr_review.time, "sleep"): + self.assertEqual(code, self.cli(["wait", "7", "--timeout", "1"])) + self.assertIn(reason, self.out.getvalue()) + if code == 0: + self.assertNotIn("status=CHECKS_MISSING", self.out.getvalue()) + + def test_a_head_no_attestation_covers_reads_no_rules_for_exit_45(self) -> None: + """A Copilot round covering the head ends the wait on its own verdict, judging no check.""" + self.answer(payload([review()], merge="DIRTY", checks=[])) + with mock.patch.object(pr_review, "required_contexts") as read: + self.assertEqual(0, self.cli(["wait", "7"])) + read.assert_not_called() + + def test_a_released_merge_exits_0_where_the_unregistered_check_is_unproven(self) -> None: + """Unread rules, a cut rollup, and a DRAFT merge each leave registration unjudged.""" + cut = self.into(payload([review(oid=OLD)], merge="BEHIND", checks=[check()]), attest=True) + cut["commits"]["nodes"][0]["commit"]["statusCheckRollup"]["contexts"]["pageInfo"] = { + "hasNextPage": True + } + cases = { + "unread": ( + self.into( + payload([review(oid=OLD)], merge="BEHIND", checks=[check()]), attest=True + ), + False, + ), + "truncated": (cut, True), + "draft": ( + self.into(payload([review(oid=OLD)], merge="DRAFT", checks=[check()]), attest=True), + True, + ), + } + for label, (pr, rules) in cases.items(): + with self.subTest(label): + pr_review.required_contexts.cache_clear() + if rules: + self.rules("Check pull request workflow status job", "Later workflow") + self.answer(pr) + self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) + with mock.patch.object(pr_review.time, "sleep"): + self.assertEqual(0, self.cli(["wait", "7", "--timeout", "1"])) + + def test_a_released_merge_with_every_required_check_registered_exits_0(self) -> None: + self.rules("Check pull request workflow status job") + for merge in ("BEHIND", "DIRTY"): + with self.subTest(merge=merge): + pr = self.into( + payload([review(oid=OLD)], merge=merge, checks=[check()]), attest=True + ) + self.answer(pr) + self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) + with mock.patch.object(pr_review.time, "sleep"): + self.assertEqual(0, self.cli(["wait", "7", "--timeout", "1"])) + def test_an_unread_rules_read_keeps_the_old_result_and_says_so(self) -> None: """The suite's default `gh_rest` fails, which must never read as nothing being required.""" pr = self.into(payload([review(oid=OLD)], merge="BLOCKED", checks=[check()]), attest=True) @@ -3932,13 +4050,18 @@ def test_a_required_check_running_long_holds_the_poll_rather_than_ending_it(self self.assertIn("status=CHECKS_PENDING", self.out.getvalue()) def test_an_unreadable_rollup_is_not_held_as_one_not_yet_registered(self) -> None: - """A rollup read off another commit is empty here, and waiting does not clear that.""" + """A rollup read off another commit is empty here, and waiting does not clear that. + + The release names the unread payload rather than exiting 0 over checks it never read. + """ pr = payload([review(oid=OLD)], merge="BLOCKED", checks=[check()], rollup_oid="d" * 40) self.answer(self.into(pr, attest=True)) self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) with mock.patch.object(pr_review.time, "sleep") as slept: - self.assertEqual(0, self.cli(["wait", "7", "--timeout", "1"])) + self.assertEqual(45, self.cli(["wait", "7", "--timeout", "1"])) slept.assert_not_called() + self.assertIn("status=CHECKS_MISSING", self.out.getvalue()) + self.assertIn("no commit matching the head", self.out.getvalue()) def test_an_outside_answer_outranks_pending_after_a_mid_poll_request(self) -> None: running = check(status="IN_PROGRESS", conclusion="", started=real_ago(60)) @@ -6072,6 +6195,32 @@ def test_blocked_with_a_required_context_absent_holds(self) -> None: clean = {**pr, "mergeStateStatus": "CLEAN"} self.assertFalse(pr_review_checks.held_checks_open(clean, nodes, now, 600, 3600, want)) + def test_checks_never_ran_judges_only_an_open_gated_merge(self) -> None: + pr = payload([], merge="DIRTY", checks=[]) + self.assertIn("no check has posted", pr_review_checks.checks_never_ran(pr, [], frozenset())) + nodes = pr_review_checks.check_nodes(payload([], merge="DIRTY", checks=[check()])) + self.assertEqual("", pr_review_checks.checks_never_ran(pr, nodes, frozenset())) + later = pr_review_checks.checks_never_ran(pr, nodes, frozenset({("Later", None)})) + self.assertIn("has not registered or came from another app, Later", later) + for ungated in ({**pr, "mergeStateStatus": "CLEAN"}, {**pr, "state": "MERGED"}): + self.assertEqual("", pr_review_checks.checks_never_ran(ungated, [], frozenset())) + other = payload([], merge="BLOCKED", checks=[check()], rollup_oid="f" * 40) + self.assertIn( + "no commit matching", pr_review_checks.checks_never_ran(other, [], frozenset()) + ) + + def test_required_unregistered_judges_only_a_rollup_read_whole(self) -> None: + """A cut rollup can hold the absent context past its window, so it proves nothing.""" + want = frozenset({("Later", None)}) + pr = payload([], merge="BEHIND", checks=[check()]) + nodes = pr_review_checks.check_nodes(pr) + self.assertEqual(["Later"], pr_review_checks.required_unregistered(pr, nodes, want)) + self.assertEqual([], pr_review_checks.required_unregistered(pr, nodes, frozenset())) + pr["commits"]["nodes"][0]["commit"]["statusCheckRollup"]["contexts"]["pageInfo"] = { + "hasNextPage": True + } + self.assertEqual([], pr_review_checks.required_unregistered(pr, nodes, want)) + def test_a_rollup_not_read_whole_never_holds_on_an_absent_context(self) -> None: now = datetime.now(UTC) want = frozenset({("Later", None)}) @@ -6124,11 +6273,13 @@ def test_an_unbound_entry_is_met_by_a_same_name_node_of_either_type(self) -> Non self.assertFalse(self.held([status_context("Gate")], ("Gate", None))) def test_the_unmet_context_is_named(self) -> None: - nodes = pr_review_checks.check_nodes( - payload([], merge="BLOCKED", checks=[check(name="Gate")]) - ) - self.assertEqual("", pr_review_checks._unmet_note(frozenset({("Gate", None)}), nodes)) - self.assertIn("Gate", pr_review_checks._unmet_note(frozenset({("Gate", 9)}), nodes)) + pr = payload([], merge="BLOCKED", checks=[check(name="Gate")]) + nodes = pr_review_checks.check_nodes(pr) + self.assertEqual("", pr_review_checks._unmet_note(pr, frozenset({("Gate", None)}), nodes)) + self.assertIn("Gate", pr_review_checks._unmet_note(pr, frozenset({("Gate", 9)}), nodes)) + rollup = pr["commits"]["nodes"][0]["commit"]["statusCheckRollup"] + rollup["contexts"]["pageInfo"] = {"hasNextPage": True} + self.assertEqual("", pr_review_checks._unmet_note(pr, frozenset({("Gate", 9)}), nodes)) def test_the_rules_are_read_only_for_a_blocked_merge(self) -> None: with mock.patch.object(pr_review, "gh_rest") as rest: @@ -6136,6 +6287,16 @@ def test_the_rules_are_read_only_for_a_blocked_merge(self) -> None: self.assertEqual(frozenset(), pr_review.required_for(clean, "o", "r")) rest.assert_not_called() + def test_the_rules_for_exit_45_are_read_only_for_a_dirty_or_behind_merge(self) -> None: + gated = frozenset({"DIRTY", "BEHIND"}) + with mock.patch.object(pr_review, "gh_rest") as rest: + for merge in ("CLEAN", "UNSTABLE", "HAS_HOOKS", "BLOCKED", "DRAFT"): + pr = payload([], merge=merge, checks=[check()]) + self.assertEqual(frozenset(), pr_review.required_for(pr, "o", "r", gated)) + closed = {**payload([], merge="DIRTY", checks=[check()]), "state": "CLOSED"} + self.assertEqual(frozenset(), pr_review.required_for(closed, "o", "r", gated)) + rest.assert_not_called() + class TestRunsOpen(unittest.TestCase): """A head's workflow run that has not completed holds a held wait whatever the merge reads."""