mirror of
https://github.com/ossf/scorecard.git
synced 2024-09-11 08:55:27 +03:00
✨ enforce check scores are between the min and max (#3769)
* enforce check scores are between the min and max if the score is invalid, the Error field is set and the score is replaced with an inconclusive result score. Signed-off-by: Spencer Schrock <sschrock@google.com> * exclude inconclusive result score Callers who want the score should use the CreateInconclusiveResult function. The goal is partly to enforce a consistent coding style, and partly to limit proportions which score to -1 accidentally. Signed-off-by: Spencer Schrock <sschrock@google.com> --------- Signed-off-by: Spencer Schrock <sschrock@google.com>
This commit is contained in:
parent
b556d932a4
commit
0dcad3a960
@ -20,6 +20,7 @@ import (
|
||||
"fmt"
|
||||
"math"
|
||||
|
||||
sce "github.com/ossf/scorecard/v4/errors"
|
||||
"github.com/ossf/scorecard/v4/finding"
|
||||
"github.com/ossf/scorecard/v4/rule"
|
||||
)
|
||||
@ -175,8 +176,15 @@ func NormalizeReason(reason string, score int) string {
|
||||
|
||||
// CreateResultWithScore is used when
|
||||
// the check runs without runtime errors, and we want to assign a
|
||||
// specific score.
|
||||
// specific score. The score must be between [MinResultScore] and [MaxResultScore].
|
||||
// Callers who want [InconclusiveResultScore] must use [CreateInconclusiveResult] instead.
|
||||
//
|
||||
// Passing an invalid score results in a runtime error result as if created by [CreateRuntimeErrorResult].
|
||||
func CreateResultWithScore(name, reason string, score int) CheckResult {
|
||||
if score < MinResultScore || score > MaxResultScore {
|
||||
err := sce.CreateInternal(sce.ErrScorecardInternal, fmt.Sprintf("invalid score (%d), please report this", score))
|
||||
return CreateRuntimeErrorResult(name, err)
|
||||
}
|
||||
return CheckResult{
|
||||
Name: name,
|
||||
Version: 2,
|
||||
@ -193,15 +201,8 @@ func CreateResultWithScore(name, reason string, score int) CheckResult {
|
||||
// the number of tests that succeeded.
|
||||
func CreateProportionalScoreResult(name, reason string, b, t int) CheckResult {
|
||||
score := CreateProportionalScore(b, t)
|
||||
return CheckResult{
|
||||
Name: name,
|
||||
// Old structure.
|
||||
// New structure.
|
||||
Version: 2,
|
||||
Error: nil,
|
||||
Score: score,
|
||||
Reason: NormalizeReason(reason, score),
|
||||
}
|
||||
reason = NormalizeReason(reason, score)
|
||||
return CreateResultWithScore(name, reason, score)
|
||||
}
|
||||
|
||||
// CreateMaxScoreResult is used when
|
||||
|
@ -19,6 +19,9 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/google/go-cmp/cmp"
|
||||
"github.com/google/go-cmp/cmp/cmpopts"
|
||||
|
||||
sce "github.com/ossf/scorecard/v4/errors"
|
||||
)
|
||||
|
||||
func TestAggregateScores(t *testing.T) {
|
||||
@ -493,12 +496,58 @@ func TestCreateResultWithScore(t *testing.T) {
|
||||
Score: 1,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "inconclusive score is not valid",
|
||||
args: args{
|
||||
name: "name",
|
||||
reason: "reason",
|
||||
score: InconclusiveResultScore,
|
||||
},
|
||||
want: CheckResult{
|
||||
Name: "name",
|
||||
Reason: "internal error: invalid score (-1), please report this",
|
||||
Version: 2,
|
||||
Score: -1,
|
||||
Error: sce.ErrScorecardInternal,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "score too low",
|
||||
args: args{
|
||||
name: "name",
|
||||
reason: "reason",
|
||||
score: -3,
|
||||
},
|
||||
want: CheckResult{
|
||||
Name: "name",
|
||||
Reason: "internal error: invalid score (-3), please report this",
|
||||
Version: 2,
|
||||
Score: -1,
|
||||
Error: sce.ErrScorecardInternal,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "score too high",
|
||||
args: args{
|
||||
name: "name",
|
||||
reason: "reason",
|
||||
score: MaxResultScore + 2,
|
||||
},
|
||||
want: CheckResult{
|
||||
Name: "name",
|
||||
Reason: "internal error: invalid score (12), please report this",
|
||||
Version: 2,
|
||||
Score: -1,
|
||||
Error: sce.ErrScorecardInternal,
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
tt := tt
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
if got := CreateResultWithScore(tt.args.name, tt.args.reason, tt.args.score); !cmp.Equal(got, tt.want) {
|
||||
got := CreateResultWithScore(tt.args.name, tt.args.reason, tt.args.score)
|
||||
if !cmp.Equal(got, tt.want, cmpopts.EquateErrors()) {
|
||||
t.Errorf("CreateResultWithScore() = %v, want %v", got, cmp.Diff(got, tt.want))
|
||||
}
|
||||
})
|
||||
@ -548,12 +597,29 @@ func TestCreateProportionalScoreResult(t *testing.T) {
|
||||
Version: 2,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "negative proportion, score too low",
|
||||
args: args{
|
||||
name: "name",
|
||||
reason: "reason",
|
||||
b: -2,
|
||||
t: 1,
|
||||
},
|
||||
want: CheckResult{
|
||||
Name: "name",
|
||||
Reason: "internal error: invalid score (-20), please report this",
|
||||
Version: 2,
|
||||
Score: -1,
|
||||
Error: sce.ErrScorecardInternal,
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
tt := tt
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
if got := CreateProportionalScoreResult(tt.args.name, tt.args.reason, tt.args.b, tt.args.t); !cmp.Equal(got, tt.want) {
|
||||
got := CreateProportionalScoreResult(tt.args.name, tt.args.reason, tt.args.b, tt.args.t)
|
||||
if !cmp.Equal(got, tt.want, cmpopts.EquateErrors()) {
|
||||
t.Errorf("CreateProportionalScoreResult() = %v, want %v", got, cmp.Diff(got, tt.want))
|
||||
}
|
||||
})
|
||||
|
Loading…
Reference in New Issue
Block a user