diff --git a/tools/merge-guard.py b/tools/merge-guard.py index ebabc66..b71b164 100644 --- a/tools/merge-guard.py +++ b/tools/merge-guard.py @@ -207,13 +207,26 @@ def leg(name, ok, detail): detail += f" ⛔ net-negative in {len(losers)} file(s): {top}{more}" leg("4 three-dot diff", add >= dele, detail) - created = d.get("createdAt") or "" - try: - age = (datetime.now(timezone.utc) - - datetime.fromisoformat(created.replace("Z", "+00:00"))).total_seconds() - leg("5 age at merge", age >= 120, f"{int(age)}s since creation") - except Exception: - leg("5 age at merge", False, f"UNESTABLISHED — unparseable createdAt {created!r}") + # ⛔ --shape-only OMITS LEG 5 TOO — the THIRD merge-time leg in this tool, and the one I + # left behind after fixing the other two. Measured on PR #599: + # ⛔ 5 age at merge 9s since creation -> the advisory job went red + # A workflow triggered BY the PR's creation observes an age of seconds BY CONSTRUCTION, + # and it is not merging, so "was this old enough at merge?" is not a question it can ask. + # ⇒ Legs 0, 2 and 5 are facts about THE MERGE EVENT; only 1, 3 and 4 are facts about the + # PR's SHAPE. ⚠ The name was right from the start and the implementation kept not catching + # up: I fixed leg 0, then leg 2, and each time treated it as a one-off instead of reading + # the remaining legs for the same property. Three instances is a class, not a coincidence. + if shape_only: + leg("5 age at merge", True, "⚠ SKIPPED — --shape-only. A run triggered BY this PR " + "cannot judge its age at a merge that has not happened.") + else: + created = d.get("createdAt") or "" + try: + age = (datetime.now(timezone.utc) + - datetime.fromisoformat(created.replace("Z", "+00:00"))).total_seconds() + leg("5 age at merge", age >= 120, f"{int(age)}s since creation") + except Exception: + leg("5 age at merge", False, f"UNESTABLISHED — unparseable createdAt {created!r}") return legs diff --git a/tools/test_merge_guard.py b/tools/test_merge_guard.py index f662d39..7af042c 100644 --- a/tools/test_merge_guard.py +++ b/tools/test_merge_guard.py @@ -15,6 +15,7 @@ import importlib.util import io import sys +from datetime import datetime, timezone import unittest from contextlib import redirect_stderr, redirect_stdout from pathlib import Path @@ -32,6 +33,9 @@ def load(): return mod +NOW_ISO = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + + def prd(base="main", gate="SUCCESS", reviews=None, state="OPEN", created="2020-01-01T00:00:00Z", sha="deadbee"): return { @@ -222,6 +226,24 @@ def test_shape_only_SKIPS_leg0_and_says_so(self): self.assertIn("SKIPPED", out) self.assertIn("establishes\nNOTHING about who may merge".replace("\n", " "), out) + def test_shape_only_SKIPS_leg5_because_a_NEW_pr_is_seconds_old(self): + """⛔ THE KNOWN-POSITIVE, measured on PR #599: `5 age at merge 9s since creation`. + + A workflow triggered by the PR's own creation sees an age of seconds by construction. + It is not merging, so it cannot ask whether the PR was old enough AT MERGE. This is the + third merge-time leg in this tool (0 holder, 2 gate, 5 age) and the last to be fixed.""" + rc, out, _ = drive(self.mod, prd(created=NOW_ISO), ["1", "--shape-only"], session=OTHER) + self.assertEqual(rc, 0, "a seconds-old PR must not fail an ADVISORY shape check") + self.assertIn("a merge that has not happened", out) + + def test_the_SAME_new_pr_is_still_BLOCKED_from_a_real_merge(self): + """★ THE KNOWN-NEGATIVE. Identical input, --shape-only removed. #224 measured 25 of 100 + PRs merged inside 60s of creation; that guard must survive this exemption intact.""" + rc, out, _ = drive(self.mod, prd(created=NOW_ISO), ["1"]) + self.assertEqual(rc, 1, "a seconds-old PR is still BLOCKED from merging") + self.assertIn("5 age at merge", out) + self.assertNotIn("a merge that has not happened", out) + def test_shape_only_does_NOT_need_the_authority_file(self): """⛔ Found by CodeRabbit reviewing this PR, confirmed by measurement first.