Skip to content

Commit

Permalink
Add Orgs sharding for Blunderbuss
Browse files Browse the repository at this point in the history 
Using a pointer lets sharding determine whether a
config had been provided so they can be properly
compared. If one is not provided, this also
instantiates one in `setDefaults()`, which is
called following the sharding logic.

Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com>
  • Loading branch information
@dhaiducek
dhaiducek committed Aug 16, 2024
1 parent 2b7da0b commit 569d41b
Show file tree
Hide file tree
Showing 8 changed files with 599 additions and 31 deletions.
5 changes: 3 additions & 2 deletions cmd/deck/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -888,8 +888,9 @@ func TestHandlePluginConfig(t *testing.T) {
}},
},
Blunderbuss: plugins.Blunderbuss{
ExcludeApprovers: true,
},
BlunderbussConfig: &plugins.BlunderbussConfig{
ExcludeApprovers: true,
}},
}
pluginAgent := &plugins.ConfigAgent{}
pluginAgent.Set(&c)
Expand Down
3 changes: 3 additions & 0 deletions pkg/genyaml/genyaml.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,9 @@ func fieldType(field *ast.Field, recurse bool) (string, bool) {
case *ast.SelectorExpr:
// SelectorExpr are not object types yet reference one, thus continue with DFS.
isSelect = true
case *ast.StarExpr:
// Encountered a pointer--overwrite typeName with ast.Expr
typeName = fmt.Sprint(x.X)
}

return recurse || isSelect
Expand Down
45 changes: 35 additions & 10 deletions pkg/plugins/blunderbuss/blunderbuss.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,18 +64,43 @@ func helpProvider(config *plugins.Configuration, _ []config.OrgRepo) (*pluginhel
two := 2
yamlSnippet, err := plugins.CommentMap.GenYaml(&plugins.Configuration{
Blunderbuss: plugins.Blunderbuss{
ReviewerCount: &two,
MaxReviewerCount: 3,
ExcludeApprovers: true,
UseStatusAvailability: true,
IgnoreAuthors: []string{},
BlunderbussConfig: &plugins.BlunderbussConfig{
ReviewerCount: &two,
MaxReviewerCount: 3,
ExcludeApprovers: true,
UseStatusAvailability: true,
IgnoreAuthors: []string{},
},
Orgs: map[string]plugins.BlunderbussOrgConfig{
"": {
BlunderbussConfig: &plugins.BlunderbussConfig{
ReviewerCount: &two,
MaxReviewerCount: 3,
ExcludeApprovers: true,
UseStatusAvailability: true,
IgnoreAuthors: []string{},
},
Repos: map[string]plugins.BlunderbussRepoConfig{
"": {
BlunderbussConfig: plugins.BlunderbussConfig{
ReviewerCount: &two,
MaxReviewerCount: 3,
ExcludeApprovers: true,
UseStatusAvailability: true,
IgnoreAuthors: []string{},
},
},
},
},
},
},
})
if err != nil {
logrus.WithError(err).Warnf("cannot generate comments for %s plugin", PluginName)
}
pluginHelp := &pluginhelp.PluginHelp{
Description: "The blunderbuss plugin automatically requests reviews from reviewers when a new PR is created. The reviewers are selected based on the reviewers specified in the OWNERS files that apply to the files modified by the PR.",
Description: "The blunderbuss plugin automatically requests reviews from reviewers when a new PR is created. " +
"The reviewers are selected based on the reviewers specified in the OWNERS files that apply to the files modified by the PR.",
Config: map[string]string{
"": configString(reviewCount),
},
Expand Down Expand Up @@ -137,14 +162,14 @@ func handlePullRequestEvent(pc plugins.Agent, pre github.PullRequestEvent) error
pc.GitHubClient,
pc.OwnersClient,
pc.Logger,
pc.PluginConfig.Blunderbuss,
pc.PluginConfig.BlunderbussFor(pre.Repo.Owner.Login, pre.Repo.Name),
pre.Action,
&pre.PullRequest,
&pre.Repo,
)
}

func handlePullRequest(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.Blunderbuss, action github.PullRequestEventAction, pr *github.PullRequest, repo *github.Repo) error {
func handlePullRequest(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.BlunderbussConfig, action github.PullRequestEventAction, pr *github.PullRequest, repo *github.Repo) error {
if !(action == github.PullRequestActionOpened || action == github.PullRequestActionReadyForReview) || assign.CCRegexp.MatchString(pr.Body) {
return nil
}
Expand Down Expand Up @@ -176,7 +201,7 @@ func handleGenericCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent)
pc.GitHubClient,
pc.OwnersClient,
pc.Logger,
pc.PluginConfig.Blunderbuss,
pc.PluginConfig.BlunderbussFor(ce.Repo.Owner.Login, ce.Repo.Name),
ce.Action,
ce.IsPR,
ce.Number,
Expand All @@ -186,7 +211,7 @@ func handleGenericCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent)
)
}

