Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Critical: Fix banded scoring within individual bands #1385

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Critical: Fix banded scoring within individual bands #1385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions policy/score_calculator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type ScoreCalculator interface {
Add(score *Score, impact *explorer.Impact)
Calculate() *Score
Init()
String() string
}

type averageScoreCalculator struct {
Expand All @@ -32,6 +33,10 @@ type averageScoreCalculator struct {
featureFlagFailErrors bool
}

func (c *averageScoreCalculator) String() string {
return "Average"
}

func (c *averageScoreCalculator) Init() {
c.value = 0
c.weight = 0
Expand Down Expand Up @@ -216,6 +221,10 @@ type weightedScoreCalculator struct {
featureFlagFailErrors bool
}

func (c *weightedScoreCalculator) String() string {
return "Weighted Average"
}

func (c *weightedScoreCalculator) Init() {
c.value = 0
c.weight = 0
Expand Down Expand Up @@ -327,6 +336,10 @@ type worstScoreCalculator struct {
featureFlagFailErrors bool
}

func (c *worstScoreCalculator) String() string {
return "Highest Impact"
}

func (c *worstScoreCalculator) Init() {
c.value = 100
c.weight = 0
Expand Down Expand Up @@ -450,6 +463,10 @@ type bandedScoreCalculator struct {
featureFlagFailErrors bool
}

func (c *bandedScoreCalculator) String() string {
return "Banded"
}

func (c *bandedScoreCalculator) Init() {
c.minscore = 100
c.value = 100
Expand Down Expand Up @@ -570,21 +587,21 @@ func (c *bandedScoreCalculator) Calculate() *Score {

pcrLow := float64(1)
if c.lowMax != 0 {
pcrLow = float64(c.low) / float64(c.lowMax)
pcrLow = float64(c.lowMax-c.low) / float64(c.lowMax)
}
fMid := float64(1)
if c.midMax != 0 {
fMid = float64(c.mid) / float64(c.midMax)
fMid = float64(c.midMax-c.mid) / float64(c.midMax)
}
pcrMid := (3 + pcrLow) / 4 * fMid
fHigh := float64(1)
if c.highMax != 0 {
fHigh = float64(c.high) / float64(c.highMax)
fHigh = float64(c.highMax-c.high) / float64(c.highMax)
}
pcrHigh := (1 + pcrMid) / 2 * fHigh
fCrit := float64(1)
if c.critMax != 0 {
fCrit = float64(c.crit) / float64(c.critMax)
fCrit = float64(c.critMax-c.crit) / float64(c.critMax)
}
pcrCrit := (1 + 4*pcrHigh) / 5 * fCrit

Expand Down Expand Up @@ -622,6 +639,10 @@ type decayedScoreCalculator struct {
featureFlagFailErrors bool
}

func (c *decayedScoreCalculator) String() string {
return "Decayed"
}

var gravity float64 = 10

func (c *decayedScoreCalculator) Init() {
Expand Down
133 changes: 130 additions & 3 deletions policy/score_calculator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package policy

import (
"fmt"
"strconv"
"testing"

Expand Down Expand Up @@ -222,7 +223,7 @@ func TestBandedScores(t *testing.T) {
{Value: &explorer.ImpactValue{Value: 100}},
{Action: explorer.Action_IGNORE},
},
out: &Score{Value: 22, ScoreCompletion: 100, DataCompletion: 66, Weight: 10, Type: ScoreType_Result},
out: &Score{Value: 25, ScoreCompletion: 100, DataCompletion: 66, Weight: 10, Type: ScoreType_Result},
},
{
in: []*Score{
Expand All @@ -239,7 +240,7 @@ func TestBandedScores(t *testing.T) {
// 10 high checks
{Value: &explorer.ImpactValue{Value: 80}},
},
out: &Score{Value: 1, ScoreCompletion: 100, DataCompletion: 66, Weight: 20, Type: ScoreType_Result},
out: &Score{Value: 45, ScoreCompletion: 100, DataCompletion: 66, Weight: 20, Type: ScoreType_Result},
},
{
in: []*Score{
Expand Down Expand Up @@ -273,7 +274,7 @@ func TestBandedScores(t *testing.T) {
// 10 high checks
{Value: &explorer.ImpactValue{Value: 80}},
},
out: &Score{Value: 5, ScoreCompletion: 100, DataCompletion: 66, Weight: 20, Type: ScoreType_Result},
out: &Score{Value: 9, ScoreCompletion: 100, DataCompletion: 66, Weight: 20, Type: ScoreType_Result},
},
})
}
Expand Down Expand Up @@ -404,3 +405,129 @@ func TestImpact(t *testing.T) {
require.EqualValues(t, 80, int(s.Value))
})
}

func addMultipleScores(impact uint32, failed int, passed int, calculator ScoreCalculator) {
for i := 0; i < failed; i++ {
calculator.Add(&Score{
Value: 100 - impact,
ScoreCompletion: 100,
DataCompletion: 100,
DataTotal: 1,
Weight: 1,
Type: ScoreType_Result,
}, &explorer.Impact{
Value: &explorer.ImpactValue{
Value: int32(impact),
},
})
}
for i := 0; i < passed; i++ {
calculator.Add(&Score{
Value: 100,
ScoreCompletion: 100,
DataCompletion: 100,
DataTotal: 1,
Weight: 1,
Type: ScoreType_Result,
}, &explorer.Impact{
Value: &explorer.ImpactValue{
Value: int32(impact),
},
})
}
}

func addTestResults(critFail, critPass, highFail, highPass, midFail, midPass, lowFail, lowPass int, calculator ScoreCalculator) {
addMultipleScores(100, critFail, critPass, calculator)
addMultipleScores(80, highFail, highPass, calculator)
addMultipleScores(55, midFail, midPass, calculator)
addMultipleScores(20, lowFail, lowPass, calculator)
}

func testEachScoreCalculator(critFail, critCnt, highFail, highCnt, midFail, midCnt, lowFail, lowCnt int, t *testing.T) []*Score {
require.LessOrEqual(t, critFail, critCnt)
require.LessOrEqual(t, highFail, highCnt)
require.LessOrEqual(t, midFail, midCnt)
require.LessOrEqual(t, lowFail, lowCnt)

fmt.Printf("Results: %d/%d CRIT %d/%d HIGH %d/%d MID %d/%d LOW %d/%d Total\n",
critFail, critCnt,
highFail, highCnt,
midFail, midCnt,
lowFail, lowCnt,
critFail+highFail+midFail+lowFail, critCnt+highCnt+midCnt+lowCnt,
)
calculators := []ScoreCalculator{
&averageScoreCalculator{},
&bandedScoreCalculator{},
&decayedScoreCalculator{},
&worstScoreCalculator{},
}

var results []*Score
for i := range calculators {
calculator := calculators[i]
calculator.Init()

addTestResults(critFail, critCnt-critFail, highFail, highCnt-highFail, midFail, midCnt-midFail, lowFail, lowCnt-lowFail, calculator)

score := calculator.Calculate()
results = append(results, score)
require.NotNil(t, score)
fmt.Printf(" %20s: %3d (%s)\n", calculator.String(), score.Value, score.Rating().FailureLabel())
}
fmt.Println("")

return results
}

func TestCalculatorBehavior_Banded(t *testing.T) {
var res []*Score
t.Run("critical band", func(t *testing.T) {
res = testEachScoreCalculator(1, 10, 0, 10, 0, 10, 0, 10, t)
assert.Equal(t, 45, int(res[1].Value))
res = testEachScoreCalculator(5, 10, 0, 10, 0, 10, 0, 10, t)
assert.Equal(t, 25, int(res[1].Value))
res = testEachScoreCalculator(10, 10, 0, 10, 0, 10, 0, 10, t)
assert.Equal(t, 0, int(res[1].Value))
})
t.Run("critical band with high band variation", func(t *testing.T) {
res = testEachScoreCalculator(1, 10, 1, 10, 0, 10, 0, 10, t)
assert.Equal(t, 41, int(res[1].Value))
res = testEachScoreCalculator(1, 10, 5, 10, 0, 10, 0, 10, t)
assert.Equal(t, 27, int(res[1].Value))
res = testEachScoreCalculator(1, 10, 10, 10, 0, 10, 0, 10, t)
assert.Equal(t, 9, int(res[1].Value))
})
t.Run("high band", func(t *testing.T) {
res = testEachScoreCalculator(0, 10, 1, 10, 0, 10, 0, 10, t)
assert.Equal(t, 55, int(res[1].Value))
res = testEachScoreCalculator(0, 10, 5, 10, 0, 10, 0, 10, t)
assert.Equal(t, 35, int(res[1].Value))
res = testEachScoreCalculator(0, 10, 10, 10, 0, 10, 0, 10, t)
assert.Equal(t, 10, int(res[1].Value))
})
}

// This is a toolchain that can be used to see what the scoring mechanism would produce under different scenarios
func TestScoreMechanisms(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been added to have a playground for experimenting with scoring mechanisms. Uncomment the last line to force the failure to be printed

testEachScoreCalculator(1, 2, 1, 2, 2, 3, 1, 3, t)

testEachScoreCalculator(5, 10, 10, 15, 2, 30, 1, 45, t)

testEachScoreCalculator(5, 10, 5, 15, 2, 30, 1, 45, t)

testEachScoreCalculator(2, 10, 4, 15, 2, 30, 1, 45, t)

testEachScoreCalculator(1, 10, 4, 15, 2, 30, 1, 45, t)

testEachScoreCalculator(10, 10, 4, 15, 2, 30, 1, 45, t)

testEachScoreCalculator(0, 1, 1, 9, 0, 23, 0, 0, t)

testEachScoreCalculator(0, 1, 3, 9, 0, 23, 0, 0, t)

testEachScoreCalculator(0, 2, 1, 2, 2, 3, 1, 3, t)

// t.FailNow()
}
Loading