⚠️ Switch Outcome type to string (#4006)

* convert outcome constants to strings

Originally, these were introduced as ints to enable ordering between them.
Today, I don't see the value in doing that, and it makes the output less readable.

Signed-off-by: Spencer Schrock <[email protected]>

* explicitly mention negative outcome for some tests

previously, OutcomeNegative had the integer value of 0. So some tests
didnt specify the outcome and happened to pass due to the zero value.
This also fixes the tests names while I was here.

Signed-off-by: Spencer Schrock <[email protected]>

* match expected probe output with new string values

this change demonstrates the reason for this PR.
Human readable outcomes are good!

Signed-off-by: Spencer Schrock <[email protected]>

---------

Signed-off-by: Spencer Schrock <[email protected]>

finding/finding.go

@@ -54,43 +54,25 @@ type Location struct {
 }
 
 // Outcome is the result of a finding.
-type Outcome int
+type Outcome string
 
 // TODO(#2928): re-visit the finding definitions.
 const (
-	// NOTE: The additional '_' are intended for future use.
-	// This allows adding outcomes without breaking the values
-	// of existing outcomes.
 	// OutcomeNegative indicates a negative outcome.
-	OutcomeNegative Outcome = iota
-	_
-	_
-	_
+	OutcomeNegative Outcome = "Negative"
 	// OutcomeNotAvailable indicates an unavailable outcome,
 	// typically because an API call did not return an answer.
-	OutcomeNotAvailable
-	_
-	_
-	_
+	OutcomeNotAvailable Outcome = "NotAvailable"
 	// OutcomeError indicates an errors while running.
 	// The results could not be determined.
-	OutcomeError
-	_
-	_
-	_
+	OutcomeError Outcome = "Error"
 	// OutcomePositive indicates a positive outcome.
-	OutcomePositive
-	_
-	_
-	_
+	OutcomePositive Outcome = "Positive"
 	// OutcomeNotSupported indicates a non-supported outcome.
-	OutcomeNotSupported
-	_
-	_
-	_
+	OutcomeNotSupported Outcome = "NotSupported"
 	// OutcomeNotApplicable indicates if a finding should not
 	// be considered in evaluation.
-	OutcomeNotApplicable
+	OutcomeNotApplicable Outcome = "NotApplicable"
 )
 
 // Finding represents a finding.

pkg/scorecard_test.go

@@ -235,6 +235,7 @@ func TestExperimentalRunProbes(t *testing.T) {
 				Findings: []finding.Finding{
 					{
 						Probe:   fuzzed.Probe,
+						Outcome: finding.OutcomeNegative,
 						Message: "no fuzzer integrations found",
 						Remediation: &probe.Remediation{
 							Effort: probe.RemediationEffortHigh,

pkg/testdata/probe1.json

@@ -16,12 +16,12 @@
             },
             "probe": "check for X",
             "message": "found X",
-            "outcome": 12
+            "outcome": "Positive"
         },
         {
             "probe": "check for Y",
             "message": "did not find Y",
-            "outcome": 0
+            "outcome": "Negative"
         }
     ]
 }

probes/sastToolRunsOnAllCommits/impl_test.go

@@ -36,7 +36,7 @@ func Test_Run(t *testing.T) {
 		expectedFindings []finding.Finding
 	}{
 		{
-			name: "sonar present",
+			name: "any unchecked commits leads to negative outcome",
 			err:  nil,
 			raw: &checker.RawResults{
 				SASTResults: checker.SASTData{
@@ -57,6 +57,7 @@ func Test_Run(t *testing.T) {
 				{
 					Probe:   Probe,
 					Message: "1 commits out of 2 are checked with a SAST tool",
+					Outcome: finding.OutcomeNegative,
 					Values: map[string]string{
 						AnalyzedPRsKey: "1",
 						TotalPRsKey:    "2",
@@ -65,7 +66,7 @@ func Test_Run(t *testing.T) {
 			},
 		},
 		{
-			name: "sonar present",
+			name: "all commits checked is positive outcome",
 			err:  nil,
 			raw: &checker.RawResults{
 				SASTResults: checker.SASTData{