🌱 Enhance test output and management in ValidateTestReturn (#3810)

* test failures should print the details they receive this makes debugging failing tests easier. Signed-off-by: Spencer Schrock <sschrock@google.com> * use GinkgoTB so the test helpers work instead of panicing Signed-off-by: Spencer Schrock <sschrock@google.com> * ValidateTestReturn will fail the test directly, no need for the bool return Signed-off-by: Spencer Schrock <sschrock@google.com> * clarify diff details Signed-off-by: Spencer Schrock <sschrock@google.com> --------- Signed-off-by: Spencer Schrock <sschrock@google.com>
ossf · Jan 30, 2024 · 83ff808 · 83ff808
1 parent 19047e8
commit 83ff808
Show file tree

Hide file tree

Showing 47 changed files with 109 additions and 163 deletions.
diff --git a/checks/branch_protection_test.go b/checks/branch_protection_test.go
@@ -431,9 +431,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) {
 				RepoClient: mockRepoClient,
 			}
 			r := BranchProtection(&req)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl) {
-				t.Fail()
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl)
 			ctrl.Finish()
 		})
 	}

diff --git a/checks/cii_best_practices_test.go b/checks/cii_best_practices_test.go
@@ -122,9 +122,7 @@ func TestCIIBestPractices(t *testing.T) {
 			}
 			res := CIIBestPractices(&req)
 			dl := scut.TestDetailLogger{}
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, &dl) {
-				t.Fail()
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, &dl)
 			ctrl.Finish()
 		})
 	}

diff --git a/checks/dependency_update_tool_test.go b/checks/dependency_update_tool_test.go
@@ -155,9 +155,7 @@ func TestDependencyUpdateTool(t *testing.T) {
 			}
 			res := DependencyUpdateTool(c)
 
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, &dl) {
-				t.Fail()
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, &dl)
 		})
 	}
 }

