Evaluate PR Body independently from comments (#69)

* Evaluate PR Body independently from comments

Right now we compare comment substrings on both comments and the PR body. In some cases,
it should be possible to lock down evaluation to only the PR body, instead of comments
_and_ the pr body.

Users who submit PRs do not have write access to the repo, so they cannot add a label.
For comments, anyone with read access to the repo can add a comment on any PR.

However, only someone with write access (or the Author) can edit the PR description.

* Revert breaking changes to body matching

add further test cases

* Update wording in the README

The existing phrasing doesn't accurately describe current operation

Author: asvoboda

README.md

@@ -33,9 +33,12 @@ merge:
     # "labels" is a list of labels that must be matched to whitelist a PR for merging (case-insensitive)
     labels: ["merge when ready"]
 
-    # "comment_substrings" matches on substrings in comments
+    # "comment_substrings" matches on substrings in comments or the pull request body
     comment_substrings: ["==MERGE_WHEN_READY=="]
 
+    # "pr_body_substrings" matches on substrings in the PR body
+    pr_body_substrings: ["==MERGE_WHEN_READY=="]
+
   # "blacklist" defines how to exclude PRs from evaluation and merging
   blacklist:
 

bulldozer/config_v1.go

@@ -31,6 +31,7 @@ type Signals struct {
 	Labels            []string `yaml:"labels"`
 	CommentSubstrings []string `yaml:"comment_substrings"`
 	Comments          []string `yaml:"comments"`
+	PRBodySubstrings  []string `yaml:"pr_body_substrings"`
 }
 
 func (s *Signals) Enabled() bool {

bulldozer/evaluate.go

@@ -69,6 +69,12 @@ func IsPRBlacklisted(ctx context.Context, pullCtx pull.Context, config Signals)
 		}
 	}
 
+	for _, blacklistedSubstring := range config.PRBodySubstrings {
+		if strings.Contains(body, blacklistedSubstring) {
+			return true, fmt.Sprintf("PR body matches one of specified blacklist substrings: %q", blacklistedSubstring), nil
+		}
+	}
+
 	return false, "no matching blacklist found", nil
 }
 
@@ -116,6 +122,12 @@ func IsPRWhitelisted(ctx context.Context, pullCtx pull.Context, config Signals)
 		}
 	}
 
+	for _, whitelistedSubstring := range config.PRBodySubstrings {
+		if strings.Contains(body, whitelistedSubstring) {
+			return true, fmt.Sprintf("PR body matches one of specified whitelist substrings: %q", whitelistedSubstring), nil
+		}
+	}
+
 	return false, "no matching whitelist found", nil
 }
 

bulldozer/evaluate_test.go

@@ -31,11 +31,13 @@ func TestSimpleXListed(t *testing.T) {
 			Labels:            []string{"LABEL_MERGE"},
 			Comments:          []string{"FULL_COMMENT_PLZ_MERGE"},
 			CommentSubstrings: []string{":+1:"},
+			PRBodySubstrings:  []string{"BODY_MERGE_PLZ"},
 		},
 		Blacklist: Signals{
 			Labels:            []string{"LABEL_NOMERGE"},
 			Comments:          []string{"NO_WAY"},
 			CommentSubstrings: []string{":-1:"},
+			PRBodySubstrings:  []string{"BODY_NOMERGE"},
 		},
 	}
 
@@ -167,6 +169,114 @@ func TestSimpleXListed(t *testing.T) {
 		assert.True(t, actualWhitelist)
 		assert.Equal(t, "PR label matches one of specified whitelist labels: \"LABEL_MERGE\"", actualWhitelistReason)
 	})
