Exclude release branch configs

Also use base and head for calculating the diff from a PR Signed-off-by: Daniel Hiller <[email protected]>
kubevirt · Jun 23, 2023 · 3e7a4c6 · 3e7a4c6
1 parent dca64ec
commit 3e7a4c6
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 9 deletions.
diff --git a/external-plugins/botreview/review/image_update.go b/external-plugins/botreview/review/image_update.go
@@ -38,10 +38,14 @@ These are the suspicious hunks I found:
 `
 )
 
-var prowJobImageUpdateHunkBodyMatcher *regexp.Regexp
+var (
+	prowJobImageUpdateHunkBodyMatcher   *regexp.Regexp
+	prowJobReleaseBranchFileNameMatcher *regexp.Regexp
+)
 
 func init() {
 	prowJobImageUpdateHunkBodyMatcher = regexp.MustCompile(`(?m)^(-[\s]+- image: [^\s]+$[\n]^\+[\s]+- image: [^\s]+|-[\s]+image: [^\s]+$[\n]^\+[\s]+image: [^\s]+)$`)
+	prowJobReleaseBranchFileNameMatcher = regexp.MustCompile(`.*\/[\w-]+-[0-9-\.]+\.yaml`)
 }
 
 type ProwJobImageUpdateResult struct {
@@ -78,7 +82,8 @@ func (t *ProwJobImageUpdate) AddIfRelevant(fileDiff *diff.FileDiff) {
 	//  * who are not yaml
 	if strings.TrimPrefix(fileDiff.OrigName, "a/") != fileName ||
 		!strings.HasSuffix(fileName, ".yaml") ||
-		!strings.HasPrefix(fileName, "github/ci/prow-deploy/files/jobs") {
+		!strings.HasPrefix(fileName, "github/ci/prow-deploy/files/jobs") ||
+		prowJobReleaseBranchFileNameMatcher.MatchString(fileName) {
 		return
 	}
 

diff --git a/external-plugins/botreview/review/image_update_test.go b/external-plugins/botreview/review/image_update_test.go
@@ -87,3 +87,76 @@ func TestProwJobImageUpdate_Review(t1 *testing.T) {
 		})
 	}
 }
+
+func TestProwJobImageUpdate_AddIfRelevant(t1 *testing.T) {
+	type fields struct {
+		relevantFileDiffs []*diff.FileDiff
+		notMatchingHunks  []*diff.Hunk
+	}
+	type args struct {
+		fileDiff *diff.FileDiff
+	}
+	tests := []struct {
+		name                      string
+		fields                    fields
+		args                      args
+		expectedRelevantFileDiffs []*diff.FileDiff
+	}{
+		{
+			name: "release branch config is ignored",
+			fields: fields{
+				relevantFileDiffs: nil,
+				notMatchingHunks:  nil,
+			},
+			args: args{
+				fileDiff: &diff.FileDiff{
+					OrigName: "a/github/ci/prow-deploy/files/jobs/kubevirt/kubevirt/kubevirt-presubmits-0.54.yaml",
+					OrigTime: nil,
+					NewName:  "b/github/ci/prow-deploy/files/jobs/kubevirt/kubevirt/kubevirt-presubmits-0.54.yaml",
+					NewTime:  nil,
+					Extended: nil,
+					Hunks:    nil,
+				},
+			},
+		},
+		{
+			name: "non-release branch config is added",
+			fields: fields{
+				relevantFileDiffs: nil,
+				notMatchingHunks:  nil,
+			},
+			args: args{
+				fileDiff: &diff.FileDiff{
+					OrigName: "a/github/ci/prow-deploy/files/jobs/kubevirt/kubevirt/kubevirt-presubmits.yaml",
+					OrigTime: nil,
+					NewName:  "b/github/ci/prow-deploy/files/jobs/kubevirt/kubevirt/kubevirt-presubmits.yaml",
+					NewTime:  nil,
+					Extended: nil,
+					Hunks:    nil,
+				},
+			},
+			expectedRelevantFileDiffs: []*diff.FileDiff{
+				{
+					OrigName: "a/github/ci/prow-deploy/files/jobs/kubevirt/kubevirt/kubevirt-presubmits.yaml",
+					OrigTime: nil,
+					NewName:  "b/github/ci/prow-deploy/files/jobs/kubevirt/kubevirt/kubevirt-presubmits.yaml",
+					NewTime:  nil,
+					Extended: nil,
+					Hunks:    nil,
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t1.Run(tt.name, func(t1 *testing.T) {
+			t := &ProwJobImageUpdate{
+				relevantFileDiffs: tt.fields.relevantFileDiffs,
+				notMatchingHunks:  tt.fields.notMatchingHunks,
+			}
+			t.AddIfRelevant(tt.args.fileDiff)
+			if !reflect.DeepEqual(tt.expectedRelevantFileDiffs, t.relevantFileDiffs) {
+				t1.Errorf("expectedRelevantFileDiffs not equal: %v\n, was\n%v", tt.expectedRelevantFileDiffs, t.relevantFileDiffs)
+			}
+		})
+	}
+}
diff --git a/external-plugins/botreview/review/review.go b/external-plugins/botreview/review/review.go
@@ -71,13 +71,14 @@ func (n BasicResult) String() string {
 }
 
 type Reviewer struct {
-	l      *logrus.Entry
-	org    string
-	repo   string
-	num    int
-	user   string
-	action github.PullRequestEventAction
-	dryRun bool
+	l       *logrus.Entry
+	org     string
+	repo    string
+	num     int
+	user    string
+	action  github.PullRequestEventAction
+	dryRun  bool
+	BaseSHA string
 }
 
 func NewReviewer(l *logrus.Entry, action github.PullRequestEventAction, org string, repo string, num int, user string, dryRun bool) *Reviewer {
@@ -113,6 +114,9 @@ func (r *Reviewer) ReviewLocalCode() ([]BotReviewResult, error) {
 	r.info("preparing review")
 
 	diffCommand := exec.Command("git", "diff", "..main")
+	if r.BaseSHA != "" {
+		diffCommand = exec.Command("git", "diff", fmt.Sprintf("%s..%s", r.BaseSHA, "HEAD"))
+	}
 	output, err := diffCommand.Output()
 	if err != nil {
 		r.fatalF("could not fetch diff output: %v", err)

diff --git a/robots/cmd/botreview/main.go b/robots/cmd/botreview/main.go
@@ -120,6 +120,7 @@ func main() {
 
 	// Perform review
 	reviewer := review.NewReviewer(log, github.PullRequestActionEdited, o.org, o.repo, o.pullRequestNumber, user.Login, o.dryRun)
+	reviewer.BaseSHA = pullRequest.Base.SHA
 	botReviewResults, err := reviewer.ReviewLocalCode()
 	if err != nil {
 		log.Info("no review results, cancelling review comments")