Skip to content

Commit

Permalink
ignore case when evaluating labels (#64)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmcampanini authored Nov 14, 2018
1 parent c368ee1 commit 76d281f
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 11 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ merge:
# "whitelist" defines how to select PRs to evaluate and merge
whitelist:

# "labels" is a list of labels that must be matched to whitelist a PR for merging
# "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
Expand All @@ -39,7 +39,7 @@ merge:
# "blacklist" defines how to exclude PRs from evaluation and merging
blacklist:

# similar as above, "labels" defines a list of labels. In this case, matched labels cause exclusion.
# similar as above, "labels" defines a list of labels. In this case, matched labels cause exclusion. (case-insensitive)
labels: ["do not merge"]

# "comment_substrings" matches substrings in comments. In this case, matched substrings cause exclusion.
Expand Down
10 changes: 5 additions & 5 deletions bulldozer/config_fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,12 @@ func (cf *ConfigFetcher) unmarshalConfigV0(bytes []byte) (*Config, error) {
Version: 1,
Update: UpdateConfig{
Whitelist: Signals{
Labels: []string{"update me", "Update Me", "UPDATE ME", "update-me", "Update-Me", "UPDATE-ME", "update_me", "Update_Me", "UPDATE_ME"},
Labels: []string{"update me", "update-me", "update_me"},
},
},
Merge: MergeConfig{
Whitelist: Signals{
Labels: []string{"merge when ready", "Merge When Ready", "MERGE WHEN READY", "merge-when-ready", "Merge-When-Ready", "MERGE-WHEN-READY", "merge_when_ready", "Merge_When_Ready", "MERGE_WHEN_READY"},
Labels: []string{"merge when ready", "merge-when-ready", "merge_when_ready"},
},
DeleteAfterMerge: configv0.DeleteAfterMerge,
Method: configv0.Strategy,
Expand All @@ -185,12 +185,12 @@ func (cf *ConfigFetcher) unmarshalConfigV0(bytes []byte) (*Config, error) {
Version: 1,
Update: UpdateConfig{
Whitelist: Signals{
Labels: []string{"update me", "Update Me", "UPDATE ME", "update-me", "Update-Me", "UPDATE-ME", "update_me", "Update_Me", "UPDATE_ME"},
Labels: []string{"update me", "update-me", "update_me"},
},
},
Merge: MergeConfig{
Blacklist: Signals{
Labels: []string{"do not merge", "Do Not Merge", "DO NOT MERGE", "wip", "WIP", "do-not-merge", "Do-Not-Merge", "DO-NOT-MERGE", "do_not_merge", "Do_Not_Merge", "DO_NOT_MERGE"},
Labels: []string{"wip", "do not merge", "do-not-merge", "do_not_merge"},
},
DeleteAfterMerge: configv0.DeleteAfterMerge,
Method: configv0.Strategy,
Expand All @@ -204,7 +204,7 @@ func (cf *ConfigFetcher) unmarshalConfigV0(bytes []byte) (*Config, error) {
Version: 1,
Update: UpdateConfig{
Whitelist: Signals{
Labels: []string{"update me", "Update Me", "UPDATE ME", "update-me", "Update-Me", "UPDATE-ME", "update_me", "Update_Me", "UPDATE_ME"},
Labels: []string{"update me", "update-me", "update_me"},
},
},
Merge: MergeConfig{
Expand Down
16 changes: 14 additions & 2 deletions bulldozer/evaluate.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func IsPRBlacklisted(ctx context.Context, pullCtx pull.Context, config Signals)
return true, "unable to list PR labels", err
}

if inSlice, idx := anyInSlice(labels, config.Labels); inSlice {
if inSlice, idx := anyInSliceCaseInsensitive(labels, config.Labels); inSlice {
return true, fmt.Sprintf("PR label matches one of specified blacklist labels: %q", config.Labels[idx]), nil
}

Expand Down Expand Up @@ -80,7 +80,7 @@ func IsPRWhitelisted(ctx context.Context, pullCtx pull.Context, config Signals)
return false, "unable to list PR labels", err
}

if inSlice, idx := anyInSlice(labels, config.Labels); inSlice {
if inSlice, idx := anyInSliceCaseInsensitive(labels, config.Labels); inSlice {
return true, fmt.Sprintf("PR label matches one of specified whitelist labels: %q", config.Labels[idx]), nil
}

Expand Down Expand Up @@ -130,6 +130,18 @@ func anyInSlice(testValues []string, elements []string) (bool, int) {
return false, -1
}

func anyInSliceCaseInsensitive(testValues []string, elements []string) (bool, int) {
for _, testValue := range testValues {
for index, element := range elements {
if strings.EqualFold(testValue, element) {
return true, index
}
}
}

return false, -1
}

// setDifference returns all elements in set1 that
// are not in set2.
func setDifference(set1, set2 []string) []string {
Expand Down
53 changes: 51 additions & 2 deletions bulldozer/evaluate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,40 @@ func TestSimpleXListed(t *testing.T) {
require.Nil(t, err)
assert.True(t, actualBlacklist)
assert.Equal(t, "PR label matches one of specified blacklist labels: \"LABEL_NOMERGE\"", actualBlacklistReason)
})

t.Run("labelCausesBlacklistCaseInsensitive", func(t *testing.T) {
pc := &pulltest.MockPullContext{
LabelValue: []string{"LABEL_nomERGE"},
}

actualBlacklist, actualBlacklistReason, err := IsPRBlacklisted(ctx, pc, mergeConfig.Blacklist)
require.Nil(t, err)
assert.True(t, actualBlacklist)
assert.Equal(t, "PR label matches one of specified blacklist labels: \"LABEL_NOMERGE\"", actualBlacklistReason)
})

t.Run("labelCausesWhitelist", func(t *testing.T) {
pc := &pulltest.MockPullContext{
LabelValue: []string{"LABEL_MERGE"},
}

actualWhitelist, actualWhitelistReason, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist)
require.Nil(t, err)
assert.False(t, actualWhitelist)
assert.Equal(t, "no matching whitelist found", actualWhitelistReason)
assert.True(t, actualWhitelist)
assert.Equal(t, "PR label matches one of specified whitelist labels: \"LABEL_MERGE\"", actualWhitelistReason)
})

t.Run("labelCausesWhitelistCaseInsensitive", func(t *testing.T) {
pc := &pulltest.MockPullContext{
LabelValue: []string{"LABEL_meRGE"},
}

actualWhitelist, actualWhitelistReason, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist)
require.Nil(t, err)
assert.True(t, actualWhitelist)
assert.Equal(t, "PR label matches one of specified whitelist labels: \"LABEL_MERGE\"", actualWhitelistReason)
})
}

func TestShouldMerge(t *testing.T) {
Expand Down Expand Up @@ -191,6 +218,17 @@ func TestShouldMerge(t *testing.T) {
assert.True(t, actualShouldMerge)
})

t.Run("labelShouldMergeCaseInsensitive", func(t *testing.T) {
pc := &pulltest.MockPullContext{
LabelValue: []string{"LABEL_merGE"},
}

actualShouldMerge, err := ShouldMergePR(ctx, pc, mergeConfig)

require.Nil(t, err)
assert.True(t, actualShouldMerge)
})

t.Run("noContextShouldntMerge", func(t *testing.T) {
pc := &pulltest.MockPullContext{
LabelValue: []string{"NOT_A_LABEL"},
Expand Down Expand Up @@ -235,6 +273,17 @@ func TestShouldMerge(t *testing.T) {
assert.False(t, actualShouldMerge)
})

t.Run("labelCausesBlacklistCaseInsensitive", func(t *testing.T) {
pc := &pulltest.MockPullContext{
LabelValue: []string{"LABEL_nomERGE"},
}

actualShouldMerge, err := ShouldMergePR(ctx, pc, mergeConfig)

require.Nil(t, err)
assert.False(t, actualShouldMerge)
})

t.Run("substringCausesWhitelist", func(t *testing.T) {
pc := &pulltest.MockPullContext{
LabelValue: []string{"NOT_A_LABEL"},
Expand Down

0 comments on commit 76d281f

Please sign in to comment.