diff --git a/checks/evaluation/binary_artifacts_test.go b/checks/evaluation/binary_artifacts_test.go
@@ -142,9 +142,7 @@ func TestBinaryArtifacts(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := BinaryArtifacts(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/branch_protection_test.go b/checks/evaluation/branch_protection_test.go
@@ -573,9 +573,7 @@ func TestIsBranchProtected(t *testing.T) {
 				Score: score,
 				Error: err,
 			}
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, actual, &dl) {
-				t.Fail()
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, actual, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/ci_tests_test.go b/checks/evaluation/ci_tests_test.go
@@ -111,9 +111,7 @@ func TestCITests(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := CITests(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/cii_best_practices_test.go b/checks/evaluation/cii_best_practices_test.go
@@ -143,9 +143,7 @@ func TestCIIBestPractices(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := CIIBestPractices(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/code_review_test.go b/checks/evaluation/code_review_test.go
@@ -216,9 +216,7 @@ func TestCodeReview(t *testing.T) {
 
 			dl := &scut.TestDetailLogger{}
 			res := CodeReview(tt.name, dl, tt.rawData)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, dl) {
-				t.Error()
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, dl)
 		})
 	}
 }
diff --git a/checks/evaluation/contributors_test.go b/checks/evaluation/contributors_test.go
@@ -83,9 +83,7 @@ func TestContributors(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := Contributors(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Error(tt.name)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/dangerous_workflow_test.go b/checks/evaluation/dangerous_workflow_test.go
@@ -252,9 +252,7 @@ func TestDangerousWorkflow(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := DangerousWorkflow(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/dependency_update_tool_test.go b/checks/evaluation/dependency_update_tool_test.go
@@ -164,9 +164,7 @@ func TestDependencyUpdateTool(t *testing.T) {
 
 			dl := scut.TestDetailLogger{}
 			got := DependencyUpdateTool(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/fuzzing_test.go b/checks/evaluation/fuzzing_test.go
@@ -217,9 +217,7 @@ func TestFuzzing(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := Fuzzing(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/license_test.go b/checks/evaluation/license_test.go
@@ -155,9 +155,7 @@ func TestLicense(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := License(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/maintained_test.go b/checks/evaluation/maintained_test.go
@@ -127,9 +127,7 @@ func TestMaintained(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := Maintained(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/packaging_test.go b/checks/evaluation/packaging_test.go
@@ -88,9 +88,7 @@ func TestPackaging(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := Packaging(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/pinned_dependencies_test.go b/checks/evaluation/pinned_dependencies_test.go
@@ -841,9 +841,7 @@ func Test_PinningDependencies(t *testing.T) {
 					ProcessingErrors: tt.processingErrors,
 				})
 
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &actual, &dl) {
-				t.Fail()
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, &actual, &dl)
 		})
 	}
 }

diff --git a/checks/evaluation/sast_test.go b/checks/evaluation/sast_test.go
@@ -301,9 +301,7 @@ func TestSAST(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := SAST(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/security_policy_test.go b/checks/evaluation/security_policy_test.go
@@ -190,9 +190,7 @@ func TestSecurityPolicy(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := SecurityPolicy(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/signed_releases_test.go b/checks/evaluation/signed_releases_test.go
@@ -303,9 +303,7 @@ func TestSignedReleases(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := SignedReleases(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }

diff --git a/checks/evaluation/vulnerabilities_test.go b/checks/evaluation/vulnerabilities_test.go
@@ -139,9 +139,7 @@ func TestVulnerabilities(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := Vulnerabilities(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/evaluation/webhooks_test.go b/checks/evaluation/webhooks_test.go
@@ -229,9 +229,7 @@ func TestWebhooks(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
 			got := Webhooks(tt.name, tt.findings, &dl)
-			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
-				t.Errorf("got %v, expected %v", got, tt.result)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl)
 		})
 	}
 }
diff --git a/checks/fuzzing_test.go b/checks/fuzzing_test.go
@@ -169,9 +169,7 @@ func TestFuzzing(t *testing.T) {
 				return
 			}
 
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &result, &dl) {
-				t.Fatalf(tt.name, tt.expected)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, &result, &dl)
 		})
 	}
 }
diff --git a/checks/license_test.go b/checks/license_test.go
@@ -92,9 +92,7 @@ func TestLicenseFileSubdirectory(t *testing.T) {
 
 			res := License(&req)
 
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, &dl) {
-				t.Fail()
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, &dl)
 
 			ctrl.Finish()
 		})

diff --git a/checks/permissions_test.go b/checks/permissions_test.go
@@ -458,9 +458,7 @@ func TestGithubTokenPermissions(t *testing.T) {
 
 			res := TokenPermissions(&c)
 
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, &dl) {
-				t.Errorf("test failed: log message not present: %+v\n%+v", tt.expected, dl)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, &res, &dl)
 		})
 	}
 }

diff --git a/checks/raw/security_policy_test.go b/checks/raw/security_policy_test.go
@@ -165,9 +165,7 @@ func TestSecurityPolicy(t *testing.T) {
 
 			res, err := SecurityPolicy(&c)
 
-			if !scut.ValidateTestReturn(t, tt.name, &tt.want, &checker.CheckResult{}, &dl) {
-				t.Errorf("test failed: log message not present: %+v , for test %v", tt.want, tt.name)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.want, &checker.CheckResult{}, &dl)
 
 			if (err != nil) != tt.wantErr {
 				t.Errorf("SecurityPolicy() error = %v, wantErr %v", err, tt.wantErr)

diff --git a/checks/raw/vulnerabilities_test.go b/checks/raw/vulnerabilities_test.go
@@ -114,9 +114,7 @@ func TestVulnerabilities(t *testing.T) {
 				}
 			}
 
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &checker.CheckResult{}, &dl) {
-				t.Fatalf("Test %s failed", tt.name)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, &checker.CheckResult{}, &dl)
 		})
 	}
 }
diff --git a/checks/raw/webhooks_test.go b/checks/raw/webhooks_test.go
@@ -130,9 +130,7 @@ func TestWebhooks(t *testing.T) {
 				}
 			}
 
-			if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &checker.CheckResult{}, &dl) {
-				t.Fatalf("Test %s failed", tt.name)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.expected, &checker.CheckResult{}, &dl)
 		})
 	}
 }
diff --git a/checks/security_policy_test.go b/checks/security_policy_test.go
@@ -197,9 +197,7 @@ func TestSecurityPolicy(t *testing.T) {
 
 			res := SecurityPolicy(c)
 
-			if !scut.ValidateTestReturn(t, tt.name, &tt.want, &res, &dl) {
-				t.Errorf("test failed: log message not present: %+v on %+v", tt.want, res)
-			}
+			scut.ValidateTestReturn(t, tt.name, &tt.want, &res, &dl)
 		})
 	}
 }
