More test fixes

* Break out GitLab tests into a separate workflow step/make target
* repo_client coverage
* branch_protection tests

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

.github/workflows/integration.yml

@@ -71,3 +71,19 @@ jobs:
         with:
          files: ./e2e-coverage.out
          verbose: true
+
+      - name: Run GitLab E2E  #using retry because the GitHub token is being throttled.
+        uses: nick-invision/retry@943e742917ac94714d2f408a0e8320f2d1fcafcd
+        env:
+          GITLAB_AUTH_TOKEN: ${{ secrets.GITLAB_AUTH_TOKEN }}
+        with:
+          max_attempts: 3
+          retry_on: error
+          timeout_minutes: 30
+          command: make e2e-gitlab
+
+      - name: codecov
+        uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # 2.1.0
+        with:
+         files: ./e2e-coverage.out
+         verbose: true

Makefile

@@ -335,7 +335,7 @@ e2e-gh-token: build-scorecard check-env | $(GINKGO)
 	TOKEN_TYPE="GITHUB_TOKEN" $(GINKGO) --race -p -v -cover -coverprofile=e2e-coverage.out --keep-separate-coverprofiles ./...
 
 e2e-gitlab: ## Runs e2e tests for GitLab only. TOKEN_TYPE is arbitrary, but must be set to something
-	TOKEN_TYPE="GITHUB_TOKEN" $(GINKGO) --race -p -vv --focus '.*GitLab' ./...
+	TOKEN_TYPE="GITLAB_PAT" $(GINKGO) --race -p -vv --focus '.*GitLab' ./...
 
 e2e-attestor: ## Runs e2e tests for scorecard-attestor
 	cd attestor/e2e; go test -covermode=atomic -coverprofile=e2e-coverage.out; cd ../..

clients/githubrepo/repo_test.go

@@ -97,6 +97,10 @@ func TestRepoURL_IsValid(t *testing.T) {
 			if !tt.wantErr && !cmp.Equal(tt.expected, r, cmp.AllowUnexported(repoURL{})) {
 				t.Errorf("Got diff: %s", cmp.Diff(tt.expected, r))
 			}
+
+			if !cmp.Equal(r.Host(), tt.expected.host) {
+				t.Errorf("%s expected host: %s got host %s", tt.inputURL, tt.expected.host, r.Host())
+			}
 		})
 	}
 }

clients/gitlabrepo/repo_test.go

@@ -34,7 +34,7 @@ func TestRepoURL_IsValid(t *testing.T) {
 			name: "github repository",
 			expected: repoURL{
 				scheme:  "https",
-				host:    "github.com",
+				host:    "https://github.com",
 				owner:   "ossf",
 				project: "scorecard",
 			},
@@ -45,7 +45,7 @@ func TestRepoURL_IsValid(t *testing.T) {
 			name: "GitHub project with 'gitlab.' in the title",
 			expected: repoURL{
 				scheme:  "http",
-				host:    "github.com",
+				host:    "http://github.com",
 				owner:   "foo",
 				project: "gitlab.test",
 			},
@@ -55,18 +55,18 @@ func TestRepoURL_IsValid(t *testing.T) {
 		{
 			name: "valid gitlab project",
 			expected: repoURL{
-				host:    "gitlab.com",
+				host:    "https://gitlab.com",
 				owner:   "ossf-test",
 				project: "scorecard-check-binary-artifacts-e2e",
 			},
-			inputURL: "gitlab.com/ossf-test/scorecard-check-binary-artifacts-e2e",
+			inputURL: "https://gitlab.com/ossf-test/scorecard-check-binary-artifacts-e2e",
 			wantErr:  false,
 		},
 		{
 			name: "valid https address with trailing slash",
 			expected: repoURL{
 				scheme:  "https",
-				host:    "gitlab.com",
+				host:    "https://gitlab.com",
 				owner:   "ossf-test",
 				project: "scorecard-check-binary-artifacts-e2e",
 			},
@@ -78,7 +78,7 @@ func TestRepoURL_IsValid(t *testing.T) {
 			name: "valid hosted gitlab project",
 			expected: repoURL{
 				scheme:  "https",
-				host:    "salsa.debian.org",
+				host:    "https://salsa.debian.org",
 				owner:   "webmaster-team",
 				project: "webml",
 			},
@@ -107,6 +107,9 @@ func TestRepoURL_IsValid(t *testing.T) {
 				fmt.Println("expected: " + tt.expected.project + " GOT: " + r.project)
 				t.Errorf("Got diff: %s", cmp.Diff(tt.expected, r))
 			}
+			if !cmp.Equal(r.Host(), tt.expected.host) {
+				t.Errorf("%s expected host: %s got host %s", tt.inputURL, tt.expected.host, r.Host())
+			}
 		})
 	}
 }

e2e/binary_artifacts_test.go

@@ -212,6 +212,8 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 			Expect(x.Close()).Should(BeNil())
 		})
 		It("Should return binary artifacts present in source code - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/scorecard-check-binary-artifacts-e2e")
 			Expect(err).Should(BeNil())
