⚠️ Removing the error field from result

- Removing the error field from result - #1393 Signed-off-by: naveensrinivasan <[email protected]>
ossf · Apr 21, 2022 · 5cb7f63 · 5cb7f63
1 parent 72e2486
commit 5cb7f63
Show file tree

Hide file tree

Showing 17 changed files with 4 additions and 38 deletions.
diff --git a/checker/check_result.go b/checker/check_result.go
@@ -79,7 +79,6 @@ const (
 // nolint:govet
 type CheckResult struct {
 	// TODO(#1393): Remove old structure after deprecation.
-	Error      error `json:"-"`
 	Name       string
 	Details    []string
 	Confidence int
@@ -161,7 +160,6 @@ func CreateResultWithScore(name, reason string, score int) CheckResult {
 	return CheckResult{
 		Name: name,
 		// Old structure.
-		Error:      nil,
 		Confidence: MaxResultScore,
 		Pass:       pass,
 		// New structure.
@@ -186,7 +184,6 @@ func CreateProportionalScoreResult(name, reason string, b, t int) CheckResult {
 	return CheckResult{
 		Name: name,
 		// Old structure.
-		Error:      nil,
 		Confidence: MaxResultConfidence,
 		Pass:       pass,
 		// New structure.
@@ -232,7 +229,6 @@ func CreateRuntimeErrorResult(name string, e error) CheckResult {
 	return CheckResult{
 		Name: name,
 		// Old structure.
-		Error:      e,
 		Confidence: 0,
 		Pass:       false,
 		// New structure.

diff --git a/checker/check_runner.go b/checker/check_runner.go
@@ -76,7 +76,7 @@ func logStats(ctx context.Context, startTime time.Time, result *CheckResult) err
 	runTimeInSecs := time.Now().Unix() - startTime.Unix()
 	opencensusstats.Record(ctx, stats.CheckRuntimeInSec.M(runTimeInSecs))
 
-	if result.Error != nil {
+	if result.Error2 != nil {
 		ctx, err := tag.New(ctx, tag.Upsert(stats.ErrorName, sce.GetName(result.Error2)))
 		if err != nil {
 			return sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("tag.New: %v", err))

diff --git a/checks/evaluation/branch_protection_test.go b/checks/evaluation/branch_protection_test.go
@@ -285,7 +285,6 @@ func TestIsBranchProtected(t *testing.T) {
 			score, err := testScore(tt.branch, &dl)
 			actual := &checker.CheckResult{
 				Score: score,
-				Error: err,
 			}
 			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, actual, &dl) {
 				t.Fail()

diff --git a/checks/vulnerabilities_test.go b/checks/vulnerabilities_test.go
@@ -67,9 +67,9 @@ func TestVulnerabilities(t *testing.T) {
 				VulnerabilitiesClient: mockVulnClient,
 			}
 			res := Vulnerabilities(&req)
-			if !tt.isError && res.Error != nil {
+			if !tt.isError && res.Error2 != nil {
 				t.Fail()
-			} else if tt.isError && res.Error == nil {
+			} else if tt.isError && res.Error2 == nil {
 				t.Fail()
 			}
 			ctrl.Finish()

diff --git a/e2e/binary_artifacts_test.go b/e2e/binary_artifacts_test.go
@@ -86,7 +86,6 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 			result := checks.BinaryArtifacts(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "binary artifacts", &expected, &result, &dl)).Should(BeTrue())
@@ -117,7 +116,6 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 			result := checks.BinaryArtifacts(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "binary artifacts", &expected, &result, &dl)).Should(BeTrue())

diff --git a/e2e/branch_protection_test.go b/e2e/branch_protection_test.go
@@ -54,7 +54,6 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			result := checks.BranchProtection(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 
 			// New version.
@@ -86,7 +85,6 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			result := checks.BranchProtection(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 
 			// New version.
@@ -118,7 +116,6 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			result := checks.BranchProtection(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 
 			// New version.
@@ -146,9 +143,9 @@ var _ = Describe("E2E TEST GITHUB_TOKEN:"+checks.CheckBranchProtection, func() {
 			}
 
 			result := checks.BranchProtection(&req)
+			Expect(result.Error2).ShouldNot(BeNil())
 			// There should be an error with the GITHUB_TOKEN, until it's supported
 			// byt GitHub.
-			Expect(result.Error).ShouldNot(BeNil())
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 	})

diff --git a/e2e/cii_best_practices_test.go b/e2e/cii_best_practices_test.go
@@ -52,7 +52,6 @@ var _ = Describe("E2E TEST:"+checks.CheckCIIBestPractices, func() {
 			result := checks.CIIBestPractices(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "passing badge", &expected, &result, &dl)).Should(BeTrue())

diff --git a/e2e/contributors_test.go b/e2e/contributors_test.go
@@ -52,7 +52,6 @@ var _ = Describe("E2E TEST:"+checks.CheckContributors, func() {
 			result := checks.Contributors(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "several contributors", &expected, &result, &dl)).Should(BeTrue())

diff --git a/e2e/dangerous_workflow_test.go b/e2e/dangerous_workflow_test.go
@@ -56,7 +56,6 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
 			// UPGRADEv2: to remove.
 			// Old version.
 
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "dangerous workflow", &expected, &result, &dl)).Should(BeTrue())
@@ -85,7 +84,6 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
 			// UPGRADEv2: to remove.
 			// Old version.
 
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "dangerous workflow", &expected, &result, &dl)).Should(BeTrue())
@@ -126,7 +124,6 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
 			// UPGRADEv2: to remove.
 			// Old version.
 
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "dangerous workflow", &expected, &result, &dl)).Should(BeTrue())

diff --git a/e2e/dependency_update_tool_test.go b/e2e/dependency_update_tool_test.go
@@ -56,7 +56,6 @@ var _ = Describe("E2E TEST:"+checks.CheckDependencyUpdateTool, func() {
 			result := checks.DependencyUpdateTool(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "dependabot", &expected, &result, &dl)).Should(BeTrue())
@@ -86,7 +85,6 @@ var _ = Describe("E2E TEST:"+checks.CheckDependencyUpdateTool, func() {
 			result := checks.DependencyUpdateTool(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "renovabot", &expected, &result, &dl)).Should(BeTrue())

diff --git a/e2e/license_test.go b/e2e/license_test.go
@@ -54,7 +54,6 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() {
 			}
 			result := checks.License(&req)
 
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 
 			Expect(scut.ValidateTestReturn(nil, "license found", &expected, &result,
@@ -82,7 +81,6 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() {
 			}
 			result := checks.License(&req)
 
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 
 			Expect(scut.ValidateTestReturn(nil, "license found", &expected, &result,
@@ -122,7 +120,6 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() {
 			}
 			result := checks.License(&req)
 
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 
 			Expect(scut.ValidateTestReturn(nil, "license found", &expected, &result,

diff --git a/e2e/maintained_test.go b/e2e/maintained_test.go
@@ -52,7 +52,6 @@ var _ = Describe("E2E TEST:"+checks.CheckMaintained, func() {
 			result := checks.Maintained(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "active repo", &expected, &result, &dl)).Should(BeTrue())

diff --git a/e2e/permissions_test.go b/e2e/permissions_test.go
@@ -56,7 +56,6 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
 			// UPGRADEv2: to remove.
 			// Old version.
 
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "token permissions", &expected, &result, &dl)).Should(BeTrue())
@@ -86,7 +85,6 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
 			// UPGRADEv2: to remove.
 			// Old version.
 
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "token permissions", &expected, &result, &dl)).Should(BeTrue())
@@ -128,7 +126,6 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
 			// UPGRADEv2: to remove.
 			// Old version.
 
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "token permissions", &expected, &result, &dl)).Should(BeTrue())

diff --git a/e2e/sast_test.go b/e2e/sast_test.go
@@ -52,7 +52,6 @@ var _ = Describe("E2E TEST:"+checks.CheckSAST, func() {
 			result := checks.SAST(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "sast used", &expected, &result, &dl)).Should(BeTrue())

diff --git a/e2e/security_policy_test.go b/e2e/security_policy_test.go
@@ -56,7 +56,6 @@ var _ = Describe("E2E TEST:"+checks.CheckSecurityPolicy, func() {
 			result := checks.SecurityPolicy(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "policy found", &expected, &result, &dl)).Should(BeTrue())
@@ -86,7 +85,6 @@ var _ = Describe("E2E TEST:"+checks.CheckSecurityPolicy, func() {
 			result := checks.SecurityPolicy(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "policy found", &expected, &result, &dl)).Should(BeTrue())
@@ -116,7 +114,6 @@ var _ = Describe("E2E TEST:"+checks.CheckSecurityPolicy, func() {
 			result := checks.SecurityPolicy(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "policy found", &expected, &result, &dl)).Should(BeTrue())
@@ -146,7 +143,6 @@ var _ = Describe("E2E TEST:"+checks.CheckSecurityPolicy, func() {
 			result := checks.SecurityPolicy(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "policy found", &expected, &result, &dl)).Should(BeTrue())
@@ -187,7 +183,6 @@ var _ = Describe("E2E TEST:"+checks.CheckSecurityPolicy, func() {
 			result := checks.SecurityPolicy(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "policy found", &expected, &result, &dl)).Should(BeTrue())

diff --git a/e2e/signedreleases_test.go b/e2e/signedreleases_test.go
@@ -52,7 +52,6 @@ var _ = Describe("E2E TEST:"+checks.CheckSignedReleases, func() {
 			result := checks.SignedReleases(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "verified release", &expected, &result, &dl)).Should(BeTrue())

diff --git a/e2e/vulnerabilities_test.go b/e2e/vulnerabilities_test.go
@@ -55,7 +55,6 @@ var _ = Describe("E2E TEST:"+checks.CheckVulnerabilities, func() {
 			result := checks.Vulnerabilities(&req)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeTrue())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "no osv vulnerabilities", &expected, &result, &dl)).Should(BeTrue())
@@ -87,7 +86,6 @@ var _ = Describe("E2E TEST:"+checks.CheckVulnerabilities, func() {
 			result := checks.Vulnerabilities(&checkRequest)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "osv vulnerabilities", &expected, &result, &dl)).Should(BeTrue())
@@ -118,7 +116,6 @@ var _ = Describe("E2E TEST:"+checks.CheckVulnerabilities, func() {
 			result := checks.Vulnerabilities(&checkRequest)
 			// UPGRADEv2: to remove.
 			// Old version.
-			Expect(result.Error).Should(BeNil())
 			Expect(result.Pass).Should(BeFalse())
 			// New version.
 			Expect(scut.ValidateTestReturn(nil, "osv vulnerabilities", &expected, &result, &dl)).Should(BeTrue())