diff --git a/pkg/api/componentreadiness/component_report.go b/pkg/api/componentreadiness/component_report.go
index 1af2643358..11a0deda70 100644
--- a/pkg/api/componentreadiness/component_report.go
+++ b/pkg/api/componentreadiness/component_report.go
@@ -14,7 +14,7 @@ import (
"cloud.google.com/go/bigquery"
"github.com/apache/thrift/lib/go/thrift"
fischer "github.com/glycerine/golang-fisher-exact"
- "github.com/openshift/sippy/pkg/db/models"
+ "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/regressiontracker"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
"google.golang.org/api/iterator"
@@ -40,10 +40,6 @@ import (
const (
triagedIncidentsTableID = "triaged_incidents"
- // openRegressionConfidenceAdjustment is subtracted from the requested confidence for regressed tests that have
- // an open regression.
- openRegressionConfidenceAdjustment = 5
-
explanationNoRegression = "No significant regressions found"
)
@@ -149,7 +145,6 @@ type ComponentReportGenerator struct {
dbc *db.DB
triagedIssues *resolvedissues.TriagedIncidentsForRelease
ReqOptions crtype.RequestOptions
- openRegressions []*models.TestRegression
variantJunitTableOverrides []configv1.VariantJunitTableOverride
middlewares []middleware.Middleware
}
@@ -248,6 +243,11 @@ func (c *ComponentReportGenerator) initializeMiddleware() {
if c.ReqOptions.AdvancedOption.IncludeMultiReleaseAnalysis || c.ReqOptions.BaseOverrideRelease.Release != c.ReqOptions.BaseRelease.Release {
c.middlewares = append(c.middlewares, releasefallback.NewReleaseFallbackMiddleware(c.client, c.ReqOptions))
}
+ if c.dbc != nil {
+ c.middlewares = append(c.middlewares, regressiontracker.NewRegressionTrackerMiddleware(c.dbc, c.ReqOptions))
+ } else {
+ log.Warnf("no db connection provided, skipping regressiontracker middleware")
+ }
}
// GenerateReport is the main entry point for generation of a component readiness report.
@@ -261,30 +261,7 @@ func (c *ComponentReportGenerator) GenerateReport(ctx context.Context) (crtype.C
return crtype.ComponentReport{}, errs
}
- // Allow all middlware a chance to transform the base/sample TestStatuses before we analyze:
var err error
- for _, mw := range c.middlewares {
- err = mw.Transform(&componentReportTestStatus)
- if err != nil {
- return crtype.ComponentReport{}, []error{err}
- }
- }
-
- // Load current regression data from bigquery, used to enhance the response with information such as how long
- // this regression has been appearing in a tracked view.
- // We only execute this if we were given a postgres database connection, it is still possible to run
- // component readiness without postgresql, you just won't have regression tracking.
- if c.dbc != nil {
- bqs := NewPostgresRegressionStore(c.dbc)
- c.openRegressions, err = bqs.ListCurrentRegressionsForRelease(c.ReqOptions.SampleRelease.Release)
- if err != nil {
- log.WithError(err).Error("error listing current regressions")
- errs = append(errs, err)
- return crtype.ComponentReport{}, errs
- }
- } else {
- log.Warnf("no postgres connection for ComponentReportGenerator, regression tracking data will not be included")
- }
// generateComponentTestReport modifies SampleStatus removing matches from BaseStatus
// resulting in erroneous sample results count
@@ -647,8 +624,7 @@ type cellStatus struct {
func getNewCellStatus(testID crtype.ReportTestIdentification,
testStats crtype.ReportTestStats,
existingCellStatus *cellStatus,
- triagedIncidents []crtype.TriagedIncident,
- openRegressions []*models.TestRegression) cellStatus {
+ triagedIncidents []crtype.TriagedIncident) cellStatus {
var newCellStatus cellStatus
if existingCellStatus != nil {
if (testStats.ReportStatus < crtype.NotSignificant && testStats.ReportStatus < existingCellStatus.status) ||
@@ -670,14 +646,6 @@ func getNewCellStatus(testID crtype.ReportTestIdentification,
ReportTestIdentification: testID,
ReportTestStats: testStats,
}
- if len(openRegressions) > 0 {
- view := openRegressions[0].View // grab view from first regression, they were queried only for sample release
- or := FindOpenRegression(view, rt.TestID, rt.Variants, openRegressions)
- if or != nil {
- rt.Opened = &or.Opened
- rt.RegressionID = int(or.ID)
- }
- }
newCellStatus.regressedTests = append(newCellStatus.regressedTests, rt)
} else if testStats.ReportStatus < crtype.MissingSample {
ti := crtype.TriageIncidentSummary{
@@ -686,15 +654,6 @@ func getNewCellStatus(testID crtype.ReportTestIdentification,
ReportTestIdentification: testID,
ReportTestStats: testStats,
}}
- if len(openRegressions) > 0 {
- view := openRegressions[0].View // grab view from first regression, they were queried only for sample release
- or := FindOpenRegression(view, ti.ReportTestSummary.TestID,
- ti.ReportTestSummary.Variants, openRegressions)
- if or != nil {
- ti.ReportTestSummary.Opened = &or.Opened
- ti.ReportTestSummary.RegressionID = int(or.ID)
- }
- }
newCellStatus.triagedIncidents = append(newCellStatus.triagedIncidents, ti)
}
return newCellStatus
@@ -707,8 +666,7 @@ func updateCellStatus(rowIdentifications []crtype.RowIdentification,
status map[crtype.RowIdentification]map[crtype.ColumnID]cellStatus,
allRows map[crtype.RowIdentification]struct{},
allColumns map[crtype.ColumnID]struct{},
- triagedIncidents []crtype.TriagedIncident,
- openRegressions []*models.TestRegression) {
+ triagedIncidents []crtype.TriagedIncident) {
for _, columnIdentification := range columnIdentifications {
if _, ok := allColumns[columnIdentification]; !ok {
allColumns[columnIdentification] = struct{}{}
@@ -728,16 +686,16 @@ func updateCellStatus(rowIdentifications []crtype.RowIdentification,
if !ok {
row = map[crtype.ColumnID]cellStatus{}
for _, columnIdentification := range columnIdentifications {
- row[columnIdentification] = getNewCellStatus(testID, testStats, nil, triagedIncidents, openRegressions)
+ row[columnIdentification] = getNewCellStatus(testID, testStats, nil, triagedIncidents)
status[rowIdentification] = row
}
} else {
for _, columnIdentification := range columnIdentifications {
existing, ok := row[columnIdentification]
if !ok {
- row[columnIdentification] = getNewCellStatus(testID, testStats, nil, triagedIncidents, openRegressions)
+ row[columnIdentification] = getNewCellStatus(testID, testStats, nil, triagedIncidents)
} else {
- row[columnIdentification] = getNewCellStatus(testID, testStats, &existing, triagedIncidents, openRegressions)
+ row[columnIdentification] = getNewCellStatus(testID, testStats, &existing, triagedIncidents)
}
}
}
@@ -1056,31 +1014,45 @@ func (c *ComponentReportGenerator) triagedIncidentsFor(ctx context.Context,
return impactedRuns, activeProductRegression, triagedIncidents
}
-// getRequiredConfidence returns the required certainty of a regression before we include it in the report as a
-// regressed test. This is to introduce some hysteresis into the process so once a regression creeps over the 95%
-// confidence we typically use, dropping to 94.9% should not make the cell immediately green.
-//
-// Instead, once you cross the confidence threshold and a regression begins tracking in the openRegressions list,
-// we'll require less confidence for that test until the regression is closed. (-5%) Once the certainty drops below that
-// modified confidence, the regression will be closed and the -5% adjuster is gone.
-//
-// ie. if the request was for 95% confidence, but we see that a test has an open regression (meaning at some point recently
-// we were over 95% certain of a regression), we're going to only require 90% certainty to mark that test red.
-func (c *ComponentReportGenerator) getRequiredConfidence(testID string, variants map[string]string) int {
- if len(c.openRegressions) > 0 {
- view := c.openRegressions[0].View // grab view from first regression, they were queried only for sample release
- or := FindOpenRegression(view, testID, variants, c.openRegressions)
- if or != nil {
- log.Debugf("adjusting required regression confidence from %d to %d because %s (%v) has an open regression since %s",
- c.ReqOptions.AdvancedOption.Confidence,
- c.ReqOptions.AdvancedOption.Confidence-openRegressionConfidenceAdjustment,
- testID,
- variants,
- or.Opened)
- return c.ReqOptions.AdvancedOption.Confidence - openRegressionConfidenceAdjustment
+func initTestAnalysisStruct(
+ testStats *crtype.ReportTestStats,
+ reqOptions crtype.RequestOptions,
+ sampleStats crtype.TestStatus,
+ baseStats *crtype.TestStatus) {
+
+ // Default to required confidence from request, middleware may adjust later.
+ testStats.RequiredConfidence = reqOptions.AdvancedOption.Confidence
+
+ successFailCount := sampleStats.TotalCount - sampleStats.FlakeCount - sampleStats.SuccessCount
+ testStats.SampleStats = crtype.TestDetailsReleaseStats{
+ Release: reqOptions.SampleRelease.Release,
+ Start: &reqOptions.SampleRelease.Start,
+ End: &reqOptions.SampleRelease.End,
+ TestDetailsTestStats: crtype.TestDetailsTestStats{
+ SuccessRate: utils.CalculatePassRate(sampleStats.SuccessCount, successFailCount, sampleStats.FlakeCount, reqOptions.AdvancedOption.FlakeAsFailure),
+ SuccessCount: sampleStats.SuccessCount,
+ FlakeCount: sampleStats.FlakeCount,
+ FailureCount: successFailCount,
+ },
+ }
+ if baseStats != nil {
+ baseRelease := reqOptions.BaseRelease.Release
+ baseStart := reqOptions.BaseRelease.Start
+ baseEnd := reqOptions.BaseRelease.End
+
+ failCount := baseStats.TotalCount - baseStats.FlakeCount - baseStats.SuccessCount
+ testStats.BaseStats = &crtype.TestDetailsReleaseStats{
+ Release: baseRelease,
+ Start: &baseStart,
+ End: &baseEnd,
+ TestDetailsTestStats: crtype.TestDetailsTestStats{
+ SuccessRate: utils.CalculatePassRate(baseStats.SuccessCount, failCount, baseStats.FlakeCount, reqOptions.AdvancedOption.FlakeAsFailure),
+ SuccessCount: baseStats.SuccessCount,
+ FlakeCount: baseStats.FlakeCount,
+ FailureCount: failCount,
+ },
}
}
- return c.ReqOptions.AdvancedOption.Confidence
}
// TODO: break this function down and remove this nolint
@@ -1121,21 +1093,25 @@ func (c *ComponentReportGenerator) generateComponentTestReport(ctx context.Conte
if !ok {
testStats.ReportStatus = crtype.MissingSample
} else {
- // requiredConfidence is lowered for on-going regressions to prevent cells from flapping:
- requiredConfidence := c.getRequiredConfidence(testKey.TestID, testKey.Variants)
resolvedIssueCompensation, activeProductRegression, triagedIncidents = c.triagedIncidentsFor(ctx, testKey) // triaged job run failures to ignore
- // Check if the TestStatus is decorated with info indicating its release was overridden, and use that data if so
- matchedBaseRelease := c.ReqOptions.BaseRelease.Release
- var baseStart, baseEnd *time.Time
- if baseStats.Release != nil {
- matchedBaseRelease = baseStats.Release.Release
- baseStart = baseStats.Release.Start
- baseEnd = baseStats.Release.End
+ // Initialize the test analysis before we start passing it around to the middleware
+ initTestAnalysisStruct(&testStats, c.ReqOptions, sampleStats, &baseStats)
+
+ // Give middleware their chance to adjust parameters prior to analysis
+ for _, mw := range c.middlewares {
+ err = mw.PreAnalysis(testKey, &testStats)
+ if err != nil {
+ return crtype.ComponentReport{}, err
+ }
}
- testStats = c.assessComponentStatus(requiredConfidence, sampleStats.TotalCount, sampleStats.SuccessCount,
- sampleStats.FlakeCount, baseStats.TotalCount, baseStats.SuccessCount,
- baseStats.FlakeCount, nil, activeProductRegression, resolvedIssueCompensation, matchedBaseRelease, baseStart, baseEnd)
+
+ c.assessComponentStatus(
+ &testStats,
+ nil,
+ activeProductRegression,
+ resolvedIssueCompensation,
+ )
if !sampleStats.LastFailure.IsZero() {
testStats.LastFailure = &sampleStats.LastFailure
@@ -1167,7 +1143,8 @@ func (c *ComponentReportGenerator) generateComponentTestReport(ctx context.Conte
if err != nil {
return crtype.ComponentReport{}, err
}
- updateCellStatus(rowIdentifications, columnIdentifications, testKey, testStats, aggregatedStatus, allRows, allColumns, triagedIncidents, c.openRegressions)
+ updateCellStatus(rowIdentifications, columnIdentifications, testKey, testStats, aggregatedStatus,
+ allRows, allColumns, triagedIncidents)
}
// Anything we saw in the basis was removed above, all that remains are tests with no basis, typically new
@@ -1185,10 +1162,22 @@ func (c *ComponentReportGenerator) generateComponentTestReport(ctx context.Conte
var activeProductRegression bool
resolvedIssueCompensation, activeProductRegression, triagedIncidents = c.triagedIncidentsFor(ctx, testID)
- requiredConfidence := 0 // irrelevant for pass rate comparison
- testStats = c.assessComponentStatus(requiredConfidence, sampleStats.TotalCount, sampleStats.SuccessCount,
- sampleStats.FlakeCount, 0, 0, 0, // pass 0s for base stats
- nil, activeProductRegression, resolvedIssueCompensation, "", nil, nil)
+ // Initialize the test analysis before we start passing it around to the middleware and eventual assess:
+ initTestAnalysisStruct(&testStats, c.ReqOptions, sampleStats, nil)
+
+ // Give middleware their chance to adjust parameters prior to analysis
+ for _, mw := range c.middlewares {
+ err = mw.PreAnalysis(testID, &testStats)
+ if err != nil {
+ return crtype.ComponentReport{}, err
+ }
+ }
+
+ c.assessComponentStatus(&testStats,
+ nil,
+ activeProductRegression,
+ resolvedIssueCompensation,
+ )
if testStats.IsTriaged() {
// we are within the triage range
@@ -1218,7 +1207,8 @@ func (c *ComponentReportGenerator) generateComponentTestReport(ctx context.Conte
if err != nil {
return crtype.ComponentReport{}, err
}
- updateCellStatus(rowIdentifications, columnIdentification, testID, testStats, aggregatedStatus, allRows, allColumns, nil, c.openRegressions)
+ updateCellStatus(rowIdentifications, columnIdentification, testID, testStats, aggregatedStatus,
+ allRows, allColumns, nil)
}
// Sort the row identifications
@@ -1322,7 +1312,7 @@ func (c *ComponentReportGenerator) getTestStatusPassRate(testStatus crtype.TestS
}
func (c *ComponentReportGenerator) getPassRate(success, failure, flake int) float64 {
- return utils.CalculatePassRate(c.ReqOptions, success, failure, flake)
+ return utils.CalculatePassRate(success, failure, flake, c.ReqOptions.AdvancedOption.FlakeAsFailure)
}
func getRegressionStatus(basisPassPercentage, samplePassPercentage float64, isTriage bool) crtype.Status {
@@ -1363,31 +1353,24 @@ func (c *ComponentReportGenerator) getEffectivePityFactor(basisPassPercentage fl
// (fishers, pass rate, bayes (future)) and the middlewares (fallback, intentional regressions,
// cross variant compare, rarely run jobs, etc.)
func (c *ComponentReportGenerator) assessComponentStatus(
- requiredConfidence,
- sampleTotal,
- sampleSuccess,
- sampleFlake,
- baseTotal,
- baseSuccess,
- baseFlake int,
+ testStats *crtype.ReportTestStats,
approvedRegression *regressionallowances.IntentionalRegression,
activeProductRegression bool,
numberOfIgnoredSampleJobRuns int, // count for triaged failures we can safely omit and ignore
- baseRelease string,
- baseStart,
- baseEnd *time.Time) crtype.ReportTestStats {
-
- // if we don't have a valid set of start and end dates we default to the baseRelease values
- if baseStart == nil || baseEnd == nil {
- baseStart = &c.ReqOptions.BaseRelease.Start
- baseEnd = &c.ReqOptions.BaseRelease.End
+) {
+ // Catch unset required confidence, typically unit tests
+ if testStats.RequiredConfidence == 0 {
+ testStats.RequiredConfidence = c.ReqOptions.AdvancedOption.Confidence
}
+
+ // TODO: move to triage middleware Analyze eventually
// preserve the initial sampleTotal, so we can check
// to see if numberOfIgnoredSampleJobRuns impacts the status
+ sampleTotal := testStats.SampleStats.SuccessCount + testStats.SampleStats.FailureCount + testStats.SampleStats.FlakeCount
initialSampleTotal := sampleTotal
adjustedSampleTotal := sampleTotal - numberOfIgnoredSampleJobRuns
- if adjustedSampleTotal < sampleSuccess {
- log.Warnf("adjustedSampleTotal is too small: sampleTotal=%d, numberOfIgnoredSampleJobRuns=%d, sampleSuccess=%d", sampleTotal, numberOfIgnoredSampleJobRuns, sampleSuccess)
+ if adjustedSampleTotal < testStats.SampleStats.SuccessCount {
+ log.Warnf("adjustedSampleTotal is too small: sampleTotal=%d, numberOfIgnoredSampleJobRuns=%d, sampleSuccess=%d", sampleTotal, numberOfIgnoredSampleJobRuns, testStats.SampleStats.SuccessCount)
// due to differences in sample query times reflecting 'modified_time' and the use of job_run start / completion_time we can include
// some triaged job runs that are outside our sample window resulting in removing too many runs
// we see this often for triaged runs on the oldest day of the sample window
@@ -1396,98 +1379,61 @@ func (c *ComponentReportGenerator) assessComponentStatus(
// the original intent was to err on the side of caution since we didn't understand why this would happen
// now we see the negative consequence flipping triaged records back to regressions
// in this case we will remove all failures and only report the successes and flakes
- sampleTotal = sampleSuccess + sampleFlake
+ sampleTotal = testStats.SampleStats.SuccessCount + testStats.SampleStats.FlakeCount
} else {
sampleTotal = adjustedSampleTotal
}
+ // Adjust failure count for triaged runs
+ testStats.SampleStats.FailureCount = sampleTotal - testStats.SampleStats.SuccessCount - testStats.SampleStats.FlakeCount
+ if testStats.SampleStats.FailureCount < 0 {
+ // The adjusted total for ignored runs can push failure count into the negatives if there were
+ // more ignored runs than actual failures. (or no failures at all)
+ testStats.SampleStats.FailureCount = 0
+ }
- sampleFailure := sampleTotal - sampleSuccess - sampleFlake
- // The adjusted total for ignored runs can push failure count into the negatives if there were
- // more ignored runs than actual failures. (or no failures at all)
- if sampleFailure < 0 {
- sampleFailure = 0
+ var baseSuccess, baseFailure, baseFlake, baseTotal int
+ if testStats.BaseStats != nil {
+ baseSuccess = testStats.BaseStats.SuccessCount
+ baseFailure = testStats.BaseStats.FailureCount
+ baseFlake = testStats.BaseStats.FlakeCount
+ baseTotal = baseSuccess + baseFailure + baseFlake
}
- baseFailure := baseTotal - baseSuccess - baseFlake
if baseTotal == 0 && c.ReqOptions.AdvancedOption.PassRateRequiredNewTests > 0 {
// If we have no base stats, fall back to a raw pass rate comparison for new or improperly renamed tests:
- // Swap out sample with approvedRegression if we have it
- if approvedRegression != nil {
- sampleFailure = approvedRegression.PreviousFailures
- sampleFlake = approvedRegression.PreviousFlakes
- sampleSuccess = approvedRegression.PreviousSuccesses
- }
- testStats := c.buildPassRateTestStats(sampleSuccess, sampleFailure, sampleFlake,
- float64(c.ReqOptions.AdvancedOption.PassRateRequiredNewTests))
+ c.buildPassRateTestStats(testStats, float64(c.ReqOptions.AdvancedOption.PassRateRequiredNewTests))
// If a new test reports no regression, and we're not using pass rate mode for all tests, we alter
// status to be missing basis for the pre-existing Fisher Exact behavior:
if testStats.ReportStatus == crtype.NotSignificant && c.ReqOptions.AdvancedOption.PassRateRequiredAllTests == 0 {
testStats.ReportStatus = crtype.MissingBasis
}
- return testStats
+ return
} else if c.ReqOptions.AdvancedOption.PassRateRequiredAllTests > 0 {
// If requested, switch to pass rate only testing to see what does not meet the criteria:
- // Swap out sample with approvedRegression if we have it
- if approvedRegression != nil {
- sampleFailure = approvedRegression.PreviousFailures
- sampleFlake = approvedRegression.PreviousFlakes
- sampleSuccess = approvedRegression.PreviousSuccesses
- }
- testStats := c.buildPassRateTestStats(sampleSuccess, sampleFailure, sampleFlake,
+ c.buildPassRateTestStats(testStats,
float64(c.ReqOptions.AdvancedOption.PassRateRequiredAllTests))
- // include base stats even though we didn't do fishers exact here, this is helpful
- // for the test details page to give a visual on how the test behaved in the basis
- testStats.BaseStats = &crtype.TestDetailsReleaseStats{
- Release: baseRelease,
- Start: baseStart,
- End: baseEnd,
- TestDetailsTestStats: crtype.TestDetailsTestStats{
- SuccessRate: c.getPassRate(baseSuccess, baseFailure, baseFlake),
- SuccessCount: baseSuccess,
- FailureCount: baseFailure,
- FlakeCount: baseFlake,
- },
- }
-
- return testStats
+ return
}
// Otherwise we fall back to default behavior of Fishers Exact test:
- testStats := c.buildFisherExactTestStats(requiredConfidence, sampleTotal, sampleSuccess, sampleFlake, sampleFailure, baseTotal, baseSuccess, baseFlake, baseFailure, approvedRegression, activeProductRegression, initialSampleTotal, baseRelease, baseStart, baseEnd)
- return testStats
+ c.buildFisherExactTestStats(
+ testStats,
+ approvedRegression,
+ activeProductRegression,
+ initialSampleTotal)
}
-func (c *ComponentReportGenerator) buildFisherExactTestStats(requiredConfidence, sampleTotal, sampleSuccess, sampleFlake, sampleFailure, baseTotal, baseSuccess, baseFlake, baseFailure int, approvedRegression *regressionallowances.IntentionalRegression, activeProductRegression bool, initialSampleTotal int, baseRelease string, baseStart, baseEnd *time.Time) crtype.ReportTestStats {
+func (c *ComponentReportGenerator) buildFisherExactTestStats(testStats *crtype.ReportTestStats,
+ approvedRegression *regressionallowances.IntentionalRegression,
+ activeProductRegression bool,
+ initialSampleTotal int) {
+
+ sampleSuccess := testStats.SampleStats.SuccessCount
+ sampleFlake := testStats.SampleStats.FlakeCount
fisherExact := 0.0
- baseStats := &crtype.TestDetailsReleaseStats{
- Release: baseRelease,
- Start: baseStart,
- End: baseEnd,
- TestDetailsTestStats: crtype.TestDetailsTestStats{
- SuccessRate: c.getPassRate(baseSuccess, baseFailure, baseFlake),
- SuccessCount: baseSuccess,
- FailureCount: baseFailure,
- FlakeCount: baseFlake,
- },
- }
+ testStats.Comparison = crtype.FisherExact
- testStats := crtype.ReportTestStats{
- Comparison: crtype.FisherExact,
- SampleStats: crtype.TestDetailsReleaseStats{
- Release: c.ReqOptions.SampleRelease.Release,
- Start: &c.ReqOptions.SampleRelease.Start,
- End: &c.ReqOptions.SampleRelease.End,
- TestDetailsTestStats: crtype.TestDetailsTestStats{
- SuccessRate: c.getPassRate(sampleSuccess, sampleFailure, sampleFlake),
- SuccessCount: sampleSuccess,
- FailureCount: sampleFailure,
- FlakeCount: sampleFlake,
- },
- },
- BaseStats: baseStats,
- Explanations: []string{explanationNoRegression},
- }
status := crtype.MissingBasis
// if the unadjusted sample was 0 then nothing to do
if initialSampleTotal == 0 {
@@ -1496,24 +1442,26 @@ func (c *ComponentReportGenerator) buildFisherExactTestStats(requiredConfidence,
} else {
status = crtype.MissingSample
}
- } else if baseTotal != 0 {
- // see if we had a significant regression prior to adjusting
- basePass := baseSuccess + baseFlake
+ } else if testStats.BaseStats != nil && testStats.BaseStats.Total() != 0 {
+ // see if we had a significant regression prior to adjusting for triage
+ basePass := testStats.BaseStats.SuccessCount + testStats.BaseStats.FlakeCount
samplePass := sampleSuccess + sampleFlake
if c.ReqOptions.AdvancedOption.FlakeAsFailure {
- basePass = baseSuccess
+ basePass = testStats.BaseStats.SuccessCount
samplePass = sampleSuccess
}
- basisPassPercentage := float64(basePass) / float64(baseTotal)
+ basisPassPercentage := float64(basePass) / float64(testStats.BaseStats.Total())
initialPassPercentage := float64(samplePass) / float64(initialSampleTotal)
effectivePityFactor := c.getEffectivePityFactor(basisPassPercentage, approvedRegression)
wasSignificant := false
// only consider wasSignificant if the sampleTotal has been changed and our sample
// pass percentage is below the basis
- if initialSampleTotal > sampleTotal && initialPassPercentage < basisPassPercentage {
+ // SampleStats had it's failure count decremented earlier when we calculated adjusted sampleTotal and subtracted it
+ if initialSampleTotal > testStats.SampleStats.Total() && initialPassPercentage < basisPassPercentage {
if basisPassPercentage-initialPassPercentage > float64(c.ReqOptions.AdvancedOption.PityFactor)/100 {
- wasSignificant, _ = c.fischerExactTest(requiredConfidence, initialSampleTotal-samplePass, samplePass, baseTotal-basePass, basePass)
+ wasSignificant, _ = c.fischerExactTest(testStats.RequiredConfidence, initialSampleTotal-samplePass, samplePass,
+ testStats.BaseStats.Total()-basePass, basePass)
}
// if it was significant without the adjustment use
// ExtremeTriagedRegression or SignificantTriagedRegression
@@ -1522,7 +1470,7 @@ func (c *ComponentReportGenerator) buildFisherExactTestStats(requiredConfidence,
}
}
- if sampleTotal == 0 {
+ if testStats.SampleStats.Total() == 0 {
if !wasSignificant {
if c.ReqOptions.AdvancedOption.IgnoreMissing {
status = crtype.NotSignificant
@@ -1531,12 +1479,10 @@ func (c *ComponentReportGenerator) buildFisherExactTestStats(requiredConfidence,
status = crtype.MissingSample
}
}
- return crtype.ReportTestStats{
- Comparison: crtype.FisherExact,
- ReportStatus: status,
- FisherExact: thrift.Float64Ptr(0.0),
- Explanations: []string{explanationNoRegression},
- }
+ testStats.ReportStatus = status
+ testStats.FisherExact = thrift.Float64Ptr(0.0)
+ testStats.Explanations = append(testStats.Explanations, explanationNoRegression)
+ return
}
// if we didn't detect a significant regression prior to adjusting set our default here
@@ -1545,32 +1491,32 @@ func (c *ComponentReportGenerator) buildFisherExactTestStats(requiredConfidence,
}
// now that we know sampleTotal is non zero
- samplePassPercentage := float64(samplePass) / float64(sampleTotal)
+ samplePassPercentage := float64(samplePass) / float64(testStats.SampleStats.Total())
// did we remove enough failures that we are below the MinimumFailure threshold?
- if c.ReqOptions.AdvancedOption.MinimumFailure != 0 && (sampleTotal-samplePass) < c.ReqOptions.AdvancedOption.MinimumFailure {
+ if c.ReqOptions.AdvancedOption.MinimumFailure != 0 &&
+ (testStats.SampleStats.Total()-samplePass) < c.ReqOptions.AdvancedOption.MinimumFailure {
if status <= crtype.SignificantTriagedRegression {
- testStats.Explanations = []string{
- fmt.Sprintf("%s regression detected.", crtype.StringForStatus(status)),
- }
+ testStats.Explanations = append(testStats.Explanations,
+ fmt.Sprintf("%s regression detected.", crtype.StringForStatus(status)))
}
testStats.ReportStatus = status
testStats.FisherExact = thrift.Float64Ptr(0.0)
- return testStats
+ return
}
significant := false
improved := samplePassPercentage >= basisPassPercentage
if improved {
// flip base and sample when improved
- significant, fisherExact = c.fischerExactTest(requiredConfidence, baseTotal-basePass, basePass, sampleTotal-samplePass, samplePass)
+ significant, fisherExact = c.fischerExactTest(testStats.RequiredConfidence, testStats.BaseStats.Total()-basePass, basePass, testStats.SampleStats.Total()-samplePass, samplePass)
} else if basisPassPercentage-samplePassPercentage > float64(effectivePityFactor)/100 {
- significant, fisherExact = c.fischerExactTest(requiredConfidence, sampleTotal-samplePass, samplePass, baseTotal-basePass, basePass)
+ significant, fisherExact = c.fischerExactTest(testStats.RequiredConfidence, testStats.SampleStats.Total()-samplePass, samplePass, testStats.BaseStats.Total()-basePass, basePass)
}
if significant {
if improved {
// only show improvements if we are not dropping out triaged results
- if initialSampleTotal == sampleTotal {
+ if initialSampleTotal == testStats.SampleStats.Total() {
status = crtype.SignificantImprovement
}
} else {
@@ -1585,28 +1531,26 @@ func (c *ComponentReportGenerator) buildFisherExactTestStats(requiredConfidence,
if testStats.ReportStatus <= crtype.SignificantTriagedRegression {
if testStats.ReportStatus <= crtype.SignificantRegression {
- testStats.Explanations = []string{
- fmt.Sprintf("%s regression detected.", crtype.StringForStatus(testStats.ReportStatus)),
- fmt.Sprintf("Fishers Exact probability of a regression: %.2f%%.", float64(100)-*testStats.FisherExact),
+ testStats.Explanations = append(testStats.Explanations,
+ fmt.Sprintf("%s regression detected.", crtype.StringForStatus(testStats.ReportStatus)))
+ testStats.Explanations = append(testStats.Explanations,
+ fmt.Sprintf("Fishers Exact probability of a regression: %.2f%%.", float64(100)-*testStats.FisherExact))
+ testStats.Explanations = append(testStats.Explanations,
fmt.Sprintf("Test pass rate dropped from %.2f%% to %.2f%%.",
testStats.BaseStats.SuccessRate*float64(100),
- testStats.SampleStats.SuccessRate*float64(100)),
- }
+ testStats.SampleStats.SuccessRate*float64(100)))
} else {
- testStats.Explanations = []string{
- fmt.Sprintf("%s regression detected.", crtype.StringForStatus(testStats.ReportStatus)),
- }
- }
- // check for override
- if baseRelease != c.ReqOptions.BaseRelease.Release {
- testStats.Explanations = append(testStats.Explanations, fmt.Sprintf("Overrode base stats using release %s", baseRelease))
+ testStats.Explanations = append(testStats.Explanations,
+ fmt.Sprintf("%s regression detected.", crtype.StringForStatus(testStats.ReportStatus)))
}
}
-
- return testStats
}
-func (c *ComponentReportGenerator) buildPassRateTestStats(sampleSuccess, sampleFailure, sampleFlake int, requiredSuccessRate float64) crtype.ReportTestStats {
+func (c *ComponentReportGenerator) buildPassRateTestStats(testStats *crtype.ReportTestStats, requiredSuccessRate float64) {
+ sampleSuccess := testStats.SampleStats.SuccessCount
+ sampleFailure := testStats.SampleStats.FailureCount
+ sampleFlake := testStats.SampleStats.FlakeCount
+
successRate := c.getPassRate(sampleSuccess, sampleFailure, sampleFlake)
// Assume 2x our allowed failure rate = an extreme regression.
@@ -1622,29 +1566,16 @@ func (c *ComponentReportGenerator) buildPassRateTestStats(sampleSuccess, sampleF
if successRate*100 < severeRegressionSuccessRate {
rStatus = crtype.ExtremeRegression
}
- return crtype.ReportTestStats{
- ReportStatus: rStatus,
- Explanations: []string{
- fmt.Sprintf("Test has a %.2f%% pass rate, but %.2f%% is required.", successRate*100, requiredSuccessRate),
- },
- Comparison: crtype.PassRate,
- SampleStats: crtype.TestDetailsReleaseStats{
- Release: c.ReqOptions.SampleRelease.Release,
- Start: &c.ReqOptions.SampleRelease.Start,
- End: &c.ReqOptions.SampleRelease.End,
- TestDetailsTestStats: crtype.TestDetailsTestStats{
- SuccessRate: successRate,
- SuccessCount: sampleSuccess,
- FailureCount: sampleFailure,
- FlakeCount: sampleFlake,
- },
- },
- }
- }
- return crtype.ReportTestStats{
- ReportStatus: crtype.NotSignificant,
- Explanations: []string{explanationNoRegression},
+ testStats.ReportStatus = rStatus
+ testStats.Explanations = append(testStats.Explanations,
+ fmt.Sprintf("Test has a %.2f%% pass rate, but %.2f%% is required.", successRate*100, requiredSuccessRate))
+ testStats.Comparison = crtype.PassRate
+ testStats.SampleStats.SuccessRate = successRate
+ return
}
+
+ testStats.ReportStatus = crtype.NotSignificant
+ testStats.Explanations = append(testStats.Explanations, explanationNoRegression)
}
func (c *ComponentReportGenerator) fischerExactTest(confidenceRequired, sampleFailure, sampleSuccess, baseFailure, baseSuccess int) (bool, float64) {
diff --git a/pkg/api/componentreadiness/component_report_test.go b/pkg/api/componentreadiness/component_report_test.go
index 2a4f659f6f..15ec2e81e1 100644
--- a/pkg/api/componentreadiness/component_report_test.go
+++ b/pkg/api/componentreadiness/component_report_test.go
@@ -491,7 +491,8 @@ func TestGenerateComponentReport(t *testing.T) {
},
},
ReportTestStats: crtype.ReportTestStats{
- Comparison: crtype.FisherExact,
+ RequiredConfidence: 95,
+ Comparison: crtype.FisherExact,
Explanations: []string{
"Extreme regression detected.",
"Fishers Exact probability of a regression: 100.00%.",
@@ -532,7 +533,8 @@ func TestGenerateComponentReport(t *testing.T) {
},
},
ReportTestStats: crtype.ReportTestStats{
- Comparison: crtype.FisherExact,
+ RequiredConfidence: 95,
+ Comparison: crtype.FisherExact,
Explanations: []string{
"Significant regression detected.",
"Fishers Exact probability of a regression: 100.00%.",
@@ -828,7 +830,8 @@ func TestGenerateComponentReport(t *testing.T) {
},
},
ReportTestStats: crtype.ReportTestStats{
- Comparison: crtype.FisherExact,
+ RequiredConfidence: 90,
+ Comparison: crtype.FisherExact,
Explanations: []string{
"Significant regression detected.",
"Fishers Exact probability of a regression: 99.92%.",
@@ -1056,7 +1059,8 @@ func TestGenerateComponentReport(t *testing.T) {
},
},
ReportTestStats: crtype.ReportTestStats{
- Comparison: crtype.FisherExact,
+ RequiredConfidence: 95,
+ Comparison: crtype.FisherExact,
Explanations: []string{
"Extreme regression detected.",
"Fishers Exact probability of a regression: 100.00%.",
@@ -1097,7 +1101,8 @@ func TestGenerateComponentReport(t *testing.T) {
},
},
ReportTestStats: crtype.ReportTestStats{
- Comparison: crtype.FisherExact,
+ RequiredConfidence: 95,
+ Comparison: crtype.FisherExact,
Explanations: []string{
"Significant regression detected.",
"Fishers Exact probability of a regression: 100.00%.",
@@ -1171,7 +1176,7 @@ func TestGenerateComponentReport(t *testing.T) {
for ic := range report.Rows[ir].Columns {
assert.Equal(t, len(tc.expectedReport.Rows[ir].Columns[ic].RegressedTests), len(report.Rows[ir].Columns[ic].RegressedTests))
for it, regTest := range report.Rows[ir].Columns[ic].RegressedTests {
- assert.InDelta(t, *tc.expectedReport.Rows[ir].Columns[ic].RegressedTests[it].FisherExact, *regTest.FisherExact, 0.000001)
+ assert.InDelta(t, *tc.expectedReport.Rows[ir].Columns[ic].RegressedTests[it].FisherExact, *regTest.FisherExact, 0.000001, regTest.TestName)
tc.expectedReport.Rows[ir].Columns[ic].RegressedTests[it].FisherExact = nil
report.Rows[ir].Columns[ic].RegressedTests[it].FisherExact = nil
@@ -1242,8 +1247,6 @@ func TestGenerateComponentTestDetailsReport(t *testing.T) {
FailureCount: 200,
FlakeCount: 100,
},
- Start: &time.Time{},
- End: &time.Time{},
}
sampleTestStatsHigh := crtype.TestDetailsTestStats{
SuccessRate: 0.9203539823008849,
@@ -1288,8 +1291,6 @@ func TestGenerateComponentTestDetailsReport(t *testing.T) {
FailureCount: 100,
FlakeCount: 50,
},
- Start: &time.Time{},
- End: &time.Time{},
}
sampleReleaseStatsOneLow := crtype.TestDetailsReleaseStats{
Release: testDetailsGenerator.ReqOptions.SampleRelease.Release,
@@ -1310,8 +1311,6 @@ func TestGenerateComponentTestDetailsReport(t *testing.T) {
FailureCount: 600,
FlakeCount: 50,
},
- Start: &time.Time{},
- End: &time.Time{},
}
tests := []struct {
name string
@@ -1887,16 +1886,33 @@ func Test_componentReportGenerator_assessComponentStatus(t *testing.T) {
c.ReqOptions.AdvancedOption.PassRateRequiredAllTests = tt.requiredPassRateForAllTests
c.ReqOptions.AdvancedOption.MinimumFailure = tt.minFail
- testStats := c.assessComponentStatus(0, tt.sampleTotal, tt.sampleSuccess, tt.sampleFlake, tt.baseTotal, tt.baseSuccess, tt.baseFlake, nil, false, tt.numberOfIgnoredSamples, "dummyRelease", nil, nil)
- assert.Equalf(t, tt.expectedStatus, testStats.ReportStatus, "assessComponentStatus expected status not equal")
+ testAnalysis := &crtype.ReportTestStats{
+ SampleStats: crtype.TestDetailsReleaseStats{
+ TestDetailsTestStats: crtype.TestDetailsTestStats{
+ SuccessCount: tt.sampleSuccess,
+ FlakeCount: tt.sampleFlake,
+ FailureCount: tt.sampleTotal - tt.sampleSuccess - tt.sampleFlake,
+ },
+ },
+ BaseStats: &crtype.TestDetailsReleaseStats{
+ TestDetailsTestStats: crtype.TestDetailsTestStats{
+ SuccessCount: tt.baseSuccess,
+ FlakeCount: tt.baseFlake,
+ FailureCount: tt.baseTotal - tt.baseSuccess - tt.baseFlake,
+ },
+ },
+ }
+
+ c.assessComponentStatus(testAnalysis, nil, false, tt.numberOfIgnoredSamples)
+ assert.Equalf(t, tt.expectedStatus, testAnalysis.ReportStatus, "assessComponentStatus expected status not equal")
if tt.expectedFischers != nil {
// Mac and Linux do not matchup on floating point precision, so lets approximate the comparison:
assert.Equalf(t,
fmt.Sprintf("%.4f", *tt.expectedFischers),
- fmt.Sprintf("%.4f", *testStats.FisherExact),
+ fmt.Sprintf("%.4f", *testAnalysis.FisherExact),
"assessComponentStatus expected fischers value not equal")
} else {
- assert.Nil(t, testStats.FisherExact)
+ assert.Nil(t, testAnalysis.FisherExact)
}
})
diff --git a/pkg/api/componentreadiness/middleware/interface.go b/pkg/api/componentreadiness/middleware/interface.go
index 178f4eacca..7e7a5cd673 100644
--- a/pkg/api/componentreadiness/middleware/interface.go
+++ b/pkg/api/componentreadiness/middleware/interface.go
@@ -20,15 +20,13 @@ type Middleware interface {
// QueryTestDetails phase allow middleware to load data that will later be used.
QueryTestDetails(ctx context.Context, wg *sync.WaitGroup, errCh chan error, allJobVariants crtype.JobVariants)
- // Transform gives middleware opportunity to adjust the queried base and sample TestStatuses before we
- // proceed to analysis.
- Transform(status *crtype.ReportTestStatus) error
+ // PreAnalysis gives middleware opportunity to adjust test analysis data prior to running analysis.
+ // Implementations can alter base/sample data as needed, request confidence levels, and add explanations for
+ // what they did.
+ // NOTE: due to differences in test details reports, this function is not used there.
+ PreAnalysis(testKey crtype.ReportTestIdentification, testStats *crtype.ReportTestStats) error
- // TransformTestDetails gives middleware opportunity to adjust the queried base and sample job run data
- // before we proceed to analysis.
- TransformTestDetails(status *crtype.JobRunTestReportStatus) error
-
- // TestDetailsAnalyze gives middleware opportunity to analyze data and adjust the final report before
- // being returned over the API.
- TestDetailsAnalyze(report *crtype.ReportTestDetails) error
+ // PreTestDetailsAnalysis gives middleware the opportunity to adjust inputs to the report status
+ // prior to analysis.
+ PreTestDetailsAnalysis(status *crtype.JobRunTestReportStatus) error
}
diff --git a/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances.go b/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances.go
index 586e47e2a4..19b84fa041 100644
--- a/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances.go
+++ b/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances.go
@@ -40,20 +40,11 @@ func (r *RegressionAllowances) Query(_ context.Context, _ *sync.WaitGroup, _ crt
// unused
}
-// Transform iterates the base status looking for any with an accepted regression in the basis release, and if found
+// PreAnalysis iterates the base status looking for any with an accepted regression in the basis release, and if found
// swaps out the stats with the better pass rate data specified in the intentional regression allowance.
-func (r *RegressionAllowances) Transform(status *crtype.ReportTestStatus) error {
- for testKeyStr, baseStats := range status.BaseStatus {
- testKey, err := utils.DeserializeTestKey(baseStats, testKeyStr)
- if err != nil {
- return err
- }
+func (r *RegressionAllowances) PreAnalysis(testKey crtype.ReportTestIdentification, testStats *crtype.ReportTestStats) error {
- newBaseStatus, newBaseRelease := r.matchBaseRegression(testKey, r.reqOptions.BaseRelease.Release, baseStats)
- if newBaseRelease != r.reqOptions.BaseRelease.Release {
- status.BaseStatus[testKeyStr] = newBaseStatus
- }
- }
+ r.matchBaseRegression(testKey, r.reqOptions.BaseRelease.Release, testStats)
return nil
}
@@ -62,18 +53,25 @@ func (r *RegressionAllowances) Transform(status *crtype.ReportTestStatus) error
// in an intentional regression that accepted a lower threshold but maintains the higher
// threshold when used as a basis.
// It will return the original testStatus if there is no intentional regression.
-func (r *RegressionAllowances) matchBaseRegression(testID crtype.ReportTestIdentification, baseRelease string, baseStats crtype.TestStatus) (crtype.TestStatus, string) {
- var baseRegression *regressionallowances.IntentionalRegression
+func (r *RegressionAllowances) matchBaseRegression(testID crtype.ReportTestIdentification, baseRelease string, testStats *crtype.ReportTestStats) {
+ // Nothing to do for tests with no basis. (i.e. new tests)
+ if testStats.BaseStats == nil {
+ return
+ }
- // TODO: knowledge of variant cross compare here would be nice to eliminate
+ var baseRegression *regressionallowances.IntentionalRegression
if len(r.reqOptions.VariantOption.VariantCrossCompare) == 0 {
// only really makes sense when not cross-comparing variants:
// look for corresponding regressions we can account for in the analysis
// only if we are ignoring fallback, otherwise we will let fallback determine the threshold
baseRegression = r.regressionGetterFunc(baseRelease, testID.ColumnIdentification, testID.TestID)
- _, success, fail, flake := baseStats.GetTotalSuccessFailFlakeCounts()
- basePassRate := utils.CalculatePassRate(r.reqOptions, success, fail, flake)
+ baseStats := testStats.BaseStats
+
+ success := baseStats.SuccessCount
+ fail := baseStats.FailureCount
+ flake := baseStats.FlakeCount
+ basePassRate := utils.CalculatePassRate(success, fail, flake, r.reqOptions.AdvancedOption.FlakeAsFailure)
if baseRegression != nil && baseRegression.PreviousPassPercentage(r.reqOptions.AdvancedOption.FlakeAsFailure) > basePassRate {
// override with the basis regression previous values
// testStats will reflect the expected threshold, not the computed values from the release with the allowed regression
@@ -81,31 +79,26 @@ func (r *RegressionAllowances) matchBaseRegression(testID crtype.ReportTestIdent
if err != nil {
log.WithError(err).Error("Failed to determine the previous release for baseRegression")
} else {
- // create a clone since we might be updating a cached item though the same regression would likely apply each time...
- updatedStats := crtype.TestStatus{TestName: baseStats.TestName, TestSuite: baseStats.TestSuite, Capabilities: baseStats.Capabilities,
- Component: baseStats.Component, Variants: baseStats.Variants,
- FlakeCount: baseRegression.PreviousFlakes,
+ testStats.BaseStats.Release = baseRegressionPreviousRelease
+ testStats.BaseStats.TestDetailsTestStats = crtype.TestDetailsTestStats{
SuccessCount: baseRegression.PreviousSuccesses,
- TotalCount: baseRegression.PreviousFailures + baseRegression.PreviousFlakes + baseRegression.PreviousSuccesses,
+ FailureCount: baseRegression.PreviousFailures,
+ FlakeCount: baseRegression.PreviousFlakes,
+ SuccessRate: utils.CalculatePassRate(baseRegression.PreviousSuccesses, baseRegression.PreviousFailures,
+ baseRegression.PreviousFlakes, r.reqOptions.AdvancedOption.FlakeAsFailure),
}
- baseStats = updatedStats
- baseRelease = baseRegressionPreviousRelease
- log.Infof("BaseRegression - PreviousPassPercentage overrides baseStats. Release: %s, Successes: %d, Flakes: %d, Total: %d",
- baseRelease, baseStats.SuccessCount, baseStats.FlakeCount, baseStats.TotalCount)
+
+ log.Infof("BaseRegression - PreviousPassPercentage overrides baseStats. Release: %s, Successes: %d, Flakes: %d",
+ baseRegressionPreviousRelease, baseStats.SuccessCount, baseStats.FlakeCount)
}
}
}
- return baseStats, baseRelease
}
func (r *RegressionAllowances) QueryTestDetails(ctx context.Context, wg *sync.WaitGroup, errCh chan error, allJobVariants crtype.JobVariants) {
}
-func (r *RegressionAllowances) TransformTestDetails(status *crtype.JobRunTestReportStatus) error {
- return nil
-}
-
-func (r *RegressionAllowances) TestDetailsAnalyze(details *crtype.ReportTestDetails) error {
+func (r *RegressionAllowances) PreTestDetailsAnalysis(status *crtype.JobRunTestReportStatus) error {
return nil
}
diff --git a/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances_test.go b/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances_test.go
index e7bb4c49a4..bc5f57eca6 100644
--- a/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances_test.go
+++ b/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances_test.go
@@ -1,10 +1,10 @@
package regressionallowances
import (
- "encoding/json"
"reflect"
"testing"
+ "github.com/openshift/sippy/pkg/api/componentreadiness/utils"
crtype "github.com/openshift/sippy/pkg/apis/api/componentreport"
"github.com/openshift/sippy/pkg/regressionallowances"
"github.com/stretchr/testify/assert"
@@ -19,9 +19,9 @@ func Test_Transform(t *testing.T) {
regressionGetter := func(releaseString string, variant crtype.ColumnIdentification, testID string) *regressionallowances.IntentionalRegression {
if releaseString == "4.18" && reflect.DeepEqual(variant.Variants, variants) && testID == test1ID {
return ®ressionallowances.IntentionalRegression{
- JiraComponent: "foo",
+ JiraComponent: "",
TestID: test1ID,
- TestName: "test1",
+ TestName: "test 1",
Variant: variant,
PreviousSuccesses: 100,
PreviousFailures: 0,
@@ -40,74 +40,80 @@ func Test_Transform(t *testing.T) {
},
AdvancedOption: crtype.RequestAdvancedOptions{IncludeMultiReleaseAnalysis: true},
}
- test1Variants := []string{"Arch:amd64", "Platform:aws"}
- test1Key := crtype.TestWithVariantsKey{
- TestID: test1ID,
- Variants: variants,
+
+ test1Key := crtype.ReportTestIdentification{
+ RowIdentification: crtype.RowIdentification{
+ Component: "",
+ Capability: "",
+ TestName: "test 1",
+ TestSuite: "",
+ TestID: test1ID,
+ },
+ ColumnIdentification: crtype.ColumnIdentification{
+ Variants: variants,
+ },
}
- test1KeyBytes, err := json.Marshal(test1Key)
- test1KeyStr := string(test1KeyBytes)
- assert.NoError(t, err)
test2ID := "test2ID"
- test2Key := crtype.TestWithVariantsKey{
- TestID: test2ID,
- Variants: variants,
+ test2Key := crtype.ReportTestIdentification{
+ RowIdentification: crtype.RowIdentification{
+ Component: "",
+ Capability: "",
+ TestName: "test 2",
+ TestSuite: "",
+ TestID: test2ID,
+ },
+ ColumnIdentification: crtype.ColumnIdentification{
+ Variants: variants,
+ },
}
- test2KeyBytes, err := json.Marshal(test2Key)
- test2KeyStr := string(test2KeyBytes)
- assert.NoError(t, err)
tests := []struct {
name string
+ testKey crtype.ReportTestIdentification
reqOpts crtype.RequestOptions
- baseStatus map[string]crtype.TestStatus
- expectedStatus map[string]crtype.TestStatus
+ baseStatus *crtype.ReportTestStats
+ expectedStatus *crtype.ReportTestStats
}{
{
- name: "swap base stats using regression allowance",
- reqOpts: reqOpts419,
- baseStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 75, 0, nil),
- },
- expectedStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 100, 0, nil),
- },
+ name: "swap base stats using regression allowance",
+ reqOpts: reqOpts419,
+ testKey: test1Key,
+ baseStatus: buildTestStatus(100, 75, 0, "4.18"),
+ expectedStatus: buildTestStatus(100, 100, 0, "4.17"),
},
{
- name: "do not swap base stats if no regression allowance",
- reqOpts: reqOpts419,
- baseStatus: map[string]crtype.TestStatus{
- test2KeyStr: buildTestStatus("test2", test1Variants, 100, 75, 0, nil),
- },
- expectedStatus: map[string]crtype.TestStatus{
- test2KeyStr: buildTestStatus("test2", test1Variants, 100, 75, 0, nil),
- },
+ name: "do not swap base stats if no regression allowance",
+ reqOpts: reqOpts419,
+ testKey: test2Key,
+ baseStatus: buildTestStatus(100, 75, 0, "4.18"),
+ expectedStatus: buildTestStatus(100, 75, 0, "4.18"),
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
rfb := NewRegressionAllowancesMiddleware(test.reqOpts)
rfb.regressionGetterFunc = regressionGetter
- status := &crtype.ReportTestStatus{BaseStatus: test.baseStatus}
- err := rfb.Transform(status)
+ err := rfb.PreAnalysis(test.testKey, test.baseStatus)
assert.NoError(t, err)
- assert.Equal(t, test.expectedStatus, status.BaseStatus)
+ assert.Equal(t, *test.expectedStatus, *test.baseStatus)
})
}
}
//nolint:unparam
-func buildTestStatus(testName string, variants []string, total, success, flake int, release *crtype.Release) crtype.TestStatus {
- return crtype.TestStatus{
- TestName: testName,
- TestSuite: "conformance",
- Component: "foo",
- Capabilities: nil,
- Variants: variants,
- TotalCount: total,
- SuccessCount: success,
- FlakeCount: flake,
- Release: release,
+func buildTestStatus(total, success, flake int, baseRelease string) *crtype.ReportTestStats {
+ fails := total - success - flake
+ ts := &crtype.ReportTestStats{
+ BaseStats: &crtype.TestDetailsReleaseStats{
+ Release: baseRelease,
+ TestDetailsTestStats: crtype.TestDetailsTestStats{
+ FailureCount: fails,
+ SuccessCount: success,
+ FlakeCount: flake,
+ SuccessRate: utils.CalculatePassRate(success, fails, flake, false),
+ },
+ },
}
+ return ts
}
diff --git a/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go b/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go
new file mode 100644
index 0000000000..6c9f44badd
--- /dev/null
+++ b/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go
@@ -0,0 +1,123 @@
+package regressiontracker
+
+import (
+ "context"
+ "strings"
+ "sync"
+
+ "github.com/openshift/sippy/pkg/api/componentreadiness/middleware"
+ crtype "github.com/openshift/sippy/pkg/apis/api/componentreport"
+ "github.com/openshift/sippy/pkg/db"
+ "github.com/openshift/sippy/pkg/db/models"
+ log "github.com/sirupsen/logrus"
+)
+
+const (
+ // openRegressionConfidenceAdjustment is subtracted from the requested confidence for regressed tests that have
+ // an open regression.
+ openRegressionConfidenceAdjustment = 5
+)
+
+var _ middleware.Middleware = &RegressionTracker{}
+
+func NewRegressionTrackerMiddleware(dbc *db.DB, reqOptions crtype.RequestOptions) *RegressionTracker {
+ return &RegressionTracker{
+ log: log.WithField("middleware", "RegressionTracker"),
+ reqOptions: reqOptions,
+ dbc: dbc,
+ }
+}
+
+// RegressionTracker middleware loads all known regressions for this release from the db, and will
+// inject details onto regressed test stats if they match known regressions.
+type RegressionTracker struct {
+ log log.FieldLogger
+ reqOptions crtype.RequestOptions
+ dbc *db.DB
+ openRegressions []*models.TestRegression
+}
+
+func (r *RegressionTracker) Query(ctx context.Context, wg *sync.WaitGroup, allJobVariants crtype.JobVariants, baseStatusCh, sampleStatusCh chan map[string]crtype.TestStatus, errCh chan error) {
+ // Load all known regressions for this release:
+ r.openRegressions = make([]*models.TestRegression, 0)
+ q := r.dbc.DB.Table("test_regressions").
+ Where("release = ?", r.reqOptions.SampleRelease.Release).
+ Where("closed IS NULL")
+ res := q.Scan(&r.openRegressions)
+ if res.Error != nil {
+ errCh <- res.Error
+ return
+ }
+ r.log.Infof("Found %d open regressions", len(r.openRegressions))
+}
+
+func (r *RegressionTracker) QueryTestDetails(ctx context.Context, wg *sync.WaitGroup, errCh chan error, allJobVariants crtype.JobVariants) {
+}
+
+func (r *RegressionTracker) PreAnalysis(testKey crtype.ReportTestIdentification, testStats *crtype.ReportTestStats) error {
+ if len(r.openRegressions) > 0 {
+ view := r.openRegressions[0].View // grab view from first regression, they were queried only for sample release
+ or := FindOpenRegression(view, testKey.TestID, testKey.Variants, r.openRegressions)
+ if or != nil {
+ testStats.Regression = or
+
+ // Adjust the required certainty of a regression before we include it in the report as a
+ // regressed test. This is to introduce some hysteresis into the process so once a regression creeps over the 95%
+ // confidence we typically use, dropping to 94.9% should not make the cell immediately green.
+ //
+ // Instead, once you cross the confidence threshold and a regression begins tracking in the openRegressions list,
+ // we'll require less confidence for that test until the regression is closed. (-5%) Once the certainty drops below that
+ // modified confidence, the regression will be closed and the -5% adjuster is gone.
+ //
+ // ie. if the request was for 95% confidence, but we see that a test has an open regression (meaning at some point recently
+ // we were over 95% certain of a regression), we're going to only require 90% certainty to mark that test red.
+ testStats.RequiredConfidence = r.reqOptions.AdvancedOption.Confidence - openRegressionConfidenceAdjustment
+ }
+ }
+ return nil
+}
+
+// FindOpenRegression scans the list of open regressions for any that match the given test summary.
+func FindOpenRegression(view string,
+ testID string,
+ variants map[string]string,
+ regressions []*models.TestRegression) *models.TestRegression {
+
+ for _, tr := range regressions {
+ if tr.View != view {
+ continue
+ }
+
+ // We compare test ID not name, as names can change.
+ if tr.TestID != testID {
+ continue
+ }
+ found := true
+ for key, value := range variants {
+ if value != findVariant(key, tr) {
+ found = false
+ break
+ }
+ }
+ if !found {
+ continue
+ }
+ // If we made it this far, this appears to be a match:
+ return tr
+ }
+ return nil
+}
+
+func findVariant(variantName string, testReg *models.TestRegression) string {
+ for _, v := range testReg.Variants {
+ keyVal := strings.Split(v, ":")
+ if keyVal[0] == variantName {
+ return keyVal[1]
+ }
+ }
+ return ""
+}
+
+func (r *RegressionTracker) PreTestDetailsAnalysis(status *crtype.JobRunTestReportStatus) error {
+ return nil
+}
diff --git a/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go b/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go
index 8dd857f6a6..bd6e4a8ee3 100644
--- a/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go
+++ b/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go
@@ -2,6 +2,8 @@ package releasefallback
import (
"context"
+ "encoding/json"
+ "fmt"
"sync"
"time"
@@ -52,6 +54,10 @@ type ReleaseFallback struct {
baseOverrideStatus map[string][]crtype.JobRunTestStatusRow
}
+func (r *ReleaseFallback) Analyze(testID string, variants map[string]string, report *crtype.ReportTestStats) error {
+ return nil
+}
+
func (r *ReleaseFallback) Query(ctx context.Context, wg *sync.WaitGroup, allJobVariants crtype.JobVariants,
_, _ chan map[string]crtype.TestStatus, errCh chan error) {
wg.Add(1)
@@ -73,37 +79,32 @@ func (r *ReleaseFallback) Query(ctx context.Context, wg *sync.WaitGroup, allJobV
}()
}
-// Transform iterates the base status looking for any statuses that had a better pass rate in the prior releases
-// we queried earlier.
-func (r *ReleaseFallback) Transform(status *crtype.ReportTestStatus) error {
- for testKeyStr, baseStats := range status.BaseStatus {
- newBaseStatus := r.matchBestBaseStats(testKeyStr, r.reqOptions.BaseRelease.Release, baseStats)
- if newBaseStatus.Release != nil && newBaseStatus.Release.Release != r.reqOptions.BaseRelease.Release {
- status.BaseStatus[testKeyStr] = newBaseStatus
- }
+// PreAnalysis looks for a better pass rate across our fallback releases for the given test stats.
+// It then swaps them out and leaves an explanation before handing back to the core for analysis.
+func (r *ReleaseFallback) PreAnalysis(testKey crtype.ReportTestIdentification, testStats *crtype.ReportTestStats) error {
+ // Nothing to do for tests without a basis, i.e. new tests.
+ if testStats.BaseStats == nil {
+ return nil
}
-
- return nil
-}
-
-// matchBestBaseStats returns the testStatus, release and reportTestStatus
-// that has the highest threshold across the basis release and previous releases included
-// in fallback comparison.
-func (r *ReleaseFallback) matchBestBaseStats(
- testKeyStr, baseRelease string,
- baseStats crtype.TestStatus) crtype.TestStatus {
+ testIDVariantsKey := crtype.TestWithVariantsKey{
+ TestID: testKey.TestID,
+ Variants: testKey.Variants,
+ }
+ testIDBytes, _ := json.Marshal(testIDVariantsKey)
+ testKeyStr := string(testIDBytes)
if !r.reqOptions.AdvancedOption.IncludeMultiReleaseAnalysis {
- return baseStats
+ // nothing to do if this feature is disabled
+ return nil
}
if r.cachedFallbackTestStatuses == nil {
- r.log.Errorf("Invalid fallback test statuses")
- return baseStats
+ return fmt.Errorf("Invalid fallback test statuses")
}
- var priorRelease = baseRelease
+ var priorRelease = testStats.BaseStats.Release
var err error
+ var swappedExplanation string
for err == nil {
var cachedReleaseTestStatuses crtype.ReleaseTestMap
var cTestStats crtype.TestStatus
@@ -111,12 +112,12 @@ func (r *ReleaseFallback) matchBestBaseStats(
priorRelease, err = utils.PreviousRelease(priorRelease)
// if we fail to determine the previous release then stop
if err != nil {
- return baseStats
+ break
}
// if we hit a missing release then stop
if cachedReleaseTestStatuses, ok = r.cachedFallbackTestStatuses.Releases[priorRelease]; !ok {
- return baseStats
+ break
}
// it's ok if we don't have a testKeyStr for this release
@@ -126,29 +127,44 @@ func (r *ReleaseFallback) matchBestBaseStats(
// what is our base total compared to the original base
// this happens when jobs shift like sdn -> ovn
// if we get below threshold that's a sign we are reducing our base signal
- if float64(cTestStats.TotalCount)/float64(baseStats.TotalCount) < .6 {
+ if float64(cTestStats.TotalCount)/float64(testStats.BaseStats.Total()) < .6 {
r.log.Debugf("Fallback base total: %d to low for fallback analysis compared to original: %d",
- cTestStats.TotalCount, baseStats.TotalCount)
- return baseStats
+ cTestStats.TotalCount, testStats.BaseStats.Total())
+ return nil
}
- _, success, fail, flake := baseStats.GetTotalSuccessFailFlakeCounts()
- basePassRate := utils.CalculatePassRate(r.reqOptions, success, fail, flake)
+ success := testStats.BaseStats.SuccessCount
+ fail := testStats.BaseStats.FailureCount
+ flake := testStats.BaseStats.FlakeCount
+ basePassRate := utils.CalculatePassRate(success, fail, flake, r.reqOptions.AdvancedOption.FlakeAsFailure)
_, success, fail, flake = cTestStats.GetTotalSuccessFailFlakeCounts()
- cPassRate := utils.CalculatePassRate(r.reqOptions, success, fail, flake)
+ cPassRate := utils.CalculatePassRate(success, fail, flake, r.reqOptions.AdvancedOption.FlakeAsFailure)
if cPassRate > basePassRate {
- baseStats = cTestStats
- // If we swapped out base stats for better ones from a prior release, we need to communicate
- // this back to the core report generator so it can include the adjusted release/start/end dates in
- // the report, and ultimately the UI.
- baseStats.Release = &cachedReleaseTestStatuses.Release
- r.log.Debugf("Overrode base stats (%.4f) using release %s (%.4f) for test: %s - %s",
- basePassRate, baseStats.Release.Release, cPassRate, baseStats.TestName, testKeyStr)
+ // We've found a better pass rate in a prior release with enough runs to qualify.
+ // Adjust the stats and keep looking for an even better one.
+ testStats.BaseStats = &crtype.TestDetailsReleaseStats{
+ Release: priorRelease,
+ Start: cachedReleaseTestStatuses.Start,
+ End: cachedReleaseTestStatuses.End,
+ TestDetailsTestStats: crtype.TestDetailsTestStats{
+ SuccessCount: success,
+ FailureCount: fail,
+ FlakeCount: flake,
+ SuccessRate: cPassRate,
+ },
+ }
+ swappedExplanation = fmt.Sprintf("Overrode base stats (%.4f) using release %s (%.4f)",
+ basePassRate, testStats.BaseStats.Release, cPassRate)
+ r.log.Debugf("%s for test %s", swappedExplanation, testKey.TestName)
}
}
}
+ // Add an explanation for the user why we fell back for the final release data:
+ if swappedExplanation != "" {
+ testStats.Explanations = append(testStats.Explanations, swappedExplanation)
+ }
- return baseStats
+ return nil
}
func (r *ReleaseFallback) getFallbackBaseQueryStatus(ctx context.Context,
@@ -211,14 +227,13 @@ func (r *ReleaseFallback) QueryTestDetails(ctx context.Context, wg *sync.WaitGro
}
-func (r *ReleaseFallback) TransformTestDetails(status *crtype.JobRunTestReportStatus) error {
- // Simply attach the override base stats to the status
- r.log.Infof("Transforming fallback test details")
- status.BaseOverrideStatus = r.baseOverrideStatus
+func (r *ReleaseFallback) TestDetailsAnalyze(report *crtype.ReportTestDetails) error {
return nil
}
-func (r *ReleaseFallback) TestDetailsAnalyze(report *crtype.ReportTestDetails) error {
+func (r *ReleaseFallback) PreTestDetailsAnalysis(status *crtype.JobRunTestReportStatus) error {
+ // Add our baseOverrideStatus to the report, unfortunate hack we have to live with for now.
+ status.BaseOverrideStatus = r.baseOverrideStatus
return nil
}
@@ -272,30 +287,7 @@ func (f *fallbackTestQueryReleasesGenerator) getTestFallbackReleases(ctx context
// currently gets current base plus previous 3
// current base is just for testing but use could be
// extended to no longer require the base query
- var selectedReleases []*crtype.Release
- fallbackRelease := f.BaseRelease
-
- // Get up to 3 fallback releases
- for i := 0; i < 3; i++ {
- var crRelease *crtype.Release
-
- fallbackRelease, err := utils.PreviousRelease(fallbackRelease)
- if err != nil {
- log.WithError(err).Errorf("Failure determining fallback release for %s", fallbackRelease)
- break
- }
-
- for i := range releases {
- if releases[i].Release == fallbackRelease {
- crRelease = &releases[i]
- break
- }
- }
-
- if crRelease != nil {
- selectedReleases = append(selectedReleases, crRelease)
- }
- }
+ selectedReleases := calculateFallbackReleases(f.BaseRelease, releases)
for _, crRelease := range selectedReleases {
@@ -304,8 +296,8 @@ func (f *fallbackTestQueryReleasesGenerator) getTestFallbackReleases(ctx context
// we want our base release validation to match the base release report dates
if crRelease.Release != f.BaseRelease && crRelease.End != nil && crRelease.Start != nil {
- end = *crRelease.End
start = *crRelease.Start
+ end = *crRelease.End
}
wg.Add(1)
@@ -331,6 +323,35 @@ func (f *fallbackTestQueryReleasesGenerator) getTestFallbackReleases(ctx context
return &f.CachedFallbackTestStatuses, nil
}
+func calculateFallbackReleases(startingRelease string, releases []crtype.Release) []*crtype.Release {
+ var selectedReleases []*crtype.Release
+ fallbackRelease := startingRelease
+
+ // Get up to 3 fallback releases
+ for i := 0; i < 3; i++ {
+ var crRelease *crtype.Release
+
+ var err error
+ fallbackRelease, err = utils.PreviousRelease(fallbackRelease)
+ if err != nil {
+ log.WithError(err).Errorf("Failure determining fallback release for %s", fallbackRelease)
+ break
+ }
+
+ for i := range releases {
+ if releases[i].Release == fallbackRelease {
+ crRelease = &releases[i]
+ break
+ }
+ }
+
+ if crRelease != nil {
+ selectedReleases = append(selectedReleases, crRelease)
+ }
+ }
+ return selectedReleases
+}
+
func (f *fallbackTestQueryReleasesGenerator) updateTestStatuses(release crtype.Release, updateStatuses map[string]crtype.TestStatus) {
var testStatuses crtype.ReleaseTestMap
@@ -351,8 +372,8 @@ func (f *fallbackTestQueryReleasesGenerator) updateTestStatuses(release crtype.R
func (f *fallbackTestQueryReleasesGenerator) getTestFallbackRelease(ctx context.Context, client *bqcachedclient.Client, release string, start, end time.Time) (crtype.ReportTestStatus, []error) {
generator := newFallbackBaseQueryGenerator(client, f.ReqOptions, f.allJobVariants, release, start, end)
-
- testStatuses, errs := api.GetDataFromCacheOrGenerate[crtype.ReportTestStatus](ctx, f.client.Cache, generator.cacheOption, api.GetPrefixedCacheKey("FallbackBaseTestStatus~", generator), generator.getTestFallbackRelease, crtype.ReportTestStatus{})
+ cacheKey := api.GetPrefixedCacheKey("FallbackBaseTestStatus~", generator)
+ testStatuses, errs := api.GetDataFromCacheOrGenerate[crtype.ReportTestStatus](ctx, f.client.Cache, generator.cacheOption, cacheKey, generator.getTestFallbackRelease, crtype.ReportTestStatus{})
if len(errs) > 0 {
return crtype.ReportTestStatus{}, errs
diff --git a/pkg/api/componentreadiness/middleware/releasefallback/releasefallback_test.go b/pkg/api/componentreadiness/middleware/releasefallback/releasefallback_test.go
index 6b9677d05c..67819f745a 100644
--- a/pkg/api/componentreadiness/middleware/releasefallback/releasefallback_test.go
+++ b/pkg/api/componentreadiness/middleware/releasefallback/releasefallback_test.go
@@ -5,11 +5,12 @@ import (
"testing"
"time"
+ "github.com/openshift/sippy/pkg/api/componentreadiness/utils"
crtype "github.com/openshift/sippy/pkg/apis/api/componentreport"
"github.com/stretchr/testify/assert"
)
-func Test_Transform(t *testing.T) {
+func Test_PreAnalysis(t *testing.T) {
reqOpts419 := crtype.RequestOptions{
BaseRelease: crtype.RequestReleaseOptions{
Release: "4.19",
@@ -17,17 +18,39 @@ func Test_Transform(t *testing.T) {
AdvancedOption: crtype.RequestAdvancedOptions{IncludeMultiReleaseAnalysis: true},
}
test1ID := "test1ID"
- test1Variants := []string{"Arch:amd64", "Platform:aws"}
- test1Key := crtype.TestWithVariantsKey{
- TestID: test1ID,
- Variants: map[string]string{
- "Arch": "amd64",
- "Platform": "aws",
- },
+ test1Variants := map[string]string{
+ "Arch": "amd64",
+ "Platform": "aws",
+ }
+ test1VariantsFlattened := []string{"Arch:amd64", "Platform:aws"}
+ test1MapKey := crtype.TestWithVariantsKey{
+ TestID: test1ID,
+ Variants: test1Variants,
}
- test1KeyBytes, err := json.Marshal(test1Key)
+ test1KeyBytes, err := json.Marshal(test1MapKey)
test1KeyStr := string(test1KeyBytes)
assert.NoError(t, err)
+ test1RTI := crtype.ReportTestIdentification{
+ RowIdentification: crtype.RowIdentification{
+ Component: "",
+ Capability: "",
+ TestName: "test 1",
+ TestSuite: "",
+ TestID: test1ID,
+ },
+ ColumnIdentification: crtype.ColumnIdentification{
+ Variants: test1Variants,
+ },
+ }
+
+ // 4.19 will be our assumed requested base release, which may trigger fallback to 4.18 or 4.17 in these tests
+ start419 := time.Date(2025, 3, 2, 0, 0, 0, 0, time.UTC)
+ end419 := time.Date(2025, 4, 30, 0, 0, 0, 0, time.UTC)
+ release419 := crtype.Release{
+ Release: "4.19",
+ Start: &start419,
+ End: &end419,
+ }
start418 := time.Date(2025, 2, 1, 0, 0, 0, 0, time.UTC)
end418 := time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC)
@@ -39,7 +62,7 @@ func Test_Transform(t *testing.T) {
fallbackMap418 := crtype.ReleaseTestMap{
Release: release418,
Tests: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 95, 0, nil),
+ test1KeyStr: buildTestStatus("test1", test1VariantsFlattened, 100, 95, 0),
},
}
@@ -53,111 +76,139 @@ func Test_Transform(t *testing.T) {
fallbackMap417 := crtype.ReleaseTestMap{
Release: release417,
Tests: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 98, 0, nil),
+ test1KeyStr: buildTestStatus("test1", test1VariantsFlattened, 100, 98, 0),
},
}
tests := []struct {
name string
reqOpts crtype.RequestOptions
+ testKey crtype.ReportTestIdentification
fallbackReleases crtype.FallbackReleases
- baseStatus map[string]crtype.TestStatus
- expectedStatus map[string]crtype.TestStatus
+ testStats *crtype.ReportTestStats
+ expectedStatus *crtype.ReportTestStats
}{
{
name: "fallback to prior release",
reqOpts: reqOpts419,
+ testKey: test1RTI,
fallbackReleases: crtype.FallbackReleases{
Releases: map[string]crtype.ReleaseTestMap{
fallbackMap418.Release.Release: fallbackMap418,
},
},
- baseStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 93, 0, nil),
- },
- expectedStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 95, 0, &release418),
- },
+ testStats: buildTestStats(100, 93, release419, nil),
+ expectedStatus: buildTestStats(100, 95, release418, []string{"Overrode base stats (0.9300) using release 4.18 (0.9500)"}),
},
{
name: "fallback twice to prior release",
reqOpts: reqOpts419,
+ testKey: test1RTI,
fallbackReleases: crtype.FallbackReleases{
Releases: map[string]crtype.ReleaseTestMap{
fallbackMap418.Release.Release: fallbackMap418,
fallbackMap417.Release.Release: fallbackMap417, // 4.17 improves even further
},
},
- baseStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 93, 0, nil),
- },
- expectedStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 98, 0, &release417),
- },
+ testStats: buildTestStats(100, 93, release419, nil),
+ expectedStatus: buildTestStats(100, 98, release417, []string{"Overrode base stats (0.9500) using release 4.17 (0.9800)"}),
},
{
name: "fallback once to two releases ago",
reqOpts: reqOpts419,
+ testKey: test1RTI,
fallbackReleases: crtype.FallbackReleases{
Releases: map[string]crtype.ReleaseTestMap{
fallbackMap418.Release.Release: fallbackMap418,
fallbackMap417.Release.Release: fallbackMap417, // 4.17 improves even further
},
},
- baseStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 97, 0, nil),
- },
- expectedStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 98, 0, &release417),
- },
+ testStats: buildTestStats(100, 97, release419, nil),
+ expectedStatus: buildTestStats(100, 98, release417, []string{"Overrode base stats (0.9700) using release 4.17 (0.9800)"}),
},
{
name: "don't fallback to prior release",
reqOpts: reqOpts419,
+ testKey: test1RTI,
fallbackReleases: crtype.FallbackReleases{
Releases: map[string]crtype.ReleaseTestMap{
fallbackMap418.Release.Release: fallbackMap418,
},
},
- baseStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 100, 0, nil),
- },
- expectedStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 100, 100, 0, nil),
- },
+ testStats: buildTestStats(100, 100, release419, nil),
+ expectedStatus: buildTestStats(100, 100, release419, nil),
},
{
name: "don't fallback to prior release with insufficient runs",
reqOpts: reqOpts419,
+ testKey: test1RTI,
fallbackReleases: crtype.FallbackReleases{
Releases: map[string]crtype.ReleaseTestMap{
fallbackMap418.Release.Release: fallbackMap418,
fallbackMap417.Release.Release: fallbackMap417,
},
},
- baseStatus: map[string]crtype.TestStatus{
- test1KeyStr: buildTestStatus("test1", test1Variants, 10000, 9700, 0, nil),
- },
- expectedStatus: map[string]crtype.TestStatus{
- // No fallback release had at least 60% of our run count
- test1KeyStr: buildTestStatus("test1", test1Variants, 10000, 9700, 0, nil),
- },
+ testStats: buildTestStats(10000, 9700, release419, nil),
+ // No fallback release had at least 60% of our run count
+ expectedStatus: buildTestStats(10000, 9700, release419, nil),
},
}
for i, test := range tests {
t.Run(test.name, func(t *testing.T) {
rfb := NewReleaseFallbackMiddleware(nil, test.reqOpts)
rfb.cachedFallbackTestStatuses = &tests[i].fallbackReleases
- status := &crtype.ReportTestStatus{BaseStatus: test.baseStatus}
- err := rfb.Transform(status)
+ err := rfb.PreAnalysis(test.testKey, test.testStats)
assert.NoError(t, err)
- assert.Equal(t, test.expectedStatus, status.BaseStatus)
+ assert.Equal(t, *test.expectedStatus, *test.testStats)
})
}
}
+func TestCalculateFallbackReleases(t *testing.T) {
+ start419 := time.Date(2025, 3, 2, 0, 0, 0, 0, time.UTC)
+ end419 := time.Date(2025, 4, 30, 0, 0, 0, 0, time.UTC)
+ release419 := crtype.Release{
+ Release: "4.19",
+ Start: &start419,
+ End: &end419,
+ }
+
+ start418 := time.Date(2025, 2, 1, 0, 0, 0, 0, time.UTC)
+ end418 := time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC)
+ release418 := crtype.Release{
+ Release: "4.18",
+ Start: &start418,
+ End: &end418,
+ }
+
+ start417 := time.Date(2024, 12, 1, 0, 0, 0, 0, time.UTC)
+ end417 := time.Date(2024, 12, 31, 0, 0, 0, 0, time.UTC)
+ release417 := crtype.Release{
+ Release: "4.17",
+ Start: &start417,
+ End: &end417,
+ }
+
+ start416 := time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC)
+ end416 := time.Date(2024, 6, 30, 0, 0, 0, 0, time.UTC)
+ release416 := crtype.Release{
+ Release: "4.16",
+ Start: &start416,
+ End: &end416,
+ }
+
+ allReleases := []crtype.Release{release419, release418, release417, release416}
+ expectedReleases := []crtype.Release{release419, release418, release417}
+
+ fallbackReleases := calculateFallbackReleases("4.20", allReleases)
+ for i := range expectedReleases {
+ assert.Equal(t, expectedReleases[i].Release, fallbackReleases[i].Release)
+ assert.Equal(t, expectedReleases[i].Start, fallbackReleases[i].Start)
+ assert.Equal(t, expectedReleases[i].End, fallbackReleases[i].End)
+ }
+}
//nolint:unparam
-func buildTestStatus(testName string, variants []string, total, success, flake int, release *crtype.Release) crtype.TestStatus {
+func buildTestStatus(testName string, variants []string, total, success, flake int) crtype.TestStatus {
return crtype.TestStatus{
TestName: testName,
TestSuite: "conformance",
@@ -167,6 +218,26 @@ func buildTestStatus(testName string, variants []string, total, success, flake i
TotalCount: total,
SuccessCount: success,
FlakeCount: flake,
- Release: release,
}
}
+
+func buildTestStats(total, success int, baseRelease crtype.Release, explanations []string) *crtype.ReportTestStats {
+ fails := total - success
+ ts := &crtype.ReportTestStats{
+ BaseStats: &crtype.TestDetailsReleaseStats{
+ Release: baseRelease.Release,
+ Start: baseRelease.Start,
+ End: baseRelease.End,
+ TestDetailsTestStats: crtype.TestDetailsTestStats{
+ FailureCount: fails,
+ SuccessCount: success,
+ FlakeCount: 0,
+ SuccessRate: utils.CalculatePassRate(success, fails, 0, false),
+ },
+ },
+ }
+ if explanations != nil {
+ ts.Explanations = explanations
+ }
+ return ts
+}
diff --git a/pkg/api/componentreadiness/regressiontracker.go b/pkg/api/componentreadiness/regressiontracker.go
index 1262ce9129..3ee2484664 100644
--- a/pkg/api/componentreadiness/regressiontracker.go
+++ b/pkg/api/componentreadiness/regressiontracker.go
@@ -8,6 +8,7 @@ import (
"strings"
"time"
+ "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/regressiontracker"
"github.com/openshift/sippy/pkg/db/models"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
@@ -300,7 +301,7 @@ func (rt *RegressionTracker) SyncRegressionsForReport(ctx context.Context, view
matchedOpenRegressions := []*models.TestRegression{} // all the matches we found, used to determine what had no match
rLog.Infof("syncing %d open regressions", len(allRegressedTests))
for _, regTest := range allRegressedTests {
- if openReg := FindOpenRegression(view.Name, regTest.TestID, regTest.Variants, regressions); openReg != nil {
+ if openReg := regressiontracker.FindOpenRegression(view.Name, regTest.TestID, regTest.Variants, regressions); openReg != nil {
if openReg.Closed.Valid {
// if the regression returned has a closedRegs date, we found a recently closedRegs
// regression for this test. We'll re-use it to limit churn as sometimes tests may drop
@@ -365,44 +366,3 @@ func (rt *RegressionTracker) SyncRegressionsForReport(ctx context.Context, view
return nil
}
-
-// FindOpenRegression scans the list of open regressions for any that match the given test summary.
-func FindOpenRegression(view string,
- testID string,
- variants map[string]string,
- regressions []*models.TestRegression) *models.TestRegression {
-
- for _, tr := range regressions {
- if tr.View != view {
- continue
- }
-
- // We compare test ID not name, as names can change.
- if tr.TestID != testID {
- continue
- }
- found := true
- for key, value := range variants {
- if value != findVariant(key, tr) {
- found = false
- break
- }
- }
- if !found {
- continue
- }
- // If we made it this far, this appears to be a match:
- return tr
- }
- return nil
-}
-
-func findVariant(variantName string, testReg *models.TestRegression) string {
- for _, v := range testReg.Variants {
- keyVal := strings.Split(v, ":")
- if keyVal[0] == variantName {
- return keyVal[1]
- }
- }
- return ""
-}
diff --git a/pkg/api/componentreadiness/test_details.go b/pkg/api/componentreadiness/test_details.go
index 897e2da9f0..8a3f9f3214 100644
--- a/pkg/api/componentreadiness/test_details.go
+++ b/pkg/api/componentreadiness/test_details.go
@@ -65,27 +65,6 @@ func (c *ComponentReportGenerator) GenerateTestDetailsReport(ctx context.Context
now := time.Now()
componentJobRunTestReportStatus.GeneratedAt = &now
- // Allow all middleware a chance to transform the job run test statuses:
- var err error
- for _, mw := range c.middlewares {
- err = mw.TransformTestDetails(&componentJobRunTestReportStatus)
- if err != nil {
- return crtype.ReportTestDetails{}, []error{err}
- }
- }
-
- // We only execute this if we were given a postgres database connection, it is still possible to run
- // component readiness without postgresql, you just won't have regression tracking.
- if c.dbc != nil {
- var err error
- bqs := NewPostgresRegressionStore(c.dbc)
- c.openRegressions, err = bqs.ListCurrentRegressionsForRelease(c.ReqOptions.SampleRelease.Release)
- if err != nil {
- errs = append(errs, err)
- return crtype.ReportTestDetails{}, errs
- }
- }
-
// Generate the report for the main release that was originally requested:
report := c.internalGenerateTestDetailsReport(ctx,
componentJobRunTestReportStatus.BaseStatus,
@@ -95,19 +74,24 @@ func (c *ComponentReportGenerator) GenerateTestDetailsReport(ctx context.Context
componentJobRunTestReportStatus.SampleStatus)
report.GeneratedAt = componentJobRunTestReportStatus.GeneratedAt
- for _, mw := range c.middlewares {
- err = mw.TestDetailsAnalyze(&report)
- if err != nil {
- return crtype.ReportTestDetails{}, []error{err}
- }
- }
-
// Generate the report for the fallback release if one was found:
- // Move all this to the middleware.
+ // TODO: this belongs in the releasefallback middleware, but our goal to return and display multiple
+ // reports means the PreAnalysis state cannot be used for test details. The second call to
+ // internalGenerateTestDetailsReport does not extract easily off "c". We cannot pass a ref to "c" due
+ // to a circular dep. This is an unfortunate compormise in the middleware goal I didn't have time to unwind.
+ // For now, the middleware does the querying for test details, and passes the override status out
+ // by adding it to componentJobRunTestReportStatus.BaseOverrideStatus.
var baseOverrideReport *crtype.ReportTestDetails
if c.ReqOptions.BaseOverrideRelease.Release != "" &&
c.ReqOptions.BaseOverrideRelease.Release != c.ReqOptions.BaseRelease.Release {
+ for _, mw := range c.middlewares {
+ err := mw.PreTestDetailsAnalysis(&componentJobRunTestReportStatus)
+ if err != nil {
+ return report, []error{err}
+ }
+ }
+
overrideReport := c.internalGenerateTestDetailsReport(ctx,
componentJobRunTestReportStatus.BaseOverrideStatus,
c.ReqOptions.BaseOverrideRelease.Release,
@@ -338,18 +322,20 @@ func (c *ComponentReportGenerator) internalGenerateTestDetailsReport(ctx context
sampleStatusCopy[k] = v
}
- result := crtype.ReportTestDetails{
- ReportTestIdentification: crtype.ReportTestIdentification{
- RowIdentification: crtype.RowIdentification{
- Component: c.ReqOptions.TestIDOption.Component,
- Capability: c.ReqOptions.TestIDOption.Capability,
- TestID: c.ReqOptions.TestIDOption.TestID,
- },
- ColumnIdentification: crtype.ColumnIdentification{
- Variants: c.ReqOptions.VariantOption.RequestedVariants,
- },
+ testKey := crtype.ReportTestIdentification{
+ RowIdentification: crtype.RowIdentification{
+ Component: c.ReqOptions.TestIDOption.Component,
+ Capability: c.ReqOptions.TestIDOption.Capability,
+ TestID: c.ReqOptions.TestIDOption.TestID,
+ },
+ ColumnIdentification: crtype.ColumnIdentification{
+ Variants: c.ReqOptions.VariantOption.RequestedVariants,
},
}
+
+ result := crtype.ReportTestDetails{
+ ReportTestIdentification: testKey,
+ }
var resolvedIssueCompensation int
var incidents []crtype.TriagedIncident
approvedRegression := regressionallowances.IntentionalRegressionFor(c.ReqOptions.SampleRelease.Release, result.ColumnIdentification, c.ReqOptions.TestIDOption.TestID)
@@ -502,23 +488,46 @@ func (c *ComponentReportGenerator) internalGenerateTestDetailsReport(ctx context
}
}
- requiredConfidence := c.getRequiredConfidence(c.ReqOptions.TestIDOption.TestID, c.ReqOptions.VariantOption.RequestedVariants)
+ testStats := crtype.ReportTestStats{
+ RequiredConfidence: c.ReqOptions.AdvancedOption.Confidence,
+ SampleStats: crtype.TestDetailsReleaseStats{
+ Release: c.ReqOptions.SampleRelease.Release,
+ Start: &c.ReqOptions.SampleRelease.Start,
+ End: &c.ReqOptions.SampleRelease.End,
+ TestDetailsTestStats: crtype.TestDetailsTestStats{
+ SuccessRate: utils.CalculatePassRate(totalSampleSuccess, totalSampleFailure, totalSampleFlake, c.ReqOptions.AdvancedOption.FlakeAsFailure),
+ SuccessCount: totalSampleSuccess,
+ FlakeCount: totalSampleFlake,
+ FailureCount: totalSampleFailure,
+ },
+ },
+ BaseStats: &crtype.TestDetailsReleaseStats{
+ Release: baseRelease,
+ Start: baseStart,
+ End: baseEnd,
+ TestDetailsTestStats: crtype.TestDetailsTestStats{
+ SuccessRate: utils.CalculatePassRate(totalBaseSuccess, totalBaseFailure, totalBaseFlake, c.ReqOptions.AdvancedOption.FlakeAsFailure),
+ SuccessCount: totalBaseSuccess,
+ FlakeCount: totalBaseFlake,
+ FailureCount: totalBaseFailure,
+ },
+ },
+ }
+
+ for _, mw := range c.middlewares {
+ err := mw.PreAnalysis(testKey, &testStats)
+ if err != nil {
+ logrus.WithError(err).Error("Failure from middleware analysis")
+ }
+ }
- report.ReportTestStats = c.assessComponentStatus(
- requiredConfidence,
- totalSampleSuccess+totalSampleFailure+totalSampleFlake,
- totalSampleSuccess,
- totalSampleFlake,
- totalBaseSuccess+totalBaseFailure+totalBaseFlake,
- totalBaseSuccess,
- totalBaseFlake,
+ c.assessComponentStatus(
+ &testStats,
approvedRegression,
activeProductRegression,
resolvedIssueCompensation,
- baseRelease,
- baseStart,
- baseEnd,
)
+ report.ReportTestStats = testStats
result.Analyses = []crtype.TestDetailsAnalysis{report}
return result
diff --git a/pkg/api/componentreadiness/utils/utils.go b/pkg/api/componentreadiness/utils/utils.go
index ae7bb51c34..ea87c1970e 100644
--- a/pkg/api/componentreadiness/utils/utils.go
+++ b/pkg/api/componentreadiness/utils/utils.go
@@ -95,12 +95,12 @@ func DeserializeTestKey(stats componentreport.TestStatus, testKeyStr string) (co
return testID, nil
}
-func CalculatePassRate(reqOptions componentreport.RequestOptions, success, failure, flake int) float64 {
+func CalculatePassRate(success, failure, flake int, treatFlakeAsFailure bool) float64 {
total := success + failure + flake
if total == 0 {
return 0.0
}
- if reqOptions.AdvancedOption.FlakeAsFailure {
+ if treatFlakeAsFailure {
return float64(success) / float64(total)
}
return float64(success+flake) / float64(total)
diff --git a/pkg/apis/api/componentreport/types.go b/pkg/apis/api/componentreport/types.go
index e4cbe5f1c8..06f6f81171 100644
--- a/pkg/apis/api/componentreport/types.go
+++ b/pkg/apis/api/componentreport/types.go
@@ -7,6 +7,7 @@ import (
"cloud.google.com/go/bigquery"
"cloud.google.com/go/civil"
"github.com/openshift/sippy/pkg/apis/cache"
+ "github.com/openshift/sippy/pkg/db/models"
"github.com/openshift/sippy/pkg/util/sets"
)
@@ -47,9 +48,10 @@ type RequestReleaseOptions struct {
// for the point in time when the request was made. This is used in the UI to pre-populate the
// date picks to transition from view based to custom reporting.
type RequestRelativeReleaseOptions struct {
- RequestReleaseOptions `json:",inline" yaml:",inline"` //nolint:revive // inline is a known option
- RelativeStart string `json:"relative_start,omitempty" yaml:"relative_start,omitempty"`
- RelativeEnd string `json:"relative_end,omitempty" yaml:"relative_end,omitempty"`
+ RequestReleaseOptions `json:",inline" yaml:",inline"` //nolint:revive
+ // inline is a known option
+ RelativeStart string `json:"relative_start,omitempty" yaml:"relative_start,omitempty"`
+ RelativeEnd string `json:"relative_end,omitempty" yaml:"relative_end,omitempty"`
}
type RequestTestIdentificationOptions struct {
@@ -136,9 +138,6 @@ type TestStatus struct {
SuccessCount int `json:"success_count"`
FlakeCount int `json:"flake_count"`
LastFailure time.Time `json:"last_failure"`
- // Release provides info on the release this test status was pulled from for base TestStatus.
- // If nil, assume the base/sample release from the request options. (used for ReleaseFallback)
- Release *Release `json:"release"`
}
func (ts TestStatus) GetTotalSuccessFailFlakeCounts() (int, int, int, int) {
@@ -197,20 +196,8 @@ type ReportTestIdentification struct {
}
type ReportTestSummary struct {
+ // TODO: really feels like this could just be moved ReportTestStats, eliminating the need for ReportTestSummary
ReportTestIdentification
-
- // Opened will be set to the time we first recorded this test went regressed.
- // TODO: This is largely a hack right now, the sippy metrics loop sets this as soon as it notices
- // the regression with it's *default view* query. However we always include it in the response (if that test
- // is regressed per the query params used). Eventually we should only include these details if the default view
- // is being used, without overriding the start/end dates.
- Opened *time.Time `json:"opened"`
- RegressionID int `json:"regression_id"`
-
- // Links contains REST links for clients to follow for this specific triage. Most notably "self".
- // These are injected by the API and not stored in the DB.
- Links map[string]string `json:"links"`
-
ReportTestStats
}
@@ -238,6 +225,11 @@ type ReportTestStats struct {
SampleStats TestDetailsReleaseStats `json:"sample_stats"`
+ // RequiredConfidence is the confidence required from Fishers to consider a regression.
+ // Typically, it is as defined in the request options, but middleware may choose to adjust.
+ // 95 = 95% confidence of a regression required.
+ RequiredConfidence int `json:"-"`
+
// Optional fields depending on the Comparison mode
// FisherExact indicates the confidence of a regression after applying Fisher's Exact Test.
@@ -248,6 +240,10 @@ type ReportTestStats struct {
// LastFailure is the last time the regressed test failed.
LastFailure *time.Time `json:"last_failure"`
+
+ // Regression is populated with data on when we first detected this regression. If unset it implies
+ // the regression tracker has not yet run to find it, or you're using report params/a view without regression tracking.
+ Regression *models.TestRegression `json:"regression,omitempty"`
}
// IsTriaged returns true if this tests status is within the triaged regression range.
@@ -286,10 +282,16 @@ type TestDetailsReleaseStats struct {
}
type TestDetailsTestStats struct {
- SuccessRate float64 `json:"success_rate"`
- SuccessCount int `json:"success_count"`
- FailureCount int `json:"failure_count"`
- FlakeCount int `json:"flake_count"`
+ // TODO: should be a function not a field, calculated from the three below
+ SuccessRate float64 `json:"success_rate"`
+
+ SuccessCount int `json:"success_count"`
+ FailureCount int `json:"failure_count"`
+ FlakeCount int `json:"flake_count"`
+}
+
+func (tdts TestDetailsTestStats) Total() int {
+ return tdts.SuccessCount + tdts.FailureCount + tdts.FlakeCount
}
type TestDetailsJobStats struct {
diff --git a/sippy-ng/src/component_readiness/RegressedTestsPanel.js b/sippy-ng/src/component_readiness/RegressedTestsPanel.js
index 5ae7437bac..d96645e4e4 100644
--- a/sippy-ng/src/component_readiness/RegressedTestsPanel.js
+++ b/sippy-ng/src/component_readiness/RegressedTestsPanel.js
@@ -267,25 +267,25 @@ export default function RegressedTestsPanel(props) {
renderCell: (param) => {param.value}
,
},
{
- field: 'opened',
+ field: 'regression',
headerName: 'Regressed Since',
flex: 12,
valueGetter: (params) => {
- if (!params.row.opened) {
+ if (!params.row.regression?.opened) {
// For a regression we haven't yet detected:
return null
}
- return new Date(params.row.opened).getTime()
+ return new Date(params.row.regression.opened).getTime()
},
renderCell: (params) => {
if (!params.value) return ''
- const regressedSinceDate = new Date(params.value)
+ const regressedSinceDate = new Date(params.row.regression.opened)
return (
- copyTestToClipboard(event, params.row.regression_id)
+ copyTestToClipboard(event, params.row.regression.id)
}
>