diff --git a/e2e/binary_artifacts_test.go b/e2e/binary_artifacts_test.go
@@ -57,7 +57,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 			}
 
 			result := checks.BinaryArtifacts(&req)
-			Expect(scut.ValidateTestReturn(nil, "no binary artifacts", &expected, &result, &dl)).Should(BeTrue())
+			scut.ValidateTestReturn(GinkgoTB(), "no binary artifacts", &expected, &result, &dl)
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return binary artifacts present in source code", func() {
@@ -84,7 +84,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 				NumberOfDebug: 0,
 			}
 			result := checks.BinaryArtifacts(&req)
-			Expect(scut.ValidateTestReturn(nil, "binary artifacts", &expected, &result, &dl)).Should(BeTrue())
+			scut.ValidateTestReturn(GinkgoTB(), "binary artifacts", &expected, &result, &dl)
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return binary artifacts present at commit in source code", func() {
@@ -111,7 +111,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 				NumberOfDebug: 0,
 			}
 			result := checks.BinaryArtifacts(&req)
-			Expect(scut.ValidateTestReturn(nil, "binary artifacts", &expected, &result, &dl)).Should(BeTrue())
+			scut.ValidateTestReturn(GinkgoTB(), "binary artifacts", &expected, &result, &dl)
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return no binary artifacts present in source code", func() {
@@ -139,7 +139,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 			}
 			result := checks.BinaryArtifacts(&req)
 			// New version.
-			Expect(scut.ValidateTestReturn(nil, "binary artifacts", &expected, &result, &dl)).Should(BeTrue())
+			scut.ValidateTestReturn(GinkgoTB(), "binary artifacts", &expected, &result, &dl)
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return binary artifacts present at commit in source code", func() {
@@ -167,7 +167,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 			}
 			result := checks.BinaryArtifacts(&req)
 			// New version.
-			Expect(scut.ValidateTestReturn(nil, "binary artifacts", &expected, &result, &dl)).Should(BeTrue())
+			scut.ValidateTestReturn(GinkgoTB(), "binary artifacts", &expected, &result, &dl)
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should return binary artifacts present at commit in source code when using local repoClient", func() {
@@ -206,7 +206,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
 			}
 			result := checks.BinaryArtifacts(&req)
 			// New version.
-			Expect(scut.ValidateTestReturn(nil, "binary artifacts", &expected, &result, &dl)).Should(BeTrue())
+			scut.ValidateTestReturn(GinkgoTB(), "binary artifacts", &expected, &result, &dl)
 			Expect(x.Close()).Should(BeNil())
 		})
 	})

diff --git a/e2e/branch_protection_test.go b/e2e/branch_protection_test.go
@@ -56,7 +56,7 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			// Old version.
 
 			// New version.
-			Expect(scut.ValidateTestReturn(nil, "branch protection accessible", &expected, &result, &dl)).Should(BeTrue())
+			scut.ValidateTestReturn(GinkgoTB(), "branch protection accessible", &expected, &result, &dl)
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should fail to return branch protection on other repositories", func() {
@@ -84,7 +84,7 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			result := checks.BranchProtection(&req)
 
 			// New version.
-			Expect(scut.ValidateTestReturn(nil, "branch protection accessible none", &expected, &result, &dl)).Should(BeTrue())
+			scut.ValidateTestReturn(GinkgoTB(), "branch protection accessible none", &expected, &result, &dl)
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 		It("Should fail to return branch protection on other repositories patch", func() {
@@ -112,7 +112,7 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
 			result := checks.BranchProtection(&req)
 
 			// New version.
-			Expect(scut.ValidateTestReturn(nil, "branch protection accessible patch", &expected, &result, &dl)).Should(BeTrue())
+			scut.ValidateTestReturn(GinkgoTB(), "branch protection accessible patch", &expected, &result, &dl)
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 	})
@@ -169,7 +169,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBranchProtection+" (repo rules)", func(
 			}
 			result := checks.BranchProtection(&req)
 			Expect(result.Error).Should(BeNil())
-			Expect(scut.ValidateTestReturn(nil, "repo rules accessible", &expected, &result, &dl)).Should(BeTrue())
+			scut.ValidateTestReturn(GinkgoTB(), "repo rules accessible", &expected, &result, &dl)
 			Expect(repoClient.Close()).Should(BeNil())
 		})
 	})