From e8d37bfbe5c555c86a7f2b65b1d4c50774e94ad7 Mon Sep 17 00:00:00 2001 From: Dominik Richter Date: Thu, 18 Jul 2024 09:24:00 -0700 Subject: [PATCH] critical: fix banded scoring on variation within bands Variations within bands were inverted and led to incorrect results on the edges of those variations i.e. if all scores within a band were failing or only one was failing. The tests reflect have been added to test out this behavior of variation within bands and will be further expanded in the future. Signed-off-by: Dominik Richter --- policy/score_calculator.go | 29 +++++++- policy/score_calculator_test.go | 127 ++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 4 deletions(-) diff --git a/policy/score_calculator.go b/policy/score_calculator.go index 4223d2da..647cba39 100644 --- a/policy/score_calculator.go +++ b/policy/score_calculator.go @@ -17,6 +17,7 @@ type ScoreCalculator interface { Add(score *Score, impact *explorer.Impact) Calculate() *Score Init() + String() string } type averageScoreCalculator struct { @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -622,6 +639,10 @@ type decayedScoreCalculator struct { featureFlagFailErrors bool } +func (c *decayedScoreCalculator) String() string { + return "Decayed" +} + var gravity float64 = 10 func (c *decayedScoreCalculator) Init() { diff --git a/policy/score_calculator_test.go b/policy/score_calculator_test.go index bb5bc40a..1498679a 100644 --- a/policy/score_calculator_test.go +++ b/policy/score_calculator_test.go @@ -4,6 +4,7 @@ package policy import ( + "fmt" "strconv" "testing" @@ -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) { + 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() +}