feat(bot): merge.block_on_neutral_required_check_runs (#785)

When `merge.block_on_neutral_required_check_runs` is enabled, we won't merge the pull request if a required check run has a neutral conclusion.

related: #780

CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## 0.50.0 - 2022-02-03
+
+### Added
+- Added `merge.block_on_neutral_required_check_runs` option to stop Kodiak from merging a pull request if a require check run has a neutral conclusion. (#785)
+
 ## 0.49.0 - 2021-11-26
 
 ### Fixed

bot/kodiak/config.py

@@ -88,6 +88,8 @@ class Merge(BaseModel):
     #
     # block merging if there are outstanding review requests
     block_on_reviews_requested: bool = False
+    # block merging if there are neutral required check runs.
+    block_on_neutral_required_check_runs: bool = False
     # comment on merge conflict and remove automerge label
     notify_on_conflict: bool = True
     # don't wait for status checks to run before updating branch

bot/kodiak/evaluation.py

@@ -824,6 +824,23 @@ async def set_status(msg: str, markdown_content: Optional[str] = None) -> None:
         # status checks. we may want to handle this via config
         pass
 
+    required_status_checks = set(branch_protection.requiredStatusCheckContexts)
+    if config.merge.block_on_neutral_required_check_runs:
+        neutral_check_runs = {
+            check_run.name
+            for check_run in deduplicate_check_runs(check_runs)
+            if check_run.conclusion == CheckConclusionState.NEUTRAL
+        }
+
+        failing_required_status_checks = neutral_check_runs & required_status_checks
+        if failing_required_status_checks:
+            await block_merge(
+                api,
+                pull_request,
+                f"neutral required check runs: {failing_required_status_checks!r}",
+            )
+            return
+
     wait_for_checks = False
     if pull_request.mergeStateStatus in (
         MergeStateStatus.BLOCKED,
@@ -857,15 +874,13 @@ async def set_status(msg: str, markdown_content: Optional[str] = None) -> None:
             await block_merge(api, pull_request, "unresolved conversations")
             return
 
-        required: Set[str] = set()
         passing: Set[str] = set()
 
         if branch_protection.requiresStatusChecks:
             skippable_contexts: List[str] = []
             failing_contexts: List[str] = []
             pending_contexts: List[str] = []
             passing_contexts: List[str] = []
-            required = set(branch_protection.requiredStatusCheckContexts)
             for status_context in contexts:
                 # handle dont_wait_on_status_checks. We want to consider a
                 # status_check failed if it is incomplete and in the
@@ -911,13 +926,13 @@ async def set_status(msg: str, markdown_content: Optional[str] = None) -> None:
             passing = set(passing_contexts)
             failing = set(failing_contexts)
             # we have failing statuses that are required
-            failing_required_status_checks = failing & required
+            failing_required_status_checks = failing & required_status_checks
             # GitHub has undocumented logic for travis-ci checks in GitHub
             # branch protection rules. GitHub compresses
             # "continuous-integration/travis-ci/{pr,push}" to
             # "continuous-integration/travis-ci". There is only special handling
             # for these specific checks.
-            if "continuous-integration/travis-ci" in required:
+            if "continuous-integration/travis-ci" in required_status_checks:
                 if "continuous-integration/travis-ci/pr" in failing:
                     failing_required_status_checks.add(
                         "continuous-integration/travis-ci/pr"
@@ -932,7 +947,7 @@ async def set_status(msg: str, markdown_content: Optional[str] = None) -> None:
                     "continuous-integration/travis-ci/pr" in passing
                     or "continuous-integration/travis-ci/push" in passing
                 ):
-                    required.remove("continuous-integration/travis-ci")
+                    required_status_checks.remove("continuous-integration/travis-ci")
             if failing_required_status_checks:
                 # NOTE(chdsbd): We need to skip this PR because it would block
                 # the merge queue. We may be able to bump it to the back of the
@@ -963,7 +978,7 @@ async def set_status(msg: str, markdown_content: Optional[str] = None) -> None:
                 )
                 return
 
-        missing_required_status_checks = required - passing
+        missing_required_status_checks = required_status_checks - passing
         wait_for_checks = bool(
             branch_protection.requiresStatusChecks and missing_required_status_checks
         )

bot/kodiak/test/fixtures/config/config-schema.json

@@ -27,6 +27,7 @@
         "method": null,
         "delete_branch_on_merge": false,
         "block_on_reviews_requested": false,
+        "block_on_neutral_required_check_runs": false,
         "notify_on_conflict": true,
         "optimistic_updates": true,
         "message": {
@@ -305,6 +306,11 @@
           "default": false,
           "type": "boolean"
         },
+        "block_on_neutral_required_check_runs": {
+          "title": "Block On Neutral Required Check Runs",
+          "default": false,
+          "type": "boolean"
+        },
         "notify_on_conflict": {
           "title": "Notify On Conflict",
           "default": true,

bot/kodiak/tests/evaluation/test_check_runs.py

@@ -602,3 +602,32 @@ async def test_duplicate_check_suites() -> None:
     )
     assert "enqueued for merge" in api.set_status.calls[0]["msg"]
     assert api.queue_for_merge.call_count == 1
+
+
+@pytest.mark.asyncio
+async def test_neutral_required_check_runs() -> None:
+    """
+    When merge.block_on_neutral_required_check_runs is enabled, we should block
+    merge if a required check run has a neutral conclusion.
+    """
+    api = create_api()
+    config = create_config()
+    config.merge.block_on_neutral_required_check_runs = True
+    branch_protection = create_branch_protection()
+    branch_protection.requiresStatusChecks = True
+    branch_protection.requiredStatusCheckContexts = ["Pre-merge checks"]
+
+    mergeable = create_mergeable()
+    await mergeable(
+        api=api,
+        branch_protection=branch_protection,
+        config=config,
+        check_runs=[
+            create_check_run(
+                name="Pre-merge checks", conclusion=CheckConclusionState.NEUTRAL
+            ),
+        ],
+    )
+    assert "neutral required check runs" in api.set_status.calls[0]["msg"]
+    assert api.queue_for_merge.call_count == 0
+    assert api.dequeue.call_count == 1

docs/docs/config-reference.md

@@ -111,6 +111,15 @@ versions = ["minor", "patch"]
 usernames = ["dependabot", "renovate"]
 ```
 
+### `merge.block_on_neutral_required_check_runs`
+
+- **type:** `boolean`
+- **default:** `false`
+
+GitHub considers a "neutral" check run conclusion as passing GitHub branch protection requirements.
+
+When enabled, pull requests won't be merged if a required check run has a neutral conclusion.
+
 <span id="mergeblacklist_title_regex"/> <!-- handle old links -->
 
 ### `merge.blocking_title_regex`