✨ checks/evaluation logs findings (#3409)

* checks/validation logs findings Signed-off-by: laurentsimon <laurentsimon@google.com> * gofmt file Signed-off-by: laurentsimon <laurentsimon@google.com> * linter Signed-off-by: laurentsimon <laurentsimon@google.com> * revert go.sum Signed-off-by: laurentsimon <laurentsimon@google.com> * typo Signed-off-by: laurentsimon <laurentsimon@google.com> * add unit tests and address comments Signed-off-by: laurentsimon <laurentsimon@google.com> * update comment Signed-off-by: laurentsimon <laurentsimon@google.com> * missing file Signed-off-by: laurentsimon <laurentsimon@google.com> * use option 1 Signed-off-by: laurentsimon <laurentsimon@google.com> * use got / want in test Signed-off-by: laurentsimon <laurentsimon@google.com> * missing tests updates Signed-off-by: laurentsimon <laurentsimon@google.com> --------- Signed-off-by: laurentsimon <laurentsimon@google.com>
ossf · Sep 12, 2023 · 8b096ad · 8b096ad
1 parent 52a4843
commit 8b096ad
Show file tree

Hide file tree

Showing 16 changed files with 261 additions and 249 deletions.
diff --git a/checker/check_result.go b/checker/check_result.go
@@ -195,7 +195,7 @@ func CreateRuntimeErrorResult(name string, e error) CheckResult {
 }
 
 // LogFindings logs the list of findings.
-func LogFindings(findings []finding.Finding, dl DetailLogger) error {
+func LogFindings(findings []finding.Finding, dl DetailLogger) {
 	for i := range findings {
 		f := &findings[i]
 		switch f.Outcome {
@@ -213,6 +213,4 @@ func LogFindings(findings []finding.Finding, dl DetailLogger) error {
 			})
 		}
 	}
-
-	return nil
 }
diff --git a/checks/dependency_update_tool.go b/checks/dependency_update_tool.go
@@ -20,6 +20,7 @@ import (
 	"github.com/ossf/scorecard/v4/checks/raw"
 	sce "github.com/ossf/scorecard/v4/errors"
 	"github.com/ossf/scorecard/v4/probes"
+	"github.com/ossf/scorecard/v4/probes/zrunner"
 )
 
 // CheckDependencyUpdateTool is the exported name for Automatic-Depdendency-Update.
@@ -49,12 +50,12 @@ func DependencyUpdateTool(c *checker.CheckRequest) checker.CheckResult {
 	pRawResults.DependencyUpdateToolResults = rawData
 
 	// Evaluate the probes.
-	findings, err := evaluateProbes(c, pRawResults, probes.DependencyToolUpdates)
+	findings, err := zrunner.Run(pRawResults, probes.DependencyToolUpdates)
 	if err != nil {
 		e := sce.WithMessage(sce.ErrScorecardInternal, err.Error())
 		return checker.CreateRuntimeErrorResult(CheckDependencyUpdateTool, e)
 	}
 
 	// Return the score evaluation.
-	return evaluation.DependencyUpdateTool(CheckDependencyUpdateTool, findings)
+	return evaluation.DependencyUpdateTool(CheckDependencyUpdateTool, findings, c.Dlogger)
 }
diff --git a/checks/dependency_update_tool_test.go b/checks/dependency_update_tool_test.go
@@ -51,7 +51,7 @@ func TestDependencyUpdateTool(t *testing.T) {
 			CallSearchCommits: 0,
 			expected: scut.TestReturn{
 				NumberOfInfo: 1,
-				NumberOfWarn: 3,
+				NumberOfWarn: 0,
 				Score:        10,
 			},
 		},
@@ -64,7 +64,7 @@ func TestDependencyUpdateTool(t *testing.T) {
 			CallSearchCommits: 0,
 			expected: scut.TestReturn{
 				NumberOfInfo: 1,
-				NumberOfWarn: 3,
+				NumberOfWarn: 0,
 				Score:        10,
 			},
 		},
@@ -103,7 +103,7 @@ func TestDependencyUpdateTool(t *testing.T) {
 			CallSearchCommits: 1,
 			expected: scut.TestReturn{
 				NumberOfInfo: 1,
-				NumberOfWarn: 3,
+				NumberOfWarn: 0,
 				Score:        10,
 			},
 		},
@@ -118,7 +118,7 @@ func TestDependencyUpdateTool(t *testing.T) {
 			CallSearchCommits: 1,
 			expected: scut.TestReturn{
 				NumberOfInfo: 1,
-				NumberOfWarn: 3,
+				NumberOfWarn: 0,
 				Score:        10,
 			},
 		},
@@ -136,7 +136,7 @@ func TestDependencyUpdateTool(t *testing.T) {
 			CallSearchCommits: 1,
 			expected: scut.TestReturn{
 				NumberOfInfo: 1,
-				NumberOfWarn: 3,
+				NumberOfWarn: 0,
 				Score:        10,
 			},
 		},

diff --git a/checks/evaluation/dependency_update_tool.go b/checks/evaluation/dependency_update_tool.go
@@ -24,9 +24,10 @@ import (
 	"github.com/ossf/scorecard/v4/probes/toolSonatypeLiftInstalled"
 )
 
-// DependencyUpdateTool applies the score policy for the Dependency-Update-Tool check.
+// DependencyUpdateTool applies the score policy and logs the details
+// for the Dependency-Update-Tool check.
 func DependencyUpdateTool(name string,
-	findings []finding.Finding,
+	findings []finding.Finding, dl checker.DetailLogger,
 ) checker.CheckResult {
 	expectedProbes := []string{
 		toolDependabotInstalled.Probe,
@@ -42,9 +43,13 @@ func DependencyUpdateTool(name string,
 	for i := range findings {
 		f := &findings[i]
 		if f.Outcome == finding.OutcomePositive {
+			// Log all findings except the negative ones.
+			checker.LogFindings(nonNegativeFindings(findings), dl)
 			return checker.CreateMaxScoreResult(name, "update tool detected")
 		}
 	}
 
+	// Log all findings.
+	checker.LogFindings(findings, dl)
 	return checker.CreateMinScoreResult(name, "no update tool detected")
 }
diff --git a/checks/evaluation/dependency_update_tool_test.go b/checks/evaluation/dependency_update_tool_test.go
@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"github.com/ossf/scorecard/v4/checker"
+	sce "github.com/ossf/scorecard/v4/errors"
 	"github.com/ossf/scorecard/v4/finding"
 	scut "github.com/ossf/scorecard/v4/utests"
 )
@@ -28,9 +29,7 @@ func TestDependencyUpdateTool(t *testing.T) {
 	tests := []struct {
 		name     string
 		findings []finding.Finding
-		err      bool
-		want     checker.CheckResult
-		expected scut.TestReturn
+		result   scut.TestReturn
 	}{
 		{
 			name: "dependabot",
@@ -52,8 +51,9 @@ func TestDependencyUpdateTool(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 			},
-			want: checker.CheckResult{
-				Score: 10,
+			result: scut.TestReturn{
+				Score:        checker.MaxResultScore,
+				NumberOfInfo: 1,
 			},
 		},
 		{
@@ -76,8 +76,9 @@ func TestDependencyUpdateTool(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 			},
-			want: checker.CheckResult{
-				Score: 10,
+			result: scut.TestReturn{
+				Score:        checker.MaxResultScore,
+				NumberOfInfo: 1,
 			},
 		},
 		{
@@ -100,8 +101,9 @@ func TestDependencyUpdateTool(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 			},
-			want: checker.CheckResult{
-				Score: 10,
+			result: scut.TestReturn{
+				Score:        checker.MaxResultScore,
+				NumberOfInfo: 1,
 			},
 		},
 		{
@@ -128,8 +130,9 @@ func TestDependencyUpdateTool(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 			},
-			want: checker.CheckResult{
-				Score: 10,
+			result: scut.TestReturn{
+				Score:        checker.MaxResultScore,
+				NumberOfInfo: 1,
 			},
 		},
 		{
@@ -152,8 +155,9 @@ func TestDependencyUpdateTool(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 			},
-			want: checker.CheckResult{
-				Score: 0,
+			result: scut.TestReturn{
+				Score:        checker.MinResultScore,
+				NumberOfWarn: 4,
 			},
 		},
 		{
@@ -172,9 +176,9 @@ func TestDependencyUpdateTool(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 			},
-			err: true,
-			want: checker.CheckResult{
-				Score: -1,
+			result: scut.TestReturn{
+				Score: checker.InconclusiveResultScore,
+				Error: sce.ErrScorecardInternal,
 			},
 		},
 		{
@@ -201,8 +205,9 @@ func TestDependencyUpdateTool(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 			},
-			want: checker.CheckResult{
-				Score: -1,
+			result: scut.TestReturn{
+				Score: checker.InconclusiveResultScore,
+				Error: sce.ErrScorecardInternal,
 			},
 		},
 	}
@@ -211,13 +216,10 @@ func TestDependencyUpdateTool(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			got := DependencyUpdateTool(tt.name, tt.findings)
-			if tt.want.Score != got.Score {
-				t.Errorf("DependencyUpdateTool() got Score = %v, want %v for %v", got.Score, tt.want.Score, tt.name)
-			}
-			if tt.err && got.Error == nil {
-				t.Errorf("DependencyUpdateTool() error = %v, want %v for %v", got.Error, tt.want.Error, tt.name)
-				return
+			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)
 			}
 		})
 	}

diff --git a/checks/evaluation/finding.go b/checks/evaluation/finding.go
@@ -0,0 +1,31 @@
+// Copyright 2023 OpenSSF Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package evaluation
+
+import (
+	"github.com/ossf/scorecard/v4/finding"
+)
+
+func nonNegativeFindings(findings []finding.Finding) []finding.Finding {
+	var ff []finding.Finding
+	for i := range findings {
+		f := &findings[i]
+		if f.Outcome == finding.OutcomeNegative {
+			continue
+		}
+		ff = append(ff, *f)
+	}
+	return ff
+}
diff --git a/checks/evaluation/fuzzing.go b/checks/evaluation/fuzzing.go
@@ -29,7 +29,7 @@ import (
 
 // Fuzzing applies the score policy for the Fuzzing check.
 func Fuzzing(name string,
-	findings []finding.Finding,
+	findings []finding.Finding, dl checker.DetailLogger,
 ) checker.CheckResult {
 	// We have 7 unique probes, each should have a finding.
 	expectedProbes := []string{
@@ -51,8 +51,12 @@ func Fuzzing(name string,
 	for i := range findings {
 		f := &findings[i]
 		if f.Outcome == finding.OutcomePositive {
+			// Log all findings except the negative ones.
+			checker.LogFindings(nonNegativeFindings(findings), dl)
 			return checker.CreateMaxScoreResult(name, "project is fuzzed")
 		}
 	}
+	// Log all findings.
+	checker.LogFindings(findings, dl)
 	return checker.CreateMinScoreResult(name, "project is not fuzzed")
 }