@@ -240,6 +242,8 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return binary artifacts present at commit in source code - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/scorecard-check-binary-artifacts-e2e")
 			Expect(err).Should(BeNil())
@@ -269,6 +273,8 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return no binary artifacts present in source code - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/scorecard-check-binary-artifacts-e2e-4-binaries")
 			Expect(err).Should(BeNil())
@@ -298,6 +304,8 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return binary artifacts present at commit in source code - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/scorecard-check-binary-artifacts-e2e-4-binaries")
 			Expect(err).Should(BeNil())

e2e/branch_protection_test.go

@@ -118,7 +118,7 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should get non-admin branch protection on other repositories - GitLab", func() {
-			skipIfTokenIsNot(patTokenType, "GitLab pac only")
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab PAT only")
 
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/scorecard-check-branch-protection-e2e")
@@ -138,7 +138,7 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 				Score:         4,
 				NumberOfWarn:  3,
 				NumberOfInfo:  5,
-				NumberOfDebug: 0,
+				NumberOfDebug: 1,
 			}
 			result := checks.BranchProtection(&req)
 			// UPGRADEv2: to remove.
@@ -149,7 +149,7 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should fail to return branch protection on other repositories - GitLab", func() {
-			skipIfTokenIsNot(patTokenType, "GitLab pac only")
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab PAT only")
 
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/scorecard-check-branch-protection-e2e-none")
@@ -166,10 +166,10 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			}
 			expected := scut.TestReturn{
 				Error:         nil,
-				Score:         0,
-				NumberOfWarn:  2,
-				NumberOfInfo:  0,
-				NumberOfDebug: 0,
+				Score:         4,
+				NumberOfWarn:  3,
+				NumberOfInfo:  5,
+				NumberOfDebug: 1,
 			}
 			result := checks.BranchProtection(&req)
 
@@ -178,7 +178,7 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should fail to return branch protection on other repositories - GitLab", func() {
-			skipIfTokenIsNot(patTokenType, "GitLab pac only")
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab PAT only")
 
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/scorecard-check-branch-protection-e2e-patch-1")
@@ -195,10 +195,10 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			}
 			expected := scut.TestReturn{
 				Error:         nil,
-				Score:         2,
-				NumberOfWarn:  4,
+				Score:         4,
+				NumberOfWarn:  3,
 				NumberOfInfo:  5,
-				NumberOfDebug: 0,
+				NumberOfDebug: 1,
 			}
 			result := checks.BranchProtection(&req)
 

e2e/ci_tests_test.go