func handleGenericComment(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.Blunderbuss, action github.GenericCommentEventAction, isPR bool, prNumber int, issueState string, repo *github.Repo, body string) error {
func handleGenericComment(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.BlunderbussConfig, action github.GenericCommentEventAction, isPR bool, prNumber int, issueState string, repo *github.Repo, body string) error {
if action != github.GenericCommentActionCreated || !isPR || issueState == "closed" {
return nil
}
Expand Down
192 changes: 184 additions & 8 deletions pkg/plugins/blunderbuss/blunderbuss_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ func TestHandlePullRequest(t *testing.T) {
pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}, Body: tc.body, Draft: tc.draft}
repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}
fghc := newFakeGitHubClient(&pr, tc.filesChanged)
c := plugins.Blunderbuss{
c := plugins.BlunderbussConfig{
ReviewerCount: &tc.reviewerCount,
MaxReviewerCount: 0,
ExcludeApprovers: false,
Expand All @@ -667,6 +667,70 @@ func TestHandlePullRequest(t *testing.T) {
}
}

func TestHandlePullRequestShardedConfig(t *testing.T) {
froc := &fakeRepoownersClient{
foc: &fakeOwnersClient{
owners: map[string]string{
"a.go": "1",
},
leafReviewers: map[string]sets.Set[string]{
"a.go": sets.New("al"),
},
},
}

var tc = struct {
action github.PullRequestEventAction
body string
filesChanged []string
reviewerCount int
expectedRequested []string
draft bool
ignoreDrafts bool
ignoreAuthors []string
}{
action: github.PullRequestActionOpened,
filesChanged: []string{"a.go"},
draft: false,
ignoreDrafts: true,
reviewerCount: 1,
}

pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}, Body: tc.body, Draft: tc.draft}
repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}
fghc := newFakeGitHubClient(&pr, tc.filesChanged)
c := &plugins.Configuration{
Blunderbuss: plugins.Blunderbuss{
BlunderbussConfig: &plugins.BlunderbussConfig{
ReviewerCount: &tc.reviewerCount,
MaxReviewerCount: 0,
ExcludeApprovers: false,
IgnoreDrafts: tc.ignoreDrafts,
IgnoreAuthors: tc.ignoreAuthors,
},
Orgs: map[string]plugins.BlunderbussOrgConfig{
"org": {
Repos: map[string]plugins.BlunderbussRepoConfig{
"org/repo": {
BlunderbussConfig: plugins.BlunderbussConfig{
IgnoreAuthors: []string{"author"},
}}}}}}}
bc := c.BlunderbussFor(repo.Owner.Login, repo.Name)

if err := handlePullRequest(
fghc, froc, logrus.WithField("plugin", PluginName),
bc, tc.action, &pr, &repo,
); err != nil {
t.Fatalf("unexpected error from handle: %v", err)
}

sort.Strings(fghc.requested)
sort.Strings(tc.expectedRequested)
if !reflect.DeepEqual(fghc.requested, tc.expectedRequested) {
t.Fatalf("expected the requested reviewers to be %q, but got %q.", tc.expectedRequested, fghc.requested)
}
}

