From e71dba64f1b7fc7a570b2bc1c16ac14c301badf2 Mon Sep 17 00:00:00 2001 From: mpherman2 Date: Wed, 12 Apr 2023 11:11:44 -0700 Subject: [PATCH] Revert "Feat: skip presubmit tests for revisions without code change" This reverts commit 0ee31ea3cf340d47e9fd4fda3401e3cf0b3b457d. --- go.mod | 2 +- go.sum | 4 +- prow/gerrit/adapter/adapter.go | 39 ++------- prow/gerrit/adapter/adapter_test.go | 119 ---------------------------- prow/gerrit/client/client.go | 2 +- 5 files changed, 10 insertions(+), 156 deletions(-) diff --git a/go.mod b/go.mod index 7bcd9f490fdb..75b8864b242b 100644 --- a/go.mod +++ b/go.mod @@ -29,7 +29,7 @@ require ( github.com/Azure/go-autorest/autorest/adal v0.9.20 github.com/GoogleCloudPlatform/testgrid v0.0.123 github.com/NYTimes/gziphandler v1.1.1 - github.com/andygrunwald/go-gerrit v0.0.0-20230211083816-04e01d7217b2 + github.com/andygrunwald/go-gerrit v0.0.0-20210709065208-9d38b0be0268 github.com/andygrunwald/go-jira v1.14.0 github.com/aws/aws-sdk-go v1.38.49 github.com/bazelbuild/buildtools v0.0.0-20200922170545-10384511ce98 diff --git a/go.sum b/go.sum index ff2a459ef7d4..9600094fc05e 100644 --- a/go.sum +++ b/go.sum @@ -160,8 +160,8 @@ github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRF github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= github.com/alvaroaleman/httpcache v0.0.0-20210618195546-ab9a1a3f8a38 h1:ML+zGuJv9er8cg0N8D8edCNOwqI/Kb3WoV6VsIV93SU= github.com/alvaroaleman/httpcache v0.0.0-20210618195546-ab9a1a3f8a38/go.mod h1:R4sf/p+3TZ6dvs7BxjxjozS7lIpzKtuS4jmQt4egJfU= -github.com/andygrunwald/go-gerrit v0.0.0-20230211083816-04e01d7217b2 h1:ohDU8MHAAE/FMlqs+KVlzLpVx3/gJ86rNO+8TZ9nea8= -github.com/andygrunwald/go-gerrit v0.0.0-20230211083816-04e01d7217b2/go.mod h1:SeP12EkHZxEVjuJ2HZET304NBtHGG2X6w2Gzd0QXAZw= +github.com/andygrunwald/go-gerrit v0.0.0-20210709065208-9d38b0be0268 h1:7gokoTWteZhP1t2f0OzrFFXlyL8o0+b0r4ZaRV9PXOs= +github.com/andygrunwald/go-gerrit v0.0.0-20210709065208-9d38b0be0268/go.mod h1:aqcjwEnmLLSalFNYR0p2ttnEXOVVRctIzsUMHbEcruU= github.com/andygrunwald/go-jira v1.14.0 h1:7GT/3qhar2dGJ0kq8w0d63liNyHOnxZsUZ9Pe4+AKBI= github.com/andygrunwald/go-jira v1.14.0/go.mod h1:KMo2f4DgMZA1C9FdImuLc04x4WQhn5derQpnsuBFgqE= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 h1:kFOfPq6dUM1hTo4JG6LR5AXSUEsOjtdm0kw0FtQtMJA= diff --git a/prow/gerrit/adapter/adapter.go b/prow/gerrit/adapter/adapter.go index 87792af5d09c..2c2e4a3a217d 100644 --- a/prow/gerrit/adapter/adapter.go +++ b/prow/gerrit/adapter/adapter.go @@ -21,7 +21,6 @@ import ( "context" "fmt" "net/url" - "sort" "strconv" "strings" "sync" @@ -595,25 +594,23 @@ func (c *Controller) processChange(logger logrus.FieldLogger, instance string, c logger.WithField("lastUpdate", lastUpdate).Warnf("lastUpdate not found, falling back to now") } - currentRevision := change.Revisions[change.CurrentRevision] - failedJobs := failedJobs(account.AccountID, currentRevision.Number, change.Messages...) + revision := change.Revisions[change.CurrentRevision] + failedJobs := failedJobs(account.AccountID, revision.Number, change.Messages...) failed, all := presubmitContexts(failedJobs, presubmits, logger) messages := currentMessages(change, lastUpdate) logger.WithField("failed", len(failed)).Debug("Failed jobs parsed from previous comments.") filters := []pjutil.Filter{ messageFilter(messages, failed, all, triggerTimes, logger), } - // Automatically trigger the Prow jobs if the change has a revision with code change - // since last update and the change is not in WorkInProgress. - lastRevisionWithChange := lastRevisionWithCodeChange(change, lastUpdate) - if lastRevisionWithChange != nil && !change.WorkInProgress { + // Automatically trigger the Prow jobs if the revision is new and the + // change is not in WorkInProgress. + if revision.Created.Time.After(lastUpdate) && !change.WorkInProgress { filters = append(filters, &timeAnnotationFilter{ Filter: pjutil.NewTestAllFilter(), - eventTime: lastRevisionWithChange.Created.Time, + eventTime: revision.Created.Time, triggerTimes: triggerTimes, }) } - toTrigger, err := pjutil.FilterPresubmits(pjutil.NewAggregateFilter(filters), client.ChangedFilesProvider(&change), change.Branch, presubmits, logger) if err != nil { return fmt.Errorf("filter presubmits: %w", err) @@ -704,30 +701,6 @@ func (c *Controller) processChange(logger logrus.FieldLogger, instance string, c return nil } -// lastRevisionWithCodeChange tries to find a revision with code change since last update. -// There is no point repeating tests if no code change happened since last update. -func lastRevisionWithCodeChange(change client.ChangeInfo, lastUpdate time.Time) *client.RevisionInfo { - var revisionsWithCodeChange []client.RevisionInfo - for _, revision := range change.Revisions { - if revision.Kind == gerrit.NoCodeChange { - continue - } - revisionsWithCodeChange = append(revisionsWithCodeChange, revision) - } - // sorts in reverse chronological order - sort.Slice(revisionsWithCodeChange, func(i, j int) bool { - return revisionsWithCodeChange[i].Created.Time.After(revisionsWithCodeChange[j].Created.Time) - }) - - for _, revision := range revisionsWithCodeChange { - if revision.Created.Time.After(lastUpdate) { - return &revision - } - } - - return nil -} - // isProjectOptOutHelp returns if the project is opt-out from getting help // information about how to run presubmit tests on their changes. func isProjectOptOutHelp(projectsOptOutHelp map[string]sets.String, instance, project string) bool { diff --git a/prow/gerrit/adapter/adapter_test.go b/prow/gerrit/adapter/adapter_test.go index d41cc02b7830..39dd93cb2e5f 100644 --- a/prow/gerrit/adapter/adapter_test.go +++ b/prow/gerrit/adapter/adapter_test.go @@ -19,7 +19,6 @@ package adapter import ( "errors" "fmt" - "reflect" "sync" "testing" "time" @@ -304,124 +303,6 @@ func TestCreateRefs(t *testing.T) { } } -func TestFindLatestRevisionWithoutCodeChange(t *testing.T) { - now := time.Now() - cases := []struct { - name string - change gerrit.ChangeInfo - lastUpdate time.Time - expected *gerrit.RevisionInfo - }{ - { - name: "should return nil if no code change since last update", - change: client.ChangeInfo{ - Project: "foo", - ChangeID: "33521", - CurrentRevision: "2", - Revisions: map[string]gerrit.RevisionInfo{ - "1": { - Number: 1, - Created: makeStamp(now.Add(-time.Hour)), - Kind: gerrit.MergeFirstParentUpdate, - }, - "2": { - Number: 2, - Created: makeStamp(now.Add(time.Hour)), - Kind: gerrit.NoCodeChange, - }, - }, - }, - lastUpdate: now, - expected: nil, - }, - { - name: "should return last revision with code change since last update when the last revision has no code change", - change: client.ChangeInfo{ - Project: "foo", - ChangeID: "33521", - CurrentRevision: "3", - Revisions: map[string]gerrit.RevisionInfo{ - "1": { - Number: 1, - Created: makeStamp(now.Add(-time.Hour)), - Kind: gerrit.MergeFirstParentUpdate, - }, - "2": { - Number: 2, - Created: makeStamp(now.Add(time.Hour)), - Kind: gerrit.Rework, - }, - "3": { - Number: 3, - Created: makeStamp(now.Add(2 * time.Hour)), - Kind: gerrit.NoCodeChange, - }, - }, - }, - lastUpdate: now, - expected: &client.RevisionInfo{ - Number: 2, - Created: makeStamp(now.Add(time.Hour)), - Kind: gerrit.Rework, - }, - }, - { - name: "should return last revision with code change since last update when the last revision has code change", - change: client.ChangeInfo{ - Project: "foo", - ChangeID: "33521", - CurrentRevision: "4", - Revisions: map[string]gerrit.RevisionInfo{ - "1": { - Number: 1, - Created: makeStamp(now.Add(-time.Hour)), - Kind: gerrit.MergeFirstParentUpdate, - }, - "2": { - Number: 2, - Created: makeStamp(now.Add(time.Hour)), - Kind: gerrit.NoCodeChange, - }, - "3": { - Number: 3, - Created: makeStamp(now.Add(2 * time.Hour)), - Kind: gerrit.MergeFirstParentUpdate, - }, - "4": { - Number: 4, - Created: makeStamp(now.Add(3 * time.Hour)), - Kind: gerrit.Rework, - }, - }, - }, - lastUpdate: now, - expected: &client.RevisionInfo{ - Number: 4, - Created: makeStamp(now.Add(3 * time.Hour)), - Kind: gerrit.Rework, - }, - }, - { - name: "should return nil if the change list has no revision", - change: client.ChangeInfo{ - Project: "foo", - ChangeID: "33521", - }, - lastUpdate: now, - expected: nil, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - rev := lastRevisionWithCodeChange(tc.change, tc.lastUpdate) - if !reflect.DeepEqual(rev, tc.expected) { - t.Errorf("expected revision: %+v, got: %+v", tc.expected, rev) - } - }) - } -} - func TestFailedJobs(t *testing.T) { const ( me = 314159 diff --git a/prow/gerrit/client/client.go b/prow/gerrit/client/client.go index 5540936cc8ee..dca4d8fa2d1d 100644 --- a/prow/gerrit/client/client.go +++ b/prow/gerrit/client/client.go @@ -621,7 +621,7 @@ func (h *gerritInstanceHandler) QueryChangesForProject(log logrus.FieldLogger, p var opt gerrit.QueryChangeOptions opt.Query = append(opt.Query, strings.Join(append(additionalFilters, "project:"+project), "+")) - opt.AdditionalFields = []string{"ALL_REVISIONS", "CURRENT_COMMIT", "CURRENT_FILES", "MESSAGES", "LABELS"} + opt.AdditionalFields = []string{"CURRENT_REVISION", "CURRENT_COMMIT", "CURRENT_FILES", "MESSAGES", "LABELS"} log = log.WithFields(logrus.Fields{"query": opt.Query, "additional_fields": opt.AdditionalFields}) var start int