Add option to prevent merges without required checks (#217)

This PR includes changes which seek to resolve #122, where PRs without any required checks are merged automatically, often due to mis-configuration.

A new option is added to the v1 configuration format to allow/deny merges with no required status checks. The option is nested within the `merge` object:
```yaml
version: 1

merge:
  allow_merge_with_no_checks: false
```

The v0 configuration format is not updated to support this feature. Repositories using the v0 configuration will be treated as if `allow_merge_with_no_checks` is set to the default value `false`.

README.md

@@ -148,6 +148,11 @@ merge:
   # If true, bulldozer will delete branches after their pull requests merge.
   delete_after_merge: true
 
+  # If true, bulldozer will merge pull requests with no required checks. This
+  # helps to protect against merging branches which inadvertently do not have
+  # required status checks.
+  allow_merge_with_no_checks: false
+
 # "update" defines how and when to update pull request branches. Unlike with
 # merges, if this section is missing, bulldozer will not update any pull requests.
 update:
@@ -223,6 +228,8 @@ which are to be expected, and others that may be caused by mis-configuring Bulld
 * Branch protection rules are preventing `bulldozer[bot]` from [pushing to the
   branch][push restrictions]. Github apps can be added to the list of restricted
   push users, so you can allow Bulldozer specifically for your repo.
+* The branch has no required checks and `allow_merge_with_no_checks` is set to
+  the default value (`false`).
 
 [push restrictions]: https://help.github.com/articles/about-branch-restrictions/
 [a workaround]: #can-bulldozer-work-with-push-restrictions-on-branches

bulldozer/config_fetcher.go

@@ -189,8 +189,9 @@ func (cf *ConfigFetcher) unmarshalConfigV0(bytes []byte) (*Config, error) {
 				Trigger: Signals{
 					Labels: []string{"merge when ready", "merge-when-ready", "merge_when_ready"},
 				},
-				DeleteAfterMerge: configv0.DeleteAfterMerge,
-				Method:           configv0.Strategy,
+				DeleteAfterMerge:       configv0.DeleteAfterMerge,
+				AllowMergeWithNoChecks: false,
+				Method:                 configv0.Strategy,
 			},
 		}
 		if config.Merge.Method == SquashAndMerge {
@@ -210,8 +211,9 @@ func (cf *ConfigFetcher) unmarshalConfigV0(bytes []byte) (*Config, error) {
 				Ignore: Signals{
 					Labels: []string{"wip", "do not merge", "do-not-merge", "do_not_merge"},
 				},
-				DeleteAfterMerge: configv0.DeleteAfterMerge,
-				Method:           configv0.Strategy,
+				DeleteAfterMerge:       configv0.DeleteAfterMerge,
+				AllowMergeWithNoChecks: false,
+				Method:                 configv0.Strategy,
 			},
 		}
 		if config.Merge.Method == SquashAndMerge {
@@ -231,8 +233,9 @@ func (cf *ConfigFetcher) unmarshalConfigV0(bytes []byte) (*Config, error) {
 				Trigger: Signals{
 					CommentSubstrings: []string{"==MERGE_WHEN_READY=="},
 				},
-				DeleteAfterMerge: configv0.DeleteAfterMerge,
-				Method:           configv0.Strategy,
+				DeleteAfterMerge:       configv0.DeleteAfterMerge,
+				AllowMergeWithNoChecks: false,
+				Method:                 configv0.Strategy,
 			},
 		}
 		if config.Merge.Method == SquashAndMerge {

bulldozer/config_v1.go

@@ -41,7 +41,8 @@ type MergeConfig struct {
 	Blacklist Signals `yaml:"blacklist"`
 	Whitelist Signals `yaml:"whitelist"`
 
-	DeleteAfterMerge bool `yaml:"delete_after_merge"`
+	DeleteAfterMerge       bool `yaml:"delete_after_merge"`
+	AllowMergeWithNoChecks bool `yaml:"allow_merge_with_no_checks"`
 
 	Method  MergeMethod  `yaml:"method"`
 	Options MergeOptions `yaml:"options"`

bulldozer/evaluate.go

@@ -123,6 +123,11 @@ func ShouldMergePR(ctx context.Context, pullCtx pull.Context, mergeConfig MergeC
 	}
 	requiredStatuses = append(requiredStatuses, mergeConfig.RequiredStatuses...)
 
+	if len(requiredStatuses) == 0 && !mergeConfig.AllowMergeWithNoChecks {
+		logger.Debug().Msgf("%s has 0 required status checks, but is deemed not mergeable because AllowMergeWithNoChecks is false", pullCtx.Locator())
+		return false, nil
+	}
+
 	successStatuses, err := pullCtx.CurrentSuccessStatuses(ctx)
 	if err != nil {
 		return false, errors.Wrap(err, "failed to determine currently successful status checks")

bulldozer/evaluate_test.go

@@ -98,6 +98,7 @@ func TestShouldMerge(t *testing.T) {
 			Comments:          []string{"NO_WAY"},
 			CommentSubstrings: []string{":-1:"},
 		},
+		AllowMergeWithNoChecks: true,
 	}
 
 	ctx := context.Background()

config/examples/standard.bulldozer.v1.yml

@@ -12,6 +12,7 @@ merge:
     squash:
       body: summarize_commits
   delete_after_merge: true
+  allow_merge_with_no_checks: false
 
 update:
   trigger: