maintner/maintnerd/maintapi: use inline comment posted on older patchset
A valid TRY= request can end up being left in a patchset-level comment
associated with an older patch set than the patch set that the message
with a Run-TryBot+1 vote is posted on. Fix the oversight of dismissing
such SlowBot requests.
For golang/go#46106.
Change-Id: If3ff0bbdddca5bffa5b4279755765eca2fcb0cca
Reviewed-on: https://go-review.googlesource.com/c/build/+/493855
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
diff --git a/maintner/maintnerd/maintapi/api.go b/maintner/maintnerd/maintapi/api.go
index 811edd9..f769fa8 100644
--- a/maintner/maintnerd/maintapi/api.go
+++ b/maintner/maintnerd/maintapi/api.go
@@ -146,12 +146,11 @@
// from matching patchset-level comments. They
// are posted on the magic "/PATCHSET_LEVEL" path, see https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#file-id.
for _, c := range comments["/PATCHSET_LEVEL"] {
- // It should be sufficient to match by equal time only.
- // But check that author and patch set match too in order to be more strict.
- if !c.Updated.Equal(m.Time) || c.Author.NumericID != m.Author.NumericID || c.PatchSet != m.RevisionNumber {
+ if !c.Updated.Equal(m.Time) || c.Author.NumericID != m.Author.NumericID {
+ // Not a matching time or author ID.
continue
}
- if len(w.TryMessage) > 0 && c.PatchSet < int(w.TryMessage[len(w.TryMessage)-1].Version) {
+ if len(w.TryMessage) > 0 && m.RevisionNumber < int(w.TryMessage[len(w.TryMessage)-1].Version) {
// Don't include try messages older than the latest we've seen. They're obsolete.
continue
}
@@ -162,7 +161,7 @@
w.TryMessage = append(w.TryMessage, &apipb.TryVoteMessage{
Message: tm[1],
AuthorId: c.Author.NumericID,
- Version: int32(c.PatchSet),
+ Version: int32(m.RevisionNumber),
})
}
}
diff --git a/maintner/maintnerd/maintapi/api_test.go b/maintner/maintnerd/maintapi/api_test.go
index 3088b98..ac7dfdc 100644
--- a/maintner/maintnerd/maintapi/api_test.go
+++ b/maintner/maintnerd/maintapi/api_test.go
@@ -374,6 +374,53 @@
},
},
},
+
+ // Test that TRY= request messages with an older patchset-level comment are included.
+ // See https://go-review.googlesource.com/c/go/+/493535/comments/c72580be_773332cb where
+ // a Run-TryBot+1 request is posted on PS 2 with a patchset-level comment left on PS 1.
+ {
+ proj: "go",
+ clnum: 493535,
+ ci: &gerrit.ChangeInfo{
+ CurrentRevision: "f8aa751e53d7019eb1114da68754c77cc0830163",
+ Revisions: map[string]gerrit.RevisionInfo{
+ "a2afb09fc37fcff8ff43d895def78274d6ec4d74": {PatchSetNumber: 1},
+ "f8aa751e53d7019eb1114da68754c77cc0830163": {PatchSetNumber: 2},
+ },
+ Messages: []gerrit.ChangeMessageInfo{
+ // A message posted a minute after PS 2 was uploaded.
+ {
+ Author: &gerrit.AccountInfo{NumericID: 1234},
+ Message: "Patch Set 2: Code-Review+2 Run-TryBot+1\n\n(1 comment)",
+ Time: gerrit.TimeStamp(time.Date(2023, 5, 8, 16, 14, 3, 0, time.UTC)),
+ RevisionNumber: 2,
+ },
+ },
+ },
+ comments: map[string][]gerrit.CommentInfo{
+ "/PATCHSET_LEVEL": {
+ // Its patchset-level comment is associated with PS 1.
+ {
+ PatchSet: 1,
+ Message: "TRY\u003dplan9\n\nThanks!",
+ Updated: gerrit.TimeStamp(time.Date(2023, 5, 8, 16, 14, 3, 0, time.UTC)),
+ Author: &gerrit.AccountInfo{NumericID: 1234},
+ },
+ },
+ },
+ want: &apipb.GerritTryWorkItem{
+ Project: "go",
+ Branch: "master",
+ ChangeId: "Ia30f51307cc6d07a7e3ada6bf9d60bf9951982ff",
+ Commit: "f8aa751e53d7019eb1114da68754c77cc0830163",
+ AuthorEmail: "millerresearch@gmail.com",
+ Version: 2,
+ GoVersion: []*apipb.MajorMinor{{Major: 1, Minor: 17}},
+ TryMessage: []*apipb.TryVoteMessage{
+ {Message: "plan9", AuthorId: 1234, Version: 2},
+ },
+ },
+ },
}
for _, tt := range tests {
t.Run(strconv.Itoa(int(tt.clnum)), func(t *testing.T) {
