[autoroll] Move Gerrit label application and interpretation to go/gerrit
- Add gerrit.Config struct, "constant" values for common projects.
- Use labels from gerrit.Config when setting CQ, dry run, self-approve, etc.
- Use gerrit.Config to derive CqFinished, CqSuccess, DryRunFinished, and DryRunSuccess.
- Drive-by: change labels from map[string]interface{} to map[string]int
Bug: skia:9529
Change-Id: I7d81938bbe13e9fc3d719e90200d9e0e7b04dd83
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/249984
Commit-Queue: Eric Boren <borenet@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
diff --git a/autoroll/go/autoroll-be/main.go b/autoroll/go/autoroll-be/main.go
index 65bdd14..64d12ed 100644
--- a/autoroll/go/autoroll-be/main.go
+++ b/autoroll/go/autoroll-be/main.go
@@ -210,7 +210,11 @@
if cfg.Gerrit != nil {
// Create the code review API client.
- g, err = gerrit.NewGerrit(cfg.Gerrit.URL, gitcookiesPath, nil)
+ gc, err := cfg.Gerrit.GetConfig()
+ if err != nil {
+ sklog.Fatalf("Failed to get Gerrit config: %s", err)
+ }
+ g, err = gerrit.NewGerritWithConfig(gc, cfg.Gerrit.URL, gitcookiesPath, nil)
if err != nil {
sklog.Fatalf("Failed to create Gerrit client: %s", err)
}
diff --git a/autoroll/go/codereview/codereview.go b/autoroll/go/codereview/codereview.go
index 8052e52..5501fd8 100644
--- a/autoroll/go/codereview/codereview.go
+++ b/autoroll/go/codereview/codereview.go
@@ -44,7 +44,6 @@
fullHistoryUrl string
gerritClient gerrit.GerritInterface
issueUrlBase string
- labels map[bool]map[string]interface{}
userEmail string
userName string
}
@@ -83,10 +82,7 @@
// See documentation for CodeReview interface.
func (c *gerritCodeReview) RetrieveRoll(ctx context.Context, issue *autoroll.AutoRollIssue, recent *recent_rolls.RecentRolls, rollingTo *revision.Revision, finishedCallback func(context.Context, RollImpl) error) (RollImpl, error) {
- if c.cfg.Config == GERRIT_CONFIG_ANDROID {
- return newGerritAndroidRoll(ctx, issue, c.gerritClient, recent, c.issueUrlBase, rollingTo, finishedCallback)
- }
- return newGerritRoll(ctx, issue, c.gerritClient, recent, c.issueUrlBase, rollingTo, finishedCallback)
+ return newGerritRoll(ctx, c.cfg, issue, c.gerritClient, recent, c.issueUrlBase, rollingTo, finishedCallback)
}
// See documentation for CodeReview interface.
diff --git a/autoroll/go/codereview/config.go b/autoroll/go/codereview/config.go
index 8e2fd82..daf30c2 100644
--- a/autoroll/go/codereview/config.go
+++ b/autoroll/go/codereview/config.go
@@ -3,9 +3,12 @@
import (
"errors"
"fmt"
+ "sort"
+ "strings"
"go.skia.org/infra/go/gerrit"
"go.skia.org/infra/go/github"
+ "go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/util"
)
@@ -23,47 +26,11 @@
)
var (
- // GERRIT_LABELS indicates which labels should be set on Gerrit CLs in
- // normal and dry run modes, for each Gerrit configuration.
- GERRIT_LABELS = map[string]map[bool]map[string]interface{}{
- GERRIT_CONFIG_ANDROID: {
- // Normal mode.
- false: {
- gerrit.CODEREVIEW_LABEL: "2",
- gerrit.PRESUBMIT_READY_LABEL: "1",
- gerrit.AUTOSUBMIT_LABEL: gerrit.AUTOSUBMIT_LABEL_SUBMIT,
- },
- // Dry run mode.
- true: {
- gerrit.CODEREVIEW_LABEL: "2",
- gerrit.PRESUBMIT_READY_LABEL: "1",
- gerrit.AUTOSUBMIT_LABEL: gerrit.AUTOSUBMIT_LABEL_NONE,
- },
- },
- GERRIT_CONFIG_ANGLE: {
- // Normal mode.
- false: {
- gerrit.CODEREVIEW_LABEL: gerrit.CODEREVIEW_LABEL_SELF_APPROVE,
- gerrit.COMMITQUEUE_LABEL: gerrit.COMMITQUEUE_LABEL_SUBMIT,
- },
- // Dry run mode.
- true: {
- gerrit.CODEREVIEW_LABEL: gerrit.CODEREVIEW_LABEL_SELF_APPROVE,
- gerrit.COMMITQUEUE_LABEL: gerrit.COMMITQUEUE_LABEL_DRY_RUN,
- },
- },
- GERRIT_CONFIG_CHROMIUM: {
- // Normal mode.
- false: {
- gerrit.CODEREVIEW_LABEL: gerrit.CODEREVIEW_LABEL_APPROVE,
- gerrit.COMMITQUEUE_LABEL: gerrit.COMMITQUEUE_LABEL_SUBMIT,
- },
- // Dry run mode.
- true: {
- gerrit.CODEREVIEW_LABEL: gerrit.CODEREVIEW_LABEL_APPROVE,
- gerrit.COMMITQUEUE_LABEL: gerrit.COMMITQUEUE_LABEL_DRY_RUN,
- },
- },
+ // GERRIT_CONFIGS maps Gerrit config names to gerrit.Configs.
+ GERRIT_CONFIGS = map[string]*gerrit.Config{
+ GERRIT_CONFIG_ANDROID: gerrit.CONFIG_ANDROID,
+ GERRIT_CONFIG_ANGLE: gerrit.CONFIG_ANGLE,
+ GERRIT_CONFIG_CHROMIUM: gerrit.CONFIG_CHROMIUM,
}
)
@@ -98,8 +65,13 @@
if c.Project == "" {
return errors.New("Project is required.")
}
- if c.Config != GERRIT_CONFIG_ANDROID && c.Config != GERRIT_CONFIG_ANGLE && c.Config != GERRIT_CONFIG_CHROMIUM {
- return fmt.Errorf("Config must be one of: [%s, %s, %s]", GERRIT_CONFIG_ANDROID, GERRIT_CONFIG_ANGLE, GERRIT_CONFIG_CHROMIUM)
+ if _, ok := GERRIT_CONFIGS[c.Config]; !ok {
+ validConfigs := make([]string, 0, len(GERRIT_CONFIGS))
+ for name := range GERRIT_CONFIGS {
+ validConfigs = append(validConfigs, name)
+ }
+ sort.Strings(validConfigs)
+ return fmt.Errorf("Config must be one of: [%s]", strings.Join(validConfigs, ", "))
}
return nil
}
@@ -109,9 +81,18 @@
return newGerritCodeReview(c, gerritClient)
}
-// GetLabels returns the labels needed for a given CL.
-func (c *GerritConfig) GetLabels(dryRun bool) map[string]interface{} {
- return GERRIT_LABELS[c.Config][dryRun]
+// GetConfig returns the gerrit.Config referenced by the GerritConfig.
+func (c *GerritConfig) GetConfig() (*gerrit.Config, error) {
+ cfg, ok := GERRIT_CONFIGS[c.Config]
+ if !ok {
+ return nil, skerr.Fmt("Unknown Gerrit config %q", c.Config)
+ }
+ return cfg, nil
+}
+
+// CanQueryTrybots returns true if we can query for trybot results.
+func (c *GerritConfig) CanQueryTrybots() bool {
+ return c.Config != GERRIT_CONFIG_ANDROID
}
// GithubConfig provides configuration for Github.
diff --git a/autoroll/go/codereview/roll.go b/autoroll/go/codereview/roll.go
index eb817bb..2e64e99 100644
--- a/autoroll/go/codereview/roll.go
+++ b/autoroll/go/codereview/roll.go
@@ -28,12 +28,14 @@
InsertIntoDB(ctx context.Context) error
}
-func updateGerritIssue(ctx context.Context, a *autoroll.AutoRollIssue, g gerrit.GerritInterface, rollIntoAndroid bool) (*gerrit.ChangeInfo, error) {
+// updateIssueFromGerrit loads details about the issue from the Gerrit API and
+// updates the AutoRollIssue accordingly.
+func updateIssueFromGerrit(ctx context.Context, cfg *GerritConfig, a *autoroll.AutoRollIssue, g gerrit.GerritInterface) (*gerrit.ChangeInfo, error) {
info, err := g.GetIssueProperties(ctx, a.Issue)
if err != nil {
return nil, fmt.Errorf("Failed to get issue properties: %s", err)
}
- if !rollIntoAndroid {
+ if cfg.CanQueryTrybots() {
// Use try results from the most recent non-trivial patchset.
if len(info.Patchsets) == 0 {
return nil, fmt.Errorf("Issue %d has no patchsets!", a.Issue)
@@ -56,12 +58,42 @@
}
a.TryResults = tryResults
}
- if err := a.UpdateFromGerritChangeInfo(info, rollIntoAndroid); err != nil {
+ if err := updateIssueFromGerritChangeInfo(a, info, g.Config()); err != nil {
return nil, fmt.Errorf("Failed to convert issue format: %s", err)
}
return info, nil
}
+// updateIssueFromGerritChangeInfo updates the AutoRollIssue instance based on
+// the given gerrit.ChangeInfo.
+func updateIssueFromGerritChangeInfo(i *autoroll.AutoRollIssue, ci *gerrit.ChangeInfo, gc *gerrit.Config) error {
+ if i.Issue != ci.Issue {
+ return fmt.Errorf("CL ID %d differs from existing issue number %d!", ci.Issue, i.Issue)
+ }
+ i.CqFinished = !i.IsDryRun && !gc.CqRunning(ci)
+ i.CqSuccess = !i.IsDryRun && gc.CqSuccess(ci)
+ i.DryRunFinished = i.IsDryRun && !gc.DryRunRunning(ci)
+ i.DryRunSuccess = i.IsDryRun && gc.DryRunSuccess(ci, gc.DryRunUsesTryjobResults && i.AllTrybotsSucceeded())
+
+ ps := make([]int64, 0, len(ci.Patchsets))
+ for _, p := range ci.Patchsets {
+ ps = append(ps, p.Number)
+ }
+ i.Closed = ci.IsClosed()
+ i.Committed = ci.Committed
+ i.Created = ci.Created
+ i.Modified = ci.Updated
+ i.Patchsets = ps
+ i.Subject = ci.Subject
+ i.Result = autoroll.RollResult(i)
+ // TODO(borenet): If this validation fails, it's likely that it will
+ // continue to fail indefinitely, resulting in a stuck roller.
+ // Additionally, this AutoRollIssue instance persists in the AutoRoller
+ // for its entire lifetime; it's possible to partially fail to update
+ // it and end up in an inconsistent state.
+ return i.Validate()
+}
+
// gerritRoll is an implementation of RollImpl.
type gerritRoll struct {
ci *gerrit.ChangeInfo
@@ -77,8 +109,8 @@
// newGerritRoll obtains a gerritRoll instance from the given Gerrit issue
// number.
-func newGerritRoll(ctx context.Context, issue *autoroll.AutoRollIssue, g gerrit.GerritInterface, recent *recent_rolls.RecentRolls, issueUrlBase string, rollingTo *revision.Revision, cb func(context.Context, RollImpl) error) (RollImpl, error) {
- ci, err := updateGerritIssue(ctx, issue, g, false)
+func newGerritRoll(ctx context.Context, cfg *GerritConfig, issue *autoroll.AutoRollIssue, g gerrit.GerritInterface, recent *recent_rolls.RecentRolls, issueUrlBase string, rollingTo *revision.Revision, cb func(context.Context, RollImpl) error) (RollImpl, error) {
+ ci, err := updateIssueFromGerrit(ctx, cfg, issue, g)
if err != nil {
return nil, err
}
@@ -90,7 +122,7 @@
g: g,
recent: recent,
retrieveRoll: func(ctx context.Context) (*gerrit.ChangeInfo, error) {
- return updateGerritIssue(ctx, issue, g, false)
+ return updateIssueFromGerrit(ctx, cfg, issue, g)
},
rollingTo: rollingTo,
}, nil
@@ -239,86 +271,6 @@
return r.issueUrl
}
-// Special type for Android rolls.
-type gerritAndroidRoll struct {
- *gerritRoll
-}
-
-// newGerritAndroidRoll obtains a gerritAndroidRoll instance from the given Gerrit issue number.
-func newGerritAndroidRoll(ctx context.Context, issue *autoroll.AutoRollIssue, g gerrit.GerritInterface, recent *recent_rolls.RecentRolls, issueUrlBase string, rollingTo *revision.Revision, cb func(context.Context, RollImpl) error) (RollImpl, error) {
- ci, err := updateGerritIssue(ctx, issue, g, true)
- if err != nil {
- return nil, err
- }
- return &gerritAndroidRoll{&gerritRoll{
- ci: ci,
- issue: issue,
- issueUrl: fmt.Sprintf("%s%d", issueUrlBase, issue.Issue),
- finishedCallback: cb,
- g: g,
- recent: recent,
- retrieveRoll: func(ctx context.Context) (*gerrit.ChangeInfo, error) {
- return updateGerritIssue(ctx, issue, g, true)
- },
- rollingTo: rollingTo,
- }}, nil
-}
-
-// See documentation for state_machine.RollCLImpl interface.
-func (r *gerritAndroidRoll) IsDryRunFinished() bool {
- return r.issue.DryRunFinished
-}
-
-// See documentation for state_machine.RollCLImpl interface.
-func (r *gerritAndroidRoll) IsDryRunSuccess() bool {
- return r.issue.DryRunSuccess
-}
-
-// See documentation for state_machine.RollCLImpl interface.
-func (r *gerritAndroidRoll) SwitchToDryRun(ctx context.Context) error {
- return r.withModify(ctx, "switch the CL to dry run", func() error {
- if err := r.g.SetReview(ctx, r.ci, "Mode was changed to dry run", map[string]interface{}{gerrit.AUTOSUBMIT_LABEL: gerrit.AUTOSUBMIT_LABEL_NONE}, nil); err != nil {
- return err
- }
- r.issue.IsDryRun = true
- return nil
- })
-}
-
-// See documentation for state_machine.RollCLImpl interface.
-func (r *gerritAndroidRoll) SwitchToNormal(ctx context.Context) error {
- return r.withModify(ctx, "switch the CL out of dry run", func() error {
- if err := r.g.SetReview(ctx, r.ci, "Mode was changed to normal", map[string]interface{}{gerrit.AUTOSUBMIT_LABEL: gerrit.AUTOSUBMIT_LABEL_SUBMIT}, nil); err != nil {
- return err
- }
- r.issue.IsDryRun = false
- return nil
- })
-}
-
-// See documentation for state_machine.RollCLImpl interface.
-func (r *gerritAndroidRoll) RetryCQ(ctx context.Context) error {
- return r.withModify(ctx, "retry TH", func() error {
- if err := r.g.SetReview(ctx, r.ci, "TH failed but there are no new commits. Retrying...", map[string]interface{}{gerrit.PRESUBMIT_READY_LABEL: "1", gerrit.AUTOSUBMIT_LABEL: gerrit.AUTOSUBMIT_LABEL_SUBMIT}, nil); err != nil {
- return err
- }
- r.issue.IsDryRun = false
- return nil
-
- })
-}
-
-// See documentation for state_machine.RollCLImpl interface.
-func (r *gerritAndroidRoll) RetryDryRun(ctx context.Context) error {
- return r.withModify(ctx, "retry the TH (dry run)", func() error {
- if err := r.g.SetReview(ctx, r.ci, "Dry run failed but there are no new commits. Retrying...", map[string]interface{}{gerrit.PRESUBMIT_READY_LABEL: "1"}, nil); err != nil {
- return err
- }
- r.issue.IsDryRun = true
- return nil
- })
-}
-
// githubRoll is an implementation of RollImpl.
// TODO(rmistry): Add tests after a code-review abstraction later exists.
type githubRoll struct {
@@ -337,7 +289,9 @@
t *travisci.TravisCI
}
-func updateGithubPullRequest(ctx context.Context, a *autoroll.AutoRollIssue, g *github.GitHub, checksNum int, checksWaitFor []string, mergeMethodURL string) (*github_api.PullRequest, error) {
+// updateIssueFromGitHub loads details about the pull request from the GitHub
+// API and updates the AutoRollIssue accordingly.
+func updateIssueFromGitHub(ctx context.Context, a *autoroll.AutoRollIssue, g *github.GitHub, checksNum int, checksWaitFor []string, mergeMethodURL string) (*github_api.PullRequest, error) {
// Retrieve the pull request from github.
pullRequest, err := g.GetPullRequest(int(a.Issue))
@@ -403,7 +357,7 @@
}
a.TryResults = tryResults
- if err := a.UpdateFromGitHubPullRequest(pullRequest); err != nil {
+ if err := updateIssueFromGitHubPullRequest(a, pullRequest); err != nil {
return nil, fmt.Errorf("Failed to convert issue format: %s", err)
}
@@ -471,7 +425,7 @@
if pullRequestModified {
// Update Autoroll issue to show the current state of the PR.
- if err := a.UpdateFromGitHubPullRequest(pullRequest); err != nil {
+ if err := updateIssueFromGitHubPullRequest(a, pullRequest); err != nil {
return nil, fmt.Errorf("Failed to convert issue format: %s", err)
}
}
@@ -479,6 +433,42 @@
return pullRequest, nil
}
+// updateIssueFromGitHubPullRequest updates the AutoRollIssue instance based on the
+// given PullRequest.
+func updateIssueFromGitHubPullRequest(i *autoroll.AutoRollIssue, pullRequest *github_api.PullRequest) error {
+ prNum := int64(pullRequest.GetNumber())
+ if i.Issue != prNum {
+ return fmt.Errorf("Pull request number %d differs from existing issue number %d!", prNum, i.Issue)
+ }
+ i.CqFinished = pullRequest.GetState() == github.CLOSED_STATE || pullRequest.GetMerged()
+ i.CqSuccess = pullRequest.GetMerged()
+ if i.IsDryRun {
+ i.DryRunFinished = i.AllTrybotsFinished() || pullRequest.GetState() == github.CLOSED_STATE || pullRequest.GetMerged()
+ i.DryRunSuccess = (i.DryRunFinished && i.AllTrybotsSucceeded()) || pullRequest.GetMerged()
+ } else {
+ i.DryRunFinished = false
+ i.DryRunSuccess = false
+ }
+
+ ps := make([]int64, 0, *pullRequest.Commits)
+ for i := 1; i <= *pullRequest.Commits; i++ {
+ ps = append(ps, int64(i))
+ }
+ i.Closed = pullRequest.GetState() == github.CLOSED_STATE
+ i.Committed = pullRequest.GetMerged()
+ i.Created = pullRequest.GetCreatedAt()
+ i.Modified = pullRequest.GetUpdatedAt()
+ i.Patchsets = ps
+ i.Subject = pullRequest.GetTitle()
+ i.Result = autoroll.RollResult(i)
+ // TODO(borenet): If this validation fails, it's likely that it will
+ // continue to fail indefinitely, resulting in a stuck roller.
+ // Additionally, this AutoRollIssue instance persists in the AutoRoller
+ // for its entire lifetime; it's possible to partially fail to update
+ // it and end up in an inconsistent state.
+ return i.Validate()
+}
+
func shouldStateBeMerged(mergeableState string) bool {
// Allow "clean" and "unstable" mergeable state.
// "unstable" is allowed (for now) because of a bug in Github where a race condition makes
@@ -490,7 +480,7 @@
// newGithubRoll obtains a githubRoll instance from the given Gerrit issue number.
func newGithubRoll(ctx context.Context, issue *autoroll.AutoRollIssue, g *github.GitHub, recent *recent_rolls.RecentRolls, issueUrlBase string, config *GithubConfig, rollingTo *revision.Revision, cb func(context.Context, RollImpl) error) (RollImpl, error) {
- pullRequest, err := updateGithubPullRequest(ctx, issue, g, config.ChecksNum, config.ChecksWaitFor, config.MergeMethodURL)
+ pullRequest, err := updateIssueFromGitHub(ctx, issue, g, config.ChecksNum, config.ChecksWaitFor, config.MergeMethodURL)
if err != nil {
return nil, err
}
@@ -504,7 +494,7 @@
pullRequest: pullRequest,
recent: recent,
retrieveRoll: func(ctx context.Context) (*github_api.PullRequest, error) {
- return updateGithubPullRequest(ctx, issue, g, config.ChecksNum, config.ChecksWaitFor, config.MergeMethodURL)
+ return updateIssueFromGitHub(ctx, issue, g, config.ChecksNum, config.ChecksWaitFor, config.MergeMethodURL)
},
rollingTo: rollingTo,
}, nil
diff --git a/autoroll/go/codereview/roll_test.go b/autoroll/go/codereview/roll_test.go
index 66a9598..db67843 100644
--- a/autoroll/go/codereview/roll_test.go
+++ b/autoroll/go/codereview/roll_test.go
@@ -9,21 +9,24 @@
"time"
"github.com/golang/protobuf/ptypes"
+ github_api "github.com/google/go-github/github"
"github.com/stretchr/testify/require"
buildbucketpb "go.chromium.org/luci/buildbucket/proto"
"go.skia.org/infra/autoroll/go/recent_rolls"
"go.skia.org/infra/autoroll/go/revision"
"go.skia.org/infra/go/autoroll"
+ "go.skia.org/infra/go/deepequal"
"go.skia.org/infra/go/ds"
"go.skia.org/infra/go/ds/testutil"
"go.skia.org/infra/go/gerrit"
gerrit_testutils "go.skia.org/infra/go/gerrit/testutils"
+ "go.skia.org/infra/go/github"
"go.skia.org/infra/go/mockhttpclient"
"go.skia.org/infra/go/testutils"
"go.skia.org/infra/go/testutils/unittest"
)
-func makeFakeRoll(issueNum int64, from, to string, dryRun, android bool) (*gerrit.ChangeInfo, *autoroll.AutoRollIssue) {
+func makeFakeRoll(t *testing.T, cfg *GerritConfig, issueNum int64, from, to string, dryRun bool) (*gerrit.ChangeInfo, *autoroll.AutoRollIssue) {
// Gerrit API only has millisecond precision.
now := time.Now().UTC().Round(time.Millisecond)
description := fmt.Sprintf(`Roll src/third_party/skia/ %s..%s (42 commits).
@@ -37,23 +40,13 @@
CreatedString: now.Format(gerrit.TIME_FORMAT),
Created: now,
}
- cqLabel := gerrit.COMMITQUEUE_LABEL_SUBMIT
- if android {
- cqLabel = gerrit.AUTOSUBMIT_LABEL_SUBMIT
- }
- if dryRun {
- if android {
- cqLabel = gerrit.AUTOSUBMIT_LABEL_NONE
- } else {
- cqLabel = gerrit.COMMITQUEUE_LABEL_DRY_RUN
- }
- }
roll := &gerrit.ChangeInfo{
Created: now,
CreatedString: now.Format(gerrit.TIME_FORMAT),
Subject: description,
ChangeId: fmt.Sprintf("%d", issueNum),
Issue: issueNum,
+ Labels: map[string]*gerrit.LabelEntry{},
Owner: &gerrit.Owner{
Email: "fake-deps-roller@chromium.org",
},
@@ -65,35 +58,20 @@
Updated: now,
UpdatedString: now.Format(gerrit.TIME_FORMAT),
}
- if android {
- roll.Labels = map[string]*gerrit.LabelEntry{
- gerrit.PRESUBMIT_VERIFIED_LABEL: {
- All: []*gerrit.LabelDetail{},
- },
- gerrit.AUTOSUBMIT_LABEL: {
- All: []*gerrit.LabelDetail{
- {
- Value: cqLabel,
- },
- },
- },
+ gc, err := cfg.GetConfig()
+ require.NoError(t, err)
+ cqLabels := gc.SetCqLabels
+ if dryRun {
+ cqLabels = gc.SetDryRunLabels
+ }
+ for k, v := range cqLabels {
+ roll.Labels[k] = &gerrit.LabelEntry{
+ All: []*gerrit.LabelDetail{{Value: v}},
}
- } else {
- roll.Labels = map[string]*gerrit.LabelEntry{
- gerrit.CODEREVIEW_LABEL: {
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.CODEREVIEW_LABEL_APPROVE,
- },
- },
- },
- gerrit.COMMITQUEUE_LABEL: {
- All: []*gerrit.LabelDetail{
- {
- Value: cqLabel,
- },
- },
- },
+ }
+ for k, v := range gc.SelfApproveLabels {
+ roll.Labels[k] = &gerrit.LabelEntry{
+ All: []*gerrit.LabelDetail{{Value: v}},
}
}
return roll, &autoroll.AutoRollIssue{
@@ -104,7 +82,7 @@
}
}
-func TestGerritRoll(t *testing.T) {
+func testGerritRoll(t *testing.T, cfg *GerritConfig) {
unittest.LargeTest(t)
tmp, err := ioutil.TempDir("", "")
@@ -113,7 +91,9 @@
testutil.InitDatastore(t, ds.KIND_AUTOROLL_ROLL)
- g := gerrit_testutils.NewGerrit(t, tmp, false)
+ gc, err := cfg.GetConfig()
+ require.NoError(t, err)
+ g := gerrit_testutils.NewGerritWithConfig(t, gc, tmp)
ctx := context.Background()
recent, err := recent_rolls.NewRecentRolls(ctx, "test-roller")
require.NoError(t, err)
@@ -125,10 +105,12 @@
Id: to,
Description: "rolling to fghi",
}
- ci, issue := makeFakeRoll(123, from, to, false, false)
+ ci, issue := makeFakeRoll(t, cfg, 123, from, to, false)
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
- gr, err := newGerritRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
+ gr, err := newGerritRoll(ctx, cfg, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
require.NoError(t, err)
require.False(t, issue.IsDryRun)
require.False(t, gr.IsFinished())
@@ -159,16 +141,12 @@
require.False(t, gr.IsDryRunSuccess())
// Set dry run.
- g.MockSetDryRun(ci, "Mode was changed to dry run")
- ci.Labels[gerrit.COMMITQUEUE_LABEL] = &gerrit.LabelEntry{
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.COMMITQUEUE_LABEL_DRY_RUN,
- },
- },
- }
+ g.MockPost(ci, "Mode was changed to dry run", gc.SetDryRunLabels)
+ gerrit.SetLabels(ci, gc.SetDryRunLabels)
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
require.NoError(t, gr.SwitchToDryRun(ctx))
g.AssertEmpty()
require.True(t, issue.IsDryRun)
@@ -178,16 +156,12 @@
require.False(t, gr.IsDryRunSuccess())
// Set normal.
- g.MockSetCQ(ci, "Mode was changed to normal")
- ci.Labels[gerrit.COMMITQUEUE_LABEL] = &gerrit.LabelEntry{
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.COMMITQUEUE_LABEL_SUBMIT,
- },
- },
- }
+ g.MockPost(ci, "Mode was changed to normal", gc.SetCqLabels)
+ gerrit.SetLabels(ci, gc.SetCqLabels)
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
require.NoError(t, gr.SwitchToNormal(ctx))
g.AssertEmpty()
require.False(t, issue.IsDryRun)
@@ -207,7 +181,9 @@
ci.Revisions[fmt.Sprintf("%d", rev.Number)] = rev
ci.Patchsets = append(ci.Patchsets, rev)
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
require.NoError(t, gr.Update(ctx))
require.False(t, issue.IsDryRun)
require.True(t, gr.IsFinished())
@@ -217,32 +193,35 @@
require.Nil(t, recent.CurrentRoll())
// Upload and retrieve another roll, dry run this time.
- ts, err := ptypes.TimestampProto(time.Now().UTC().Round(time.Millisecond))
- require.NoError(t, err)
- ci, issue = makeFakeRoll(124, from, to, true, false)
+ ci, issue = makeFakeRoll(t, cfg, 124, from, to, true)
g.MockGetIssueProperties(ci)
- tryjob := &buildbucketpb.Build{
- Builder: &buildbucketpb.BuilderID{
- Project: "skia",
- Bucket: "fake",
- Builder: "fake-builder",
- },
- Id: 99999,
- CreateTime: ts,
- Status: buildbucketpb.Status_STARTED,
- Tags: []*buildbucketpb.StringPair{
- {
- Key: "user_agent",
- Value: "cq",
+ var tryjob *buildbucketpb.Build
+ if cfg.CanQueryTrybots() {
+ ts, err := ptypes.TimestampProto(time.Now().UTC().Round(time.Millisecond))
+ require.NoError(t, err)
+ tryjob = &buildbucketpb.Build{
+ Builder: &buildbucketpb.BuilderID{
+ Project: "skia",
+ Bucket: "fake",
+ Builder: "fake-builder",
},
- {
- Key: "cq_experimental",
- Value: "false",
+ Id: 99999,
+ CreateTime: ts,
+ Status: buildbucketpb.Status_STARTED,
+ Tags: []*buildbucketpb.StringPair{
+ {
+ Key: "user_agent",
+ Value: "cq",
+ },
+ {
+ Key: "cq_experimental",
+ Value: "false",
+ },
},
- },
+ }
+ g.MockGetTrybotResults(ci, 1, []*buildbucketpb.Build{tryjob})
}
- g.MockGetTrybotResults(ci, 1, []*buildbucketpb.Build{tryjob})
- gr, err = newGerritRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
+ gr, err = newGerritRoll(ctx, cfg, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
require.NoError(t, err)
require.True(t, issue.IsDryRun)
require.False(t, gr.IsFinished())
@@ -262,16 +241,15 @@
g.AssertEmpty()
// Success.
- tryjob.Status = buildbucketpb.Status_SUCCESS
- ci.Labels[gerrit.COMMITQUEUE_LABEL] = &gerrit.LabelEntry{
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.COMMITQUEUE_LABEL_NONE,
- },
- },
+ if len(gc.DryRunSuccessLabels) > 0 {
+ gerrit.SetLabels(ci, gc.DryRunSuccessLabels)
}
+ gerrit.UnsetLabels(ci, gc.DryRunActiveLabels)
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, []*buildbucketpb.Build{tryjob})
+ if cfg.CanQueryTrybots() {
+ tryjob.Status = buildbucketpb.Status_SUCCESS
+ g.MockGetTrybotResults(ci, 1, []*buildbucketpb.Build{tryjob})
+ }
require.NoError(t, gr.Update(ctx))
require.True(t, issue.IsDryRun)
require.False(t, gr.IsFinished())
@@ -283,51 +261,59 @@
// Close for cleanup.
ci.Status = gerrit.CHANGE_STATUS_ABANDONED
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, []*buildbucketpb.Build{tryjob})
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, []*buildbucketpb.Build{tryjob})
+ }
require.NoError(t, gr.Update(ctx))
// Verify that all of the mutation functions handle a conflict (eg.
// someone closed the CL) gracefully.
// 1. SwitchToDryRun.
- ci, issue = makeFakeRoll(125, from, to, false, false)
+ ci, issue = makeFakeRoll(t, cfg, 125, from, to, false)
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
- gr, err = newGerritRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
+ gr, err = newGerritRoll(ctx, cfg, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
require.NoError(t, err)
require.NoError(t, gr.InsertIntoDB(ctx))
- url, reqBytes := g.MakePostRequest(ci, "Mode was changed to dry run", map[string]int{
- gerrit.COMMITQUEUE_LABEL: gerrit.COMMITQUEUE_LABEL_DRY_RUN,
- })
+ url, reqBytes := g.MakePostRequest(ci, "Mode was changed to dry run", gc.SetDryRunLabels)
g.Mock.MockOnce(url, mockhttpclient.MockPostError("application/json", reqBytes, "CONFLICT", http.StatusConflict))
ci.Status = gerrit.CHANGE_STATUS_ABANDONED
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
require.NoError(t, gr.SwitchToDryRun(ctx))
g.AssertEmpty()
// 2. SwitchToNormal
- ci, issue = makeFakeRoll(126, from, to, false, false)
+ ci, issue = makeFakeRoll(t, cfg, 126, from, to, false)
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
- gr, err = newGerritRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
+ gr, err = newGerritRoll(ctx, cfg, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
require.NoError(t, err)
require.NoError(t, gr.InsertIntoDB(ctx))
- url, reqBytes = g.MakePostRequest(ci, "Mode was changed to normal", map[string]int{
- gerrit.COMMITQUEUE_LABEL: gerrit.COMMITQUEUE_LABEL_SUBMIT,
- })
+ url, reqBytes = g.MakePostRequest(ci, "Mode was changed to normal", gc.SetCqLabels)
g.Mock.MockOnce(url, mockhttpclient.MockPostError("application/json", reqBytes, "CONFLICT", http.StatusConflict))
ci.Status = gerrit.CHANGE_STATUS_ABANDONED
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
require.NoError(t, gr.SwitchToNormal(ctx))
g.AssertEmpty()
// 3. Close.
- ci, issue = makeFakeRoll(127, from, to, false, false)
+ ci, issue = makeFakeRoll(t, cfg, 127, from, to, false)
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
- gr, err = newGerritRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
+ gr, err = newGerritRoll(ctx, cfg, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
require.NoError(t, err)
require.NoError(t, gr.InsertIntoDB(ctx))
url = fmt.Sprintf("%s/a/changes/%d/abandon", gerrit_testutils.FAKE_GERRIT_URL, ci.Issue)
@@ -339,15 +325,19 @@
g.Mock.MockOnce(url, mockhttpclient.MockPostError("application/json", []byte(req), "CONFLICT", http.StatusConflict))
ci.Status = gerrit.CHANGE_STATUS_ABANDONED
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
require.NoError(t, gr.Close(ctx, autoroll.ROLL_RESULT_FAILURE, "close it!"))
g.AssertEmpty()
// Verify that we set the correct status when abandoning a CL.
- ci, issue = makeFakeRoll(128, from, to, false, false)
+ ci, issue = makeFakeRoll(t, cfg, 128, from, to, false)
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
- gr, err = newGerritRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
+ gr, err = newGerritRoll(ctx, cfg, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
require.NoError(t, err)
require.NoError(t, gr.InsertIntoDB(ctx))
url = fmt.Sprintf("%s/a/changes/%d/abandon", gerrit_testutils.FAKE_GERRIT_URL, ci.Issue)
@@ -359,7 +349,9 @@
g.Mock.MockOnce(url, mockhttpclient.MockPostDialogue("application/json", []byte(req), nil))
ci.Status = gerrit.CHANGE_STATUS_ABANDONED
g.MockGetIssueProperties(ci)
- g.MockGetTrybotResults(ci, 1, nil)
+ if cfg.CanQueryTrybots() {
+ g.MockGetTrybotResults(ci, 1, nil)
+ }
require.NoError(t, gr.Close(ctx, autoroll.ROLL_RESULT_DRY_RUN_SUCCESS, "close it!"))
g.AssertEmpty()
issue, err = recent.Get(ctx, 128)
@@ -367,206 +359,325 @@
require.Equal(t, issue.Result, autoroll.ROLL_RESULT_DRY_RUN_SUCCESS)
}
+func TestGerritRoll(t *testing.T) {
+ testGerritRoll(t, &GerritConfig{
+ URL: "???",
+ Project: "???",
+ Config: GERRIT_CONFIG_CHROMIUM,
+ })
+}
+
func TestGerritAndroidRoll(t *testing.T) {
- unittest.LargeTest(t)
+ testGerritRoll(t, &GerritConfig{
+ URL: "???",
+ Project: "???",
+ Config: GERRIT_CONFIG_ANDROID,
+ })
+}
- tmp, err := ioutil.TempDir("", "")
- require.NoError(t, err)
- defer testutils.RemoveAll(t, tmp)
+func testUpdateFromGerritChangeInfo(t *testing.T, cfg *gerrit.Config) {
+ unittest.SmallTest(t)
- testutil.InitDatastore(t, ds.KIND_AUTOROLL_ROLL)
+ now := time.Now()
- g := gerrit_testutils.NewGerrit(t, tmp, true)
-
- ctx := context.Background()
- recent, err := recent_rolls.NewRecentRolls(ctx, "test-roller")
- require.NoError(t, err)
-
- // Upload and retrieve the roll.
- from := "abcde12345abcde12345abcde12345abcde12345"
- to := "fghij67890fghij67890fghij67890fghij67890"
- toRev := &revision.Revision{
- Id: to,
- Description: "rolling to fghi",
+ a := &autoroll.AutoRollIssue{
+ Issue: 123,
+ RollingFrom: "abc123",
+ RollingTo: "def456",
}
- ci, issue := makeFakeRoll(123, from, to, false, true)
- g.MockGetIssueProperties(ci)
- gr, err := newGerritAndroidRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
- require.NoError(t, err)
- require.False(t, issue.IsDryRun)
- require.False(t, gr.IsFinished())
- require.False(t, gr.IsSuccess())
- require.False(t, gr.IsDryRunFinished())
- require.False(t, gr.IsDryRunSuccess())
- g.AssertEmpty()
- require.Equal(t, toRev, gr.RollingTo())
- // Insert into DB.
- current := recent.CurrentRoll()
- require.Nil(t, current)
- require.NoError(t, gr.InsertIntoDB(ctx))
- current = recent.CurrentRoll()
- require.NotNil(t, current)
- require.Equal(t, current.Issue, ci.Issue)
- g.AssertEmpty()
+ // Ensure that we don't overwrite the issue number.
+ require.EqualError(t, updateIssueFromGerritChangeInfo(a, &gerrit.ChangeInfo{}, cfg), "CL ID 0 differs from existing issue number 123!")
- // Add a comment.
- msg := "Here's a comment"
- g.MockAddComment(ci, msg)
- require.NoError(t, gr.AddComment(ctx, msg))
- g.AssertEmpty()
- require.False(t, issue.IsDryRun)
- require.False(t, gr.IsFinished())
- require.False(t, gr.IsSuccess())
- require.False(t, gr.IsDryRunFinished())
- require.False(t, gr.IsDryRunSuccess())
+ // Normal, in-progress CL.
+ rev := &gerrit.Revision{
+ ID: "1",
+ Number: 1,
+ Created: now,
+ CreatedString: now.Format(gerrit.TIME_FORMAT),
+ }
+ ci := &gerrit.ChangeInfo{
+ Created: now,
+ CreatedString: now.Format(gerrit.TIME_FORMAT),
+ Subject: "roll the deps",
+ ChangeId: fmt.Sprintf("%d", a.Issue),
+ Issue: a.Issue,
+ Labels: map[string]*gerrit.LabelEntry{},
+ Owner: &gerrit.Owner{
+ Email: "fake@chromium.org",
+ },
+ Project: "skia",
+ Revisions: map[string]*gerrit.Revision{
+ rev.ID: rev,
+ },
+ Patchsets: []*gerrit.Revision{rev},
+ Status: gerrit.CHANGE_STATUS_NEW,
+ Updated: now,
+ UpdatedString: now.Format(gerrit.TIME_FORMAT),
+ }
+ gerrit.SetLabels(ci, cfg.SelfApproveLabels)
+ gerrit.SetLabels(ci, cfg.SetCqLabels)
+ require.NoError(t, updateIssueFromGerritChangeInfo(a, ci, cfg))
+ expect := &autoroll.AutoRollIssue{
+ Created: now,
+ Issue: 123,
+ Modified: now,
+ Patchsets: []int64{1},
+ Result: autoroll.ROLL_RESULT_IN_PROGRESS,
+ RollingFrom: "abc123",
+ RollingTo: "def456",
+ Subject: "roll the deps",
+ }
+ deepequal.AssertDeepEqual(t, expect, a)
- // Set dry run.
- g.MockSetDryRun(ci, "Mode was changed to dry run")
- g.MockGetIssueProperties(ci)
- require.NoError(t, gr.SwitchToDryRun(ctx))
- g.AssertEmpty()
- require.True(t, issue.IsDryRun)
- require.False(t, gr.IsFinished())
- require.False(t, gr.IsSuccess())
- require.False(t, gr.IsDryRunFinished())
- require.False(t, gr.IsDryRunSuccess())
+ // CQ failed.
+ if len(cfg.CqFailureLabels) > 0 {
+ gerrit.SetLabels(ci, cfg.CqFailureLabels)
+ }
+ gerrit.UnsetLabels(ci, cfg.CqActiveLabels)
+ expect.CqFinished = true
+ expect.Result = autoroll.ROLL_RESULT_FAILURE
+ require.NoError(t, updateIssueFromGerritChangeInfo(a, ci, cfg))
+ deepequal.AssertDeepEqual(t, expect, a)
- // Set normal.
- g.MockSetCQ(ci, "Mode was changed to normal")
- g.MockGetIssueProperties(ci)
- require.NoError(t, gr.SwitchToNormal(ctx))
- g.AssertEmpty()
- require.False(t, issue.IsDryRun)
- require.False(t, gr.IsFinished())
- require.False(t, gr.IsSuccess())
- require.False(t, gr.IsDryRunFinished())
- require.False(t, gr.IsDryRunSuccess())
-
- // Update.
+ // CQ succeeded.
+ if len(cfg.CqFailureLabels) > 0 {
+ gerrit.UnsetLabels(ci, cfg.CqFailureLabels)
+ }
+ if len(cfg.CqSuccessLabels) > 0 {
+ gerrit.SetLabels(ci, cfg.CqSuccessLabels)
+ } else {
+ gerrit.UnsetLabels(ci, cfg.CqActiveLabels)
+ }
+ ci.Committed = true
ci.Status = gerrit.CHANGE_STATUS_MERGED
- g.MockGetIssueProperties(ci)
- require.NoError(t, gr.Update(ctx))
- require.False(t, issue.IsDryRun)
- require.True(t, gr.IsFinished())
- require.True(t, gr.IsSuccess())
- require.False(t, gr.IsDryRunFinished())
- require.False(t, gr.IsDryRunSuccess())
- require.Nil(t, recent.CurrentRoll())
+ expect.Closed = true
+ expect.Committed = true
+ expect.CqSuccess = true
+ expect.Result = autoroll.ROLL_RESULT_SUCCESS
+ require.NoError(t, updateIssueFromGerritChangeInfo(a, ci, cfg))
+ deepequal.AssertDeepEqual(t, expect, a)
- // Upload and retrieve another roll, dry run this time.
- ci, issue = makeFakeRoll(124, from, to, true, true)
- g.MockGetIssueProperties(ci)
- gr, err = newGerritAndroidRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
- require.NoError(t, err)
- require.True(t, issue.IsDryRun)
- require.False(t, gr.IsFinished())
- require.False(t, gr.IsSuccess())
- require.False(t, gr.IsDryRunFinished())
- require.False(t, gr.IsDryRunSuccess())
- g.AssertEmpty()
- require.Equal(t, toRev, gr.RollingTo())
+ // CL was abandoned while CQ was running.
+ if len(cfg.CqSuccessLabels) > 0 {
+ gerrit.UnsetLabels(ci, cfg.CqSuccessLabels)
+ } else {
+ gerrit.SetLabels(ci, cfg.CqActiveLabels)
+ }
+ ci.Committed = false
+ ci.Status = gerrit.CHANGE_STATUS_ABANDONED
+ expect.Committed = false
+ expect.CqFinished = true // Not really, but the CL is finished.
+ expect.CqSuccess = false
+ expect.Result = autoroll.ROLL_RESULT_FAILURE
+ require.NoError(t, updateIssueFromGerritChangeInfo(a, ci, cfg))
+ deepequal.AssertDeepEqual(t, expect, a)
- // Insert into DB.
- current = recent.CurrentRoll()
- require.Nil(t, current)
- require.NoError(t, gr.InsertIntoDB(ctx))
- current = recent.CurrentRoll()
- require.NotNil(t, current)
- require.Equal(t, current.Issue, ci.Issue)
- g.AssertEmpty()
+ // Dry run active.
+ ci.Status = gerrit.CHANGE_STATUS_NEW
+ gerrit.UnsetLabels(ci, cfg.SetCqLabels)
+ gerrit.SetLabels(ci, cfg.SetDryRunLabels)
+ expect.Closed = false
+ expect.CqFinished = false
+ expect.IsDryRun = true
+ expect.Result = autoroll.ROLL_RESULT_DRY_RUN_IN_PROGRESS
+ a.IsDryRun = true
+ require.NoError(t, updateIssueFromGerritChangeInfo(a, ci, cfg))
+ deepequal.AssertDeepEqual(t, expect, a)
- // Success.
- ci.Labels[gerrit.PRESUBMIT_VERIFIED_LABEL] = &gerrit.LabelEntry{
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.PRESUBMIT_VERIFIED_LABEL_ACCEPTED,
- },
+ // Dry run failed.
+ if len(cfg.DryRunFailureLabels) > 0 {
+ gerrit.SetLabels(ci, cfg.DryRunFailureLabels)
+ }
+ gerrit.UnsetLabels(ci, cfg.DryRunActiveLabels)
+ expect.DryRunFinished = true
+ expect.Result = autoroll.ROLL_RESULT_DRY_RUN_FAILURE
+ expect.TryResults = []*autoroll.TryResult{
+ {
+ Builder: "fake",
+ Category: autoroll.TRYBOT_CATEGORY_CQ,
+ Result: autoroll.TRYBOT_RESULT_FAILURE,
+ Status: autoroll.TRYBOT_STATUS_COMPLETED,
},
}
- g.MockGetIssueProperties(ci)
- require.NoError(t, gr.Update(ctx))
- require.True(t, issue.IsDryRun)
- require.False(t, gr.IsFinished())
- require.False(t, gr.IsSuccess())
- require.True(t, gr.IsDryRunFinished())
- require.True(t, gr.IsDryRunSuccess())
- g.AssertEmpty()
+ a.TryResults = expect.TryResults
+ require.NoError(t, updateIssueFromGerritChangeInfo(a, ci, cfg))
+ deepequal.AssertDeepEqual(t, expect, a)
- // Close for cleanup.
+ // The CL was abandoned while the dry run was running.
+ expect.TryResults[0].Result = ""
+ expect.TryResults[0].Status = autoroll.TRYBOT_STATUS_SCHEDULED
ci.Status = gerrit.CHANGE_STATUS_ABANDONED
- g.MockGetIssueProperties(ci)
- require.NoError(t, gr.Update(ctx))
+ expect.Closed = true
+ expect.DryRunFinished = true
+ expect.Result = autoroll.ROLL_RESULT_DRY_RUN_FAILURE
+ require.NoError(t, updateIssueFromGerritChangeInfo(a, ci, cfg))
+ deepequal.AssertDeepEqual(t, expect, a)
- // Verify that all of the mutation functions handle a conflict (eg.
- // someone closed the CL) gracefully.
+ // The CL was landed while the dry run was running.
+ ci.Committed = true
+ ci.Status = gerrit.CHANGE_STATUS_MERGED
+ expect.Committed = true
+ expect.DryRunSuccess = true
+ expect.Result = autoroll.ROLL_RESULT_DRY_RUN_SUCCESS
+ require.NoError(t, updateIssueFromGerritChangeInfo(a, ci, cfg))
+ deepequal.AssertDeepEqual(t, expect, a)
- // 1. SwitchToDryRun.
- ci, issue = makeFakeRoll(125, from, to, false, true)
- g.MockGetIssueProperties(ci)
- gr, err = newGerritAndroidRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
- require.NoError(t, err)
- require.NoError(t, gr.InsertIntoDB(ctx))
- url, reqBytes := g.MakePostRequest(ci, "Mode was changed to dry run", map[string]int{
- gerrit.AUTOSUBMIT_LABEL: gerrit.AUTOSUBMIT_LABEL_NONE,
- })
- g.Mock.MockOnce(url, mockhttpclient.MockPostError("application/json", reqBytes, "CONFLICT", http.StatusConflict))
- ci.Status = gerrit.CHANGE_STATUS_ABANDONED
- g.MockGetIssueProperties(ci)
- require.NoError(t, gr.SwitchToDryRun(ctx))
- g.AssertEmpty()
+ // Dry run success.
+ if len(cfg.DryRunSuccessLabels) > 0 {
+ gerrit.SetLabels(ci, cfg.DryRunSuccessLabels)
+ }
+ ci.Committed = false
+ ci.Status = gerrit.CHANGE_STATUS_NEW
+ expect.Closed = false
+ expect.Committed = false
+ expect.CqFinished = false
+ expect.CqSuccess = false
+ expect.DryRunSuccess = true
+ expect.Result = autoroll.ROLL_RESULT_DRY_RUN_SUCCESS
+ expect.TryResults[0].Result = autoroll.TRYBOT_RESULT_SUCCESS
+ expect.TryResults[0].Status = autoroll.TRYBOT_STATUS_COMPLETED
+ require.NoError(t, updateIssueFromGerritChangeInfo(a, ci, cfg))
+ deepequal.AssertDeepEqual(t, expect, a)
+}
- // 2. SwitchToNormal
- ci, issue = makeFakeRoll(126, from, to, false, true)
- g.MockGetIssueProperties(ci)
- gr, err = newGerritAndroidRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
- require.NoError(t, err)
- require.NoError(t, gr.InsertIntoDB(ctx))
- url, reqBytes = g.MakePostRequest(ci, "Mode was changed to normal", map[string]int{
- gerrit.AUTOSUBMIT_LABEL: gerrit.AUTOSUBMIT_LABEL_SUBMIT,
- })
- g.Mock.MockOnce(url, mockhttpclient.MockPostError("application/json", reqBytes, "CONFLICT", http.StatusConflict))
- ci.Status = gerrit.CHANGE_STATUS_ABANDONED
- g.MockGetIssueProperties(ci)
- require.NoError(t, gr.SwitchToNormal(ctx))
- g.AssertEmpty()
+func TestUpdateFromGerritChangeInfoAndroid(t *testing.T) {
+ testUpdateFromGerritChangeInfo(t, gerrit.CONFIG_ANDROID)
+}
- // 3. Close.
- ci, issue = makeFakeRoll(127, from, to, false, true)
- g.MockGetIssueProperties(ci)
- gr, err = newGerritAndroidRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
- require.NoError(t, err)
- require.NoError(t, gr.InsertIntoDB(ctx))
- url = fmt.Sprintf("%s/a/changes/%d/abandon", gerrit_testutils.FAKE_GERRIT_URL, ci.Issue)
- req := testutils.MarshalJSON(t, &struct {
- Message string `json:"message"`
- }{
- Message: "close it!",
- })
- g.Mock.MockOnce(url, mockhttpclient.MockPostError("application/json", []byte(req), "CONFLICT", http.StatusConflict))
- ci.Status = gerrit.CHANGE_STATUS_ABANDONED
- g.MockGetIssueProperties(ci)
- require.NoError(t, gr.Close(ctx, autoroll.ROLL_RESULT_FAILURE, "close it!"))
- g.AssertEmpty()
+func TestUpdateFromGerritChangeInfoANGLE(t *testing.T) {
+ testUpdateFromGerritChangeInfo(t, gerrit.CONFIG_ANGLE)
+}
- // Verify that we set the correct status when abandoning a CL.
- ci, issue = makeFakeRoll(128, from, to, false, true)
- g.MockGetIssueProperties(ci)
- gr, err = newGerritAndroidRoll(ctx, issue, g.Gerrit, recent, "http://issue/", toRev, nil)
- require.NoError(t, err)
- require.NoError(t, gr.InsertIntoDB(ctx))
- url = fmt.Sprintf("%s/a/changes/%d/abandon", gerrit_testutils.FAKE_GERRIT_URL, ci.Issue)
- req = testutils.MarshalJSON(t, &struct {
- Message string `json:"message"`
- }{
- Message: "close it!",
- })
- g.Mock.MockOnce(url, mockhttpclient.MockPostDialogue("application/json", []byte(req), nil))
- ci.Status = gerrit.CHANGE_STATUS_ABANDONED
- g.MockGetIssueProperties(ci)
- require.NoError(t, gr.Close(ctx, autoroll.ROLL_RESULT_DRY_RUN_SUCCESS, "close it!"))
- g.AssertEmpty()
- issue, err = recent.Get(ctx, 128)
- require.NoError(t, err)
- require.Equal(t, issue.Result, autoroll.ROLL_RESULT_DRY_RUN_SUCCESS)
+func TestUpdateFromGerritChangeInfoChromium(t *testing.T) {
+ testUpdateFromGerritChangeInfo(t, gerrit.CONFIG_CHROMIUM)
+}
+
+func TestUpdateFromGitHubPullRequest(t *testing.T) {
+ unittest.SmallTest(t)
+
+ now := time.Now()
+
+ intPtr := func(v int) *int {
+ return &v
+ }
+ stringPtr := func(v string) *string {
+ return &v
+ }
+ boolPtr := func(v bool) *bool {
+ return &v
+ }
+
+ a := &autoroll.AutoRollIssue{
+ Issue: 123,
+ RollingFrom: "abc123",
+ RollingTo: "def456",
+ }
+
+ // Ensure that we don't overwrite the issue number.
+ require.EqualError(t, updateIssueFromGitHubPullRequest(a, &github_api.PullRequest{}), "Pull request number 0 differs from existing issue number 123!")
+
+ // Normal, in-progress CL.
+ pr := &github_api.PullRequest{
+ Number: intPtr(int(a.Issue)),
+ State: stringPtr(""),
+ Commits: intPtr(1),
+ Title: stringPtr("roll the deps"),
+ CreatedAt: &now,
+ UpdatedAt: &now,
+ }
+ require.NoError(t, updateIssueFromGitHubPullRequest(a, pr))
+ expect := &autoroll.AutoRollIssue{
+ Created: now,
+ Issue: 123,
+ Modified: now,
+ Patchsets: []int64{1},
+ Result: autoroll.ROLL_RESULT_IN_PROGRESS,
+ RollingFrom: "abc123",
+ RollingTo: "def456",
+ Subject: "roll the deps",
+ }
+ deepequal.AssertDeepEqual(t, expect, a)
+
+ // CQ failed.
+ pr.State = &github.CLOSED_STATE
+ expect.Closed = true // if the CQ fails, we close the PR.
+ expect.CqFinished = true
+ expect.Result = autoroll.ROLL_RESULT_FAILURE
+ require.NoError(t, updateIssueFromGitHubPullRequest(a, pr))
+ deepequal.AssertDeepEqual(t, expect, a)
+
+ // CQ succeeded.
+ pr.Merged = boolPtr(true)
+ expect.Closed = true
+ expect.Committed = true
+ expect.CqSuccess = true
+ expect.Result = autoroll.ROLL_RESULT_SUCCESS
+ require.NoError(t, updateIssueFromGitHubPullRequest(a, pr))
+ deepequal.AssertDeepEqual(t, expect, a)
+
+ // CL was abandoned while CQ was running.
+ // (the above includes this case)
+
+ // Dry run active.
+ pr.Merged = boolPtr(false)
+ pr.State = stringPtr("")
+ expect.TryResults = []*autoroll.TryResult{
+ {
+ Builder: "fake",
+ Category: autoroll.TRYBOT_CATEGORY_CQ,
+ Status: autoroll.TRYBOT_STATUS_SCHEDULED,
+ },
+ }
+ expect.Closed = false
+ expect.Committed = false
+ expect.CqFinished = false
+ expect.CqSuccess = false
+ expect.IsDryRun = true
+ expect.Result = autoroll.ROLL_RESULT_DRY_RUN_IN_PROGRESS
+ a.IsDryRun = true
+ a.TryResults = expect.TryResults
+ require.NoError(t, updateIssueFromGitHubPullRequest(a, pr))
+ deepequal.AssertDeepEqual(t, expect, a)
+
+ // Dry run failed.
+ expect.DryRunFinished = true
+ expect.Result = autoroll.ROLL_RESULT_DRY_RUN_FAILURE
+ expect.TryResults[0].Result = autoroll.TRYBOT_RESULT_FAILURE
+ expect.TryResults[0].Status = autoroll.TRYBOT_STATUS_COMPLETED
+ a.TryResults = expect.TryResults
+ require.NoError(t, updateIssueFromGitHubPullRequest(a, pr))
+ deepequal.AssertDeepEqual(t, expect, a)
+
+ // CL was abandoned while dry run was still running.
+ expect.TryResults[0].Result = ""
+ expect.TryResults[0].Status = autoroll.TRYBOT_STATUS_SCHEDULED
+ pr.State = &github.CLOSED_STATE
+ expect.Closed = true
+ expect.CqFinished = true
+ require.NoError(t, updateIssueFromGitHubPullRequest(a, pr))
+ deepequal.AssertDeepEqual(t, expect, a)
+
+ // CL was landed while dry run was still running.
+ pr.Merged = boolPtr(true)
+ expect.Committed = true
+ expect.CqSuccess = true
+ expect.DryRunSuccess = true
+ expect.Result = autoroll.ROLL_RESULT_DRY_RUN_SUCCESS
+ require.NoError(t, updateIssueFromGitHubPullRequest(a, pr))
+ deepequal.AssertDeepEqual(t, expect, a)
+
+ // Dry run success.
+ pr.Merged = boolPtr(false)
+ pr.State = stringPtr("")
+ expect.Closed = false
+ expect.Committed = false
+ expect.CqFinished = false
+ expect.CqSuccess = false
+ expect.DryRunSuccess = true
+ expect.Result = autoroll.ROLL_RESULT_DRY_RUN_SUCCESS
+ expect.TryResults[0].Result = autoroll.TRYBOT_RESULT_SUCCESS
+ expect.TryResults[0].Status = autoroll.TRYBOT_STATUS_COMPLETED
+ require.NoError(t, updateIssueFromGitHubPullRequest(a, pr))
+ deepequal.AssertDeepEqual(t, expect, a)
}
diff --git a/autoroll/go/repo_manager/android_repo_manager.go b/autoroll/go/repo_manager/android_repo_manager.go
index b9f5c18..54dc375 100644
--- a/autoroll/go/repo_manager/android_repo_manager.go
+++ b/autoroll/go/repo_manager/android_repo_manager.go
@@ -247,12 +247,6 @@
return r.g.SetTopic(context.TODO(), topic, changeNum)
}
-// setChangeLabels sets the appropriate labels on the Gerrit change, according
-// to the Gerrit config.
-func (r *androidRepoManager) setChangeLabels(change *gerrit.ChangeInfo, dryRun bool) error {
- return r.g.SetReview(context.TODO(), change, "Roller setting labels to auto-land change.", r.codereview.Config().(*codereview.GerritConfig).GetLabels(dryRun), nil)
-}
-
func ExtractBugNumbers(line string) map[string]bool {
bugs := map[string]bool{}
re := regexp.MustCompile("(?m)^(BUG|Bug) *[ :=] *b/([0-9]+) *$")
@@ -483,7 +477,12 @@
}
// Set labels.
- if err := r.setChangeLabels(change, dryRun); err != nil {
+ labels := r.g.Config().SetCqLabels
+ if dryRun {
+ labels = r.g.Config().SetDryRunLabels
+ }
+ labels = gerrit.MergeLabels(labels, r.g.Config().SelfApproveLabels)
+ if err = r.g.SetReview(ctx, change, "Roller setting labels to auto-land change.", labels, emails); err != nil {
// Only throw exception here if parentBranch is master. This is
// because other branches will not have permissions setup for the
// bot to run CR+2.
diff --git a/autoroll/go/repo_manager/android_repo_manager_test.go b/autoroll/go/repo_manager/android_repo_manager_test.go
index 2cb32d5..9bd7e5a 100644
--- a/autoroll/go/repo_manager/android_repo_manager_test.go
+++ b/autoroll/go/repo_manager/android_repo_manager_test.go
@@ -116,6 +116,7 @@
ctx, wd, cleanup := setupAndroid(t)
defer cleanup()
g := &gerrit_mocks.SimpleGerritInterface{IssueID: androidIssueNum}
+ g.On("Config").Return(gerrit.CONFIG_ANDROID)
rm, err := NewAndroidRepoManager(ctx, androidCfg(), wd, g, "fake.server.com", "fake-service-account", nil, androidGerrit(t, g), false)
require.NoError(t, err)
require.NoError(t, SetStrategy(ctx, rm, strategy.ROLL_STRATEGY_BATCH))
@@ -133,6 +134,7 @@
defer cleanup()
g := &gerrit_mocks.SimpleGerritInterface{IssueID: androidIssueNum}
+ g.On("Config").Return(gerrit.CONFIG_ANDROID)
rm, err := NewAndroidRepoManager(ctx, androidCfg(), wd, g, "fake.server.com", "fake-service-account", nil, androidGerrit(t, g), false)
require.NoError(t, err)
require.NoError(t, SetStrategy(ctx, rm, strategy.ROLL_STRATEGY_BATCH))
@@ -201,6 +203,7 @@
defer cleanup()
g := &gerrit_mocks.SimpleGerritInterface{IssueID: androidIssueNum}
+ g.On("Config").Return(gerrit.CONFIG_ANDROID)
rm, err := NewAndroidRepoManager(ctx, androidCfg(), wd, g, "fake.server.com", "fake-service-account", nil, androidGerrit(t, g), false)
require.NoError(t, err)
require.NoError(t, SetStrategy(ctx, rm, strategy.ROLL_STRATEGY_BATCH))
diff --git a/autoroll/go/repo_manager/fuchsia_sdk_android_repo_manager_test.go b/autoroll/go/repo_manager/fuchsia_sdk_android_repo_manager_test.go
index 4e69dc4..95b4642 100644
--- a/autoroll/go/repo_manager/fuchsia_sdk_android_repo_manager_test.go
+++ b/autoroll/go/repo_manager/fuchsia_sdk_android_repo_manager_test.go
@@ -107,7 +107,7 @@
require.NoError(t, err)
serialized = append([]byte("abcd\n"), serialized...)
urlmock.MockOnce(gUrl+"/a/accounts/self/detail", mockhttpclient.MockGetDialogue(serialized))
- g, err := gerrit.NewGerrit(gUrl, gitcookies, urlmock.Client())
+ g, err := gerrit.NewGerritWithConfig(gerrit.CONFIG_ANDROID, gUrl, gitcookies, urlmock.Client())
require.NoError(t, err)
// Initial update, everything up-to-date.
@@ -268,7 +268,7 @@
urlmock.MockOnce("https://fake-skia-review.googlesource.com/a/changes/123/detail?o=ALL_REVISIONS", mockhttpclient.MockGetDialogue(respBody))
// Mock the request to set the CQ.
- reqBody = []byte(`{"labels":{"Autosubmit":1,"Code-Review":"2","Presubmit-Ready":"1"},"message":"","reviewers":[{"reviewer":"reviewer@chromium.org"}]}`)
+ reqBody = []byte(`{"labels":{"Autosubmit":1,"Code-Review":2,"Presubmit-Ready":1},"message":"","reviewers":[{"reviewer":"reviewer@chromium.org"}]}`)
urlmock.MockOnce("https://fake-skia-review.googlesource.com/a/changes/123/revisions/ps1/review", mockhttpclient.MockPostDialogue("application/json", reqBody, []byte("")))
issue, err := rm.CreateNewRoll(ctx, rm.LastRollRev(), rm.NextRollRev(), emails, cqExtraTrybots, false)
diff --git a/autoroll/go/repo_manager/no_checkout_repo_manager.go b/autoroll/go/repo_manager/no_checkout_repo_manager.go
index d90773a..c934988 100644
--- a/autoroll/go/repo_manager/no_checkout_repo_manager.go
+++ b/autoroll/go/repo_manager/no_checkout_repo_manager.go
@@ -124,7 +124,12 @@
}
// Set the CQ bit as appropriate.
- if err = rm.g.SetReview(ctx, ci, "", rm.gerritConfig.GetLabels(dryRun), emails); err != nil {
+ labels := rm.g.Config().SetCqLabels
+ if dryRun {
+ labels = rm.g.Config().SetDryRunLabels
+ }
+ labels = gerrit.MergeLabels(labels, rm.g.Config().SelfApproveLabels)
+ if err = rm.g.SetReview(ctx, ci, "", labels, emails); err != nil {
// TODO(borenet): Should we try to abandon the CL?
return 0, fmt.Errorf("Failed to set review: %s", err)
}
diff --git a/autoroll/go/roller/autoroller.go b/autoroll/go/roller/autoroller.go
index 85b7374..ce4b8db 100644
--- a/autoroll/go/roller/autoroller.go
+++ b/autoroll/go/roller/autoroller.go
@@ -56,8 +56,6 @@
emails []string
emailsMtx sync.RWMutex
failureThrottle *state_machine.Throttler
- gerrit *gerrit.Gerrit
- github *github.GitHub
liveness metrics2.Liveness
manualRollDB manual.DB
modeHistory *modes.ModeHistory
@@ -201,8 +199,6 @@
codereview: cr,
emails: emails,
failureThrottle: failureThrottle,
- gerrit: g,
- github: githubClient,
liveness: metrics2.NewLiveness("last_autoroll_landed", map[string]string{"roller": c.RollerName}),
manualRollDB: manualRollDB,
modeHistory: mh,
diff --git a/go/autoroll/autoroll.go b/go/autoroll/autoroll.go
index ddd22fa..40b5602 100644
--- a/go/autoroll/autoroll.go
+++ b/go/autoroll/autoroll.go
@@ -12,12 +12,9 @@
"time"
"github.com/golang/protobuf/ptypes"
- github_api "github.com/google/go-github/github"
buildbucketpb "go.chromium.org/luci/buildbucket/proto"
"go.skia.org/infra/go/buildbucket"
"go.skia.org/infra/go/comment"
- "go.skia.org/infra/go/gerrit"
- "go.skia.org/infra/go/github"
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/util"
@@ -161,117 +158,8 @@
}
}
-// UpdateFromGitHubPullRequest updates the AutoRollIssue instance based on the
-// given PullRequest. If an error is returned, the AutoRollIssue is not changed.
-func (i *AutoRollIssue) UpdateFromGitHubPullRequest(pullRequest *github_api.PullRequest) error {
- prNum := int64(pullRequest.GetNumber())
- if i.Issue == 0 {
- i.Issue = prNum
- } else if i.Issue != prNum {
- return fmt.Errorf("Pull request number %d differs from existing issue number %d!", prNum, i.Issue)
- }
- i.CqFinished = pullRequest.GetState() == github.CLOSED_STATE || pullRequest.GetMerged()
- i.CqSuccess = pullRequest.GetMerged()
- if i.IsDryRun {
- i.DryRunFinished = i.AllTrybotsFinished() || pullRequest.GetState() == github.CLOSED_STATE || pullRequest.GetMerged()
- i.DryRunSuccess = (i.DryRunFinished && i.AllTrybotsSucceeded()) || pullRequest.GetMerged()
- } else {
- i.DryRunFinished = false
- i.DryRunSuccess = false
- }
-
- ps := make([]int64, 0, *pullRequest.Commits)
- for i := 1; i <= *pullRequest.Commits; i++ {
- ps = append(ps, int64(i))
- }
- i.Closed = pullRequest.GetState() == github.CLOSED_STATE
- i.Committed = pullRequest.GetMerged()
- i.Created = pullRequest.GetCreatedAt()
- i.Modified = pullRequest.GetUpdatedAt()
- i.Patchsets = ps
- i.Subject = pullRequest.GetTitle()
- i.Result = rollResult(i)
- return i.Validate()
-}
-
-// UpdateFromGerritChangeInfo updates the AutoRollIssue instance based on the
-// given gerrit.ChangeInfo. If an error is returned, the AutoRollIssue is not
-// changed.
-func (i *AutoRollIssue) UpdateFromGerritChangeInfo(ci *gerrit.ChangeInfo, rollIntoAndroid bool) error {
- if i.Issue == 0 {
- i.Issue = ci.Issue
- } else if i.Issue != ci.Issue {
- return fmt.Errorf("CL ID %d differs from existing issue number %d!", ci.Issue, i.Issue)
- }
- cqFinished := false
- dryRunFinished := false
- dryRunSuccess := false
- if rollIntoAndroid {
- if _, ok := ci.Labels[gerrit.PRESUBMIT_VERIFIED_LABEL]; ok {
- for _, lb := range ci.Labels[gerrit.PRESUBMIT_VERIFIED_LABEL].All {
- if lb.Value == gerrit.PRESUBMIT_VERIFIED_LABEL_REJECTED {
- cqFinished = true
- dryRunFinished = true
- break
- } else if lb.Value == gerrit.PRESUBMIT_VERIFIED_LABEL_ACCEPTED {
- // Not marking cqSuccess or cqFinished
- // true here; those are only true if the
- // change is merged.
- dryRunFinished = true
- dryRunSuccess = true
- }
- }
- }
- } else {
- foundCqLabel := false
- foundDryRunLabel := false
- if _, ok := ci.Labels[gerrit.COMMITQUEUE_LABEL]; ok {
- for _, lb := range ci.Labels[gerrit.COMMITQUEUE_LABEL].All {
- if lb.Value == gerrit.COMMITQUEUE_LABEL_DRY_RUN {
- foundDryRunLabel = true
- } else if lb.Value == gerrit.COMMITQUEUE_LABEL_SUBMIT {
- foundCqLabel = true
- }
- }
- }
- if !foundCqLabel {
- cqFinished = true
- }
- if !foundDryRunLabel {
- dryRunFinished = true
- }
- if i.IsDryRun && dryRunFinished {
- dryRunSuccess = i.AllTrybotsSucceeded()
- }
- }
-
- i.CqFinished = ci.IsClosed()
- i.CqSuccess = ci.Status == gerrit.CHANGE_STATUS_MERGED
- if i.IsDryRun {
- i.DryRunFinished = dryRunFinished || ci.IsClosed()
- i.DryRunSuccess = dryRunSuccess || ci.Status == gerrit.CHANGE_STATUS_MERGED
- } else {
- i.CqFinished = i.CqFinished || cqFinished
- i.DryRunFinished = false
- i.DryRunSuccess = false
- }
-
- ps := make([]int64, 0, len(ci.Patchsets))
- for _, p := range ci.Patchsets {
- ps = append(ps, p.Number)
- }
- i.Closed = ci.IsClosed()
- i.Committed = ci.Committed
- i.Created = ci.Created
- i.Modified = ci.Updated
- i.Patchsets = ps
- i.Subject = ci.Subject
- i.Result = rollResult(i)
- return i.Validate()
-}
-
-// rollResult derives a result string for the roll.
-func rollResult(roll *AutoRollIssue) string {
+// RollResult derives a result string for the roll.
+func RollResult(roll *AutoRollIssue) string {
if roll.IsDryRun {
if roll.DryRunFinished {
if roll.DryRunSuccess {
diff --git a/go/autoroll/autoroll_test.go b/go/autoroll/autoroll_test.go
index fbb2950..20b7ae3 100644
--- a/go/autoroll/autoroll_test.go
+++ b/go/autoroll/autoroll_test.go
@@ -1,19 +1,15 @@
package autoroll
import (
- "fmt"
"testing"
"time"
"github.com/golang/protobuf/ptypes"
"github.com/golang/protobuf/ptypes/timestamp"
- github_api "github.com/google/go-github/github"
"github.com/stretchr/testify/require"
buildbucketpb "go.chromium.org/luci/buildbucket/proto"
"go.skia.org/infra/go/comment"
"go.skia.org/infra/go/deepequal"
- "go.skia.org/infra/go/gerrit"
- "go.skia.org/infra/go/github"
"go.skia.org/infra/go/testutils/unittest"
)
@@ -77,7 +73,7 @@
Patchsets: []int64{1},
Subject: "Roll src/third_party/skia abc123..def456 (3 commits).",
}
- roll.Result = rollResult(roll)
+ roll.Result = RollResult(roll)
trybot := &buildbucketpb.Build{
Builder: &buildbucketpb.BuilderID{
@@ -172,470 +168,3 @@
require.True(t, roll.AllTrybotsFinished())
require.True(t, roll.AllTrybotsSucceeded())
}
-
-func TestUpdateFromGerritChangeInfo(t *testing.T) {
- unittest.SmallTest(t)
-
- now := time.Now()
-
- a := &AutoRollIssue{
- Issue: 123,
- RollingFrom: "abc123",
- RollingTo: "def456",
- }
-
- // Ensure that we don't overwrite the issue number.
- require.EqualError(t, a.UpdateFromGerritChangeInfo(&gerrit.ChangeInfo{}, false), "CL ID 0 differs from existing issue number 123!")
-
- // Normal, in-progress CL.
- rev := &gerrit.Revision{
- ID: "1",
- Number: 1,
- Created: now,
- CreatedString: now.Format(gerrit.TIME_FORMAT),
- }
- ci := &gerrit.ChangeInfo{
- Created: now,
- CreatedString: now.Format(gerrit.TIME_FORMAT),
- Subject: "roll the deps",
- ChangeId: fmt.Sprintf("%d", a.Issue),
- Issue: a.Issue,
- Labels: map[string]*gerrit.LabelEntry{
- gerrit.CODEREVIEW_LABEL: {
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.CODEREVIEW_LABEL_APPROVE,
- },
- },
- },
- gerrit.COMMITQUEUE_LABEL: {
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.COMMITQUEUE_LABEL_SUBMIT,
- },
- },
- },
- },
- Owner: &gerrit.Owner{
- Email: "fake@chromium.org",
- },
- Project: "skia",
- Revisions: map[string]*gerrit.Revision{
- rev.ID: rev,
- },
- Patchsets: []*gerrit.Revision{rev},
- Status: gerrit.CHANGE_STATUS_NEW,
- Updated: now,
- UpdatedString: now.Format(gerrit.TIME_FORMAT),
- }
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, false))
- expect := &AutoRollIssue{
- Created: now,
- Issue: 123,
- Modified: now,
- Patchsets: []int64{1},
- Result: ROLL_RESULT_IN_PROGRESS,
- RollingFrom: "abc123",
- RollingTo: "def456",
- Subject: "roll the deps",
- }
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CQ failed.
- delete(ci.Labels, gerrit.COMMITQUEUE_LABEL)
- expect.CqFinished = true
- expect.Result = ROLL_RESULT_FAILURE
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, false))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CQ succeeded.
- ci.Committed = true
- ci.Status = gerrit.CHANGE_STATUS_MERGED
- expect.Closed = true
- expect.Committed = true
- expect.CqSuccess = true
- expect.Result = ROLL_RESULT_SUCCESS
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, false))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CL was abandoned while CQ was running.
- ci.Labels[gerrit.COMMITQUEUE_LABEL] = &gerrit.LabelEntry{
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.COMMITQUEUE_LABEL_SUBMIT,
- },
- },
- }
- ci.Committed = false
- ci.Status = gerrit.CHANGE_STATUS_ABANDONED
- expect.Committed = false
- expect.CqFinished = true // Not really, but the CL is finished.
- expect.CqSuccess = false
- expect.Result = ROLL_RESULT_FAILURE
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, false))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // Dry run active.
- ci.Status = gerrit.CHANGE_STATUS_NEW
- ci.Labels[gerrit.COMMITQUEUE_LABEL].All[0].Value = gerrit.COMMITQUEUE_LABEL_DRY_RUN
- expect.Closed = false
- expect.CqFinished = false
- expect.IsDryRun = true
- expect.Result = ROLL_RESULT_DRY_RUN_IN_PROGRESS
- a.IsDryRun = true
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, false))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // Dry run failed.
- delete(ci.Labels, gerrit.COMMITQUEUE_LABEL)
- expect.DryRunFinished = true
- expect.Result = ROLL_RESULT_DRY_RUN_FAILURE
- expect.TryResults = []*TryResult{
- {
- Builder: "fake",
- Category: TRYBOT_CATEGORY_CQ,
- Result: TRYBOT_RESULT_FAILURE,
- Status: TRYBOT_STATUS_COMPLETED,
- },
- }
- a.TryResults = expect.TryResults
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, false))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // The CL was abandoned while the dry run was running.
- expect.TryResults[0].Result = ""
- expect.TryResults[0].Status = TRYBOT_STATUS_SCHEDULED
- ci.Status = gerrit.CHANGE_STATUS_ABANDONED
- expect.Closed = true
- expect.CqFinished = true
- expect.DryRunFinished = true
- expect.Result = ROLL_RESULT_DRY_RUN_FAILURE
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // The CL was landed while the dry run was running.
- ci.Committed = true
- ci.Status = gerrit.CHANGE_STATUS_MERGED
- expect.Committed = true
- expect.CqSuccess = true
- expect.DryRunSuccess = true
- expect.Result = ROLL_RESULT_DRY_RUN_SUCCESS
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // Dry run success.
- ci.Committed = false
- ci.Status = gerrit.CHANGE_STATUS_NEW
- expect.Closed = false
- expect.Committed = false
- expect.CqFinished = false
- expect.CqSuccess = false
- expect.DryRunSuccess = true
- expect.Result = ROLL_RESULT_DRY_RUN_SUCCESS
- expect.TryResults[0].Result = TRYBOT_RESULT_SUCCESS
- expect.TryResults[0].Status = TRYBOT_STATUS_COMPLETED
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, false))
- deepequal.AssertDeepEqual(t, expect, a)
-}
-
-func TestUpdateFromGerritChangeInfoAndroid(t *testing.T) {
- unittest.SmallTest(t)
-
- now := time.Now()
-
- a := &AutoRollIssue{
- Issue: 123,
- RollingFrom: "abc123",
- RollingTo: "def456",
- }
-
- // Ensure that we don't overwrite the issue number.
- require.EqualError(t, a.UpdateFromGerritChangeInfo(&gerrit.ChangeInfo{}, true), "CL ID 0 differs from existing issue number 123!")
-
- // Normal, in-progress CL.
- rev := &gerrit.Revision{
- ID: "1",
- Number: 1,
- Created: now,
- CreatedString: now.Format(gerrit.TIME_FORMAT),
- }
- ci := &gerrit.ChangeInfo{
- Created: now,
- CreatedString: now.Format(gerrit.TIME_FORMAT),
- Subject: "roll the deps",
- ChangeId: fmt.Sprintf("%d", a.Issue),
- Issue: a.Issue,
- Labels: map[string]*gerrit.LabelEntry{
- gerrit.CODEREVIEW_LABEL: {
- All: []*gerrit.LabelDetail{
- {
- Value: 2,
- },
- },
- },
- gerrit.PRESUBMIT_READY_LABEL: {
- All: []*gerrit.LabelDetail{
- {
- Value: 1,
- },
- },
- },
- gerrit.AUTOSUBMIT_LABEL: {
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.AUTOSUBMIT_LABEL_NONE,
- },
- },
- },
- },
- Owner: &gerrit.Owner{
- Email: "fake@chromium.org",
- },
- Project: "skia",
- Revisions: map[string]*gerrit.Revision{
- rev.ID: rev,
- },
- Patchsets: []*gerrit.Revision{rev},
- Status: gerrit.CHANGE_STATUS_NEW,
- Updated: now,
- UpdatedString: now.Format(gerrit.TIME_FORMAT),
- }
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- expect := &AutoRollIssue{
- Created: now,
- Issue: 123,
- Modified: now,
- Patchsets: []int64{1},
- Result: ROLL_RESULT_IN_PROGRESS,
- RollingFrom: "abc123",
- RollingTo: "def456",
- Subject: "roll the deps",
- }
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CQ failed.
- ci.Labels[gerrit.PRESUBMIT_VERIFIED_LABEL] = &gerrit.LabelEntry{
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.PRESUBMIT_VERIFIED_LABEL_REJECTED,
- },
- },
- }
- expect.CqFinished = true
- expect.Result = ROLL_RESULT_FAILURE
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CQ succeeded.
- ci.Committed = true
- ci.Status = gerrit.CHANGE_STATUS_MERGED
- expect.Closed = true
- expect.Committed = true
- expect.CqSuccess = true
- expect.Result = ROLL_RESULT_SUCCESS
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CL was abandoned while CQ was running.
- delete(ci.Labels, gerrit.PRESUBMIT_VERIFIED_LABEL)
- ci.Committed = false
- ci.Status = gerrit.CHANGE_STATUS_ABANDONED
- expect.Committed = false
- expect.CqFinished = true // Not really, but the CL is finished.
- expect.CqSuccess = false
- expect.Result = ROLL_RESULT_FAILURE
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // Dry run active.
- ci.Status = gerrit.CHANGE_STATUS_NEW
- ci.Labels[gerrit.AUTOSUBMIT_LABEL].All[0].Value = gerrit.AUTOSUBMIT_LABEL_NONE
- expect.Closed = false
- expect.CqFinished = false
- expect.IsDryRun = true
- expect.Result = ROLL_RESULT_DRY_RUN_IN_PROGRESS
- a.IsDryRun = true
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // Dry run failed.
- ci.Labels[gerrit.PRESUBMIT_VERIFIED_LABEL] = &gerrit.LabelEntry{
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.PRESUBMIT_VERIFIED_LABEL_REJECTED,
- },
- },
- }
- expect.DryRunFinished = true
- expect.Result = ROLL_RESULT_DRY_RUN_FAILURE
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // The CL was abandoned while the dry run was running.
- ci.Labels[gerrit.PRESUBMIT_VERIFIED_LABEL].All[0].Value = gerrit.PRESUBMIT_VERIFIED_LABEL_RUNNING
- ci.Status = gerrit.CHANGE_STATUS_ABANDONED
- expect.Closed = true
- expect.CqFinished = true
- expect.DryRunFinished = true
- expect.Result = ROLL_RESULT_DRY_RUN_FAILURE
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // The CL was landed while the dry run was running.
- ci.Committed = true
- ci.Status = gerrit.CHANGE_STATUS_MERGED
- expect.Committed = true
- expect.CqSuccess = true
- expect.DryRunSuccess = true
- expect.Result = ROLL_RESULT_DRY_RUN_SUCCESS
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // Dry run success.
- ci.Labels[gerrit.PRESUBMIT_VERIFIED_LABEL] = &gerrit.LabelEntry{
- All: []*gerrit.LabelDetail{
- {
- Value: gerrit.PRESUBMIT_VERIFIED_LABEL_ACCEPTED,
- },
- },
- }
- ci.Committed = false
- ci.Status = gerrit.CHANGE_STATUS_NEW
- expect.Closed = false
- expect.CqFinished = false
- expect.CqSuccess = false
- expect.Committed = false
- expect.DryRunSuccess = true
- expect.Result = ROLL_RESULT_DRY_RUN_SUCCESS
- require.NoError(t, a.UpdateFromGerritChangeInfo(ci, true))
- deepequal.AssertDeepEqual(t, expect, a)
-}
-
-func TestUpdateFromGitHubPullRequest(t *testing.T) {
- unittest.SmallTest(t)
-
- now := time.Now()
-
- intPtr := func(v int) *int {
- return &v
- }
- stringPtr := func(v string) *string {
- return &v
- }
- boolPtr := func(v bool) *bool {
- return &v
- }
-
- a := &AutoRollIssue{
- Issue: 123,
- RollingFrom: "abc123",
- RollingTo: "def456",
- }
-
- // Ensure that we don't overwrite the issue number.
- require.EqualError(t, a.UpdateFromGitHubPullRequest(&github_api.PullRequest{}), "Pull request number 0 differs from existing issue number 123!")
-
- // Normal, in-progress CL.
- pr := &github_api.PullRequest{
- Number: intPtr(int(a.Issue)),
- State: stringPtr(""),
- Commits: intPtr(1),
- Title: stringPtr("roll the deps"),
- CreatedAt: &now,
- UpdatedAt: &now,
- }
- require.NoError(t, a.UpdateFromGitHubPullRequest(pr))
- expect := &AutoRollIssue{
- Created: now,
- Issue: 123,
- Modified: now,
- Patchsets: []int64{1},
- Result: ROLL_RESULT_IN_PROGRESS,
- RollingFrom: "abc123",
- RollingTo: "def456",
- Subject: "roll the deps",
- }
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CQ failed.
- pr.State = &github.CLOSED_STATE
- expect.Closed = true // if the CQ fails, we close the PR.
- expect.CqFinished = true
- expect.Result = ROLL_RESULT_FAILURE
- require.NoError(t, a.UpdateFromGitHubPullRequest(pr))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CQ succeeded.
- pr.Merged = boolPtr(true)
- expect.Closed = true
- expect.Committed = true
- expect.CqSuccess = true
- expect.Result = ROLL_RESULT_SUCCESS
- require.NoError(t, a.UpdateFromGitHubPullRequest(pr))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CL was abandoned while CQ was running.
- // (the above includes this case)
-
- // Dry run active.
- pr.Merged = boolPtr(false)
- pr.State = stringPtr("")
- expect.TryResults = []*TryResult{
- {
- Builder: "fake",
- Category: TRYBOT_CATEGORY_CQ,
- Status: TRYBOT_STATUS_SCHEDULED,
- },
- }
- expect.Closed = false
- expect.Committed = false
- expect.CqFinished = false
- expect.CqSuccess = false
- expect.IsDryRun = true
- expect.Result = ROLL_RESULT_DRY_RUN_IN_PROGRESS
- a.IsDryRun = true
- a.TryResults = expect.TryResults
- require.NoError(t, a.UpdateFromGitHubPullRequest(pr))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // Dry run failed.
- expect.DryRunFinished = true
- expect.Result = ROLL_RESULT_DRY_RUN_FAILURE
- expect.TryResults[0].Result = TRYBOT_RESULT_FAILURE
- expect.TryResults[0].Status = TRYBOT_STATUS_COMPLETED
- a.TryResults = expect.TryResults
- require.NoError(t, a.UpdateFromGitHubPullRequest(pr))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CL was abandoned while dry run was still running.
- expect.TryResults[0].Result = ""
- expect.TryResults[0].Status = TRYBOT_STATUS_SCHEDULED
- pr.State = &github.CLOSED_STATE
- expect.Closed = true
- expect.CqFinished = true
- require.NoError(t, a.UpdateFromGitHubPullRequest(pr))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // CL was landed while dry run was still running.
- pr.Merged = boolPtr(true)
- expect.Committed = true
- expect.CqSuccess = true
- expect.DryRunSuccess = true
- expect.Result = ROLL_RESULT_DRY_RUN_SUCCESS
- require.NoError(t, a.UpdateFromGitHubPullRequest(pr))
- deepequal.AssertDeepEqual(t, expect, a)
-
- // Dry run success.
- pr.Merged = boolPtr(false)
- pr.State = stringPtr("")
- expect.Closed = false
- expect.Committed = false
- expect.CqFinished = false
- expect.CqSuccess = false
- expect.DryRunSuccess = true
- expect.Result = ROLL_RESULT_DRY_RUN_SUCCESS
- expect.TryResults[0].Result = TRYBOT_RESULT_SUCCESS
- expect.TryResults[0].Status = TRYBOT_STATUS_COMPLETED
- require.NoError(t, a.UpdateFromGitHubPullRequest(pr))
- deepequal.AssertDeepEqual(t, expect, a)
-}
diff --git a/go/gerrit/config.go b/go/gerrit/config.go
new file mode 100644
index 0000000..c3f5b00
--- /dev/null
+++ b/go/gerrit/config.go
@@ -0,0 +1,213 @@
+package gerrit
+
+var (
+ CONFIG_ANDROID = &Config{
+ SelfApproveLabels: map[string]int{
+ CODEREVIEW_LABEL: CODEREVIEW_LABEL_SELF_APPROVE,
+ },
+ SetCqLabels: map[string]int{
+ AUTOSUBMIT_LABEL: AUTOSUBMIT_LABEL_SUBMIT,
+ PRESUBMIT_READY_LABEL: PRESUBMIT_READY_LABEL_ENABLE,
+ },
+ SetDryRunLabels: map[string]int{
+ AUTOSUBMIT_LABEL: AUTOSUBMIT_LABEL_NONE,
+ PRESUBMIT_READY_LABEL: PRESUBMIT_READY_LABEL_ENABLE,
+ },
+ NoCqLabels: map[string]int{
+ AUTOSUBMIT_LABEL: AUTOSUBMIT_LABEL_NONE,
+ PRESUBMIT_READY_LABEL: PRESUBMIT_READY_LABEL_NONE,
+ },
+ CqActiveLabels: map[string]int{
+ AUTOSUBMIT_LABEL: AUTOSUBMIT_LABEL_SUBMIT,
+ PRESUBMIT_READY_LABEL: PRESUBMIT_READY_LABEL_ENABLE,
+ },
+ CqSuccessLabels: map[string]int{
+ PRESUBMIT_VERIFIED_LABEL: PRESUBMIT_VERIFIED_LABEL_ACCEPTED,
+ },
+ CqFailureLabels: map[string]int{
+ PRESUBMIT_VERIFIED_LABEL: PRESUBMIT_VERIFIED_LABEL_REJECTED,
+ },
+ DryRunActiveLabels: map[string]int{
+ AUTOSUBMIT_LABEL: AUTOSUBMIT_LABEL_NONE,
+ PRESUBMIT_READY_LABEL: PRESUBMIT_READY_LABEL_ENABLE,
+ },
+ DryRunSuccessLabels: map[string]int{
+ PRESUBMIT_VERIFIED_LABEL: PRESUBMIT_VERIFIED_LABEL_ACCEPTED,
+ },
+ DryRunFailureLabels: map[string]int{
+ PRESUBMIT_VERIFIED_LABEL: PRESUBMIT_VERIFIED_LABEL_REJECTED,
+ },
+ DryRunUsesTryjobResults: false,
+ }
+
+ CONFIG_ANGLE = &Config{
+ SelfApproveLabels: map[string]int{
+ CODEREVIEW_LABEL: CODEREVIEW_LABEL_SELF_APPROVE,
+ },
+ SetCqLabels: map[string]int{
+ COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_SUBMIT,
+ },
+ SetDryRunLabels: map[string]int{
+ COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_DRY_RUN,
+ },
+ NoCqLabels: map[string]int{
+ COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_NONE,
+ },
+ CqActiveLabels: map[string]int{
+ COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_SUBMIT,
+ },
+ CqSuccessLabels: map[string]int{},
+ CqFailureLabels: map[string]int{},
+ DryRunActiveLabels: map[string]int{
+ COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_DRY_RUN,
+ },
+ DryRunSuccessLabels: map[string]int{},
+ DryRunFailureLabels: map[string]int{},
+ DryRunUsesTryjobResults: true,
+ }
+
+ CONFIG_CHROMIUM = &Config{
+ SelfApproveLabels: map[string]int{
+ CODEREVIEW_LABEL: CODEREVIEW_LABEL_APPROVE,
+ },
+ SetCqLabels: map[string]int{
+ COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_SUBMIT,
+ },
+ SetDryRunLabels: map[string]int{
+ COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_DRY_RUN,
+ },
+ NoCqLabels: map[string]int{
+ COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_NONE,
+ },
+ CqActiveLabels: map[string]int{
+ COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_SUBMIT,
+ },
+ CqSuccessLabels: map[string]int{},
+ CqFailureLabels: map[string]int{},
+ DryRunActiveLabels: map[string]int{
+ COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_DRY_RUN,
+ },
+ DryRunSuccessLabels: map[string]int{},
+ DryRunFailureLabels: map[string]int{},
+ DryRunUsesTryjobResults: true,
+ }
+)
+
+type Config struct {
+ // Labels to set to self-approve a change. For some projects this is the
+ // same as a normal approval.
+ SelfApproveLabels map[string]int
+
+ // Labels to set to run the Commit Queue.
+ SetCqLabels map[string]int
+ // Labels to set to run the Commit Queue in dry run mode.
+ SetDryRunLabels map[string]int
+ // Labels to set to remove from the Commit Queue.
+ NoCqLabels map[string]int
+
+ // If the issue is open and all of these labels are set, the Commit
+ // Queue is considered active.
+ CqActiveLabels map[string]int
+ // If the issue is merged or all of these labels are set, the Commit
+ // Queue is considered to have finished successfully.
+ CqSuccessLabels map[string]int
+ // If the issue is abandoned or all of these labels are set, the Commit
+ // Queue is considered to have failed.
+ CqFailureLabels map[string]int
+
+ // If the issue is open and all of these labels are set, the dry run is
+ // considered active.
+ DryRunActiveLabels map[string]int
+ // If the issue is merged or all of these labels are set, the dry run is
+ // considered to have finished successfuly.
+ DryRunSuccessLabels map[string]int
+ // If the issue is abandoned or all of these labels are set, the dry run
+ // is considered to have failed.
+ DryRunFailureLabels map[string]int
+ // DryRunUsesTryjobResults is true if tryjob results should be factored
+ // into dry run success.
+ DryRunUsesTryjobResults bool
+}
+
+// all returns true iff all of the given label keys and values are set on the
+// change. Returns true if the given map of labels is empty.
+func all(ci *ChangeInfo, labels map[string]int) bool {
+ for labelKey, wantValue := range labels {
+ found := false
+ if labelEntry, ok := ci.Labels[labelKey]; ok {
+ for _, labelDetail := range labelEntry.All {
+ if wantValue == labelDetail.Value {
+ found = true
+ }
+ }
+ }
+ if !found {
+ return false
+ }
+ }
+ return true
+}
+
+// MergeLabels returns a new map containing both sets of labels. Labels from the
+// second set overwrite matching labels from the first.
+func MergeLabels(a, b map[string]int) map[string]int {
+ rv := make(map[string]int, len(a)+len(b))
+ for k, v := range a {
+ rv[k] = v
+ }
+ for k, v := range b {
+ rv[k] = v
+ }
+ return rv
+}
+
+// CqRunning returns true if the commit queue is still running. Returns false if
+// the change is merged or abandoned.
+func (c *Config) CqRunning(ci *ChangeInfo) bool {
+ if ci.IsClosed() {
+ return false
+ }
+ if len(c.CqActiveLabels) > 0 && all(ci, c.CqActiveLabels) {
+ return true
+ }
+ return false
+}
+
+// CqSuccess returns true if the commit queue has finished successfully. This
+// requires that the change is merged; CqSuccess returns false if the change is
+// not merged, even if the commit queue finishes (ie. the relevant label is
+// removed) and all trybots were successful or a CqSuccessLabel is applied.
+func (c *Config) CqSuccess(ci *ChangeInfo) bool {
+ return ci.IsMerged()
+}
+
+// DryRunRunning returns true if the dry run is still running. Returns false if
+// the change is merged or abandoned.
+func (c *Config) DryRunRunning(ci *ChangeInfo) bool {
+ if ci.IsClosed() {
+ return false
+ }
+ if len(c.DryRunActiveLabels) > 0 && all(ci, c.DryRunActiveLabels) {
+ return true
+ }
+ return false
+}
+
+// DryRunSuccess returns true if the dry run succeeded. The allTrybotsSucceeded
+// parameter indicates whether or not all of the relevant trybots for this
+// change succeeded; it is unused if Config.DryRunUsesTryjobResults is false.
+func (c *Config) DryRunSuccess(ci *ChangeInfo, allTrybotsSucceeded bool) bool {
+ if c.CqRunning(ci) || c.DryRunRunning(ci) {
+ return false
+ }
+ if ci.IsClosed() {
+ return ci.IsMerged()
+ }
+ if len(c.DryRunSuccessLabels) > 0 && all(ci, c.DryRunSuccessLabels) {
+ return true
+ }
+ if len(c.DryRunFailureLabels) > 0 && all(ci, c.DryRunFailureLabels) {
+ return false
+ }
+ return c.DryRunUsesTryjobResults && allTrybotsSucceeded
+}
diff --git a/go/gerrit/config_test.go b/go/gerrit/config_test.go
new file mode 100644
index 0000000..bfaff91
--- /dev/null
+++ b/go/gerrit/config_test.go
@@ -0,0 +1,104 @@
+package gerrit
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+ "go.skia.org/infra/go/testutils/unittest"
+)
+
+func makeChangeInfo() *ChangeInfo {
+ return &ChangeInfo{
+ Labels: map[string]*LabelEntry{},
+ }
+}
+
+func testConfig(t *testing.T, cfg *Config) {
+ unittest.SmallTest(t)
+ ci := makeChangeInfo()
+
+ // Initial empty change. No CQ labels at all.
+ require.False(t, cfg.CqRunning(ci))
+ require.False(t, cfg.CqSuccess(ci))
+ require.False(t, cfg.DryRunRunning(ci))
+ // Have to use false here because ANGLE and Chromium configs do not use
+ // CQ success/failure labels, so we can't distinguish between "dry run
+ // finished" and "dry run never started".
+ require.False(t, cfg.DryRunSuccess(ci, false))
+
+ // CQ in progress.
+ SetLabels(ci, cfg.SetCqLabels)
+ require.True(t, cfg.CqRunning(ci))
+ require.False(t, cfg.CqSuccess(ci))
+ require.False(t, cfg.DryRunRunning(ci))
+ require.False(t, cfg.DryRunSuccess(ci, true))
+
+ // CQ success.
+ if len(cfg.CqSuccessLabels) > 0 {
+ SetLabels(ci, cfg.CqSuccessLabels)
+ }
+ UnsetLabels(ci, cfg.CqActiveLabels)
+ ci.Status = CHANGE_STATUS_MERGED
+ require.False(t, cfg.CqRunning(ci))
+ require.True(t, cfg.CqSuccess(ci))
+ require.False(t, cfg.DryRunRunning(ci))
+ require.True(t, cfg.DryRunSuccess(ci, false))
+
+ // CQ failed.
+ if len(cfg.CqSuccessLabels) > 0 {
+ UnsetLabels(ci, cfg.CqSuccessLabels)
+ }
+ if len(cfg.CqFailureLabels) > 0 {
+ SetLabels(ci, cfg.CqFailureLabels)
+ }
+ ci.Status = ""
+ require.False(t, cfg.CqRunning(ci))
+ require.False(t, cfg.CqSuccess(ci))
+ require.False(t, cfg.DryRunRunning(ci))
+ require.False(t, cfg.DryRunSuccess(ci, false))
+
+ // Dry run in progress.
+ if len(cfg.CqFailureLabels) > 0 {
+ UnsetLabels(ci, cfg.CqFailureLabels)
+ }
+ UnsetLabels(ci, cfg.SetCqLabels)
+ SetLabels(ci, cfg.SetDryRunLabels)
+ require.False(t, cfg.CqRunning(ci))
+ require.False(t, cfg.CqSuccess(ci))
+ require.True(t, cfg.DryRunRunning(ci))
+ require.False(t, cfg.DryRunSuccess(ci, true))
+
+ // Dry run success.
+ if len(cfg.DryRunSuccessLabels) > 0 {
+ SetLabels(ci, cfg.DryRunSuccessLabels)
+ }
+ UnsetLabels(ci, cfg.DryRunActiveLabels)
+ require.False(t, cfg.CqRunning(ci))
+ require.False(t, cfg.CqSuccess(ci))
+ require.False(t, cfg.DryRunRunning(ci))
+ require.True(t, cfg.DryRunSuccess(ci, true))
+
+ // Dry run failed.
+ if len(cfg.DryRunSuccessLabels) > 0 {
+ UnsetLabels(ci, cfg.DryRunSuccessLabels)
+ }
+ if len(cfg.DryRunFailureLabels) > 0 {
+ SetLabels(ci, cfg.DryRunFailureLabels)
+ }
+ require.False(t, cfg.CqRunning(ci))
+ require.False(t, cfg.CqSuccess(ci))
+ require.False(t, cfg.DryRunRunning(ci))
+ require.False(t, cfg.DryRunSuccess(ci, false))
+}
+
+func TestConfigAndroid(t *testing.T) {
+ testConfig(t, CONFIG_ANDROID)
+}
+
+func TestConfigANGLE(t *testing.T) {
+ testConfig(t, CONFIG_ANGLE)
+}
+
+func TestConfigChromium(t *testing.T) {
+ testConfig(t, CONFIG_CHROMIUM)
+}
diff --git a/go/gerrit/gerrit.go b/go/gerrit/gerrit.go
index 04ec3fa..9b501b1 100644
--- a/go/gerrit/gerrit.go
+++ b/go/gerrit/gerrit.go
@@ -66,6 +66,8 @@
AUTOSUBMIT_LABEL_NONE = 0
AUTOSUBMIT_LABEL_SUBMIT = 1
PRESUBMIT_READY_LABEL = "Presubmit-Ready"
+ PRESUBMIT_READY_LABEL_NONE = 0
+ PRESUBMIT_READY_LABEL_ENABLE = 1
PRESUBMIT_VERIFIED_LABEL = "Presubmit-Verified"
PRESUBMIT_VERIFIED_LABEL_REJECTED = -1
PRESUBMIT_VERIFIED_LABEL_RUNNING = 0
@@ -157,9 +159,15 @@
// IsClosed returns true iff the issue corresponding to the ChangeInfo is
// abandoned or merged.
-func (c ChangeInfo) IsClosed() bool {
- return (c.Status == CHANGE_STATUS_ABANDONED ||
- c.Status == CHANGE_STATUS_MERGED)
+func (ci *ChangeInfo) IsClosed() bool {
+ return (ci.Status == CHANGE_STATUS_ABANDONED ||
+ ci.Status == CHANGE_STATUS_MERGED)
+}
+
+// IsMerged returns true iff the issue corresponding to the ChangeInfo is
+// merged.
+func (ci *ChangeInfo) IsMerged() bool {
+ return ci.Status == CHANGE_STATUS_MERGED
}
// Owner gathers the owner information of a ChangeInfo instance. Some fields omitted.
@@ -203,6 +211,7 @@
Abandon(context.Context, *ChangeInfo, string) error
AddComment(context.Context, *ChangeInfo, string) error
Approve(context.Context, *ChangeInfo, string) error
+ Config() *Config
CreateChange(context.Context, string, string, string, string) (*ChangeInfo, error)
DeleteChangeEdit(context.Context, *ChangeInfo) error
DeleteFile(context.Context, *ChangeInfo, string) error
@@ -223,11 +232,12 @@
PublishChangeEdit(context.Context, *ChangeInfo) error
RemoveFromCQ(context.Context, *ChangeInfo, string) error
Search(context.Context, int, ...*SearchTerm) ([]*ChangeInfo, error)
+ SelfApprove(context.Context, *ChangeInfo, string) error
SendToCQ(context.Context, *ChangeInfo, string) error
SendToDryRun(context.Context, *ChangeInfo, string) error
SetCommitMessage(context.Context, *ChangeInfo, string) error
SetReadyForReview(context.Context, *ChangeInfo) error
- SetReview(context.Context, *ChangeInfo, string, map[string]interface{}, []string) error
+ SetReview(context.Context, *ChangeInfo, string, map[string]int, []string) error
SetTopic(context.Context, string, int64) error
TurnOnAuthenticatedGets()
Url(int64) string
@@ -235,6 +245,7 @@
// Gerrit is an object used for interacting with the issue tracker.
type Gerrit struct {
+ cfg *Config
client *http.Client
BuildbucketClient *buildbucket.Client
gitCookiesPath string
@@ -247,6 +258,13 @@
// instance will be in read-only mode and only return information available to
// anonymous users.
func NewGerrit(gerritUrl, gitCookiesPath string, client *http.Client) (*Gerrit, error) {
+ return NewGerritWithConfig(CONFIG_CHROMIUM, gerritUrl, gitCookiesPath, client)
+}
+
+// NewGerritWithConfig returns a new Gerrit instance which uses the given
+// Config. If gitCookiesPath is empty the instance will be in read-only mode and
+// only return information available to anonymous users.
+func NewGerritWithConfig(cfg *Config, gerritUrl, gitCookiesPath string, client *http.Client) (*Gerrit, error) {
parsedUrl, err := url.Parse(gerritUrl)
if err != nil {
return nil, skerr.Fmt("Unable to parse gerrit URL: %s", err)
@@ -262,6 +280,7 @@
client = httputils.NewTimeoutClient()
}
return &Gerrit{
+ cfg: cfg,
url: gerritUrl,
client: client,
BuildbucketClient: buildbucket.NewClient(client),
@@ -270,6 +289,11 @@
}, nil
}
+// Config returns the Config object used by this Gerrit.
+func (g *Gerrit) Config() *Config {
+ return g.cfg
+}
+
// DefaultGitCookiesPath returns the default cookie file. The return value
// can be used as the input to NewGerrit. If it cannot be retrieved an
// error will be logged and the empty string is returned.
@@ -478,7 +502,7 @@
// setReview calls the Set Review endpoint of the Gerrit API to add messages and/or set labels for
// the latest patchset.
// API documentation: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#set-review
-func (g *Gerrit) SetReview(ctx context.Context, issue *ChangeInfo, message string, labels map[string]interface{}, reviewers []string) error {
+func (g *Gerrit) SetReview(ctx context.Context, issue *ChangeInfo, message string, labels map[string]int, reviewers []string) error {
postData := map[string]interface{}{
"message": message,
"labels": labels,
@@ -499,35 +523,39 @@
// AddComment adds a message to the issue.
func (g *Gerrit) AddComment(ctx context.Context, issue *ChangeInfo, message string) error {
- return g.SetReview(ctx, issue, message, map[string]interface{}{}, nil)
+ return g.SetReview(ctx, issue, message, map[string]int{}, nil)
}
// Utility methods for interacting with the COMMITQUEUE_LABEL.
func (g *Gerrit) SendToDryRun(ctx context.Context, issue *ChangeInfo, message string) error {
- return g.SetReview(ctx, issue, message, map[string]interface{}{COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_DRY_RUN}, nil)
+ return g.SetReview(ctx, issue, message, g.cfg.SetDryRunLabels, nil)
}
func (g *Gerrit) SendToCQ(ctx context.Context, issue *ChangeInfo, message string) error {
- return g.SetReview(ctx, issue, message, map[string]interface{}{COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_SUBMIT}, nil)
+ return g.SetReview(ctx, issue, message, g.cfg.SetCqLabels, nil)
}
func (g *Gerrit) RemoveFromCQ(ctx context.Context, issue *ChangeInfo, message string) error {
- return g.SetReview(ctx, issue, message, map[string]interface{}{COMMITQUEUE_LABEL: COMMITQUEUE_LABEL_NONE}, nil)
+ return g.SetReview(ctx, issue, message, g.cfg.NoCqLabels, nil)
}
// Utility methods for interacting with the CODEREVIEW_LABEL.
func (g *Gerrit) Approve(ctx context.Context, issue *ChangeInfo, message string) error {
- return g.SetReview(ctx, issue, message, map[string]interface{}{CODEREVIEW_LABEL: CODEREVIEW_LABEL_APPROVE}, nil)
+ return g.SetReview(ctx, issue, message, map[string]int{CODEREVIEW_LABEL: CODEREVIEW_LABEL_APPROVE}, nil)
}
func (g *Gerrit) NoScore(ctx context.Context, issue *ChangeInfo, message string) error {
- return g.SetReview(ctx, issue, message, map[string]interface{}{CODEREVIEW_LABEL: CODEREVIEW_LABEL_NONE}, nil)
+ return g.SetReview(ctx, issue, message, map[string]int{CODEREVIEW_LABEL: CODEREVIEW_LABEL_NONE}, nil)
}
func (g *Gerrit) DisApprove(ctx context.Context, issue *ChangeInfo, message string) error {
- return g.SetReview(ctx, issue, message, map[string]interface{}{CODEREVIEW_LABEL: CODEREVIEW_LABEL_DISAPPROVE}, nil)
+ return g.SetReview(ctx, issue, message, map[string]int{CODEREVIEW_LABEL: CODEREVIEW_LABEL_DISAPPROVE}, nil)
+}
+
+func (g *Gerrit) SelfApprove(ctx context.Context, issue *ChangeInfo, message string) error {
+ return g.SetReview(ctx, issue, message, g.cfg.SelfApproveLabels, nil)
}
// Abandon abandons the issue with the given message.
@@ -1107,3 +1135,46 @@
func (g *Gerrit) DeleteChangeEdit(ctx context.Context, ci *ChangeInfo) error {
return g.delete(ctx, fmt.Sprintf("/a/changes/%s/edit", ci.Id))
}
+
+// Set the given label on the ChangeInfo.
+func SetLabel(ci *ChangeInfo, key string, value int) {
+ labelEntry, ok := ci.Labels[key]
+ if !ok {
+ labelEntry = &LabelEntry{
+ All: []*LabelDetail{},
+ }
+ ci.Labels[key] = labelEntry
+ }
+ labelEntry.All = append(labelEntry.All, &LabelDetail{
+ Value: value,
+ })
+}
+
+// Set the given labels on the ChangeInfo.
+func SetLabels(ci *ChangeInfo, labels map[string]int) {
+ for key, value := range labels {
+ SetLabel(ci, key, value)
+ }
+}
+
+// Unset the given label on the ChangeInfo.
+func UnsetLabel(ci *ChangeInfo, key string, value int) {
+ labelEntry, ok := ci.Labels[key]
+ if !ok {
+ return
+ }
+ newEntries := make([]*LabelDetail, 0, len(labelEntry.All))
+ for _, details := range labelEntry.All {
+ if details.Value != value {
+ newEntries = append(newEntries, details)
+ }
+ }
+ labelEntry.All = newEntries
+}
+
+// Unset the given labels on the ChangeInfo.
+func UnsetLabels(ci *ChangeInfo, labels map[string]int) {
+ for key, value := range labels {
+ UnsetLabel(ci, key, value)
+ }
+}
diff --git a/go/gerrit/gerrit_test.go b/go/gerrit/gerrit_test.go
index 28bcfae..966c444 100644
--- a/go/gerrit/gerrit_test.go
+++ b/go/gerrit/gerrit_test.go
@@ -28,7 +28,7 @@
func TestHasOpenDependency(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
dep, err := api.HasOpenDependency(context.TODO(), 52160, 1)
@@ -43,7 +43,7 @@
func TestGerritOwnerModifiedSearch(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
t_delta := time.Now().Add(-10 * 24 * time.Hour)
@@ -66,7 +66,7 @@
func TestGerritCommitSearch(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
api.TurnOnAuthenticatedGets()
@@ -90,7 +90,7 @@
func GerritPollerTest(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
cache := NewCodeReviewCache(api, 10*time.Second, 3)
@@ -106,7 +106,7 @@
func TestGetPatch(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
patch, err := api.GetPatch(context.TODO(), 2370, "current")
@@ -130,7 +130,7 @@
func TestAddComment(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
changeInfo, err := api.GetIssueProperties(context.TODO(), 2370)
@@ -142,7 +142,7 @@
func TestSendToDryRun(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
// Send to dry run.
@@ -173,7 +173,7 @@
func TestSendToCQ(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
// Send to CQ.
@@ -204,7 +204,7 @@
func TestApprove(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
// Approve.
@@ -235,7 +235,7 @@
func TestReadOnlyFailure(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, "", nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, "", nil)
require.NoError(t, err)
// Approve.
@@ -248,7 +248,7 @@
func TestDisApprove(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
// DisApprove.
@@ -279,7 +279,7 @@
func TestAbandon(t *testing.T) {
skipTestIfRequired(t)
- api, err := NewGerrit(GERRIT_SKIA_URL, "", nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, "", nil)
require.NoError(t, err)
c1 := ChangeInfo{
ChangeId: "Idb96a747c8446126f60fdf1adca361dbc2e539d5",
@@ -324,7 +324,7 @@
defer ts.Close()
- api, err := NewGerrit(ts.URL, "", nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, ts.URL, "", nil)
files, err := api.Files(context.TODO(), 12345678, "current")
require.NoError(t, err)
require.Len(t, files, 4)
@@ -373,7 +373,7 @@
defer ts.Close()
- api, err := NewGerrit(ts.URL, "", nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, ts.URL, "", nil)
files, err := api.GetFileNames(context.TODO(), 12345678, "current")
require.NoError(t, err)
require.Len(t, files, 4)
@@ -398,7 +398,7 @@
require.NoError(t, err)
}))
defer tsNoBinary.Close()
- api, err := NewGerrit(tsNoBinary.URL, "", nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, tsNoBinary.URL, "", nil)
require.NoError(t, err)
isBinaryPatch, err := api.IsBinaryPatch(context.TODO(), 4649, "3")
require.NoError(t, err)
@@ -424,7 +424,7 @@
require.NoError(t, err)
}))
defer tsBinary.Close()
- api, err = NewGerrit(tsBinary.URL, "", nil)
+ api, err = NewGerritWithConfig(CONFIG_CHROMIUM, tsBinary.URL, "", nil)
require.NoError(t, err)
isBinaryPatch, err = api.IsBinaryPatch(context.TODO(), 2370, "5")
require.NoError(t, err)
@@ -443,7 +443,7 @@
Reviewed-on: https://skia-review.googlesource.com/549319
Commit-Queue: John Doe <jdoe@example.com>
`
- api, err := NewGerrit(GERRIT_SKIA_URL, "", nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, "", nil)
require.NoError(t, err)
issueID, err := api.ExtractIssueFromCommit(cmtMsg)
require.NoError(t, err)
@@ -460,7 +460,7 @@
revision := "91740d74af689d53b9fa4d172544e0d5620de9bd"
expectedParent := "aaab3c73575d5502ae345dd71cf8748c2070ffda"
- api, err := NewGerrit(GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
+ api, err := NewGerritWithConfig(CONFIG_CHROMIUM, GERRIT_SKIA_URL, DefaultGitCookiesPath(), nil)
require.NoError(t, err)
commitInfo, err := api.GetCommit(context.TODO(), issueID, revision)
diff --git a/go/gerrit/mocks/GerritInterface.go b/go/gerrit/mocks/GerritInterface.go
index 3fd40f7..36a988f 100644
--- a/go/gerrit/mocks/GerritInterface.go
+++ b/go/gerrit/mocks/GerritInterface.go
@@ -59,6 +59,22 @@
return r0
}
+// Config provides a mock function with given fields:
+func (_m *GerritInterface) Config() *gerrit.Config {
+ ret := _m.Called()
+
+ var r0 *gerrit.Config
+ if rf, ok := ret.Get(0).(func() *gerrit.Config); ok {
+ r0 = rf()
+ } else {
+ if ret.Get(0) != nil {
+ r0 = ret.Get(0).(*gerrit.Config)
+ }
+ }
+
+ return r0
+}
+
// CreateChange provides a mock function with given fields: _a0, _a1, _a2, _a3, _a4
func (_m *GerritInterface) CreateChange(_a0 context.Context, _a1 string, _a2 string, _a3 string, _a4 string) (*gerrit.ChangeInfo, error) {
ret := _m.Called(_a0, _a1, _a2, _a3, _a4)
@@ -428,6 +444,20 @@
return r0, r1
}
+// SelfApprove provides a mock function with given fields: _a0, _a1, _a2
+func (_m *GerritInterface) SelfApprove(_a0 context.Context, _a1 *gerrit.ChangeInfo, _a2 string) error {
+ ret := _m.Called(_a0, _a1, _a2)
+
+ var r0 error
+ if rf, ok := ret.Get(0).(func(context.Context, *gerrit.ChangeInfo, string) error); ok {
+ r0 = rf(_a0, _a1, _a2)
+ } else {
+ r0 = ret.Error(0)
+ }
+
+ return r0
+}
+
// SendToCQ provides a mock function with given fields: _a0, _a1, _a2
func (_m *GerritInterface) SendToCQ(_a0 context.Context, _a1 *gerrit.ChangeInfo, _a2 string) error {
ret := _m.Called(_a0, _a1, _a2)
@@ -485,11 +515,11 @@
}
// SetReview provides a mock function with given fields: _a0, _a1, _a2, _a3, _a4
-func (_m *GerritInterface) SetReview(_a0 context.Context, _a1 *gerrit.ChangeInfo, _a2 string, _a3 map[string]interface{}, _a4 []string) error {
+func (_m *GerritInterface) SetReview(_a0 context.Context, _a1 *gerrit.ChangeInfo, _a2 string, _a3 map[string]int, _a4 []string) error {
ret := _m.Called(_a0, _a1, _a2, _a3, _a4)
var r0 error
- if rf, ok := ret.Get(0).(func(context.Context, *gerrit.ChangeInfo, string, map[string]interface{}, []string) error); ok {
+ if rf, ok := ret.Get(0).(func(context.Context, *gerrit.ChangeInfo, string, map[string]int, []string) error); ok {
r0 = rf(_a0, _a1, _a2, _a3, _a4)
} else {
r0 = ret.Error(0)
diff --git a/go/gerrit/mocks/simple_mock_gerrit.go b/go/gerrit/mocks/simple_mock_gerrit.go
index 3287920..411eda9 100644
--- a/go/gerrit/mocks/simple_mock_gerrit.go
+++ b/go/gerrit/mocks/simple_mock_gerrit.go
@@ -17,6 +17,10 @@
func (g *SimpleGerritInterface) Initialized() bool {
return true
}
+func (g *SimpleGerritInterface) Config() *gerrit.Config {
+ args := g.Called()
+ return args.Get(0).(*gerrit.Config)
+}
func (g *SimpleGerritInterface) TurnOnAuthenticatedGets() {
}
func (g *SimpleGerritInterface) Url(issueID int64) string {
@@ -37,7 +41,7 @@
func (g *SimpleGerritInterface) GetPatch(ctx context.Context, issue int64, revision string) (string, error) {
return "", nil
}
-func (g *SimpleGerritInterface) SetReview(ctx context.Context, issue *gerrit.ChangeInfo, message string, labels map[string]interface{}, reviewers []string) error {
+func (g *SimpleGerritInterface) SetReview(ctx context.Context, issue *gerrit.ChangeInfo, message string, labels map[string]int, reviewers []string) error {
return nil
}
func (g *SimpleGerritInterface) AddComment(ctx context.Context, issue *gerrit.ChangeInfo, message string) error {
@@ -61,6 +65,9 @@
func (g *SimpleGerritInterface) DisApprove(ctx context.Context, issue *gerrit.ChangeInfo, message string) error {
return nil
}
+func (g *SimpleGerritInterface) SelfApprove(ctx context.Context, issue *gerrit.ChangeInfo, message string) error {
+ return nil
+}
func (g *SimpleGerritInterface) Abandon(ctx context.Context, issue *gerrit.ChangeInfo, message string) error {
return nil
}
diff --git a/go/gerrit/testutils/testutils.go b/go/gerrit/testutils/testutils.go
index 56363b7..169d771 100644
--- a/go/gerrit/testutils/testutils.go
+++ b/go/gerrit/testutils/testutils.go
@@ -22,29 +22,33 @@
// MockGerrit is a GerritInterface implementation which mocks out requests to
// the server.
type MockGerrit struct {
- bb *bb_testutils.MockClient
- Gerrit *gerrit.Gerrit
- Mock *mockhttpclient.URLMock
- isAndroid bool
- t sktest.TestingT
+ bb *bb_testutils.MockClient
+ Gerrit *gerrit.Gerrit
+ Mock *mockhttpclient.URLMock
+ t sktest.TestingT
}
// NewGerrit returns a mocked Gerrit instance.
-func NewGerrit(t sktest.TestingT, workdir string, isAndroid bool) *MockGerrit {
+func NewGerrit(t sktest.TestingT, workdir string) *MockGerrit {
+ return NewGerritWithConfig(t, gerrit.CONFIG_CHROMIUM, workdir)
+}
+
+// NewGerritWithConfig returns a mocked Gerrit instance which uses the given
+// Config.
+func NewGerritWithConfig(t sktest.TestingT, cfg *gerrit.Config, workdir string) *MockGerrit {
gitcookies := path.Join(workdir, "gitcookies_fake")
testutils.WriteFile(t, gitcookies, FAKE_GITCOOKIES)
mock := mockhttpclient.NewURLMock()
- g, err := gerrit.NewGerrit(FAKE_GERRIT_URL, gitcookies, mock.Client())
+ g, err := gerrit.NewGerritWithConfig(cfg, FAKE_GERRIT_URL, gitcookies, mock.Client())
require.NoError(t, err)
bb := bb_testutils.NewMockClient(t)
g.BuildbucketClient = bb.Client
return &MockGerrit{
- bb: bb,
- Gerrit: g,
- Mock: mock,
- isAndroid: isAndroid,
- t: t,
+ bb: bb,
+ Gerrit: g,
+ Mock: mock,
+ t: t,
}
}
@@ -97,27 +101,11 @@
}
func (g *MockGerrit) MockSetDryRun(ci *gerrit.ChangeInfo, msg string) {
- if g.isAndroid {
- g.MockPost(ci, msg, map[string]int{
- gerrit.AUTOSUBMIT_LABEL: gerrit.AUTOSUBMIT_LABEL_NONE,
- })
- } else {
- g.MockPost(ci, msg, map[string]int{
- gerrit.COMMITQUEUE_LABEL: gerrit.COMMITQUEUE_LABEL_DRY_RUN,
- })
- }
+ g.MockPost(ci, msg, g.Gerrit.Config().SetDryRunLabels)
}
func (g *MockGerrit) MockSetCQ(ci *gerrit.ChangeInfo, msg string) {
- if g.isAndroid {
- g.MockPost(ci, msg, map[string]int{
- gerrit.AUTOSUBMIT_LABEL: gerrit.AUTOSUBMIT_LABEL_SUBMIT,
- })
- } else {
- g.MockPost(ci, msg, map[string]int{
- gerrit.COMMITQUEUE_LABEL: gerrit.COMMITQUEUE_LABEL_SUBMIT,
- })
- }
+ g.MockPost(ci, msg, g.Gerrit.Config().SetCqLabels)
}
func (g *MockGerrit) Abandon(ci *gerrit.ChangeInfo, msg string) {
diff --git a/infra/bots/task_drivers/update_go_deps/update_go_deps.go b/infra/bots/task_drivers/update_go_deps/update_go_deps.go
index a2f19f7..114c5ea 100644
--- a/infra/bots/task_drivers/update_go_deps/update_go_deps.go
+++ b/infra/bots/task_drivers/update_go_deps/update_go_deps.go
@@ -117,9 +117,9 @@
if err != nil {
return err
}
- var labels map[string]interface{}
+ var labels map[string]int
if !*local && rs.Issue == "" {
- labels = map[string]interface{}{
+ labels = map[string]int{
gerrit.CODEREVIEW_LABEL: gerrit.CODEREVIEW_LABEL_APPROVE,
gerrit.COMMITQUEUE_LABEL: gerrit.COMMITQUEUE_LABEL_SUBMIT,
}
