[tools] Improvements to auto-submit
Sometimes CQ will post a fail and pass message on the same PS. Prioritize failure to avoid repeat spamming of CQ+2 on a change that no longer builds due to changes on main.
Check that the change has no unsubmitted parents. This allows auto-submit to submit stacks of CLs without spamming CQ+2.
Change-Id: I0eb3f65b74f8252dcb92410e423d6d89755a5255
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/135261
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
diff --git a/tools/src/cmd/auto-submit/main.go b/tools/src/cmd/auto-submit/main.go
index d79a76a..051bb9a 100644
--- a/tools/src/cmd/auto-submit/main.go
+++ b/tools/src/cmd/auto-submit/main.go
@@ -123,6 +123,7 @@
Messages: true,
CurrentRevision: true,
DetailedAccounts: true,
+ Submittable: true,
},
"status:open",
"author:"+a.user,
@@ -131,7 +132,7 @@
"label:kokoro",
"repo:"+*repoFlag)
if err != nil {
- return fmt.Errorf("failed to Query changes: %w", err)
+ return fmt.Errorf("failed to query changes: %w", err)
}
for _, change := range changes {
@@ -148,13 +149,14 @@
}
isReadyToSubmit := true &&
+ change.Submittable &&
hasLabel("Kokoro", 1) &&
hasLabel("Auto-Submit", 1) &&
hasLabel("Code-Review", 2) &&
!hasLabel("Code-Review", -1) &&
!hasLabel("Code-Review", -2)
if !isReadyToSubmit {
- // Change does not have all the required labels to submit
+ // Change is not ready to be submitted
continue
}
@@ -163,6 +165,18 @@
continue
}
+ submittedTogether, err := a.ChangesSubmittedTogether(change.ChangeID)
+ if err != nil {
+ return fmt.Errorf("failed to query changes submitted together: %w", err)
+ }
+ if len(submittedTogether) > 1 { // Include the change itself
+ // Change has unsubmitted parents
+ if *verboseFlag {
+ log.Printf("%v has %v unsubmitted parents", change.ChangeID, len(submittedTogether)-1)
+ }
+ continue
+ }
+
switch parseCQStatus(change) {
case cqUnknown, cqPassed:
if *dryrunFlag {
@@ -204,12 +218,12 @@
continue
}
if msg.Author.Email == cqEmailAccount {
- if strings.Contains(msg.Message, "This CL has passed the run") {
- return cqPassed
- }
if strings.Contains(msg.Message, "This CL has failed the run") {
return cqFailed
}
+ if strings.Contains(msg.Message, "This CL has passed the run") {
+ return cqPassed
+ }
}
}
return cqUnknown
diff --git a/tools/src/gerrit/gerrit.go b/tools/src/gerrit/gerrit.go
index 119b5c4..2b333b1 100644
--- a/tools/src/gerrit/gerrit.go
+++ b/tools/src/gerrit/gerrit.go
@@ -128,6 +128,7 @@
Messages bool
CurrentRevision bool
DetailedAccounts bool
+ Submittable bool
}
// QueryChanges returns the changes that match the given query strings.
@@ -149,6 +150,9 @@
if extras.DetailedAccounts {
changeOpts.AdditionalFields = append(changeOpts.AdditionalFields, "DETAILED_ACCOUNTS")
}
+ if extras.Submittable {
+ changeOpts.AdditionalFields = append(changeOpts.AdditionalFields, "SUBMITTABLE")
+ }
for {
batch, _, err := g.client.Changes.QueryChanges(&gerrit.QueryChangeOptions{
@@ -174,6 +178,16 @@
return g.QueryChangesWith(QueryExtraData{}, queries...)
}
+// ChangesSubmittedTogether returns the changes that want to be submitted together
+// See: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#submitted-together
+func (g *Gerrit) ChangesSubmittedTogether(changeID string) (changes []gerrit.ChangeInfo, err error) {
+ info, _, err := g.client.Changes.ChangesSubmittedTogether(changeID)
+ if err != nil {
+ return nil, g.maybeWrapError(err)
+ }
+ return *info, nil
+}
+
func (g *Gerrit) AddLabel(changeID, revisionID, message, label string, value int) error {
_, _, err := g.client.Changes.SetReview(changeID, revisionID, &gerrit.ReviewInput{
Message: message,