func TestHandleGenericComment(t *testing.T) {
froc := &fakeRepoownersClient{
foc: &fakeOwnersClient{
Expand Down Expand Up @@ -740,7 +804,7 @@ func TestHandleGenericComment(t *testing.T) {
pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}}
fghc := newFakeGitHubClient(&pr, tc.filesChanged)
repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}
config := plugins.Blunderbuss{
config := plugins.BlunderbussConfig{
ReviewerCount: &tc.reviewerCount,
MaxReviewerCount: 0,
ExcludeApprovers: false,
Expand All @@ -762,17 +826,125 @@ func TestHandleGenericComment(t *testing.T) {
}
}

func TestHandleGenericCommentShardedConfig(t *testing.T) {
froc := &fakeRepoownersClient{
foc: &fakeOwnersClient{
owners: map[string]string{
"a.go": "1",
"b.go": "2",
},
leafReviewers: map[string]sets.Set[string]{
"a.go": sets.New("al"),
"b.go": sets.New("bob"),
"c.go": sets.New("sarah"),
"d.go": sets.New("busy-user"),
},
},
}

overrideOrgReviewerCount := 2
overrideRepoReviewerCount := 3
var testcases = []struct {
name string
orgConfig map[string]plugins.BlunderbussOrgConfig
expectedRequested int
}{
{
name: "overrides default config with org config",
orgConfig: map[string]plugins.BlunderbussOrgConfig{
"org": {
BlunderbussConfig: &plugins.BlunderbussConfig{
ReviewerCount: &overrideOrgReviewerCount,
MaxReviewerCount: overrideOrgReviewerCount,
}},
},
expectedRequested: 2,
},
{
name: "overrides default and org config with repo config",
orgConfig: map[string]plugins.BlunderbussOrgConfig{
"org": {
BlunderbussConfig: &plugins.BlunderbussConfig{
ReviewerCount: &overrideOrgReviewerCount,
MaxReviewerCount: overrideOrgReviewerCount,
},
Repos: map[string]plugins.BlunderbussRepoConfig{
"org/repo": {
BlunderbussConfig: plugins.BlunderbussConfig{
ReviewerCount: &overrideRepoReviewerCount,
MaxReviewerCount: overrideRepoReviewerCount,
}}}},
},
expectedRequested: 3,
},
{
name: "Uses org config with invalid repo config key",
orgConfig: map[string]plugins.BlunderbussOrgConfig{
"org": {
BlunderbussConfig: &plugins.BlunderbussConfig{
ReviewerCount: &overrideOrgReviewerCount,
MaxReviewerCount: overrideOrgReviewerCount,
},
Repos: map[string]plugins.BlunderbussRepoConfig{
"repo": {
BlunderbussConfig: plugins.BlunderbussConfig{
ReviewerCount: &overrideRepoReviewerCount,
MaxReviewerCount: overrideRepoReviewerCount,
}}}},
},
expectedRequested: 2,
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}}
fghc := newFakeGitHubClient(&pr, []string{"a.go", "b.go", "c.go", "d.go"})
defaultReviewerCount := 1
repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}

config := &plugins.Configuration{
Blunderbuss: plugins.Blunderbuss{
BlunderbussConfig: &plugins.BlunderbussConfig{
IgnoreAuthors: []string{"bob"},
ReviewerCount: &defaultReviewerCount,
UseStatusAvailability: false,
},
Orgs: tc.orgConfig,
}}
bc := config.BlunderbussFor(repo.Owner.Login, repo.Name)

if err := handleGenericComment(
fghc, froc, logrus.WithField("plugin", PluginName), bc,
github.GenericCommentActionCreated, true, pr.Number, "open", &repo, "/auto-cc",
); err != nil {
t.Fatalf("unexpected error from handle: %v", err)
}

if tc.expectedRequested != len(fghc.requested) {
t.Fatalf("expected the requested reviewers to be %d, but got %d.", tc.expectedRequested, len(fghc.requested))
}
})
}
}

func TestHandleGenericCommentEvent(t *testing.T) {
pc := plugins.Agent{
PluginConfig: &plugins.Configuration{},
PluginConfig: &plugins.Configuration{
Blunderbuss: plugins.Blunderbuss{
BlunderbussConfig: &plugins.BlunderbussConfig{},
}},
}
ce := github.GenericCommentEvent{}
handleGenericCommentEvent(pc, ce)
}

func TestHandlePullRequestEvent(t *testing.T) {
pc := plugins.Agent{
PluginConfig: &plugins.Configuration{},
PluginConfig: &plugins.Configuration{
Blunderbuss: plugins.Blunderbuss{
BlunderbussConfig: &plugins.BlunderbussConfig{},
}},
}
pre := github.PullRequestEvent{}
handlePullRequestEvent(pc, pre)
Expand All @@ -791,17 +963,21 @@ func TestHelpProvider(t *testing.T) {
configInfoIncludes []string
}{
{
name: "Empty config",
config: &plugins.Configuration{},
name: "Empty config",
config: &plugins.Configuration{
Blunderbuss: plugins.Blunderbuss{
BlunderbussConfig: &plugins.BlunderbussConfig{},
}},
enabledRepos: enabledRepos,
configInfoIncludes: []string{configString(0)},
},
{
name: "ReviewerCount specified",
config: &plugins.Configuration{
Blunderbuss: plugins.Blunderbuss{
ReviewerCount: &[]int{2}[0],
},
BlunderbussConfig: &plugins.BlunderbussConfig{
ReviewerCount: &[]int{2}[0],
}},
},
enabledRepos: enabledRepos,
configInfoIncludes: []string{configString(2)},
Expand Down
Loading

0 comments on commit 569d41b

Please sign in to comment.