⚠️ Removing the pass field from result (#1853)

- Removing the pass field from result - #1393 Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>
ossf · May 2, 2022 · b8fe364 · b8fe364
1 parent 44ad5f5
commit b8fe364
Show file tree

Hide file tree

Showing 24 changed files with 6 additions and 159 deletions.
diff --git a/checker/check_result.go b/checker/check_result.go
@@ -37,8 +37,6 @@ const (
 	// MinResultConfidence signifies no confidence in the check result.
 	// TODO(#1393): remove after deprecation.
 	MinResultConfidence = 0
-	// TODO(#1393): remove after deprecation.
-	migrationThresholdPassValue = 8
 
 	// MaxResultScore is the best score that can be given by a check.
 	MaxResultScore = 10
@@ -82,7 +80,6 @@ type CheckResult struct {
 	Name       string
 	Details    []string
 	Confidence int
-	Pass       bool
 
 	// UPGRADEv2: New structure. Omitting unchanged Name field
 	// for simplicity.
@@ -145,23 +142,18 @@ func AggregateScoresWithWeight(scores map[int]int) int {
 }
 
 // NormalizeReason - placeholder function if we want to update range of scores.
-func NormalizeReason(reason string, score int) string {
-	return fmt.Sprintf("%v -- score normalized to %d", reason, score)
+func NormalizeReason(reason string) string {
+	return fmt.Sprintf("%v -- score normalized", reason)
 }
 
 // CreateResultWithScore is used when
 // the check runs without runtime errors and we want to assign a
 // specific score.
 func CreateResultWithScore(name, reason string, score int) CheckResult {
-	pass := true
-	if score < migrationThresholdPassValue {
-		pass = false
-	}
 	return CheckResult{
 		Name: name,
 		// Old structure.
 		Confidence: MaxResultScore,
-		Pass:       pass,
 		// New structure.
 		Version: 2,
 		Error:   nil,
@@ -176,21 +168,14 @@ func CreateResultWithScore(name, reason string, score int) CheckResult {
 // multiple tests and we want to assign a score proportional
 // the the number of tests that succeeded.
 func CreateProportionalScoreResult(name, reason string, b, t int) CheckResult {
-	pass := true
-	score := CreateProportionalScore(b, t)
-	if score < migrationThresholdPassValue {
-		pass = false
-	}
 	return CheckResult{
 		Name: name,
 		// Old structure.
 		Confidence: MaxResultConfidence,
-		Pass:       pass,
 		// New structure.
 		Version: 2,
 		Error:   nil,
-		Score:   score,
-		Reason:  NormalizeReason(reason, score),
+		Reason:  NormalizeReason(reason),
 	}
 }
 
@@ -216,7 +201,6 @@ func CreateInconclusiveResult(name, reason string) CheckResult {
 		Name: name,
 		// Old structure.
 		Confidence: 0,
-		Pass:       false,
 		// New structure.
 		Version: 2,
 		Score:   InconclusiveResultScore,
@@ -230,7 +214,6 @@ func CreateRuntimeErrorResult(name string, e error) CheckResult {
 		Name: name,
 		// Old structure.
 		Confidence: 0,
-		Pass:       false,
 		// New structure.
 		Version: 2,
 		Error:   e,

diff --git a/checks/binary_artifact_test.go b/checks/binary_artifact_test.go
@@ -42,7 +42,6 @@ func TestBinaryArtifacts(t *testing.T) {
 			err:         nil,
 			expected: checker.CheckResult{
 				Score: 9,
-				Pass:  true,
 			},
 		},
 		{
@@ -51,7 +50,6 @@ func TestBinaryArtifacts(t *testing.T) {
 			err:         nil,
 			expected: checker.CheckResult{
 				Score: 10,
-				Pass:  true,
 			},
 		},
 	}
@@ -89,9 +87,6 @@ func TestBinaryArtifacts(t *testing.T) {
 			if result.Score != tt.expected.Score {
 				t.Errorf("BinaryArtifacts: %v, expected %v for tests %v", result.Score, tt.expected.Score, tt.name)
 			}
-			if result.Pass != tt.expected.Pass {
-				t.Errorf("BinaryArtifacts: %v, expected %v for tests %v", result.Pass, tt.expected.Pass, tt.name)
-			}
 
 			ctrl.Finish()
 		})

diff --git a/checks/code_review_test.go b/checks/code_review_test.go
@@ -88,7 +88,6 @@ func TestCodereview(t *testing.T) {
 			},
 			expected: checker.CheckResult{
 				Score: 10,
-				Pass:  true,
 			},
 		},
 		{
@@ -112,7 +111,6 @@ func TestCodereview(t *testing.T) {
 			},
 			expected: checker.CheckResult{
 				Score: 10,
-				Pass:  true,
 			},
 		},
 		{
@@ -136,7 +134,6 @@ func TestCodereview(t *testing.T) {
 			},
 			expected: checker.CheckResult{
 				Score: 10,
-				Pass:  true,
 			},
 		},
 		{
@@ -217,9 +214,6 @@ func TestCodereview(t *testing.T) {
 			if res.Score != tt.expected.Score {
 				t.Errorf("Expected score %d, got %d for %v", tt.expected.Score, res.Score, tt.name)
 			}
-			if res.Pass != tt.expected.Pass {
-				t.Errorf("Expected pass %t, got %t for %v", tt.expected.Pass, res.Pass, tt.name)
-			}
 			ctrl.Finish()
 		})
 	}

diff --git a/checks/contributors_test.go b/checks/contributors_test.go
@@ -134,7 +134,6 @@ func TestContributors(t *testing.T) {
 			},
 			expected: checker.CheckResult{
 				Score: 10,
-				Pass:  true,
 			},
 		},
 		{
@@ -185,9 +184,6 @@ func TestContributors(t *testing.T) {
 			if res.Score != tt.expected.Score {
 				t.Errorf("Expected score %d, got %d for %v", tt.expected.Score, res.Score, tt.name)
 			}
-			if res.Pass != tt.expected.Pass {
-				t.Errorf("Expected pass %t, got %t for %v", tt.expected.Pass, res.Pass, tt.name)
-			}
 			ctrl.Finish()
 		})
 	}

diff --git a/checks/maintained_test.go b/checks/maintained_test.go
@@ -355,9 +355,6 @@ func Test_Maintained(t *testing.T) {
 			if res.Score != tt.expected.Score {
 				t.Errorf("Expected score %d, got %d for %v", tt.expected.Score, res.Score, tt.name)
 			}
-			if res.Pass != tt.expected.Pass {
-				t.Errorf("Expected pass %t, got %t for %v", tt.expected.Pass, res.Pass, tt.name)
-			}
 			ctrl.Finish()
 		})
 	}

diff --git a/checks/sast.go b/checks/sast.go
@@ -71,7 +71,7 @@ func SAST(c *checker.CheckRequest) checker.CheckResult {
 			return checker.CreateMaxScoreResult(CheckSAST, "SAST tool is run on all commits")
 		case codeQlScore == checker.MinResultScore:
 			return checker.CreateResultWithScore(CheckSAST,
-				checker.NormalizeReason("SAST tool is not run on all commits", sastScore), sastScore)
+				checker.NormalizeReason("SAST tool is not run on all commits"), sastScore)
 
 		// codeQl is enabled and sast has 0+ (but not all) PRs checks.
 		case codeQlScore == checker.MaxResultScore:
@@ -99,7 +99,7 @@ func SAST(c *checker.CheckRequest) checker.CheckResult {
 		}
 
 		return checker.CreateResultWithScore(CheckSAST,
-			checker.NormalizeReason("SAST tool is not run on all commits", sastScore), sastScore)
+			checker.NormalizeReason("SAST tool is not run on all commits"), sastScore)
 	}
 
 	// Should never happen.