+
+	t.Run("bodyCausesWhitelist", func(t *testing.T) {
+		pc := &pulltest.MockPullContext{
+			BodyValue: "My PR Body\n\n\n BODY_MERGE_PLZ",
+		}
+
+		actualWhitelist, actualWhitelistReason, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist)
+		require.Nil(t, err)
+		assert.True(t, actualWhitelist)
+		assert.Equal(t, "PR body matches one of specified whitelist substrings: \"BODY_MERGE_PLZ\"", actualWhitelistReason)
+	})
+
+	t.Run("bodyCausesCommentSubstringWhitelist", func(t *testing.T) {
+		pc := &pulltest.MockPullContext{
+			BodyValue: "My PR Body\n\n\n :+1:",
+		}
+
+		actualWhitelist, actualWhitelistReason, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist)
+		require.Nil(t, err)
+		assert.True(t, actualWhitelist)
+		assert.Equal(t, "PR body matches one of specified whitelist comment substrings: \":+1:\"", actualWhitelistReason)
+	})
+
+	t.Run("bodyCausesCommentWhitelist", func(t *testing.T) {
+		pc := &pulltest.MockPullContext{
+			BodyValue: "FULL_COMMENT_PLZ_MERGE",
+		}
+
+		actualWhitelist, actualWhitelistReason, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist)
+		require.Nil(t, err)
+		assert.True(t, actualWhitelist)
+		assert.Equal(t, "PR body matches one of specified whitelist comments: \"FULL_COMMENT_PLZ_MERGE\"", actualWhitelistReason)
+	})
+
+	t.Run("bodyCausesBlacklist", func(t *testing.T) {
+		pc := &pulltest.MockPullContext{
+			BodyValue: "My PR Body\n\n\n BODY_NOMERGE",
+		}
+
+		actualBlacklist, actualBlacklistReason, err := IsPRBlacklisted(ctx, pc, mergeConfig.Blacklist)
+		require.Nil(t, err)
+		assert.True(t, actualBlacklist)
+		assert.Equal(t, "PR body matches one of specified blacklist substrings: \"BODY_NOMERGE\"", actualBlacklistReason)
+	})
+
+	t.Run("bodyCausesCommentSubstringBlacklist", func(t *testing.T) {
+		pc := &pulltest.MockPullContext{
+			BodyValue: "My PR Body\n\n\n :-1:",
+		}
+
+		actualBlacklist, actualBlacklistReason, err := IsPRBlacklisted(ctx, pc, mergeConfig.Blacklist)
+		require.Nil(t, err)
+		assert.True(t, actualBlacklist)
+		assert.Equal(t, "PR body matches one of specified blacklist comment substrings: \":-1:\"", actualBlacklistReason)
+	})
+
+	t.Run("bodyCausesCommentBlacklist", func(t *testing.T) {
+		pc := &pulltest.MockPullContext{
+			BodyValue: "NO_WAY",
+		}
+
+		actualBlacklist, actualBlacklistReason, err := IsPRBlacklisted(ctx, pc, mergeConfig.Blacklist)
+		require.Nil(t, err)
+		assert.True(t, actualBlacklist)
+		assert.Equal(t, "PR body matches one of specified blacklist comments: \"NO_WAY\"", actualBlacklistReason)
+	})
+
+	t.Run("errBodyFailsClosedBlacklist", func(t *testing.T) {
+		pc := &pulltest.MockPullContext{
+			BodyValue:    "My PR Body",
+			BodyErrValue: errors.New("failure"),
+		}
+
+		actualBlacklist, _, err := IsPRBlacklisted(ctx, pc, mergeConfig.Blacklist)
+		require.NotNil(t, err)
+		assert.True(t, actualBlacklist)
+	})
+
+	t.Run("errBodyFailsClosedWhitelist", func(t *testing.T) {
+		pc := &pulltest.MockPullContext{
+			BodyValue:    "My PR Body",
+			BodyErrValue: errors.New("failure"),
+		}
+
+		actualWhitelist, _, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist)
+		require.NotNil(t, err)
+		assert.False(t, actualWhitelist)
+	})
+
+	t.Run("errLabelFailsClosedWhitelist", func(t *testing.T) {
+		pc := &pulltest.MockPullContext{
+			LabelErrValue: errors.New("failure"),
+		}
+
+		actualWhitelist, _, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist)
+		require.NotNil(t, err)
+		assert.False(t, actualWhitelist)
+	})
+
+	t.Run("errCommentsFailsClosedWhitelist", func(t *testing.T) {
+		pc := &pulltest.MockPullContext{
+			CommentErrValue: errors.New("failure"),
+		}
+
+		actualWhitelist, _, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist)
+		require.NotNil(t, err)
+		assert.False(t, actualWhitelist)
+	})
 }
 
 func TestShouldMerge(t *testing.T) {