remove 'totalTested' and 'totalMerged' values from findings

Signed-off-by: Adam Korczynski <[email protected]>

checks/evaluation/ci_tests.go

@@ -52,8 +52,7 @@ func CITests(name string,
 		return checker.CreateInconclusiveResult(CheckCITests, "no pull request found")
 	}
 
-	totalMerged := findings[0].Values["totalMerged"]
-	totalTested := findings[0].Values["totalTested"]
+	totalMerged, totalTested := getMergedAndTested(findings)
 
 	if totalMerged < totalTested || len(findings) < totalTested {
 		e := sce.WithMessage(sce.ErrScorecardInternal, "invalid finding values")
@@ -63,11 +62,25 @@ func CITests(name string,
 	// Log all findings
 	checker.LogFindings(nonNegativeFindings(findings), dl)
 
-	// All findings (should) have "totalMerged" and "totalTested"
 	reason := fmt.Sprintf("%d out of %d merged PRs checked by a CI test", totalTested, totalMerged)
 	return checker.CreateProportionalScoreResult(CheckCITests, reason, totalTested, totalMerged)
 }
 
+func getMergedAndTested(findings []finding.Finding) (int, int) {
+	totalMerged := 0
+	totalTested := 0
+
+	for i := range findings {
+		f := &findings[i]
+		totalMerged++
+		if f.Outcome == finding.OutcomePositive {
+			totalTested++
+		}
+	}
+
+	return totalMerged, totalTested
+}
+
 func noPullRequestsFound(findings []finding.Finding) bool {
 	for i := range findings {
 		f := &findings[i]

checks/evaluation/ci_tests_test.go

@@ -16,7 +16,6 @@ package evaluation
 import (
 	"testing"
 
-	sce "github.com/ossf/scorecard/v4/errors"
 	"github.com/ossf/scorecard/v4/finding"
 	scut "github.com/ossf/scorecard/v4/utests"
 )
@@ -37,7 +36,6 @@ func TestCITests(t *testing.T) {
 					Outcome:  finding.OutcomePositive,
 					Probe:    "testsRunInCI",
 					Message:  "CI test found: pr: 1, context: e2e",
-					Values:   map[string]int{"totalMerged": 1, "totalTested": 1},
 					Location: &finding.Location{Type: 4},
 				},
 			},
@@ -53,28 +51,24 @@ func TestCITests(t *testing.T) {
 					Outcome:  finding.OutcomePositive,
 					Probe:    "testsRunInCI",
 					Message:  "CI test found: pr: 1, context: e2e",
-					Values:   map[string]int{"totalMerged": 4, "totalTested": 3},
 					Location: &finding.Location{Type: 4},
 				},
 				{
 					Outcome:  finding.OutcomePositive,
 					Probe:    "testsRunInCI",
 					Message:  "CI test found: pr: 1, context: e2e",
-					Values:   map[string]int{"totalMerged": 4, "totalTested": 3},
 					Location: &finding.Location{Type: 4},
 				},
 				{
 					Outcome:  finding.OutcomePositive,
 					Probe:    "testsRunInCI",
 					Message:  "CI test found: pr: 1, context: e2e",
-					Values:   map[string]int{"totalMerged": 4, "totalTested": 3},
 					Location: &finding.Location{Type: 4},
 				},
 				{
 					Outcome:  finding.OutcomeNegative,
 					Probe:    "testsRunInCI",
 					Message:  "CI test found: pr: 1, context: e2e",
-					Values:   map[string]int{"totalMerged": 4, "totalTested": 3},
 					Location: &finding.Location{Type: 4},
 				},
 			},
@@ -84,76 +78,25 @@ func TestCITests(t *testing.T) {
 				NumberOfDebug: 1,
 			},
 		},
