Skip to content

Commit

Permalink
⚠️ Removing the pass field from result (#1853)
Browse files Browse the repository at this point in the history
    - Removing the pass field from result
    - #1393

Signed-off-by: naveensrinivasan <[email protected]>
  • Loading branch information
naveensrinivasan committed May 2, 2022
1 parent 44ad5f5 commit 1566b63
Show file tree
Hide file tree
Showing 24 changed files with 6 additions and 157 deletions.
21 changes: 3 additions & 18 deletions checker/check_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ type CheckResult struct {
Name string
Details []string
Confidence int
Pass bool

// UPGRADEv2: New structure. Omitting unchanged Name field
// for simplicity.
Expand Down Expand Up @@ -145,23 +144,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,
Expand All @@ -176,21 +170,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),
}
}

Expand All @@ -216,7 +203,6 @@ func CreateInconclusiveResult(name, reason string) CheckResult {
Name: name,
// Old structure.
Confidence: 0,
Pass: false,
// New structure.
Version: 2,
Score: InconclusiveResultScore,
Expand All @@ -230,7 +216,6 @@ func CreateRuntimeErrorResult(name string, e error) CheckResult {
Name: name,
// Old structure.
Confidence: 0,
Pass: false,
// New structure.
Version: 2,
Error: e,
Expand Down
5 changes: 0 additions & 5 deletions checks/binary_artifact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ func TestBinaryArtifacts(t *testing.T) {
err: nil,
expected: checker.CheckResult{
Score: 9,
Pass: true,
},
},
{
Expand All @@ -51,7 +50,6 @@ func TestBinaryArtifacts(t *testing.T) {
err: nil,
expected: checker.CheckResult{
Score: 10,
Pass: true,
},
},
}
Expand Down Expand Up @@ -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()
})
Expand Down
6 changes: 0 additions & 6 deletions checks/code_review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ func TestCodereview(t *testing.T) {
},
expected: checker.CheckResult{
Score: 10,
Pass: true,
},
},
{
Expand All @@ -112,7 +111,6 @@ func TestCodereview(t *testing.T) {
},
expected: checker.CheckResult{
Score: 10,
Pass: true,
},
},
{
Expand All @@ -136,7 +134,6 @@ func TestCodereview(t *testing.T) {
},
expected: checker.CheckResult{
Score: 10,
Pass: true,
},
},
{
Expand Down Expand Up @@ -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()
})
}
Expand Down
4 changes: 0 additions & 4 deletions checks/contributors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ func TestContributors(t *testing.T) {
},
expected: checker.CheckResult{
Score: 10,
Pass: true,
},
},
{
Expand Down Expand Up @@ -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()
})
}
Expand Down
3 changes: 0 additions & 3 deletions checks/maintained_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
}
Expand Down
4 changes: 2 additions & 2 deletions checks/sast.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 1 addition & 9 deletions checks/sast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -76,7 +76,6 @@ func TestSAST(t *testing.T) {
},
expected: checker.CheckResult{
Score: 10,
Pass: true,
},
},
{
Expand Down Expand Up @@ -116,7 +115,6 @@ func TestSAST(t *testing.T) {
},
expected: checker.CheckResult{
Score: 7,
Pass: false,
},
},
{
Expand Down Expand Up @@ -153,7 +151,6 @@ func TestSAST(t *testing.T) {
},
expected: checker.CheckResult{
Score: 0,
Pass: false,
},
},
{
Expand All @@ -175,7 +172,6 @@ func TestSAST(t *testing.T) {
},
expected: checker.CheckResult{
Score: 0,
Pass: false,
},
},
{
Expand All @@ -198,7 +194,6 @@ func TestSAST(t *testing.T) {
},
expected: checker.CheckResult{
Score: 0,
Pass: false,
},
},
}
Expand Down Expand Up @@ -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()
})
}
Expand Down
15 changes: 0 additions & 15 deletions checks/signed_releases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ func TestSignedRelease(t *testing.T) {
{
name: "NoReleases",
expected: checker.CheckResult{
Pass: false,
Score: -1,
},
},
Expand All @@ -54,7 +53,6 @@ func TestSignedRelease(t *testing.T) {
},
},
expected: checker.CheckResult{
Pass: false,
Score: -1,
},
},
Expand All @@ -74,7 +72,6 @@ func TestSignedRelease(t *testing.T) {
},
},
expected: checker.CheckResult{
Pass: false,
Score: 0,
},
},
Expand All @@ -94,7 +91,6 @@ func TestSignedRelease(t *testing.T) {
},
},
expected: checker.CheckResult{
Pass: true,
Score: 10,
},
},
Expand All @@ -114,7 +110,6 @@ func TestSignedRelease(t *testing.T) {
},
},
expected: checker.CheckResult{
Pass: true,
Score: 10,
},
},
Expand All @@ -134,7 +129,6 @@ func TestSignedRelease(t *testing.T) {
},
},
expected: checker.CheckResult{
Pass: true,
Score: 10,
},
},
Expand All @@ -154,7 +148,6 @@ func TestSignedRelease(t *testing.T) {
},
},
expected: checker.CheckResult{
Pass: true,
Score: 10,
},
},
Expand All @@ -178,7 +171,6 @@ func TestSignedRelease(t *testing.T) {
},
},
expected: checker.CheckResult{
Pass: true,
Score: 10,
},
},
Expand Down Expand Up @@ -217,7 +209,6 @@ func TestSignedRelease(t *testing.T) {
},
},
expected: checker.CheckResult{
Pass: true,
Score: 10,
},
},
Expand Down Expand Up @@ -252,7 +243,6 @@ func TestSignedRelease(t *testing.T) {
},
},
expected: checker.CheckResult{
Pass: false,
Score: 5,
},
},
Expand Down Expand Up @@ -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"),
},
Expand Down Expand Up @@ -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()
})
}
Expand Down
7 changes: 0 additions & 7 deletions checks/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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()
})
}
Expand Down
1 change: 0 additions & 1 deletion cron/format/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ func AsJSON(r *pkg.ScorecardResult, showDetails bool, logLevel log.Level, writer
for _, checkResult := range r.Checks {
tmpResult := jsonCheckResult{
Name: checkResult.Name,
Pass: checkResult.Pass,
Confidence: checkResult.Confidence,
}
if showDetails {
Expand Down
Loading

0 comments on commit 1566b63

Please sign in to comment.