diff --git a/checks/sast_test.go b/checks/sast_test.go
@@ -53,7 +53,7 @@ func TestSAST(t *testing.T) {
 			commits:      []clients.Commit{},
 			searchresult: clients.SearchResponse{},
 			checkRuns:    []clients.CheckRun{},
-			expected:     checker.CheckResult{Score: -1, Pass: false},
+			expected:     checker.CheckResult{Score: -1},
 		},
 		{
 			name: "Successful SAST checker should return success status",
@@ -76,7 +76,6 @@ func TestSAST(t *testing.T) {
 			},
 			expected: checker.CheckResult{
 				Score: 10,
-				Pass:  true,
 			},
 		},
 		{
@@ -116,7 +115,6 @@ func TestSAST(t *testing.T) {
 			},
 			expected: checker.CheckResult{
 				Score: 7,
-				Pass:  false,
 			},
 		},
 		{
@@ -153,7 +151,6 @@ func TestSAST(t *testing.T) {
 			},
 			expected: checker.CheckResult{
 				Score: 0,
-				Pass:  false,
 			},
 		},
 		{
@@ -175,7 +172,6 @@ func TestSAST(t *testing.T) {
 			},
 			expected: checker.CheckResult{
 				Score: 0,
-				Pass:  false,
 			},
 		},
 		{
@@ -198,7 +194,6 @@ func TestSAST(t *testing.T) {
 			},
 			expected: checker.CheckResult{
 				Score: 0,
-				Pass:  false,
 			},
 		},
 	}
