🐛 Gitlab: Tests (ossf#3027)

* fix tests Signed-off-by: Raghav Kaul <raghavkaul@google.com> * use projectID instead of project where applicable Signed-off-by: Raghav Kaul <raghavkaul@google.com> * pass ref as listcommitoption Signed-off-by: Raghav Kaul <raghavkaul@google.com> * update tests * CI-Tests: check if score > 0. pull request client is limited and can't go back to arbitrary pull requests. CI-Tests don't run on forks, so this can't be pinned either. But, for active repositories, we typically expect *some* tests to be run Signed-off-by: Raghav Kaul <raghavkaul@google.com> * fix commitshandler commitSHA tests Signed-off-by: Raghav Kaul <raghavkaul@google.com> * update tests Signed-off-by: Raghav Kaul <raghavkaul@google.com> --------- Signed-off-by: Raghav Kaul <raghavkaul@google.com> Signed-off-by: raghavkaul <8695110+raghavkaul@users.noreply.github.com> Signed-off-by: Avishay <avishay.balter@gmail.com>
balteravishay · May 28, 2023 · 3ef62a2 · 3ef62a2
1 parent 035943c
commit 3ef62a2
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 16 deletions.
diff --git a/clients/gitlabrepo/client.go b/clients/gitlabrepo/client.go
@@ -164,11 +164,12 @@ func (client *Client) ListCommits() ([]clients.Commit, error) {
 		return []clients.Commit{}, err
 	}
 
+	before := commitsRaw[0].CommittedDate
 	// Get merge request details from GraphQL
 	// GitLab REST API doesn't provide a way to link Merge Requests and Commits that
 	// are within them without making a REST call for each commit (~30 by default)
 	// Making 1 GraphQL query to combine the results of 2 REST calls, we avoid this
-	mrDetails, err := client.graphql.getMergeRequestsDetail()
+	mrDetails, err := client.graphql.getMergeRequestsDetail(before)
 	if err != nil {
 		return []clients.Commit{}, err
 	}

diff --git a/clients/gitlabrepo/commits.go b/clients/gitlabrepo/commits.go
@@ -41,7 +41,12 @@ func (handler *commitsHandler) init(repourl *repoURL) {
 
 func (handler *commitsHandler) setup() error {
 	handler.once.Do(func() {
-		commits, _, err := handler.glClient.Commits.ListCommits(handler.repourl.projectID, &gitlab.ListCommitsOptions{})
+		commits, _, err := handler.glClient.Commits.ListCommits(
+			handler.repourl.projectID,
+			&gitlab.ListCommitsOptions{
+				RefName: &handler.repourl.commitSHA,
+			},
+		)
 		if err != nil {
 			handler.errSetup = fmt.Errorf("request for commits failed with %w", err)
 			return
@@ -60,7 +65,7 @@ func (handler *commitsHandler) listRawCommits() ([]*gitlab.Commit, error) {
 	return handler.commitsRaw, nil
 }
 
-// zip combines Commit and MergeRequest information from the GitLab REST API with
+// zip combines Commit information from the GitLab REST API with MergeRequests
 // information from the GitLab GraphQL API. The REST API doesn't provide any way to
 // get from Commits -> MRs that they were part of or vice-versa (MRs -> commits they
 // contain), except through a separate API call. Instead of calling the REST API

diff --git a/clients/gitlabrepo/graphql.go b/clients/gitlabrepo/graphql.go
@@ -54,7 +54,7 @@ type graphqlData struct {
 	Project struct {
 		MergeRequests struct {
 			Nodes []graphqlMergeRequestNode `graphql:"nodes"`
-		} `graphql:"mergeRequests(sort: MERGED_AT_DESC, state: merged)"`
+		} `graphql:"mergeRequests(sort: MERGED_AT_DESC, state: merged, mergedBefore: $mergedBefore)"`
 	} `graphql:"project(fullPath: $fullPath)"`
 	QueryComplexity struct {
 		Limit int `graphql:"limit"`
@@ -127,11 +127,12 @@ func (g *GitlabGID) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
-func (handler *graphqlHandler) getMergeRequestsDetail() (graphqlData, error) {
+func (handler *graphqlHandler) getMergeRequestsDetail(before *time.Time) (graphqlData, error) {
 	data := graphqlData{}
 	path := fmt.Sprintf("%s/%s", handler.repourl.owner, handler.repourl.project)
 	params := map[string]interface{}{
-		"fullPath": path,
+		"fullPath":     path,
+		"mergedBefore": before,
 	}
 	err := handler.graphClient.Query(context.Background(), &data, params)
 	if err != nil {

diff --git a/clients/gitlabrepo/graphql_test.go b/clients/gitlabrepo/graphql_test.go
@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"os"
 	"testing"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -30,13 +31,13 @@ func TestGitlabRepoE2E(t *testing.T) {
 	}
 	t.Parallel()
 	RegisterFailHandler(Fail)
-	RunSpecs(t, "Githubrepo Suite")
+	RunSpecs(t, "GitLab Repo Suite")
 }
 
 var _ = Describe("E2E TEST: gitlabrepo.graphqlHandler", func() {
 	var graphqlhandler graphqlHandler
 
-	Context("E2E TEST: Confirm query result", func() {
+	Context("E2E TEST: Confirm query result - GitLab", func() {
 		It("Should have sufficient number of merge requests", func() {
 			repo, err := MakeGitlabRepo("gitlab.com/gitlab-org/gitlab")
 			Expect(err).Should(BeNil())
@@ -46,7 +47,8 @@ var _ = Describe("E2E TEST: gitlabrepo.graphqlHandler", func() {
 
 			path := fmt.Sprintf("%s/%s", graphqlhandler.repourl.owner, graphqlhandler.repourl.project)
 			params := map[string]interface{}{
-				"fullPath": path,
+				"fullPath":     path,
+				"mergedBefore": time.Now(),
 			}
 			err = graphqlhandler.graphClient.Query(context.Background(), &data, params)
 			Expect(err).Should(BeNil())
@@ -65,7 +67,8 @@ var _ = Describe("E2E TEST: gitlabrepo.graphqlHandler", func() {
 
 			path := fmt.Sprintf("%s/%s", graphqlhandler.repourl.owner, graphqlhandler.repourl.project)
 			params := map[string]interface{}{
-				"fullPath": path,
+				"fullPath":     path,
+				"mergedBefore": time.Now(),
 			}
 			err = graphqlhandler.graphClient.Query(context.Background(), &data, params)
 			Expect(err).Should(BeNil())

diff --git a/e2e/ci_tests_test.go b/e2e/ci_tests_test.go
@@ -122,7 +122,7 @@ var _ = Describe("E2E TEST:"+checks.CheckCITests, func() {
 			}
 			expected := scut.TestReturn{
 				Error:         nil,
-				Score:         0,
+				Score:         8,
 				NumberOfWarn:  0,
 				NumberOfInfo:  0,
 				NumberOfDebug: 13,
@@ -151,7 +151,7 @@ var _ = Describe("E2E TEST:"+checks.CheckCITests, func() {
 			}
 			expected := scut.TestReturn{
 				Error:         nil,
-				Score:         3,
+				Score:         10,
 				NumberOfWarn:  0,
 				NumberOfInfo:  0,
 				NumberOfDebug: 1,

diff --git a/e2e/code_review_test.go b/e2e/code_review_test.go
@@ -169,8 +169,9 @@ var _ = Describe("E2E TEST:"+checks.CheckCodeReview, func() {
 			Dlogger:    &dl,
 		}
 		expected := scut.TestReturn{
-			Error: nil,
-			Score: 3,
+			Error:         nil,
+			Score:         0,
+			NumberOfDebug: 1,
 		}
 		result := checks.CodeReview(&req)
 		Expect(scut.ValidateTestReturn(nil, "use code reviews", &expected, &result, &dl)).Should(BeTrue())
@@ -197,8 +198,9 @@ var _ = Describe("E2E TEST:"+checks.CheckCodeReview, func() {
 			Dlogger:    &dl,
 		}
 		expected := scut.TestReturn{
-			Error: nil,
-			Score: checker.MinResultScore,
+			Error:         nil,
+			Score:         checker.MinResultScore,
+			NumberOfDebug: 1,
 		}
 		result := checks.CodeReview(&req)
 		Expect(scut.ValidateTestReturn(nil, "use code reviews", &expected, &result, &dl)).Should(BeTrue())