fix(bot): handling of duplicate check suites (#688)

Previously if a user created duplicate check suites with the same check run name, Kodiak would consider all of the check suites, not just the most recent one. So if the check suite had failed previously, but the latest check suite passed, Kodiak would incorrectly mark the PR as blocked for merging.

Now Kodiak only considers the most recent check run name.

bot/kodiak/evaluation.py

@@ -1,6 +1,9 @@
+from __future__ import annotations
+
 import asyncio
 import textwrap
 from collections import defaultdict
+from collections.abc import Iterable
 from dataclasses import dataclass
 from typing import Dict, List, MutableMapping, Optional, Sequence, Set, Union
 
@@ -260,6 +263,11 @@ def review_status(reviews: List[PRReview]) -> PRReviewState:
     return status
 
 
+def deduplicate_check_runs(check_runs: Iterable[CheckRun]) -> Iterable[CheckRun]:
+    check_run_map = {check_run.name: check_run for check_run in check_runs}
+    return check_run_map.values()
+
+
 class PRAPI(Protocol):
     async def dequeue(self) -> None:
         ...
@@ -882,7 +890,8 @@ async def set_status(msg: str, markdown_content: Optional[str] = None) -> None:
                 else:
                     assert status_context.state == StatusState.SUCCESS
                     passing_contexts.append(status_context.context)
-            for check_run in check_runs:
+
+            for check_run in deduplicate_check_runs(check_runs):
                 if (
                     check_run.name in config.merge.dont_wait_on_status_checks
                     and check_run.conclusion in (None, CheckConclusionState.NEUTRAL)

bot/kodiak/test_evaluation.py

@@ -282,8 +282,12 @@ def create_context() -> StatusContext:
     return StatusContext(context="ci/api", state=StatusState.SUCCESS)
 
 
-def create_check_run() -> CheckRun:
-    return CheckRun(name="WIP (beta)", conclusion=CheckConclusionState.SUCCESS)
+def create_check_run(
+    *,
+    name: str = "WIP (beta)",
+    conclusion: CheckConclusionState = CheckConclusionState.SUCCESS,
+) -> CheckRun:
+    return CheckRun(name=name, conclusion=conclusion)
 
 
 def create_review_request() -> PRReviewRequest:

bot/kodiak/tests/evaluation/test_check_runs.py

@@ -1,8 +1,13 @@
+"""
+Tests for flows that depend on status checks or check_runs.
+"""
 import pytest
 
 from kodiak.errors import PollForever, RetryForSkippableChecks
-from kodiak.queries import CheckConclusionState, MergeStateStatus, StatusState
+from kodiak.queries import StatusState
 from kodiak.test_evaluation import (
+    CheckConclusionState,
+    MergeStateStatus,
     create_api,
     create_branch_protection,
     create_check_run,
@@ -562,3 +567,38 @@ async def test_mergeable_update_always_no_require_automerge_label_missing_label(
     assert api.queue_for_merge.call_count == 0
     assert api.merge.call_count == 0
     assert api.dequeue.call_count == 0
+
+
+@pytest.mark.asyncio
+async def test_duplicate_check_suites() -> None:
+    """
+    Kodiak should only consider the most recent check run when evaluating a PR
+    for merging.
+
+    Regression test.
+    """
+    api = create_api()
+    pull_request = create_pull_request()
+    pull_request.mergeStateStatus = MergeStateStatus.BEHIND
+    branch_protection = create_branch_protection()
+    branch_protection.requiresStatusChecks = True
+    branch_protection.requiredStatusCheckContexts = ["Pre-merge checks"]
+    mergeable = create_mergeable()
+    await mergeable(
+        api=api,
+        pull_request=pull_request,
+        branch_protection=branch_protection,
+        check_runs=[
+            create_check_run(
+                name="Pre-merge checks", conclusion=CheckConclusionState.NEUTRAL
+            ),
+            create_check_run(
+                name="Pre-merge checks", conclusion=CheckConclusionState.FAILURE
+            ),
+            create_check_run(
+                name="Pre-merge checks", conclusion=CheckConclusionState.SUCCESS
+            ),
+        ],
+    )
+    assert "enqueued for merge" in api.set_status.calls[0]["msg"]
+    assert api.queue_for_merge.call_count == 1