diff --git a/.github/scripts/pull-request-dashboard/author_nudge.py b/.github/scripts/pull-request-dashboard/author_nudge.py index 4be719cb44a..990010f8587 100644 --- a/.github/scripts/pull-request-dashboard/author_nudge.py +++ b/.github/scripts/pull-request-dashboard/author_nudge.py @@ -16,7 +16,6 @@ run_gh, ) from dashboard_override import author_override_guidance -from dashboard_override import DASHBOARD_OVERRIDE_LABEL from pr_status_comment import ( DASHBOARD_APP_SLUG, managed_status_comments, @@ -50,12 +49,6 @@ def routing_inputs(raw: dict[str, Any]) -> dict[str, Any]: "base_branch": str(pr.get("baseRefName") or ""), "checks": raw.get("checks"), "issue_comments": issue_comments, - "labels": sorted( - label.get("name") or "" - for label in pr.get("labels") or [] - if isinstance(label, dict) - and label.get("name") == DASHBOARD_OVERRIDE_LABEL - ), "pr_text": { "body": str(pr.get("body") or "").replace("\r\n", "\n"), "title": str(pr.get("title") or ""), diff --git a/.github/scripts/pull-request-dashboard/dashboard.py b/.github/scripts/pull-request-dashboard/dashboard.py index ec0f4c2d6ea..ca30223924b 100644 --- a/.github/scripts/pull-request-dashboard/dashboard.py +++ b/.github/scripts/pull-request-dashboard/dashboard.py @@ -109,6 +109,10 @@ fetched. ci_failing_since str (iso) Earliest completion time among current required failures. + ci_uncleared_failing_count int Required failures an override + command has not cleared. + ci_uncleared_failing_since str (iso) Earliest completion time among + those uncleared failures. ci_pending_count int Merge-blocking checks only; absent when checks could not be fetched. @@ -196,10 +200,11 @@ record_copilot_review_observation, ) from dashboard_override import ( - apply_dashboard_override, - append_route_noop_reply, + append_command_ack_reply, + clear_overridden_actions, dashboard_command_body_remainder, dashboard_override_facts, + uncleared_ci_failing_count, ) from pr_status_comment import status_author_nudge_episode_id from state import ( @@ -542,17 +547,12 @@ def compute_facts( raw.get("reviews") or [], head_sha, ) - labels = { - label.get("name") or "" - for label in pr.get("labels") or [] - if isinstance(label, dict) - } facts = { "author": author, "assignees": assignees, "head_sha": head_sha, "routing_input_fingerprint": routing_input_fingerprint(raw), - **dashboard_override_facts(raw, author, labels, reviewers or set()), + **dashboard_override_facts(raw, author, reviewers or set()), "copilot_review_requested": any( is_copilot_reviewer(request) for request in (raw.get("review_requests") or []) @@ -572,6 +572,19 @@ def compute_facts( facts["ci_failing_count"] = len(failing) if failing_timestamps: facts["ci_failing_since"] = format_ts(min(failing_timestamps)) + # A failure with no completion time cannot be shown to predate the + # override command, so it counts as uncleared. So does one that shares + # the command's second, since GitHub timestamps cannot order them. + untimed = len(failing) - len(failing_timestamps) + override_since = parse_ts(facts.get("dashboard_override_since") or "") + uncleared = [ + ts + for ts in failing_timestamps + if override_since is None or ts >= override_since + ] + facts["ci_uncleared_failing_count"] = untimed + len(uncleared) + if uncleared: + facts["ci_uncleared_failing_since"] = format_ts(min(uncleared)) facts["ci_pending_count"] = len(pending) non_blocking_check_failures = sorted({ check.get("name") or "" @@ -1114,12 +1127,13 @@ def route_pr(facts: dict[str, Any], pending_actions: dict[str, dict[str, Any]], is_maintenance_bot = facts.get("is_maintenance_bot") approval_threshold = 1 if is_maintenance_bot else required_approvals # Precedence: - # 1. A required status check failure -> "author". + # 1. A required status check failure the author has not overridden -> "author". # 2. A discussion waiting on the author -> "author". # 3. If there are enough approvals and no inline or top-level feedback is # still waiting on a reviewer -> "maintainer". # 4. Otherwise the PR is still waiting on approvers. - if facts.get("ci_failing_count", 0) > 0 and not is_maintenance_bot: + ci_failing = uncleared_ci_failing_count(facts) > 0 + if ci_failing and not is_maintenance_bot: return "author" if counts["author"] and not is_maintenance_bot: return "author" @@ -1149,8 +1163,8 @@ def fallback_wait_ts(route: str, facts: dict[str, Any]) -> tuple[datetime | None if route in ("approver", "maintainer", "copilot"): return parse_ts(facts.get("last_author_activity_at") or ""), "last_author_activity" if route == "author": - if facts.get("ci_failing_count", 0) > 0: - ci_failing_since = parse_ts(facts.get("ci_failing_since") or "") + if uncleared_ci_failing_count(facts) > 0: + ci_failing_since = parse_ts(facts.get("ci_uncleared_failing_since") or "") if ci_failing_since is not None: return ci_failing_since, "ci_failure" return parse_ts(facts.get("last_author_activity_at") or ""), "last_author_activity" @@ -1288,15 +1302,9 @@ def resolve_pr_route( required_approvals: int, require_clean_copilot_review: bool, ) -> str: - # Apply the manual reviewer-routing override before the Copilot review gate - # so an overridden route (for example author -> reviewers) is still held for - # a required clean Copilot review instead of bypassing it. return apply_copilot_review_gate( facts, - apply_dashboard_override( - facts, - route_pr(facts, pending_actions, required_approvals), - ), + route_pr(facts, pending_actions, required_approvals), enabled=require_clean_copilot_review, ) @@ -1391,6 +1399,7 @@ def build_pr_result( author_comment_source_state, ) pending_actions = review_thread_pending_actions | top_level_pending_actions + pending_actions = clear_overridden_actions(facts, pending_actions) failed_classifications = [ classification for classification in ( @@ -1433,7 +1442,7 @@ def build_pr_result( previous_result, raw.get("issue_comments") or [], ) - append_route_noop_reply(raw, facts, route) + append_command_ack_reply(raw, facts, route) add_wait_age_facts(facts, route, pending_actions) facts["author_action_review_thread_urls"] = author_action_discussion_urls( review_threads, pending_actions diff --git a/.github/scripts/pull-request-dashboard/dashboard_override.py b/.github/scripts/pull-request-dashboard/dashboard_override.py index d9b383dd8de..3bc2a502288 100644 --- a/.github/scripts/pull-request-dashboard/dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/dashboard_override.py @@ -4,19 +4,15 @@ import re from typing import Any -from urllib.parse import quote from github_cli import gh_api, run_gh from state import load_dashboard_state_cache -from utils import actor_login +from utils import actor_login, parse_ts -DASHBOARD_OVERRIDE_LABEL = "dashboard:route-overridden" DASHBOARD_COMMAND_PREFIX = "/dashboard" DASHBOARD_OVERRIDE_COMMAND = "/dashboard route:reviewers" DASHBOARD_OVERRIDE_SUBCOMMAND = "route:reviewers" -DASHBOARD_OVERRIDE_LABEL_COLOR = "1D76DB" -DASHBOARD_OVERRIDE_LABEL_DESCRIPTION = "Routing manually overridden to reviewers" # Mirrors pr_status_comment.DASHBOARD_APP_SLUG; duplicated here to avoid a # circular import between the two modules. DASHBOARD_APP_SLUG = "opentelemetry-pr-dashboard" @@ -29,7 +25,6 @@ r"" ) PRE_REVIEW_ROUTES = ("author",) -REVIEWERS_OR_LATER_ROUTES = ("approver", "maintainer") def author_override_guidance(staleness_note: str = "") -> str: @@ -105,22 +100,50 @@ def latest_authorized_command( return best_id, best_user +def latest_authorized_command_at( + raw: dict[str, Any], + author: str, + reviewers: set[str] | None, +) -> str: + """Timestamp of the newest authorized override command, acknowledged or not. + + Unlike `latest_authorized_command`, acknowledged commands still count: the + watermark has to outlive the acknowledgement, or the discussions a command + cleared would come back on the next refresh. An acknowledgement is also + treated as durable proof of authorization, since approver-team membership is + resolved live and an approver can leave the team after commanding. + """ + acknowledged_ids = _acknowledged_override_command_ids(raw.get("issue_comments")) + latest = "" + for comment in raw.get("issue_comments") or []: + if parse_dashboard_command(comment) != DASHBOARD_OVERRIDE_SUBCOMMAND: + continue + try: + comment_id = int(comment.get("id")) + except (TypeError, ValueError): + comment_id = 0 + if comment_id not in acknowledged_ids and not is_authorized_commander( + actor_login(comment.get("user") or {}), author, reviewers + ): + continue + created_at = comment.get("created_at") or "" + if created_at > latest: + latest = created_at + return latest + + def dashboard_override_facts( raw: dict[str, Any], author: str, - labels: set[str], reviewers: set[str] | None = None, ) -> dict[str, Any]: - label_applied = DASHBOARD_OVERRIDE_LABEL in labels command_id, command_user = latest_authorized_command(raw, author, reviewers) - command_pending = bool(command_id) return { - "dashboard_override": label_applied, - "dashboard_override_label_applied": label_applied, "dashboard_override_command_id": command_id, "dashboard_override_command_user": command_user, - "dashboard_override_requested": command_pending and not label_applied, - "dashboard_override_release_requested": False, + "dashboard_override_since": latest_authorized_command_at( + raw, author, reviewers + ), "dashboard_command_replies": pending_command_replies(raw, author, reviewers), } @@ -152,16 +175,27 @@ def _replied_command_ids(comments: list[dict[str, Any]] | None) -> set[int]: return replied_ids -def _acknowledged_override_command_id( +def _acknowledged_override_command_ids( comments: list[dict[str, Any]] | None, -) -> int: - acknowledged_id = 0 +) -> set[int]: + """Command ids the dashboard app has already acknowledged. + + Ack markers are matched only in comments authored by the dashboard app, so a + user cannot forge one to authorize their own command. + """ + acknowledged_ids: set[int] = set() for comment in comments or []: if not _is_dashboard_app_comment(comment): continue for match in _OVERRIDE_ACK_MARKER_RE.findall(comment.get("body") or ""): - acknowledged_id = max(acknowledged_id, int(match)) - return acknowledged_id + acknowledged_ids.add(int(match)) + return acknowledged_ids + + +def _acknowledged_override_command_id( + comments: list[dict[str, Any]] | None, +) -> int: + return max(_acknowledged_override_command_ids(comments), default=0) def pending_command_replies( @@ -231,23 +265,34 @@ def render_command_reply(reply: dict[str, Any]) -> str: "only the pull request author or a member of an approving team can " "use `/dashboard route:reviewers`." ) - elif kind == "routed": - if reply.get("route") == "copilot": + elif kind in ("routed", "already_routed"): + route = reply.get("route") or "" + if route in PRE_REVIEW_ROUTES: + message = ( + "everything still open on this pull request arrived after your " + "`/dashboard route:reviewers` command, so it is still waiting " + "on you." + ) + elif kind == "already_routed": + where = ROUTE_ALREADY_ROUTED_PHRASE.get( + route, "not currently waiting on you" + ) + message = ( + f"this pull request is {where}, so `/dashboard route:reviewers` had " + "no effect." + ) + elif route == "copilot": message = ( "accepted the reviewer-routing override; the reviewer handoff " "is waiting on Copilot." ) + elif route == "maintainer": + message = ( + "accepted the reviewer-routing override; this pull request has " + "the approvals it needs and is now waiting on maintainers." + ) else: message = "routed this pull request to reviewers." - elif kind == "already_routed": - where = ROUTE_ALREADY_ROUTED_PHRASE.get( - reply.get("route") or "", "not currently waiting on you" - ) - message = ( - f"this pull request is {where}, so `/dashboard route:reviewers` had " - "no effect. The command only applies while the pull request is " - "waiting on you." - ) else: subcommand = reply.get("subcommand") or "" attempted = DASHBOARD_COMMAND_PREFIX + (f" {subcommand}" if subcommand else "") @@ -280,24 +325,6 @@ def command_reply_exists( ) -def ensure_command_reply( - repo: str, - pr_number: int, - reply: dict[str, Any], -) -> None: - comments = gh_api( - f"/repos/{repo}/issues/{pr_number}/comments?per_page=100", - paginate=True, - ) - if command_reply_exists(comments, int(reply["comment_id"])): - return - run_gh([ - "gh", "api", "--method", "POST", - f"repos/{repo}/issues/{pr_number}/comments", - "-f", f"body={render_command_reply(reply)}", - ]) - - def deliver_dashboard_command_replies(repo: str) -> list[str]: dashboard_state = load_dashboard_state_cache() if dashboard_state is None: @@ -333,108 +360,74 @@ def deliver_dashboard_command_replies(repo: str) -> list[str]: return errors -def apply_dashboard_override(facts: dict[str, Any], route: str) -> str: - label_applied = bool(facts.get("dashboard_override_label_applied")) - requested = bool(facts.get("dashboard_override_requested")) - command_pending = bool(facts.get("dashboard_override_command_id")) - # The override only takes effect before review, while automatic routing waits - # on the author. On every later route the natural routing stands. - override_applies = route in PRE_REVIEW_ROUTES and (label_applied or requested) - # A command that does not newly move the pull request to reviewers is a no-op; - # the author is told where it is routed. This covers both a non-overridable - # route and an existing label that already provides the reviewer handoff. - facts["dashboard_override_noop"] = command_pending and ( - label_applied or not override_applies - ) - if requested and not override_applies: - facts["dashboard_override_requested"] = False - facts["dashboard_override"] = override_applies - # Release the label once automatic routing reaches or passes reviewers, so a - # forgotten override cannot pin the pull request at reviewers or drag it back - # from maintainers. - facts["dashboard_override_release_requested"] = ( - label_applied and route in REVIEWERS_OR_LATER_ROUTES - ) - return "approver" if override_applies else route +def clear_overridden_actions( + facts: dict[str, Any], + pending_actions: dict[str, dict[str, Any]], +) -> dict[str, dict[str, Any]]: + """Drop the author actions an override command already answered. + + `/dashboard route:reviewers` is the author saying everything open at that + moment is handled. Clearing those actions keeps them from routing the pull + request back to the author on every later refresh, which would make the + author repeat the command each time review comes back around. Anything that + happens after the command keeps its author action, so new feedback still + reaches the author, including a reviewer reopening something the command + cleared. + """ + # Timestamps reach here in both GitHub's `...Z` form and `format_ts`'s + # `...+00:00` form, so they have to be parsed rather than compared as text. + override_since = parse_ts(facts.get("dashboard_override_since")) + facts["dashboard_override_cleared_count"] = 0 + facts["dashboard_override_cleared_ci"] = False + if override_since is None: + return pending_actions + remaining: dict[str, dict[str, Any]] = {} + cleared = 0 + for discussion_id, entry in pending_actions.items(): + since = parse_ts(entry.get("since")) + # GitHub timestamps are second-granularity, so an item sharing the + # command's second is left open rather than risking masking it. + if entry.get("action") == "author" and since and since < override_since: + cleared += 1 + continue + remaining[discussion_id] = entry + ci_failing_count = facts.get("ci_failing_count") or 0 + facts["dashboard_override_cleared_count"] = cleared + facts["dashboard_override_cleared_ci"] = uncleared_ci_failing_count(facts) < ci_failing_count + return remaining + +def uncleared_ci_failing_count(facts: dict[str, Any]) -> int: + """Failing required checks that an override command has not cleared.""" + return facts.get("ci_uncleared_failing_count") or 0 -def append_route_noop_reply( + +def append_command_ack_reply( raw: dict[str, Any], facts: dict[str, Any], route: str, ) -> None: - if not facts.get("dashboard_override_noop"): - return + """Queue the reply that acknowledges an override command. + + The reply carries the acknowledgement marker that stops the command from + being processed again, so every command gets one, whether or not it cleared + anything. `route` is already what the cleared items produced, so a command + that cleared nothing is answered with where the pull request is routed. + """ command_id = int(facts.get("dashboard_override_command_id") or 0) if not command_id: return - replied_ids = _replied_command_ids(raw.get("issue_comments") or []) - if command_id in replied_ids: + if command_id in _replied_command_ids(raw.get("issue_comments") or []): return replies = facts.setdefault("dashboard_command_replies", []) if any(reply.get("comment_id") == command_id for reply in replies): return + cleared = bool(facts.get("dashboard_override_cleared_count")) or bool( + facts.get("dashboard_override_cleared_ci") + ) replies.append({ "comment_id": command_id, - "kind": "already_routed", + "kind": "routed" if cleared else "already_routed", "user": facts.get("dashboard_override_command_user") or facts.get("author") or "", "route": route, - }) - - -def deliver_dashboard_override_requests(repo: str) -> list[str]: - dashboard_state = load_dashboard_state_cache() - if dashboard_state is None: - return [] - pr_results = sorted( - (dashboard_state.get("prs") or {}).items(), - key=lambda item: int(item[0]), - ) - if any( - ((result or {}).get("facts") or {}).get("dashboard_override_requested") - for _key, result in pr_results - ): - try: - run_gh([ - "gh", "label", "create", DASHBOARD_OVERRIDE_LABEL, - "--repo", repo, - "--color", DASHBOARD_OVERRIDE_LABEL_COLOR, - "--description", DASHBOARD_OVERRIDE_LABEL_DESCRIPTION, - "--force", - ]) - except Exception as e: - return [f"label: {e}"] - - errors: list[str] = [] - for key, result in pr_results: - facts = (result or {}).get("facts") or {} - pr_number = int(key) - if facts.get("dashboard_override_requested"): - try: - run_gh([ - "gh", "api", "--method", "POST", - f"repos/{repo}/issues/{pr_number}/labels", - "-f", f"labels[]={DASHBOARD_OVERRIDE_LABEL}", - ]) - ensure_command_reply( - repo, - pr_number, - { - "comment_id": facts["dashboard_override_command_id"], - "kind": "routed", - "route": (result or {}).get("route") or "", - "user": facts.get("dashboard_override_command_user") or "", - }, - ) - except Exception as e: - errors.append(f"PR #{pr_number}: {e}") - elif facts.get("dashboard_override_release_requested"): - try: - run_gh([ - "gh", "api", "--method", "DELETE", - f"repos/{repo}/issues/{pr_number}/labels/{quote(DASHBOARD_OVERRIDE_LABEL)}", - ]) - except Exception as e: - if "404" not in str(e): - errors.append(f"PR #{pr_number}: {e}") - return errors \ No newline at end of file + }) \ No newline at end of file diff --git a/.github/scripts/pull-request-dashboard/delivery.py b/.github/scripts/pull-request-dashboard/delivery.py index 642d963f4cb..ed5d5c6620f 100644 --- a/.github/scripts/pull-request-dashboard/delivery.py +++ b/.github/scripts/pull-request-dashboard/delivery.py @@ -12,10 +12,7 @@ from author_nudge import deliver_prepared_author_nudges from copilot_review import deliver_copilot_review_requests -from dashboard_override import ( - deliver_dashboard_command_replies, - deliver_dashboard_override_requests, -) +from dashboard_override import deliver_dashboard_command_replies from github_cli import detect_repo, gh_api, list_open_prs, normalize_repo, repo_state_key from notify_slack import notify_slack_from_state from pr_status_comment import ( @@ -74,11 +71,6 @@ def deliver_from_state( except Exception as e: errors.append(f"open pull requests: {e}") open_prs = None - run_delivery_action( - "dashboard overrides", - lambda: deliver_dashboard_override_requests(repo), - errors, - ) run_delivery_action( "dashboard command replies", lambda: deliver_dashboard_command_replies(repo), diff --git a/.github/scripts/pull-request-dashboard/pr_status_comment.py b/.github/scripts/pull-request-dashboard/pr_status_comment.py index 96220394788..486aa56d7dc 100644 --- a/.github/scripts/pull-request-dashboard/pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/pr_status_comment.py @@ -12,7 +12,7 @@ gh_api, run_gh, ) -from dashboard_override import PRE_REVIEW_ROUTES +from dashboard_override import PRE_REVIEW_ROUTES, uncleared_ci_failing_count from route_presentation import route_status_summary, status_headline from state import ( STATUS_COMMENT_REVISION, @@ -221,7 +221,7 @@ def render_status_comment( review_thread_urls = facts.get("author_action_review_thread_urls") or [] top_level_feedback_urls = facts.get("author_action_top_level_feedback_urls") or [] feedback_count = len(review_thread_urls) + len(top_level_feedback_urls) - failing_count = facts.get("ci_failing_count", 0) + failing_count = uncleared_ci_failing_count(facts) non_blocking_check_failures = facts.get("non_blocking_check_failures") or [] override_route = "" diff --git a/.github/scripts/pull-request-dashboard/render.py b/.github/scripts/pull-request-dashboard/render.py index c0a5428f833..e5c1306f6bc 100644 --- a/.github/scripts/pull-request-dashboard/render.py +++ b/.github/scripts/pull-request-dashboard/render.py @@ -8,11 +8,6 @@ from utils import actor_login, activity_age, markdown_escape, parse_ts, seconds_since -# Mirrors dashboard_override.DASHBOARD_OVERRIDE_LABEL; duplicated here to keep -# rendering free of that module's GitHub CLI dependencies. -DASHBOARD_OVERRIDE_LABEL = "dashboard:route-overridden" - - def _limit_rows(rows: list[Any], max_rows: int | None) -> tuple[list[Any], int]: if max_rows is None or max_rows <= 0 or len(rows) <= max_rows: return rows, 0 @@ -31,7 +26,7 @@ def _pr_cell_text( number = pr["number"] title = markdown_escape(pr.get("title", "")) pr_cell = f"#{number} {title}" - patterns = [*(labels_to_display or []), DASHBOARD_OVERRIDE_LABEL] + patterns = labels_to_display or [] matched_labels: list[str] = [] seen: set[str] = set() diff --git a/.github/scripts/pull-request-dashboard/state.py b/.github/scripts/pull-request-dashboard/state.py index d386fd59e06..25e483884d3 100644 --- a/.github/scripts/pull-request-dashboard/state.py +++ b/.github/scripts/pull-request-dashboard/state.py @@ -23,7 +23,7 @@ # current vector, ordinary state loaders may regenerate mismatched disposable # caches. Every constant ending in _STATE_VERSION or _REVISION is included. # dashboard-state.json: accepted PR routing results and backfill readiness. -DASHBOARD_STATE_VERSION = 6 +DASHBOARD_STATE_VERSION = 7 # backfill-state.json: round-robin cursor used by full dashboard refreshes. BACKFILL_STATE_VERSION = 3 # notification-state.json: pending and delivered Slack notification records. @@ -36,7 +36,7 @@ STATUS_COMMENT_ROLLOUT_STATE_VERSION = 1 # Rendered status-comment behavior. Increment when existing comments need to # adopt a change; hourly runs durably roll it out to all open PRs. -STATUS_COMMENT_REVISION = 14 +STATUS_COMMENT_REVISION = 15 INITIAL_BACKFILL_COMPLETE_KEY = "initial_backfill_complete" _state_dir: Path | None = None diff --git a/.github/scripts/pull-request-dashboard/test_author_nudge.py b/.github/scripts/pull-request-dashboard/test_author_nudge.py index eabb2ecad48..29146f5209e 100644 --- a/.github/scripts/pull-request-dashboard/test_author_nudge.py +++ b/.github/scripts/pull-request-dashboard/test_author_nudge.py @@ -47,27 +47,6 @@ def test_routing_fingerprint_ignores_dashboard_comments_and_tracks_author_activi }) self.assertNotEqual(baseline, author_nudge.routing_input_fingerprint(raw)) - def test_routing_fingerprint_tracks_normalized_labels(self) -> None: - raw = { - "checks": [], - "issue_comments": [], - "pr": {"labels": [{"name": "needs-triage"}]}, - "review_comments": [], - "reviews": [], - "review_threads": [], - } - baseline = author_nudge.routing_input_fingerprint(raw) - raw["pr"]["labels"].append({"name": "documentation"}) - - self.assertEqual(baseline, author_nudge.routing_input_fingerprint(raw)) - - raw["pr"]["labels"].append({"name": "dashboard:route-overridden"}) - overridden = author_nudge.routing_input_fingerprint(raw) - - self.assertNotEqual(baseline, overridden) - raw["pr"]["labels"].reverse() - self.assertEqual(overridden, author_nudge.routing_input_fingerprint(raw)) - def test_routing_fingerprint_tracks_required_check_state(self) -> None: raw = { "checks": [{"name": "build", "bucket": "fail"}], @@ -140,10 +119,7 @@ def test_fetch_current_routing_state_includes_required_checks( "baseRefName": "main", "body": "Current body", "headRefOid": "current-head", - "labels": [ - {"name": "needs-triage"}, - {"name": "dashboard:route-overridden"}, - ], + "labels": [{"name": "needs-triage"}], "title": "Current title", } gh_pr_view.return_value = pr diff --git a/.github/scripts/pull-request-dashboard/test_copilot_review.py b/.github/scripts/pull-request-dashboard/test_copilot_review.py index 8b04aec909e..820303a0a81 100644 --- a/.github/scripts/pull-request-dashboard/test_copilot_review.py +++ b/.github/scripts/pull-request-dashboard/test_copilot_review.py @@ -334,7 +334,6 @@ def test_drops_request_when_live_routing_inputs_changed( "base_branch", "checks", "issue_comments", - "labels", "pr_text", "review_comments", "reviews", @@ -459,7 +458,6 @@ def test_fingerprint_mismatch_reports_component_digests(self) -> None: "base_branch", "checks", "issue_comments", - "labels", "pr_text", "review_comments", "reviews", diff --git a/.github/scripts/pull-request-dashboard/test_dashboard.py b/.github/scripts/pull-request-dashboard/test_dashboard.py index 7a22edec160..587bf34c71a 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard.py @@ -31,18 +31,18 @@ class ResolvePrRouteTest(unittest.TestCase): - def _author_route_facts(self, **overrides: object) -> dict[str, object]: - # A failing required check routes a human-authored PR to the author. + def _cleared_ci_facts(self, **overrides: object) -> dict[str, object]: + # A failing required check that an override already cleared, which + # would otherwise route a human-authored pull request to the author. facts: dict[str, object] = { "ci_failing_count": 1, - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, + "ci_uncleared_failing_count": 0, } facts.update(overrides) return facts def test_override_is_still_gated_by_required_copilot_review(self) -> None: - facts = self._author_route_facts( + facts = self._cleared_ci_facts( copilot_review_exists=True, copilot_review_needed=True, copilot_review_requested=False, @@ -53,7 +53,7 @@ def test_override_is_still_gated_by_required_copilot_review(self) -> None: self.assertEqual("copilot", route) def test_override_reaches_reviewers_when_copilot_review_is_clean(self) -> None: - facts = self._author_route_facts( + facts = self._cleared_ci_facts( copilot_review_exists=True, copilot_review_needed=False, ) @@ -63,7 +63,7 @@ def test_override_reaches_reviewers_when_copilot_review_is_clean(self) -> None: self.assertEqual("approver", route) def test_override_reaches_reviewers_when_gate_disabled(self) -> None: - facts = self._author_route_facts() + facts = self._cleared_ci_facts() route = resolve_pr_route(facts, {}, 1, False) @@ -890,18 +890,28 @@ def test_required_check_buckets_control_ci_facts_and_author_routing(self) -> Non def test_required_ci_failure_routes_to_author_before_approval_state(self) -> None: facts = { "approval_count": 1, - "ci_failing_count": 1, + "ci_uncleared_failing_count": 1, "is_maintenance_bot": False, } self.assertEqual("author", route_pr(facts, {}, 1)) + def test_override_cleared_ci_failure_does_not_route_to_author(self) -> None: + facts = { + "approval_count": 0, + "ci_failing_count": 1, + "ci_uncleared_failing_count": 0, + "is_maintenance_bot": False, + } + + self.assertEqual("approver", route_pr(facts, {}, 1)) + def test_required_ci_failure_preserves_maintenance_bot_routing(self) -> None: for approval_count, expected_route in ((0, "approver"), (1, "maintainer")): with self.subTest(approval_count=approval_count): facts = { "approval_count": approval_count, - "ci_failing_count": 1, + "ci_uncleared_failing_count": 1, "is_maintenance_bot": True, } @@ -956,6 +966,45 @@ def test_required_ci_failure_waits_since_first_current_failure(self) -> None: self.assertEqual(waiting_since, current_facts["waiting_since"]) self.assertEqual(basis, current_facts["waiting_age_basis"]) + def test_override_command_clears_only_the_failures_that_predate_it(self) -> None: + facts = compute_facts( + { + "pr": { + "updatedAt": "2026-07-17T03:00:00Z", + "createdAt": "2026-07-14T01:00:00Z", + "author": {"login": "author"}, + "assignees": [], + "mergeStateStatus": "CLEAN", + "mergeable": "MERGEABLE", + }, + "checks": [ + {"bucket": "fail", "completed_at": "2026-07-17T01:00:00Z"}, + {"bucket": "fail", "completed_at": "2026-07-17T02:00:00Z"}, + {"bucket": "fail", "completed_at": "2026-07-17T05:00:00Z"}, + ], + "issue_comments": [ + { + "id": 7, + "user": {"login": "author"}, + "created_at": "2026-07-17T02:00:00Z", + "body": "/dashboard route:reviewers", + } + ], + }, + "author", + [], + ) + + self.assertEqual(3, facts["ci_failing_count"]) + self.assertEqual(2, facts["ci_uncleared_failing_count"]) + self.assertEqual("2026-07-17T02:00:00+00:00", facts["ci_uncleared_failing_since"]) + self.assertEqual("author", route_pr(facts, {}, 1)) + + add_wait_age_facts(facts, "author", {}) + + self.assertEqual("2026-07-17T02:00:00+00:00", facts["waiting_since"]) + self.assertEqual("ci_failure", facts["waiting_age_basis"]) + class LastActivityTest(unittest.TestCase): def _compute_last_activity_at(self, events: list[dict[str, object]]) -> object: diff --git a/.github/scripts/pull-request-dashboard/test_dashboard_override.py b/.github/scripts/pull-request-dashboard/test_dashboard_override.py index 5e07368b6df..474ffcd8429 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard_override.py @@ -156,6 +156,12 @@ def test_renders_command_replies(self) -> None: "route": "copilot", "user": "author", }) + maintainer = dashboard_override.render_command_reply({ + "comment_id": 6, + "kind": "routed", + "route": "maintainer", + "user": "author", + }) self.assertIn(dashboard_override.command_reply_marker(2), unauthorized) self.assertIn( @@ -177,6 +183,12 @@ def test_renders_command_replies(self) -> None: "handoff is waiting on Copilot.", copilot_gated, ) + self.assertIn(dashboard_override.command_reply_marker(6), maintainer) + self.assertIn( + "@author accepted the reviewer-routing override; this pull request " + "has the approvals it needs and is now waiting on maintainers.", + maintainer, + ) def test_renders_already_routed_replies_per_route(self) -> None: cases = { @@ -194,35 +206,57 @@ def test_renders_already_routed_replies_per_route(self) -> None: self.assertIn(f"@author this pull request is {phrase}, so", body) self.assertIn("`/dashboard route:reviewers` had no effect", body) - def test_appends_already_routed_reply_for_author_noop(self) -> None: + def test_renders_already_routed_reply_for_a_command_that_cleared_nothing( + self, + ) -> None: + body = dashboard_override.render_command_reply( + {"comment_id": 7, "kind": "already_routed", "user": "author", "route": "author"} + ) + + self.assertIn(dashboard_override.override_ack_marker(7), body) + self.assertIn( + "@author everything still open on this pull request arrived after " + "your `/dashboard route:reviewers` command", + body, + ) + + def test_appends_already_routed_reply_for_a_command_that_cleared_nothing(self) -> None: facts = { "author": "author", - "dashboard_override_noop": True, "dashboard_override_command_id": 12, } - dashboard_override.append_route_noop_reply({"issue_comments": []}, facts, "approver") + dashboard_override.append_command_ack_reply({"issue_comments": []}, facts, "approver") self.assertEqual( [{"comment_id": 12, "kind": "already_routed", "user": "author", "route": "approver"}], facts["dashboard_command_replies"], ) - def test_no_already_routed_reply_when_command_applies(self) -> None: + def test_appends_routed_reply_when_the_command_cleared_something(self) -> None: facts = { "author": "author", - "dashboard_override_noop": False, "dashboard_override_command_id": 12, + "dashboard_override_cleared_count": 1, } - dashboard_override.append_route_noop_reply({"issue_comments": []}, facts, "author") + dashboard_override.append_command_ack_reply({"issue_comments": []}, facts, "approver") + + self.assertEqual( + [{"comment_id": 12, "kind": "routed", "user": "author", "route": "approver"}], + facts["dashboard_command_replies"], + ) + + def test_no_ack_reply_without_a_pending_command(self) -> None: + facts = {"author": "author", "dashboard_override_command_id": 0} + + dashboard_override.append_command_ack_reply({"issue_comments": []}, facts, "author") self.assertNotIn("dashboard_command_replies", facts) def test_already_routed_reply_deduped_by_existing_marker(self) -> None: facts = { "author": "author", - "dashboard_override_noop": True, "dashboard_override_command_id": 12, } raw = { @@ -234,14 +268,13 @@ def test_already_routed_reply_deduped_by_existing_marker(self) -> None: ] } - dashboard_override.append_route_noop_reply(raw, facts, "approver") + dashboard_override.append_command_ack_reply(raw, facts, "approver") self.assertNotIn("dashboard_command_replies", facts) def test_forged_marker_does_not_dedupe_already_routed_reply(self) -> None: facts = { "author": "author", - "dashboard_override_noop": True, "dashboard_override_command_id": 12, } raw = { @@ -253,7 +286,7 @@ def test_forged_marker_does_not_dedupe_already_routed_reply(self) -> None: ] } - dashboard_override.append_route_noop_reply(raw, facts, "approver") + dashboard_override.append_command_ack_reply(raw, facts, "approver") self.assertEqual( [{"comment_id": 12, "kind": "already_routed", "user": "author", "route": "approver"}], @@ -332,13 +365,8 @@ def test_command_stays_pending_until_app_acknowledges_it(self) -> None: ] } - first = dashboard_override.dashboard_override_facts(raw, "author", set()) - retry = dashboard_override.dashboard_override_facts(raw, "author", set()) - label_observed = dashboard_override.dashboard_override_facts( - raw, - "author", - {"dashboard:route-overridden"}, - ) + first = dashboard_override.dashboard_override_facts(raw, "author") + retry = dashboard_override.dashboard_override_facts(raw, "author") acknowledged_raw = { "issue_comments": [ *raw["issue_comments"], @@ -352,16 +380,11 @@ def test_command_stays_pending_until_app_acknowledges_it(self) -> None: acknowledged = dashboard_override.dashboard_override_facts( acknowledged_raw, "author", - set(), ) - self.assertTrue(first["dashboard_override_requested"]) - self.assertTrue(retry["dashboard_override_requested"]) - self.assertTrue(label_observed["dashboard_override_label_applied"]) - self.assertEqual(3, label_observed["dashboard_override_command_id"]) - self.assertFalse(label_observed["dashboard_override_requested"]) + self.assertEqual(3, first["dashboard_override_command_id"]) + self.assertEqual(3, retry["dashboard_override_command_id"]) self.assertEqual(0, acknowledged["dashboard_override_command_id"]) - self.assertFalse(acknowledged["dashboard_override_requested"]) def test_newer_command_reapplies_removed_override(self) -> None: raw = { @@ -376,13 +399,8 @@ def test_newer_command_reapplies_removed_override(self) -> None: ] } - facts = dashboard_override.dashboard_override_facts( - raw, - "author", - set(), - ) + facts = dashboard_override.dashboard_override_facts(raw, "author") - self.assertTrue(facts["dashboard_override_requested"]) self.assertEqual(5, facts["dashboard_override_command_id"]) def test_rebuilds_unacknowledged_already_routed_reply_across_refreshes(self) -> None: @@ -393,9 +411,8 @@ def test_rebuilds_unacknowledged_already_routed_reply_across_refreshes(self) -> } for _ in range(2): - facts = dashboard_override.dashboard_override_facts(raw, "author", set()) - route = dashboard_override.apply_dashboard_override(facts, "approver") - dashboard_override.append_route_noop_reply(raw, facts, route) + facts = dashboard_override.dashboard_override_facts(raw, "author") + dashboard_override.append_command_ack_reply(raw, facts, "approver") self.assertEqual( [{"comment_id": 5, "kind": "already_routed", "user": "author", "route": "approver"}], @@ -414,14 +431,9 @@ def test_acknowledged_command_does_not_replay_after_cache_eviction(self) -> None ] } - facts = dashboard_override.dashboard_override_facts( - raw, - "author", - set(), - ) + facts = dashboard_override.dashboard_override_facts(raw, "author") self.assertEqual(0, facts["dashboard_override_command_id"]) - self.assertFalse(facts["dashboard_override_requested"]) self.assertEqual([], facts["dashboard_command_replies"]) def test_newest_acknowledgement_consumes_older_authorized_commands(self) -> None: @@ -442,145 +454,145 @@ def test_newest_acknowledgement_consumes_older_authorized_commands(self) -> None dashboard_override.latest_authorized_command(raw, "author", set()), ) - def test_fresh_command_with_label_present_is_a_noop(self) -> None: - facts = { - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, - "dashboard_override_command_id": 5, + def test_watermark_survives_acknowledgement(self) -> None: + raw = { + "issue_comments": [ + { + "id": 3, + "user": {"login": "author"}, + "created_at": "2026-07-30T12:00:00Z", + "body": "/dashboard route:reviewers", + }, + { + "id": 4, + "user": {"login": "opentelemetry-pr-dashboard[bot]"}, + "created_at": "2026-07-30T12:05:00Z", + "body": dashboard_override.override_ack_marker(3), + }, + ] } - route = dashboard_override.apply_dashboard_override(facts, "approver") + facts = dashboard_override.dashboard_override_facts(raw, "author") - self.assertEqual("approver", route) - self.assertTrue(facts["dashboard_override_noop"]) - self.assertTrue(facts["dashboard_override_release_requested"]) + self.assertEqual(0, facts["dashboard_override_command_id"]) + self.assertEqual("2026-07-30T12:00:00Z", facts["dashboard_override_since"]) - def test_fresh_command_gets_reply_when_label_already_overrides_author_route(self) -> None: + def test_watermark_survives_the_commander_leaving_the_approver_team(self) -> None: raw = { "issue_comments": [ - {"id": 5, "user": {"login": "author"}, "body": "/dashboard route:reviewers"}, + { + "id": 3, + "user": {"login": "former-approver"}, + "created_at": "2026-07-30T12:00:00Z", + "body": "/dashboard route:reviewers", + }, + { + "id": 4, + "user": {"login": "opentelemetry-pr-dashboard[bot]"}, + "created_at": "2026-07-30T12:05:00Z", + "body": dashboard_override.override_ack_marker(3), + }, ] } - facts = dashboard_override.dashboard_override_facts( - raw, - "author", - {"dashboard:route-overridden"}, - ) - route = dashboard_override.apply_dashboard_override(facts, "author") - dashboard_override.append_route_noop_reply(raw, facts, route) + facts = dashboard_override.dashboard_override_facts(raw, "author", set()) - self.assertEqual("approver", route) - self.assertTrue(facts["dashboard_override_noop"]) - self.assertEqual( - [{"comment_id": 5, "kind": "already_routed", "user": "author", "route": "approver"}], - facts["dashboard_command_replies"], - ) + self.assertEqual("2026-07-30T12:00:00Z", facts["dashboard_override_since"]) - def test_command_only_overrides_pre_review_routes(self) -> None: - for route, expected_route, expected_pending, expected_noop in ( - ("author", "approver", True, False), - ("approver", "approver", False, True), - ("maintainer", "maintainer", False, True), - ("copilot", "copilot", False, True), - ): - with self.subTest(route=route): - facts = { - "dashboard_override_label_applied": False, - "dashboard_override_requested": True, - "dashboard_override_command_id": 5, - } + def test_unacknowledged_command_needs_current_authorization(self) -> None: + raw = { + "issue_comments": [ + { + "id": 3, + "user": {"login": "former-approver"}, + "created_at": "2026-07-30T12:00:00Z", + "body": "/dashboard route:reviewers", + }, + ] + } - actual = dashboard_override.apply_dashboard_override(facts, route) + facts = dashboard_override.dashboard_override_facts(raw, "author", set()) - self.assertEqual(expected_route, actual) - self.assertEqual(expected_pending, facts["dashboard_override_requested"]) - self.assertEqual(expected_noop, facts["dashboard_override_noop"]) + self.assertEqual("", facts["dashboard_override_since"]) - def test_label_yields_to_maintainer_route_and_releases(self) -> None: - facts = { - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, + facts = {"dashboard_override_since": "2026-07-30T12:00:00Z"} + pending_actions = { + "old": {"action": "author", "since": "2026-07-30T11:00:00Z"}, + "new": {"action": "author", "since": "2026-07-30T13:00:00Z"}, + "reviewer": {"action": "reviewer", "since": "2026-07-30T11:00:00Z"}, } - route = dashboard_override.apply_dashboard_override(facts, "maintainer") + remaining = dashboard_override.clear_overridden_actions(facts, pending_actions) - self.assertEqual("maintainer", route) - self.assertFalse(facts["dashboard_override"]) - self.assertTrue(facts["dashboard_override_release_requested"]) + self.assertEqual(["new", "reviewer"], sorted(remaining)) + self.assertEqual(1, facts["dashboard_override_cleared_count"]) - def test_releases_label_once_natural_route_reaches_reviewers(self) -> None: + def test_clears_check_failures_that_predate_the_command(self) -> None: + for uncleared, expected in ((0, True), (1, False)): + with self.subTest(uncleared=uncleared): + facts = { + "dashboard_override_since": "2026-07-30T12:00:00Z", + "ci_failing_count": 1, + "ci_uncleared_failing_count": uncleared, + } + + dashboard_override.clear_overridden_actions(facts, {}) + + self.assertEqual(expected, facts["dashboard_override_cleared_ci"]) + + def test_keeps_a_check_that_fails_after_the_command(self) -> None: facts = { - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, + "dashboard_override_since": "2026-07-30T12:00:00Z", + "ci_failing_count": 2, + "ci_uncleared_failing_count": 1, } - route = dashboard_override.apply_dashboard_override(facts, "approver") + dashboard_override.clear_overridden_actions(facts, {}) - self.assertEqual("approver", route) - self.assertTrue(facts["dashboard_override_release_requested"]) + self.assertTrue(facts["dashboard_override_cleared_ci"]) + self.assertEqual(1, dashboard_override.uncleared_ci_failing_count(facts)) - def test_does_not_release_label_while_override_holds_author_route(self) -> None: + def test_keeps_items_that_share_the_command_timestamp(self) -> None: + # The command's own `...Z` timestamp is compared against `format_ts` + # output, so the same instant reaches here written two different ways. facts = { - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, + "dashboard_override_since": "2026-07-30T12:00:00Z", + "ci_failing_count": 1, + } + pending_actions = { + "earlier": {"action": "author", "since": "2026-07-30T11:59:59+00:00"}, + "same": {"action": "author", "since": "2026-07-30T12:00:00+00:00"}, + "later": {"action": "author", "since": "2026-07-30T12:00:00.500000+00:00"}, } - route = dashboard_override.apply_dashboard_override(facts, "author") + remaining = dashboard_override.clear_overridden_actions(facts, pending_actions) - self.assertEqual("approver", route) - self.assertTrue(facts["dashboard_override"]) - self.assertFalse(facts["dashboard_override_release_requested"]) + self.assertEqual(["later", "same"], sorted(remaining)) + self.assertEqual(1, facts["dashboard_override_cleared_count"]) - @patch.object(dashboard_override, "run_gh") - @patch.object( - dashboard_override, - "load_dashboard_state_cache", - return_value={ - "prs": { - "9": {"facts": {"dashboard_override_release_requested": True}}, - } - }, - ) - def test_delivery_removes_released_override_label(self, _load_state, run_gh) -> None: - errors = dashboard_override.deliver_dashboard_override_requests( - "open-telemetry/example" - ) + def test_clears_nothing_without_a_command(self) -> None: + facts = {"dashboard_override_since": ""} + pending_actions = {"old": {"action": "author", "since": "2026-07-30T11:00:00Z"}} - self.assertEqual([], errors) - self.assertEqual( - [ - call([ - "gh", "api", "--method", "DELETE", - "repos/open-telemetry/example/issues/9/labels/dashboard%3Aroute-overridden", - ]), - ], - run_gh.call_args_list, - ) + remaining = dashboard_override.clear_overridden_actions(facts, pending_actions) - @patch.object(dashboard_override, "run_gh") - @patch.object(dashboard_override, "load_dashboard_state_cache", return_value=None) - def test_does_not_create_label_without_dashboard_state(self, _load_state, run_gh) -> None: - errors = dashboard_override.deliver_dashboard_override_requests( - "open-telemetry/example" - ) + self.assertEqual(pending_actions, remaining) + self.assertEqual(0, facts["dashboard_override_cleared_count"]) - self.assertEqual([], errors) - run_gh.assert_not_called() + def test_command_that_cleared_nothing_is_acknowledged_where_it_is_routed(self) -> None: + raw = { + "issue_comments": [ + {"id": 5, "user": {"login": "author"}, "body": "/dashboard route:reviewers"}, + ] + } + facts = dashboard_override.dashboard_override_facts(raw, "author") - @patch.object(dashboard_override, "run_gh") - @patch.object( - dashboard_override, - "load_dashboard_state_cache", - return_value={"prs": {}}, - ) - def test_does_not_create_label_without_pending_command(self, _load_state, run_gh) -> None: - errors = dashboard_override.deliver_dashboard_override_requests( - "open-telemetry/example" - ) + dashboard_override.append_command_ack_reply(raw, facts, "author") - self.assertEqual([], errors) - run_gh.assert_not_called() + self.assertEqual( + [{"comment_id": 5, "kind": "already_routed", "user": "author", "route": "author"}], + facts["dashboard_command_replies"], + ) @patch.object(dashboard_override, "run_gh") @patch.object(dashboard_override, "gh_api", return_value=[]) @@ -590,46 +602,28 @@ def test_does_not_create_label_without_pending_command(self, _load_state, run_gh return_value={ "prs": { "7": { - "route": "copilot", "facts": { - "dashboard_override_requested": True, - "dashboard_override_command_id": 3, - "dashboard_override_command_user": "author", + "dashboard_command_replies": [ + { + "comment_id": 3, + "kind": "routed", + "route": "copilot", + "user": "author", + }, + ] } }, - "8": {"facts": {"dashboard_override_requested": False}}, } }, ) - def test_delivers_pending_override_label_and_acknowledges_command( - self, - _load_state, - gh_api, - run_gh, - ) -> None: - errors = dashboard_override.deliver_dashboard_override_requests( + def test_delivers_command_acknowledgement(self, _load_state, _gh_api, run_gh) -> None: + errors = dashboard_override.deliver_dashboard_command_replies( "open-telemetry/example" ) self.assertEqual([], errors) - gh_api.assert_called_once_with( - "/repos/open-telemetry/example/issues/7/comments?per_page=100", - paginate=True, - ) self.assertEqual( [ - call([ - "gh", "label", "create", "dashboard:route-overridden", - "--repo", "open-telemetry/example", - "--color", dashboard_override.DASHBOARD_OVERRIDE_LABEL_COLOR, - "--description", dashboard_override.DASHBOARD_OVERRIDE_LABEL_DESCRIPTION, - "--force", - ]), - call([ - "gh", "api", "--method", "POST", - "repos/open-telemetry/example/issues/7/labels", - "-f", "labels[]=dashboard:route-overridden", - ]), call([ "gh", "api", "--method", "POST", "repos/open-telemetry/example/issues/7/comments", @@ -639,55 +633,6 @@ def test_delivers_pending_override_label_and_acknowledges_command( run_gh.call_args_list, ) - @patch.object(dashboard_override, "run_gh") - @patch.object(dashboard_override, "gh_api", return_value=[]) - @patch.object( - dashboard_override, - "load_dashboard_state_cache", - return_value={ - "prs": { - "7": { - "facts": { - "dashboard_override_requested": True, - "dashboard_override_command_id": 3, - "dashboard_override_command_user": "author", - } - }, - } - }, - ) - def test_retry_observes_acknowledgement_after_ambiguous_post_failure( - self, - _load_state, - gh_api, - run_gh, - ) -> None: - run_gh.side_effect = [None, None, RuntimeError("comment response lost")] - - errors = dashboard_override.deliver_dashboard_override_requests( - "open-telemetry/example" - ) - - self.assertEqual(["PR #7: comment response lost"], errors) - - run_gh.reset_mock() - run_gh.side_effect = None - gh_api.return_value = [{ - "performed_via_github_app": {"slug": "opentelemetry-pr-dashboard"}, - "body": dashboard_override.render_command_reply({ - "comment_id": 3, - "kind": "routed", - "user": "author", - }), - }] - - errors = dashboard_override.deliver_dashboard_override_requests( - "open-telemetry/example" - ) - - self.assertEqual([], errors) - self.assertEqual(2, run_gh.call_count) - if __name__ == "__main__": unittest.main() \ No newline at end of file diff --git a/.github/scripts/pull-request-dashboard/test_delivery.py b/.github/scripts/pull-request-dashboard/test_delivery.py index eb58bbd9e4c..acf0122c62d 100644 --- a/.github/scripts/pull-request-dashboard/test_delivery.py +++ b/.github/scripts/pull-request-dashboard/test_delivery.py @@ -13,7 +13,6 @@ class DeliveryTest(unittest.TestCase): @patch.object(delivery, "deliver_copilot_review_requests", return_value=[]) @patch.object(delivery, "deliver_prepared_author_nudges", return_value=[]) @patch.object(delivery, "deliver_dashboard_command_replies", return_value=[]) - @patch.object(delivery, "deliver_dashboard_override_requests", return_value=[]) @patch.object(delivery, "update_status_comments_from_state", return_value=[]) @patch.object( delivery, @@ -27,7 +26,6 @@ def test_runs_all_repository_deliveries_in_order( self, _list_open, status_comments, - dashboard_overrides, dashboard_command_replies, author_nudges, copilot_reviews, @@ -40,7 +38,6 @@ def record(label: str) -> list[str]: return [] status_comments.side_effect = lambda *_args: record("status") - dashboard_overrides.side_effect = lambda *_args: record("override") dashboard_command_replies.side_effect = lambda *_args: record("replies") author_nudges.side_effect = lambda *_args: record("author") copilot_reviews.side_effect = lambda *_args: record("copilot") @@ -55,7 +52,7 @@ def record(label: str) -> list[str]: self.assertEqual([], errors) _list_open.assert_called_once_with("open-telemetry/example") self.assertEqual( - [call("override"), call("replies"), call("author"), call("status"), call("copilot"), call("slack")], + [call("replies"), call("author"), call("status"), call("copilot"), call("slack")], order.call_args_list, ) status_comments.assert_called_once_with( @@ -76,7 +73,6 @@ def record(label: str) -> list[str]: @patch.object(delivery, "deliver_copilot_review_requests", return_value=[]) @patch.object(delivery, "deliver_prepared_author_nudges", return_value=[]) @patch.object(delivery, "deliver_dashboard_command_replies", return_value=[]) - @patch.object(delivery, "deliver_dashboard_override_requests", return_value=[]) @patch.object(delivery, "update_status_comments_from_state", side_effect=RuntimeError("boom")) @patch.object( delivery, @@ -87,7 +83,6 @@ def test_failure_does_not_block_later_deliveries( self, _list_open, _status_comments, - dashboard_overrides, dashboard_command_replies, author_nudges, copilot_reviews, @@ -101,7 +96,6 @@ def test_failure_does_not_block_later_deliveries( ) self.assertIn("status comments: boom", errors) - dashboard_overrides.assert_called_once() dashboard_command_replies.assert_called_once() author_nudges.assert_called_once() copilot_reviews.assert_called_once() @@ -110,7 +104,6 @@ def test_failure_does_not_block_later_deliveries( def test_open_pr_list_failure_skips_dependent_stages(self) -> None: with ( patch.object(delivery, "list_open_prs", side_effect=RuntimeError("unavailable")), - patch.object(delivery, "deliver_dashboard_override_requests", return_value=[]) as overrides, patch.object(delivery, "deliver_dashboard_command_replies", return_value=[]) as replies, patch.object(delivery, "deliver_prepared_author_nudges", return_value=[]) as nudges, patch.object(delivery, "update_status_comments_from_state", return_value=[]) as status, @@ -125,7 +118,6 @@ def test_open_pr_list_failure_skips_dependent_stages(self) -> None: ) self.assertEqual(["open pull requests: unavailable"], errors) - overrides.assert_called_once() replies.assert_called_once() nudges.assert_called_once() copilot.assert_called_once() @@ -140,7 +132,6 @@ def test_targeted_delivery_only_processes_triggering_pr(self) -> None: "gh_api", return_value={"state": "open", "draft": False, "title": "Seven"}, ) as gh_api, - patch.object(delivery, "deliver_dashboard_override_requests", return_value=[]), patch.object(delivery, "deliver_dashboard_command_replies", return_value=[]), patch.object(delivery, "deliver_prepared_author_nudges", return_value=[]), patch.object(delivery, "update_status_comments_from_state") as bulk_status, diff --git a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py index 0860169573d..ef4a5ef578b 100644 --- a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py @@ -180,7 +180,7 @@ def test_accuracy_note_bounds_report_url_for_large_status(self) -> None: { "route": "author", "facts": { - "ci_failing_count": 1, + "ci_uncleared_failing_count": 1, "non_blocking_check_failures": [ "&" * pr_status_comment.NON_BLOCKING_CHECK_FAILURE_NAME_LIMIT for _ in range( @@ -217,7 +217,7 @@ def test_waiting_on_author_names_required_ci_failure(self) -> None: self.pr(), { "route": "author", - "facts": {"author": "alice", "ci_failing_count": 1}, + "facts": {"author": "alice", "ci_uncleared_failing_count": 1}, }, ) @@ -233,7 +233,7 @@ def test_waiting_on_author_combines_ci_and_review_feedback_reasons(self) -> None "route": "author", "facts": { "author": "alice", - "ci_failing_count": 2, + "ci_uncleared_failing_count": 2, "author_action_review_thread_urls": [ "https://github.com/open-telemetry/example/pull/1#discussion_r1", ], @@ -252,7 +252,7 @@ def test_required_ci_action_notes_configured_non_blocking_failures(self) -> None { "route": "author", "facts": { - "ci_failing_count": 2, + "ci_uncleared_failing_count": 2, "non_blocking_check_failures": [ "CodeQL", "workflow-notification", @@ -273,7 +273,7 @@ def test_required_ci_action_escapes_non_blocking_failure_names(self) -> None: { "route": "author", "facts": { - "ci_failing_count": 1, + "ci_uncleared_failing_count": 1, "non_blocking_check_failures": [ "[CodeQL]