-		{
-			name: "More tested than there are findings = error",
-			findings: []finding.Finding{
-				{
-					Outcome:  finding.OutcomePositive,
-					Probe:    "testsRunInCI",
-					Message:  "CI test found: pr: 1, context: e2e",
-					Values:   map[string]int{"totalMerged": 2, "totalTested": 2},
-					Location: &finding.Location{Type: 4},
-				},
-			},
-			result: scut.TestReturn{
-				Error:        sce.ErrScorecardInternal,
-				NumberOfInfo: 0,
-				Score:        -1,
-			},
-		},
-		{
-			name: "More tested than merged = error",
-			findings: []finding.Finding{
-				{
-					Outcome:  finding.OutcomePositive,
-					Probe:    "testsRunInCI",
-					Message:  "CI test found: pr: 1, context: e2e",
-					Values:   map[string]int{"totalMerged": 2, "totalTested": 3},
-					Location: &finding.Location{Type: 4},
-				},
-				{
-					Outcome:  finding.OutcomePositive,
-					Probe:    "testsRunInCI",
-					Message:  "CI test found: pr: 1, context: e2e",
-					Values:   map[string]int{"totalMerged": 2, "totalTested": 3},
-					Location: &finding.Location{Type: 4},
-				},
-				{
-					Outcome:  finding.OutcomePositive,
-					Probe:    "testsRunInCI",
-					Message:  "CI test found: pr: 1, context: e2e",
-					Values:   map[string]int{"totalMerged": 2, "totalTested": 3},
-					Location: &finding.Location{Type: 4},
-				},
-			},
-			result: scut.TestReturn{
-				Error:        sce.ErrScorecardInternal,
-				NumberOfInfo: 0,
-				Score:        -1,
-			},
-		},
 		{
 			name: "Tests debugging",
 			findings: []finding.Finding{
 				{
 					Outcome:  finding.OutcomeNegative,
 					Probe:    "testsRunInCI",
 					Message:  "merged PR 1 without CI test at HEAD: 1",
-					Values:   map[string]int{"totalMerged": 3, "totalTested": 0},
 					Location: &finding.Location{Type: 4},
 				},
 				{
 					Outcome:  finding.OutcomeNegative,
 					Probe:    "testsRunInCI",
 					Message:  "merged PR 1 without CI test at HEAD: 1",
-					Values:   map[string]int{"totalMerged": 3, "totalTested": 0},
 					Location: &finding.Location{Type: 4},
 				},
 				{
 					Outcome:  finding.OutcomeNegative,
 					Probe:    "testsRunInCI",
 					Message:  "merged PR 1 without CI test at HEAD: 1",
-					Values:   map[string]int{"totalMerged": 3, "totalTested": 0},
 					Location: &finding.Location{Type: 4},
 				},
 			},

probes/testsRunInCI/def.yml

@@ -19,7 +19,7 @@ motivation: >
 implementation: >
  The probe checks for tests in the projects CI jobs in the recent commits (~30).
 outcome:
-  - The probe returns one OutcomePositive for each successful test or check that ran in the projects CI. All findings include the number of merged pull requests as well as how many of these pull requests were tested by tests in the CI. These fields are called "totalMerged" and "totalTested".
+  - The probe returns one OutcomePositive for each PR that ran CI tests and one OutcomeNegative for each PR that did not run CI tests.
   - The probe returns a single OutcomeNotApplicable if the projects has had no pull requests.
 remediation:
   effort: Medium

probes/testsRunInCI/impl.go

@@ -53,20 +53,15 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 		return findings, Probe, nil
 	}
 
-	totalMerged := 0
-	totalTested := 0
 	for i := range c.CIInfo {
 		r := c.CIInfo[i]
-		totalMerged++
-
 		// GitHub Statuses.
 		prSuccessStatus, f, err := prHasSuccessStatus(r)
 		if err != nil {
 			return nil, Probe, fmt.Errorf("create finding: %w", err)
 		}
 		if prSuccessStatus {
 			findings = append(findings, *f)
-			totalTested++
 			continue
 		}
 
@@ -77,7 +72,6 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 		}
 		if prCheckSuccessful {
 			findings = append(findings, *f)
-			totalTested++
 		}
 
 		if !prSuccessStatus && !prCheckSuccessful {
@@ -91,14 +85,6 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 		}
 	}
 
-	for i := range findings {
-		f := &findings[i]
-		f.WithValues(map[string]int{
-			"totalMerged": totalMerged,
-			"totalTested": totalTested,
-		})
-	}
-
 	return findings, Probe, nil
 }
 

probes/testsRunInCI/impl_test.go

@@ -75,7 +75,6 @@ func Test_Run(t *testing.T) {
 					Outcome:  finding.OutcomePositive,
 					Probe:    Probe,
 					Message:  "CI test found: pr: 1, context: e2e",
-					Values:   map[string]int{"totalMerged": 1, "totalTested": 1},
 					Location: &finding.Location{Type: 4},
 				},
 			},
@@ -111,7 +110,6 @@ func Test_Run(t *testing.T) {
 					Outcome:  finding.OutcomePositive,
 					Probe:    Probe,
 					Message:  "CI test found: pr: HeadSHA, context: CI-Tests",
-					Values:   map[string]int{"totalMerged": 1, "totalTested": 1},
 					Location: &finding.Location{Type: 4},
 				},
 			},