🌱 Fixup: list GitHub check runs of MergeRequest.HeadSHA instead of Commit.SHA (#2333)

* Only ListCheckRuns and ListStatuses on PR HeadSHA

Unsquashed commits are unlikely to have CheckRuns on Github. This change
reduces the overall number of API queries for raw results

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Add tests

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* gofumpt

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

Signed-off-by: Raghav Kaul <raghavkaul@google.com>
Co-authored-by: Azeem Shaikh <azeemshaikh38@gmail.com>

Author: raghavkaul

checks/raw/ci_tests.go

@@ -36,7 +36,6 @@ func CITests(c clients.RepoClient) (checker.CITestData, error) {
 	prNos := make(map[string]int)
 
 	for i := range commits {
-		commit := commits[i]
 		pr := &commits[i].AssociatedMergeRequest
 
 		if pr.MergedAt.IsZero() {
@@ -45,19 +44,21 @@ func CITests(c clients.RepoClient) (checker.CITestData, error) {
 
 		prNos[pr.HeadSHA] = pr.Number
 
-		crs, err := c.ListCheckRunsForRef(commit.SHA)
-		if err != nil {
-			return checker.CITestData{}, sce.WithMessage(
-				sce.ErrScorecardInternal,
-				fmt.Sprintf("Client.Repositories.ListCheckRunsForRef: %v", err),
-			)
-		}
+		// HeadSHA is the last commit before the merge. if squashing enabled,
+		// multiple commit SHAs will map to a single HeadSHA
+		if len(runs[pr.HeadSHA]) == 0 {
+			crs, err := c.ListCheckRunsForRef(pr.HeadSHA)
+			if err != nil {
+				return checker.CITestData{}, sce.WithMessage(
+					sce.ErrScorecardInternal,
+					fmt.Sprintf("Client.Repositories.ListCheckRunsForRef: %v", err),
+				)
+			}
 
-		// Use HeadSHA instead of commit.SHA because GitHub repos that don't squash
-		// PRs will only display check runs/statuses on last commit in a changeset
-		runs[pr.HeadSHA] = append(runs[pr.HeadSHA], crs...)
+			runs[pr.HeadSHA] = crs
+		}
 
-		statuses, err := c.ListStatuses(commit.SHA)
+		statuses, err := c.ListStatuses(pr.HeadSHA)
 		if err != nil {
 			return checker.CITestData{}, sce.WithMessage(
 				sce.ErrScorecardInternal,

e2e/ci_tests_test.go

@@ -77,5 +77,29 @@ var _ = Describe("E2E TEST:"+checks.CheckCITests, func() {
 			Expect(scut.ValidateTestReturn(nil, "CI tests run", &expected, &result, &dl)).Should(BeTrue())
 			Expect(repoClient.Close()).Should(BeNil())
 		})
+		It("Should return absence of CI tests in a repo with unsquashed merges", func() {
+			dl := scut.TestDetailLogger{}
+			repo, err := githubrepo.MakeGithubRepo("duo-labs/parliament")
+			Expect(err).Should(BeNil())
+			repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
+			err = repoClient.InitRepo(repo, "1ead655ec85bdbe0739e4a4125ce36eb48a329bc")
+			Expect(err).Should(BeNil())
+			req := checker.CheckRequest{
+				Ctx:        context.Background(),
+				RepoClient: repoClient,
+				Repo:       repo,
+				Dlogger:    &dl,
+			}
+			expected := scut.TestReturn{
+				Error:         nil,
+				Score:         checker.MinResultScore,
+				NumberOfWarn:  0,
+				NumberOfInfo:  0,
+				NumberOfDebug: 12,
+			}
+			result := checks.CITests(&req)
+			Expect(scut.ValidateTestReturn(nil, "CI tests run", &expected, &result, &dl)).Should(BeTrue())
+			Expect(repoClient.Close()).Should(BeNil())
+		})
 	})
 })