add AutoMerge: last run blocked by comment and `Override AutoMerge:…

… ignore blocking comments` labels
JuliaRegistries · Aug 7, 2024 · 5c790d0 · 5c790d0
1 parent b0afec6
commit 5c790d0
Show file tree

Hide file tree

Showing 3 changed files with 118 additions and 13 deletions.
diff --git a/src/AutoMerge/cron.jl b/src/AutoMerge/cron.jl
@@ -1,5 +1,8 @@
 using GitHub: GitHub
 
+const OVERRIDE_BLOCKS_LABEL = "Override AutoMerge: ignore blocking comments"
+const BLOCKED_LABEL = "AutoMerge: last run blocked by comment"
+
 function all_specified_statuses_passed(
     api::GitHub.GitHubAPI,
     registry::GitHub.Repo,
@@ -61,14 +64,14 @@ function pr_comment_is_blocking(c::GitHub.Comment)
     return !not_blocking
 end
 
-function pr_has_no_blocking_comments(
+function pr_has_blocking_comments(
     api::GitHub.GitHubAPI,
     registry::GitHub.Repo,
     pr::GitHub.PullRequest;
     auth::GitHub.Authorization,
 )
     all_pr_comments = get_all_pull_request_comments(api, registry, pr; auth=auth)
-    return !any(pr_comment_is_blocking.(all_pr_comments))
+    return any(pr_comment_is_blocking, all_pr_comments)
 end
 
 function pr_is_old_enough(
@@ -191,7 +194,12 @@ function cron_or_api_build(
     all_check_runs::AbstractVector{<:AbstractString},
     read_only::Bool,
 )
-    # first, get a list of ALL open pull requests on this repository
+
+    # first, create `BLOCKED_LABEL` as a label in the repo if it doesn't
+    # already exist. This way we can add it to PRs as needed.
+    maybe_create_blocked_label(api, repo)
+
+    # next, get a list of ALL open pull requests on this repository
     # then, loop through each of them.
     all_currently_open_pull_requests = my_retry(
         () -> get_all_pull_requests(api, registry, "open"; auth=auth)
@@ -384,7 +392,11 @@ function cron_or_api_build(
         return nothing
     end
 
-    if !pr_has_no_blocking_comments(api, registry, pr; auth=auth)
+    blocked = pr_has_blocking_comments(api, registry, pr; auth=auth) && !has_label(pr.labels, OVERRIDE_BLOCKS_LABEL)
+    if blocked
+        # add `BLOCKED_LABEL` to communicate to users
+        # that the PR is blocked from automerging
+        GitHub.add_labels(api, registry.full_name, pr_number, [BLOCKED_LABEL])
         @info(
             string(
                 "Pull request: $(pr_number). ",
@@ -394,6 +406,13 @@ function cron_or_api_build(
             )
         )
         return nothing
+    elseif has_label(pr.labels, BLOCKED_LABEL)
+        # remove block label BLOCKED_LABEL if it exists
+        # note we use `try_remove_label` to avoid crashing the job
+        # if there is some race condition or manual intervention
+        # and the blocked label was removed at some point between
+        # when the `pr` object was created and now.
+        try_remove_label(api, registry.full_name, pr_number, BLOCKED_LABEL)
     end
 
     if pr_type == :NewPackage # it is a new package

diff --git a/src/AutoMerge/util.jl b/src/AutoMerge/util.jl
@@ -319,18 +319,64 @@ function get_all_non_jll_package_names(registry)
     return packages
 end
 
-const PACKAGE_AUTHOR_APPROVED_LABEL = "Override AutoMerge: package author approved"
 
-function has_package_author_approved_label(labels)
-    # No labels? Not approved
+function has_label(labels, target)
+    # No labels? Then no
     isnothing(labels) && return false
     for label in labels
-        if label.name === PACKAGE_AUTHOR_APPROVED_LABEL
-            # found the approval
-            @debug "Found `$(PACKAGE_AUTHOR_APPROVED_LABEL)` label"
+        if label.name === target
+            # found it
+            @debug "Found `$(target)` label"
             return true
         end
     end
-    # Did not find approval
+    # Did not find it
     return false
 end
+
+const PACKAGE_AUTHOR_APPROVED_LABEL = "Override AutoMerge: package author approved"
+
+has_package_author_approved_label(labels) = has_label(labels, PACKAGE_AUTHOR_APPROVED_LABEL)
+
+"""
+    try_remove_label(api, repo, issue, label)
+
+Uses `GitHub.remove_label` to remove the label, if it exists.
+Differs from the upstream functionality by not erroring if we receive a 404
+response indicating the label did not exist.
+
+Returns whether or not the label was removed.
+"""
+function try_remove_label(api, repo, issue, label)
+    r = GitHub.remove_label(api, repo, issue, label; handle_error = false)
+    r.status == 404 && return false
+    GitHub.handle_response_error(r)  # throw errors in other cases if necessary
+    return true
+end
+
+function repo_has_label(api, repo, name; options...)
+    path = "/repos/$(GitHub.name(repo))/labels/$name"
+    result = GitHub.gh_get_json(api, path; options..., handle_error = false)
+    result.status == 404 && return false
+    GitHub.handle_response_error(r)  # throw errors in other cases if necessary
+    return true
+end
+
+function create_label(api, repo, name::String, color::String, description::String; options...)
+    path = "/repos/$(GitHub.name(repo))/labels"
+    result = GitHub.gh_post_json(api, path; params=(; name=name, color=color, description=description), options...)
+    return Label(result)
+end
+
+"""
+    maybe_create_blocked_label(api, repo)
+
+Add the label `$BLOCKED_LABEL` to the repo if it doesn't already exist.
+
+Returns whether or not it created the label.
+"""
+function maybe_create_blocked_label(api, repo)
+    repo_has_label(api, repo, BLOCKED_LABEL) && return false
+    create_label(api, repo, BLOCKED_LABEL, "ff0000", "PR blocked by one or more comments lacking the string [noblock].")
+    return true
+end
diff --git a/test/automerge-integration.jl b/test/automerge-integration.jl
@@ -34,7 +34,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
 @testset "Integration tests" begin
     for (
         test_number,
-        (master_dir, feature_dir, public_dir, title, point_to_slack, check_license, pass, commit),
+        (master_dir, feature_dir, public_dir, title, point_to_slack, check_license, pass, commit, create_blocking_comment),
     ) in enumerate([
         (
             "master_1",
@@ -45,6 +45,18 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             true,   # check_license
             true,   # pass
             requires_commit,
+            false,  # create_blocking_comment
+        ), # OK: new package
+        (
+            "master_1",
+            "feature_1",
+            "",
+            "New package: Requires v1.0.0",
+            true,   # point_to_slack
+            true,   # check_license
+            true,   # pass
+            requires_commit,
+            true,  # create_blocking_comment
         ), # OK: new package
         (
             "master_1",
@@ -55,6 +67,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             true,   # check_license
             false,  # pass
             "659e09770ba9fda4a503f8bf281d446c9583ff3b",
+            false,  # create_blocking_comment
         ), # FAIL: wrong commit!
         (
             "master_2",
@@ -65,6 +78,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             true,   # pass
             requires_commit,
+            false,  # create_blocking_comment
         ),            # OK: new version
         (
             "master_1",
@@ -75,6 +89,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             false,  # pass
             requires_commit,
+            false,  # create_blocking_comment
         ),                # FAIL: name too short
         (
             "master_2",
@@ -85,6 +100,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             false,  # pass
             requires_commit,
+            false,  # create_blocking_comment
         ),           # FAIL: skips v2.0.0
         (
             "master_3",
@@ -95,6 +111,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             false,  # pass
             requires_commit,
+            false,  # create_blocking_comment
         ),           # FAIL: modifies extra file
         (
             "master_1",
@@ -105,6 +122,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             true,   # pass
             hello_world_commit1,
+            false,  # create_blocking_comment
         ),   # OK: new JLL package
         (
             "master_4",
@@ -115,6 +133,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             true,   # pass
             hello_world_commit2,
+            false,  # create_blocking_comment
         ),   # OK: new JLL version
         (
             "master_1",
@@ -125,6 +144,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             false,  # pass
             hello_world_commit1,
+            false,  # create_blocking_comment
         ),  # FAIL: unallowed dependency
         (
             "master_1",
@@ -135,6 +155,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             true,   # pass
             requires_commit,
+            false,  # create_blocking_comment
         ),    # OK: no UUID conflict
         (
             "master_1",
@@ -145,6 +166,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             false,  # pass
             requires_commit,
+            false,  # create_blocking_comment
         ),  # FAIL: UUID conflict, name differs
         (
             "master_1",
@@ -155,6 +177,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             false,  # pass
             requires_commit,
+            false,  # create_blocking_comment
         ),  # FAIL: UUID conflict, repo differs
         (
             "master_1",
@@ -165,6 +188,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             false,  # check_license
             true,   # pass
             requires_commit,
+            false,  # create_blocking_comment
         ),   # OK: UUID conflict but name and repo match
         (
             "master_1",
@@ -175,6 +199,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
             true,   # check_license
             false,   # pass
             requires_commit,
+            false,  # create_blocking_comment
         ), # FAIL: new package name is not a Julia identifier
     ])
         @info "Performing integration tests with settings" test_number master_dir feature_dir public_dir title point_to_slack check_license pass commit
@@ -254,6 +279,9 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
                         "TRAVIS_REPO_SLUG" => AUTOMERGE_INTEGRATION_TEST_REPO,
                     ) do
                         sleep(1)
+                        if create_blocking_comment
+                            blocking_comment = GitHub.create_comment(repo, pr, "blocking comment")
+                        end
                         AutoMerge.run(;
                             merge_new_packages=true,
                             merge_new_versions=true,
@@ -269,7 +297,7 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
                             master_branch_is_default_branch=false,
                         )
                         sleep(1)
-                        AutoMerge.run(;
+                        merge = () -> AutoMerge.run(;
                             merge_new_packages=true,
                             merge_new_versions=true,
                             new_package_waiting_period=Minute(0),
@@ -283,6 +311,18 @@ hello_world_commit2 = "57b0aec49622faa962c6752d4bc39a62b91fe37c"
                             master_branch=master,
                             master_branch_is_default_branch=false,
                         )
+                        merge()
+                        if create_blocking_comment
+                            # Check we have the blocked label
+                            labels = GitHub.labels(repo, pr)
+                            @test AutoMerge.has_label(labels, AutoMerge.BLOCKED_LABEL)
+                            # Delete the comment & rerun
+                            GitHub.delete_comment(repo, blocking_comment)
+                            merge()
+                            # Check we no longer have the blocked label
+                            labels = GitHub.labels(repo, pr)
+                            @test !AutoMerge.has_label(labels, AutoMerge.BLOCKED_LABEL)
+                        end
                     end
                 end
             end