@@ -104,6 +104,8 @@ var _ = Describe("E2E TEST:"+checks.CheckCITests, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return use of CI tests at commit - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/gitlab-org/gitlab")
 			Expect(err).Should(BeNil())
@@ -131,6 +133,8 @@ var _ = Describe("E2E TEST:"+checks.CheckCITests, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return use of CI tests at commit - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/fdroid/fdroidclient")
 			Expect(err).Should(BeNil())

e2e/code_review_test.go

@@ -108,6 +108,8 @@ var _ = Describe("E2E TEST:"+checks.CheckCodeReview, func() {
 		// GitLab doesn't seem to preserve merge requests (pull requests in github) and some users had data lost in
 		// the transfer from github so this returns a different value than the above GitHub test.
 		It("Should return use of code reviews - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/airflow")
 			Expect(err).Should(BeNil())
@@ -136,6 +138,8 @@ var _ = Describe("E2E TEST:"+checks.CheckCodeReview, func() {
 		// GitLab doesn't seem to preserve merge requests (pull requests in github) and some users had data lost in
 		// the transfer from github so this returns a different value than the above GitHub test.
 		It("Should return use of code reviews at commit - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/airflow")
 			Expect(err).Should(BeNil())

e2e/contributors_test.go

@@ -57,6 +57,8 @@ var _ = Describe("E2E TEST:"+checks.CheckContributors, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return valid project contributors - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/gitlab-org/gitlab")
 			Expect(err).Should(BeNil())

e2e/e2e_suite_test.go

@@ -40,6 +40,7 @@ type tokenType int
 const (
 	patTokenType tokenType = iota
 	githubWorkflowDefaultTokenType
+	gitlabPATTokenType
 )
 
 var tokType tokenType
@@ -58,6 +59,8 @@ var _ = BeforeSuite(func() {
 		tokType = patTokenType
 	case "GITHUB_TOKEN":
 		tokType = githubWorkflowDefaultTokenType
+	case "GITLAB_PAT":
+		tokType = gitlabPATTokenType
 	default:
 		panic(fmt.Sprintf("invald TOKEN_TYPE: %s", tt))
 	}

e2e/license_test.go

@@ -120,6 +120,8 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() {
 				&dl)).Should(BeTrue())
 		})
 		It("Should return license check works - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/scorecard-check-license-e2e")
 			Expect(err).Should(BeNil())
@@ -146,6 +148,8 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() {
 				&dl)).Should(BeTrue())
 		})
 		It("Should return license check works at commitSHA - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/scorecard-check-license-e2e")
 			Expect(err).Should(BeNil())

e2e/maintained_test.go

@@ -57,6 +57,8 @@ var _ = Describe("E2E TEST:"+checks.CheckMaintained, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return valid maintained status - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/gitlab-org/gitlab")
 			Expect(err).Should(BeNil())

e2e/pinned_dependencies_test.go

@@ -121,6 +121,8 @@ var _ = Describe("E2E TEST:"+checks.CheckPinnedDependencies, func() {
 			Expect(x.Close()).Should(BeNil())
 		})
 		It("Should return dependencies check is working - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/scorecard-check-pinned-dependencies-e2e")
 			Expect(err).Should(BeNil())
@@ -147,6 +149,8 @@ var _ = Describe("E2E TEST:"+checks.CheckPinnedDependencies, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return dependencies check at commit - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			// project url is gitlab.com/N8BWert/scorecard-check-pinned-dependencies-e2e.
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/scorecard-check-pinned-dependencies-e2e")

e2e/sast_test.go

@@ -57,6 +57,8 @@ var _ = Describe("E2E TEST:"+checks.CheckSAST, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return use of SAST tools - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			// project url is gitlab.com/N8BWert/airflow
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/airflow")

e2e/security_policy_test.go

@@ -174,6 +174,8 @@ var _ = Describe("E2E TEST:"+checks.CheckSecurityPolicy, func() {
 			Expect(x.Close()).Should(BeNil())
 		})
 		It("Should return valid security policy - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			// project url is gitlab.com/bramw/baserow.
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/bramw/baserow")
@@ -203,6 +205,8 @@ var _ = Describe("E2E TEST:"+checks.CheckSecurityPolicy, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return valid security policy at commitSHA - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			// project url is gitlab.com/bramw/baserow.
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/bramw/baserow")

e2e/signedreleases_test.go

@@ -58,6 +58,8 @@ var _ = Describe("E2E TEST:"+checks.CheckSignedReleases, func() {
 		})
 		// TODO: Make this test test a ossf repository I currently have it just testing against the gitlab project.
 		It("Should return valid signed releases - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			dl := scut.TestDetailLogger{}
 			// project url is gitlab.com/gitlab-org/gitlab
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/gitlab-org/gitlab")

e2e/vulnerabilities_test.go

@@ -115,6 +115,8 @@ var _ = Describe("E2E TEST:"+checks.CheckVulnerabilities, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return that there are vulnerabilities - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			// project url is gitlab.com/N8BWert/scorecard-check-vulnerabilities-open62541.
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/scorecard-check-vulnerabilities-open62541")
 			Expect(err).Should(BeNil())
@@ -144,6 +146,8 @@ var _ = Describe("E2E TEST:"+checks.CheckVulnerabilities, func() {
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return that there are vulnerabilities at commit - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
 			// project url is gitlab.com/N8BWert/scorecard-check-vulnerabilities-open62541.
 			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/scorecard-check-vulnerabilities-open62541")
 			Expect(err).Should(BeNil())