Skip to content

Commit

Permalink
🐛 Gitlab: Tests (ossf#3027)
Browse files Browse the repository at this point in the history
* fix tests

Signed-off-by: Raghav Kaul <[email protected]>

* use projectID instead of project where applicable

Signed-off-by: Raghav Kaul <[email protected]>

* pass ref as listcommitoption

Signed-off-by: Raghav Kaul <[email protected]>

* 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 <[email protected]>

* fix commitshandler commitSHA tests

Signed-off-by: Raghav Kaul <[email protected]>

* update tests

Signed-off-by: Raghav Kaul <[email protected]>

---------

Signed-off-by: Raghav Kaul <[email protected]>
Signed-off-by: raghavkaul <[email protected]>
  • Loading branch information
raghavkaul authored and ashearin committed Nov 13, 2023
1 parent 773f151 commit 96f29f0
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 16 deletions.
3 changes: 2 additions & 1 deletion clients/gitlabrepo/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions clients/gitlabrepo/commits.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (handler *commitsHandler) setup() error {
i := 1

for {
c, _, err := handler.glClient.Commits.ListCommits(handler.repourl.project,
c, _, err := handler.glClient.Commits.ListCommits(handler.repourl.projectID,
&gitlab.ListCommitsOptions{
ListOptions: gitlab.ListOptions{
Page: i,
Expand Down Expand Up @@ -85,7 +85,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
Expand Down
7 changes: 4 additions & 3 deletions clients/gitlabrepo/graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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 {
Expand Down
11 changes: 7 additions & 4 deletions clients/gitlabrepo/graphql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"os"
"testing"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand Down
4 changes: 2 additions & 2 deletions e2e/ci_tests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 6 additions & 4 deletions e2e/code_review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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())
Expand Down

0 comments on commit 96f29f0

Please sign in to comment.