@@ -232,9 +227,6 @@ func TestSAST(t *testing.T) {
 			if res.Score != tt.expected.Score {
 				t.Errorf("Expected score %d, got %d for %v", tt.expected.Score, res.Score, tt.name)
 			}
-			if res.Pass != tt.expected.Pass {
-				t.Errorf("Expected pass %t, got %t for %v", tt.expected.Pass, res.Pass, tt.name)
-			}
 			ctrl.Finish()
 		})
 	}

diff --git a/checks/signed_releases_test.go b/checks/signed_releases_test.go
@@ -39,7 +39,6 @@ func TestSignedRelease(t *testing.T) {
 		{
 			name: "NoReleases",
 			expected: checker.CheckResult{
-				Pass:  false,
 				Score: -1,
 			},
 		},
@@ -54,7 +53,6 @@ func TestSignedRelease(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Pass:  false,
 				Score: -1,
 			},
 		},
@@ -74,7 +72,6 @@ func TestSignedRelease(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Pass:  false,
 				Score: 0,
 			},
 		},
@@ -94,7 +91,6 @@ func TestSignedRelease(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Pass:  true,
 				Score: 10,
 			},
 		},
@@ -114,7 +110,6 @@ func TestSignedRelease(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Pass:  true,
 				Score: 10,
 			},
 		},
@@ -134,7 +129,6 @@ func TestSignedRelease(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Pass:  true,
 				Score: 10,
 			},
 		},
@@ -154,7 +148,6 @@ func TestSignedRelease(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Pass:  true,
 				Score: 10,
 			},
 		},
@@ -178,7 +171,6 @@ func TestSignedRelease(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Pass:  true,
 				Score: 10,
 			},
 		},
@@ -217,7 +209,6 @@ func TestSignedRelease(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Pass:  true,
 				Score: 10,
 			},
 		},
@@ -252,7 +243,6 @@ func TestSignedRelease(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Pass:  false,
 				Score: 5,
 			},
 		},
@@ -351,15 +341,13 @@ func TestSignedRelease(t *testing.T) {
 				},
 			},
 			expected: checker.CheckResult{
-				Pass:  true,
 				Score: 10,
 			},
 		},
 		{
 			name: "Error getting releases",
 			err:  errors.New("Error getting releases"),
 			expected: checker.CheckResult{
-				Pass:  false,
 				Score: -1,
 				Error: errors.New("Error getting releases"),
 			},
@@ -400,9 +388,6 @@ func TestSignedRelease(t *testing.T) {
 			if res.Score != tt.expected.Score {
 				t.Errorf("Expected score %d, got %d for %v", tt.expected.Score, res.Score, tt.name)
 			}
-			if res.Pass != tt.expected.Pass {
-				t.Errorf("Expected pass %t, got %t for %v", tt.expected.Pass, res.Pass, tt.name)
-			}
 			ctrl.Finish()
 		})
 	}

diff --git a/checks/webhook_test.go b/checks/webhook_test.go
@@ -40,7 +40,6 @@ func TestWebhooks(t *testing.T) {
 			name: "No Webhooks",
 			uri:  "github.com/owner/repo",
 			expected: checker.CheckResult{
-				Pass:  true,
 				Score: 10,
 			},
 			err:      nil,
@@ -50,7 +49,6 @@ func TestWebhooks(t *testing.T) {
 			name: "With Webhooks and secret set",
 			uri:  "github.com/owner/repo",
 			expected: checker.CheckResult{
-				Pass:  true,
 				Score: 10,
 			},
 			err: nil,
@@ -65,7 +63,6 @@ func TestWebhooks(t *testing.T) {
 			name: "With Webhooks and no secret set",
 			uri:  "github.com/owner/repo",
 			expected: checker.CheckResult{
-				Pass:  false,
 				Score: 0,
 			},
 			err: nil,
@@ -80,7 +77,6 @@ func TestWebhooks(t *testing.T) {
 			name: "With 2 Webhooks with and whitout secrets configured",
 			uri:  "github.com/owner/repo",
 			expected: checker.CheckResult{
-				Pass:  false,
 				Score: 5,
 			},
 			err: nil,
@@ -133,9 +129,6 @@ func TestWebhooks(t *testing.T) {
 			if res.Score != tt.expected.Score {
 				t.Errorf("Expected score %d, got %d for %v", tt.expected.Score, res.Score, tt.name)
 			}
-			if res.Pass != tt.expected.Pass {
-				t.Errorf("Expected pass %t, got %t for %v", tt.expected.Pass, res.Pass, tt.name)
-			}
 			ctrl.Finish()
 		})
 	}