diff --git a/src/AutoMerge/cron.jl b/src/AutoMerge/cron.jl index aa3a08b9..02d164c7 100644 --- 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 index c0f5a522..c57357dd 100644 --- 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 index 18bfc7bd..76eb4a75 100644 --- 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