Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Gitlab: test fixes #3027

Merged
merged 9 commits into from
May 24, 2023
Merged
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
9 changes: 7 additions & 2 deletions clients/gitlabrepo/commits.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
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