diff --git a/apm.lock.yaml b/apm.lock.yaml index ae51ccba60..ac912064cc 100644 --- a/apm.lock.yaml +++ b/apm.lock.yaml @@ -92,8 +92,8 @@ local_deployed_files: - .opencode/commands/sippy-update-ga-release-views.md - .opencode/commands/sippy-update-job-variant.md local_deployed_file_hashes: - .claude/commands/agentic-followup.md: sha256:3195764952aac75e75d85a861ec58d9f2cbb697735b9446eecbf50762241255d - .claude/commands/agentic-solve.md: sha256:cba8ca0bf1e1ee0d1175138e6648e6344d337615bf906909034b9a55b5531465 + .claude/commands/agentic-followup.md: sha256:d40313dcf0721911cae14a4af9774c021b0f16d24662a1b547524a5eef56ca86 + .claude/commands/agentic-solve.md: sha256:8df186e128d0ffac40f9494c8d46275ea12af43921981f05d52944ba072f40cd .claude/commands/sippy-dev-app.md: sha256:656276ed961940c137dde32ecdb0501427d4d811502a27125ba073adc770d266 .claude/commands/sippy-dev-frontend.md: sha256:42eae4b3bc610c9fcb43533a6fed229a6d1c409d279f3d6f93672986ede62e3a .claude/commands/sippy-dev-migrate.md: sha256:80160e88e0cc0fc09ab3dd9cc6fc496fe87dd8873800eb65d700868034d59da2 @@ -111,8 +111,8 @@ local_deployed_file_hashes: .claude/rules/general.md: sha256:997f68e86cb43485ec5f108be3417f9bbb43ae1faffd660d598f18260f5df3ce .claude/rules/mcp.md: sha256:ddfe965e7cf8cddbba1374c6ae582a20ac0af17c958bf10e1a4edff6ff2ad0b8 .claude/rules/testing.md: sha256:57834092f6732d17f8c1812d25290cfc1cfbbbeb6eae1561ba2240973c651866 - .cursor/commands/agentic-followup.md: sha256:3195764952aac75e75d85a861ec58d9f2cbb697735b9446eecbf50762241255d - .cursor/commands/agentic-solve.md: sha256:cba8ca0bf1e1ee0d1175138e6648e6344d337615bf906909034b9a55b5531465 + .cursor/commands/agentic-followup.md: sha256:d40313dcf0721911cae14a4af9774c021b0f16d24662a1b547524a5eef56ca86 + .cursor/commands/agentic-solve.md: sha256:8df186e128d0ffac40f9494c8d46275ea12af43921981f05d52944ba072f40cd .cursor/commands/sippy-dev-app.md: sha256:656276ed961940c137dde32ecdb0501427d4d811502a27125ba073adc770d266 .cursor/commands/sippy-dev-frontend.md: sha256:42eae4b3bc610c9fcb43533a6fed229a6d1c409d279f3d6f93672986ede62e3a .cursor/commands/sippy-dev-migrate.md: sha256:80160e88e0cc0fc09ab3dd9cc6fc496fe87dd8873800eb65d700868034d59da2 @@ -131,8 +131,8 @@ local_deployed_file_hashes: .cursor/rules/general.mdc: sha256:5bc6e1e12d53d85656248c9dc1239c74bcc0df29d5987f3b08e3d79e3df413b7 .cursor/rules/mcp.mdc: sha256:c02472afd46e4c89f71d4487dcd5da98b0c1bcbcf7f9cbc4d7ed4e7d3a206ec1 .cursor/rules/testing.mdc: sha256:e5ce80313a812750404d45355b462c2e3a6458f5bc20ad7996a5da1b169a4703 - .gemini/commands/agentic-followup.toml: sha256:cb017c493fb79dcfbc6eda8d2d058cd951a2aea700887e8754f0103858fd2089 - .gemini/commands/agentic-solve.toml: sha256:613044d7539a4550102760b434bad50fb8eaac76ba4a3721e7f6265dbbf2e8ed + .gemini/commands/agentic-followup.toml: sha256:81b2f8f0ddb3757d564bb4f35c4610d8cb466068a068296222b7fc01ef253977 + .gemini/commands/agentic-solve.toml: sha256:33a3850f284398fe4080c63e2501afbf2bd0fa2475fb922400111a8294e650df .gemini/commands/sippy-dev-app.toml: sha256:fc28174eeab4e440694a823bd838d429241997a018d8a13f32e0f67ca4d973c5 .gemini/commands/sippy-dev-frontend.toml: sha256:ec4ab5e1fb7581f09473e33b3ed4f53ce40509f23aac581e3b100ad1f59de5e5 .gemini/commands/sippy-dev-migrate.toml: sha256:25c98ba4bfdb95270dfcb4238ae688f9a66f0e645d8a4a6c5e03b1cf8db5cb7e @@ -143,8 +143,8 @@ local_deployed_file_hashes: .gemini/commands/sippy-generate-release-views.toml: sha256:9173d7507f469eda21a19f7e5ec47bce0c6ce87a73c1f5c10594a89417332927 .gemini/commands/sippy-update-ga-release-views.toml: sha256:662ac8d503d883cf6142c801b79e46616afb3d806e1d936e952d918b257ddf73 .gemini/commands/sippy-update-job-variant.toml: sha256:3b044dfaddeafa1a6d375918d5860785f8d315b593c355bbb7293a87c50361b8 - .opencode/commands/agentic-followup.md: sha256:3195764952aac75e75d85a861ec58d9f2cbb697735b9446eecbf50762241255d - .opencode/commands/agentic-solve.md: sha256:cba8ca0bf1e1ee0d1175138e6648e6344d337615bf906909034b9a55b5531465 + .opencode/commands/agentic-followup.md: sha256:d40313dcf0721911cae14a4af9774c021b0f16d24662a1b547524a5eef56ca86 + .opencode/commands/agentic-solve.md: sha256:8df186e128d0ffac40f9494c8d46275ea12af43921981f05d52944ba072f40cd .opencode/commands/sippy-dev-app.md: sha256:656276ed961940c137dde32ecdb0501427d4d811502a27125ba073adc770d266 .opencode/commands/sippy-dev-frontend.md: sha256:42eae4b3bc610c9fcb43533a6fed229a6d1c409d279f3d6f93672986ede62e3a .opencode/commands/sippy-dev-migrate.md: sha256:80160e88e0cc0fc09ab3dd9cc6fc496fe87dd8873800eb65d700868034d59da2 diff --git a/config/views.yaml b/config/views.yaml index fc5dad1e8d..4e04a3253e 100755 --- a/config/views.yaml +++ b/config/views.yaml @@ -9,6 +9,13 @@ component_readiness: release: "5.0" relative_start: now-7d relative_end: now + spot_check_job_samples: + spotcheck-30d: + relative_start: now-30d + relative_end: now + include_variants: + JobTier: + - spotcheck-30d variant_options: column_group_by: Network: { } diff --git a/pkg/api/componentreadiness/component_report.go b/pkg/api/componentreadiness/component_report.go index b0340e36ba..a06cf20345 100644 --- a/pkg/api/componentreadiness/component_report.go +++ b/pkg/api/componentreadiness/component_report.go @@ -3,7 +3,6 @@ package componentreadiness import ( "context" "encoding/json" - "fmt" "maps" "os" "reflect" @@ -12,11 +11,13 @@ import ( "sync" "time" - "github.com/apache/thrift/lib/go/thrift" - fischer "github.com/glycerine/golang-fisher-exact" + "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/alltestspassrate" + "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/fisherexact" "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/linkinjector" + "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/newtestpassrate" regressionallowances2 "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/regressionallowances" "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/regressiontracker" + "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/spotcheckjobs" "github.com/openshift/sippy/pkg/apis/api/componentreport/crstatus" "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" @@ -37,7 +38,6 @@ import ( ) const ( - explanationNoRegression = "No significant regressions found" ComponentReportCacheKeyPrefix = "ComponentReport~" TestDetailsReportCacheKeyPrefix = "TestDetailsReport~" ) @@ -276,6 +276,14 @@ func (c *ComponentReportGenerator) getCache() cache.Cache { func (c *ComponentReportGenerator) initializeMiddleware() { c.middlewares = middleware.List{} + + // Initialize all our middleware applicable to this request. + + // middlewares that inject synthetic tests must run first so results are in place for other middleware. + if len(c.ReqOptions.SpotCheckJobSamples) > 0 { + c.middlewares = append(c.middlewares, spotcheckjobs.NewSpotCheckJobsMiddleware(c.dataProvider, c.ReqOptions)) + } + // Initialize all our middleware applicable to this request. if c.ReqOptions.AdvancedOption.IncludeMultiReleaseAnalysis && c.ReqOptions.SampleRelease.PullRequestOptions == nil { c.middlewares = append(c.middlewares, releasefallback.NewReleaseFallbackMiddleware(c.dataProvider, c.ReqOptions, c.releaseConfigs)) @@ -287,6 +295,11 @@ func (c *ComponentReportGenerator) initializeMiddleware() { } c.middlewares = append(c.middlewares, regressionallowances2.NewRegressionAllowancesMiddleware(c.ReqOptions, c.releaseConfigs)) + // Analysis middleware ordered by priority — first responder wins. + c.middlewares = append(c.middlewares, newtestpassrate.NewNewTestPassRateMiddleware(c.ReqOptions)) + c.middlewares = append(c.middlewares, alltestspassrate.NewAllTestsPassRateMiddleware(c.ReqOptions)) + c.middlewares = append(c.middlewares, fisherexact.NewFisherExactMiddleware(c.ReqOptions)) + // Initialize LinkInjector middleware linkInjector := linkinjector.NewLinkInjectorMiddleware(c.ReqOptions, c.baseURL) c.middlewares = append(c.middlewares, linkInjector) @@ -644,7 +657,9 @@ func (c *ComponentReportGenerator) generateComponentTestReport(basisStatusMap, s return crtype.ComponentReport{}, err } - c.assessComponentStatus(&cellReport, log.NewEntry(log.New())) + if _, err := c.middlewares.Analyze(testKey, &cellReport); err != nil { + return crtype.ComponentReport{}, err + } if lastFailure := sampleStatus.LastFailure; !lastFailure.IsZero() { cellReport.LastFailure = &lastFailure // it's a copy, for pointer hygiene } @@ -747,172 +762,6 @@ func buildReport(sortedRows []crtest.RowIdentification, sortedColumns []crtest.C return regressionRows, nil } -func getRegressionStatus(basisPassPercentage, samplePassPercentage float64) crtest.Status { - if (basisPassPercentage - samplePassPercentage) > 0.15 { - return crtest.ExtremeRegression - } - - return crtest.SignificantRegression -} - -// TODO: this will eventually become the analyze step on a Middleware, or possibly a separate -// set of objects relating to analysis, as there's not a lot of overlap between the analyzers -// (fishers, pass rate, bayes (future)) and the middlewares (fallback, intentional regressions, -// cross variant compare, rarely run jobs, etc.) -func (c *ComponentReportGenerator) assessComponentStatus(testStats *testdetails.TestComparison, logger *log.Entry) { - // Catch unset required confidence, typically unit tests - opts := c.ReqOptions.AdvancedOption - if testStats.RequiredConfidence == 0 { - testStats.RequiredConfidence = opts.Confidence - } - - if (testStats.BaseStats == nil || testStats.BaseStats.Total() == 0) && opts.PassRateRequiredNewTests > 0 { - // If we have no base stats, fall back to a raw pass rate comparison for new or improperly renamed tests: - c.buildPassRateTestStats(testStats, float64(opts.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 == crtest.NotSignificant && opts.PassRateRequiredAllTests == 0 { - testStats.ReportStatus = crtest.MissingBasis - } - return - } else if opts.PassRateRequiredAllTests > 0 { - // If requested, switch to pass rate only testing to see what does not meet the criteria: - c.buildPassRateTestStats(testStats, float64(opts.PassRateRequiredAllTests)) - return - } - - // Otherwise we fall back to default behavior of Fishers Exact test: - c.buildFisherExactTestStats(testStats, logger) -} - -func (c *ComponentReportGenerator) buildFisherExactTestStats(testStats *testdetails.TestComparison, logger *log.Entry) { - - fisherExact := 0.0 - testStats.Comparison = crtest.FisherExact - - status := crtest.MissingBasis - opts := c.ReqOptions.AdvancedOption - if testStats.SampleStats.Total() == 0 { - if opts.IgnoreMissing { - status = crtest.NotSignificant - } else { - status = crtest.MissingSample - } - testStats.ReportStatus = status - testStats.FisherExact = thrift.Float64Ptr(0.0) - testStats.Explanations = append(testStats.Explanations, explanationNoRegression) - } else if testStats.BaseStats != nil && testStats.BaseStats.Total() != 0 { - samplePass := testStats.SampleStats.Passes(opts.FlakeAsFailure) - basePass := testStats.BaseStats.Passes(opts.FlakeAsFailure) - basisPassPercentage := float64(basePass) / float64(testStats.BaseStats.Total()) - effectivePityFactor := float64(opts.PityFactor) + testStats.PityAdjustment - effectiveMinimumFailure := opts.MinimumFailure - - // default starting status now that we know we have basis and sample - status = crtest.NotSignificant - - // now that we know sampleTotal is non zero - samplePassPercentage := float64(samplePass) / float64(testStats.SampleStats.Total()) - - // are we below the MinimumFailure threshold? - if effectiveMinimumFailure != 0 && - (testStats.SampleStats.Total()-samplePass) < effectiveMinimumFailure { - if status <= crtest.SignificantTriagedRegression { - testStats.Explanations = append(testStats.Explanations, - fmt.Sprintf("%s regression detected.", crtest.StringForStatus(status))) - } - testStats.ReportStatus = status - testStats.FisherExact = thrift.Float64Ptr(0.0) - return - } - significant := false - improved := samplePassPercentage >= basisPassPercentage - - if improved { - // flip base and sample when improved - significant, fisherExact = c.fischerExactTest(testStats.RequiredConfidence, testStats.BaseStats.Total()-basePass, basePass, testStats.SampleStats.Total()-samplePass, samplePass) - } else if basisPassPercentage-samplePassPercentage > effectivePityFactor/100 { - significant, fisherExact = c.fischerExactTest(testStats.RequiredConfidence, testStats.SampleStats.Total()-samplePass, samplePass, testStats.BaseStats.Total()-basePass, basePass) - } - logger.Debugf("computed Fisher info: signifcant: %v, fisherExact: %v", significant, fisherExact) - if significant { - if improved { - status = crtest.SignificantImprovement - } else { - status = getRegressionStatus(basisPassPercentage, samplePassPercentage) - } - } - } - logger.Debugf("computed status: %d", int(status)) - testStats.ReportStatus = status - testStats.FisherExact = thrift.Float64Ptr(fisherExact) - - baseRelease := "no basis" - if testStats.BaseStats != nil { - baseRelease = testStats.BaseStats.Release - } - // If we have a regression, include explanations: - if testStats.ReportStatus <= crtest.SignificantTriagedRegression { - logger.Debugf("regression detected against: %s", baseRelease) - - if testStats.ReportStatus <= crtest.SignificantRegression { - testStats.Explanations = append(testStats.Explanations, - fmt.Sprintf("%s regression detected.", crtest.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))) - } else { - testStats.Explanations = append(testStats.Explanations, - fmt.Sprintf("%s regression detected.", crtest.StringForStatus(testStats.ReportStatus))) - } - } else { - logger.Debugf("NO regression detected against: %s", baseRelease) - } -} - -func (c *ComponentReportGenerator) buildPassRateTestStats(testStats *testdetails.TestComparison, requiredSuccessRate float64) { - - effectiveSuccessReq := requiredSuccessRate + testStats.RequiredPassRateAdjustment - - // Assume 2x our allowed failure rate = an extreme regression. - // i.e. if we require 90%, extreme is anything below 80% - // if we require 95%, extreme is anything below 90% - // if an adjustment is applied, still use the configured success rate to define extreme regression. - severeRegressionSuccessRate := effectiveSuccessReq - (100 - requiredSuccessRate) - - // Require 6 runs in the sample (typically 1 week for daily jobs) for us to consider a pass rate requirement for a new test: - sufficientRuns := testStats.SampleStats.Total() >= 6 - - opts := c.ReqOptions.AdvancedOption - successRate := testStats.SampleStats.PassRate(opts.FlakeAsFailure) - if sufficientRuns && successRate*100 < effectiveSuccessReq && testStats.SampleStats.FailureCount >= opts.MinimumFailure { - rStatus := crtest.SignificantRegression - if successRate*100 < severeRegressionSuccessRate { - rStatus = crtest.ExtremeRegression - } - testStats.ReportStatus = rStatus - testStats.Explanations = append(testStats.Explanations, - fmt.Sprintf("Test has a %.2f%% pass rate, but %.2f%% is required.", successRate*100, effectiveSuccessReq)) - testStats.Comparison = crtest.PassRate - testStats.SampleStats.SuccessRate = successRate - return - } - - testStats.ReportStatus = crtest.NotSignificant - testStats.Explanations = append(testStats.Explanations, explanationNoRegression) -} - -func (c *ComponentReportGenerator) fischerExactTest(confidenceRequired, sampleFailure, sampleSuccess, baseFailure, baseSuccess int) (bool, float64) { - _, _, r, _ := fischer.FisherExactTest(sampleFailure, - sampleSuccess, - baseFailure, - baseSuccess) - return r < 1-float64(confidenceRequired)/100, r -} - func (c *ComponentReportGenerator) getUniqueJUnitColumnValuesLast60Days(ctx context.Context, field string, nested bool) ([]string, error) { diff --git a/pkg/api/componentreadiness/component_report_test.go b/pkg/api/componentreadiness/component_report_test.go index da05888b2a..75ed1daa4e 100644 --- a/pkg/api/componentreadiness/component_report_test.go +++ b/pkg/api/componentreadiness/component_report_test.go @@ -13,7 +13,6 @@ import ( "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/openshift/sippy/pkg/api/componentreadiness/utils" @@ -1196,6 +1195,7 @@ func TestGenerateComponentReport(t *testing.T) { componentAndCapabilityGetter = fakeComponentAndCapabilityGetter for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + tc.generator.initializeMiddleware() report, err := tc.generator.generateComponentTestReport(tc.baseStatus, tc.sampleStatus) assert.NoError(t, err, "error generating component report") @@ -1622,6 +1622,7 @@ func TestGenerateComponentTestDetailsReport(t *testing.T) { } t.Run(tc.name, func(t *testing.T) { + tc.generator.initializeMiddleware() report := tc.generator.internalGenerateTestDetailsReport("", nil, nil, baseStats, sampleStats, tc.generator.ReqOptions.TestIDOptions[0]) assert.Equal(t, tc.expectedReport.RowIdentification, report.RowIdentification, "expected report row identification %+v, got %+v", tc.expectedReport.RowIdentification, report.RowIdentification) assert.Equal(t, tc.expectedReport.ColumnIdentification, report.ColumnIdentification, "expected report column identification %+v, got %+v", tc.expectedReport.ColumnIdentification, report.ColumnIdentification) @@ -1682,7 +1683,7 @@ func Test_componentReportGenerator_normalizeProwJobName(t *testing.T) { } } -func Test_componentReportGenerator_assessComponentStatus(t *testing.T) { +func Test_componentReportGenerator_analyze(t *testing.T) { tests := []struct { name string sampleTotal int @@ -1824,6 +1825,7 @@ func Test_componentReportGenerator_assessComponentStatus(t *testing.T) { c.ReqOptions.AdvancedOption.PassRateRequiredNewTests = tt.requiredPassRateForNewTests c.ReqOptions.AdvancedOption.PassRateRequiredAllTests = tt.requiredPassRateForAllTests c.ReqOptions.AdvancedOption.MinimumFailure = tt.minFail + c.initializeMiddleware() testAnalysis := &testdetails.TestComparison{ SampleStats: testdetails.ReleaseStats{ @@ -1842,14 +1844,15 @@ func Test_componentReportGenerator_assessComponentStatus(t *testing.T) { }, } - c.assessComponentStatus(testAnalysis, logrus.NewEntry(logrus.New())) - assert.Equalf(t, tt.expectedStatus, testAnalysis.ReportStatus, "assessComponentStatus expected status not equal") + testKey := crtest.Identification{} + _, err := c.middlewares.Analyze(testKey, testAnalysis) + assert.NoError(t, err) + assert.Equalf(t, tt.expectedStatus, testAnalysis.ReportStatus, "Analyze 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", *testAnalysis.FisherExact), - "assessComponentStatus expected fischers value not equal") + "Analyze expected fischers value not equal") } else { assert.Nil(t, testAnalysis.FisherExact) } diff --git a/pkg/api/componentreadiness/dataprovider/bigquery/provider.go b/pkg/api/componentreadiness/dataprovider/bigquery/provider.go index 85d97523fa..15bdb1bb2d 100644 --- a/pkg/api/componentreadiness/dataprovider/bigquery/provider.go +++ b/pkg/api/componentreadiness/dataprovider/bigquery/provider.go @@ -209,7 +209,7 @@ func (p *BigQueryProvider) QueryJobRuns(ctx context.Context, reqOptions reqopts. cleanV := param.Cleanse(v) joinVariants += fmt.Sprintf( "LEFT JOIN %s.job_variants jv_%s ON jobs.prowjob_job_name = jv_%s.job_name AND jv_%s.variant_name = '%s'\n", - p.client.Dataset, cleanV, cleanV, cleanV, v) + p.client.Dataset, cleanV, cleanV, cleanV, cleanV) } variantFilters := "" @@ -374,6 +374,345 @@ func (p *BigQueryProvider) LookupJobVariants(ctx context.Context, jobName string return variants, nil } +// --- SpotCheckQuerier --- + +// spotCheckFallbackSQL provides COALESCE expressions for deriving component/capability +// from job names when the SpotCheckComponent/SpotCheckCapability variants are not yet +// populated in the job_variants table. This fallback can be removed once the variant +// syncer has run and all spot-check jobs have the new variants. +const spotCheckComponentFallback = `COALESCE(jv_SpotCheckComponent.variant_value, + CASE + WHEN LOWER(jobs.prowjob_job_name) LIKE '%%cpu-partitioning%%' THEN 'Node / Kubelet' + WHEN LOWER(jobs.prowjob_job_name) LIKE '%%etcd-scaling%%' THEN 'Etcd' + END)` + +const spotCheckCapabilityFallback = `COALESCE(jv_SpotCheckCapability.variant_value, + CASE + WHEN LOWER(jobs.prowjob_job_name) LIKE '%%cpu-partitioning%%' THEN 'CPU Partitioning' + WHEN LOWER(jobs.prowjob_job_name) LIKE '%%etcd-scaling%%' THEN 'Scaling' + END)` + +// QuerySpotCheckJobRuns queries the jobs table (not junit) for spot-check periodic/release +// jobs, grouping by component, capability, and the DB group-by variants. +// Returns aggregated pass/fail counts per group so the middleware can create +// synthetic test results without needing individual test case data. +// includeVariants specifies variant filters (ANDed) from the spot-check sample config, +// typically including JobTier to select the correct spot-check tier. +func (p *BigQueryProvider) QuerySpotCheckJobRuns(ctx context.Context, reqOptions reqopts.RequestOptions, + allJobVariants crtest.JobVariants, + spotCheckIncludeVariants map[string][]string, + start, end time.Time) ([]dataprovider.SpotCheckGroup, error) { + + groupByVariantSet := reqOptions.VariantOption.DBGroupBy + if len(groupByVariantSet) == 0 { + groupByVariantSet = sets.NewString("Platform", "Architecture", "Network") + } + + var selectVariants, joinVariants string + var groupByParts []string + for _, v := range groupByVariantSet.List() { + cleanV := param.Cleanse(v) + joinVariants += fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_%s ON jobs.prowjob_job_name = jv_%s.job_name AND jv_%s.variant_name = '%s'\n", + p.client.Dataset, cleanV, cleanV, cleanV, cleanV) + selectVariants += fmt.Sprintf("jv_%s.variant_value AS variant_%s,\n", cleanV, cleanV) + groupByParts = append(groupByParts, fmt.Sprintf("jv_%s.variant_value", cleanV)) + } + groupByVariants := strings.Join(groupByParts, ", ") + + // Always join Release, SpotCheckComponent, SpotCheckCapability + joinVariants += fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_Release ON jobs.prowjob_job_name = jv_Release.job_name AND jv_Release.variant_name = 'Release'\n", + p.client.Dataset) + joinVariants += fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_SpotCheckComponent ON jobs.prowjob_job_name = jv_SpotCheckComponent.job_name AND jv_SpotCheckComponent.variant_name = 'SpotCheckComponent'\n", + p.client.Dataset) + joinVariants += fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_SpotCheckCapability ON jobs.prowjob_job_name = jv_SpotCheckCapability.job_name AND jv_SpotCheckCapability.variant_name = 'SpotCheckCapability'\n", + p.client.Dataset) + + // Track which variant groups already have JOINs + joinedVariants := sets.NewString(groupByVariantSet.List()...) + joinedVariants.Insert("Release", "SpotCheckComponent", "SpotCheckCapability") + + // Build variant filters from both view-level and spot-check sample-level include_variants + variantFilters := "" + var params []bigquery.QueryParameter + + // Apply view-level include variant filters, but skip SpotCheck-specific variants + viewIncludeVariants := reqOptions.VariantOption.IncludeVariants + if viewIncludeVariants == nil { + viewIncludeVariants = map[string][]string{} + } + for _, group := range sortedKeys(viewIncludeVariants) { + if group == "JobTier" || group == "SpotCheckComponent" || group == "SpotCheckCapability" { + continue + } + cleanGroup := param.Cleanse(group) + if !joinedVariants.Has(group) { + joinVariants += fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_%s ON jobs.prowjob_job_name = jv_%s.job_name AND jv_%s.variant_name = '%s'\n", + p.client.Dataset, cleanGroup, cleanGroup, cleanGroup, cleanGroup) + joinedVariants.Insert(group) + } + paramName := fmt.Sprintf("variantGroup_%s", cleanGroup) + variantFilters += fmt.Sprintf(" AND (jv_%s.variant_value IN UNNEST(@%s))", cleanGroup, paramName) + params = append(params, bigquery.QueryParameter{ + Name: paramName, + Value: viewIncludeVariants[group], + }) + } + + // Apply spot-check sample-level include variant filters (e.g. JobTier: spotcheck-30d). + // During the transition period, also include 'rare' in the JobTier filter to match + // legacy jobs that haven't been reclassified yet. + for _, group := range sortedKeys(spotCheckIncludeVariants) { + cleanGroup := param.Cleanse(group) + if !joinedVariants.Has(group) { + joinVariants += fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_%s ON jobs.prowjob_job_name = jv_%s.job_name AND jv_%s.variant_name = '%s'\n", + p.client.Dataset, cleanGroup, cleanGroup, cleanGroup, cleanGroup) + joinedVariants.Insert(group) + } + values := spotCheckIncludeVariants[group] + if group == "JobTier" { + values = append(values, "rare") + } + paramName := fmt.Sprintf("spotCheck_%s", cleanGroup) + variantFilters += fmt.Sprintf(" AND (jv_%s.variant_value IN UNNEST(@%s))", cleanGroup, paramName) + params = append(params, bigquery.QueryParameter{ + Name: paramName, + Value: values, + }) + } + + // Use COALESCE fallback SQL to derive component/capability from job name substrings + // for legacy jobs that don't yet have the SpotCheckComponent/SpotCheckCapability variants. + queryString := fmt.Sprintf(` + SELECT + %s + %s AS spot_check_component, + %s AS spot_check_capability, + COUNT(DISTINCT jobs.prowjob_build_id) AS total_runs, + COUNT(DISTINCT IF(jobs.prowjob_state = 'success', jobs.prowjob_build_id, NULL)) AS successful_runs, + ARRAY_AGG(DISTINCT jobs.prowjob_job_name) AS job_names, + MAX(IF(jobs.prowjob_state != 'success', TIMESTAMP(jobs.prowjob_start), NULL)) AS last_failure + FROM %s.jobs jobs + %s + WHERE jobs.prowjob_start >= DATETIME(@From) + AND jobs.prowjob_start < DATETIME(@To) + AND jv_Release.variant_value = @Release + AND (jobs.prowjob_job_name LIKE 'periodic-%%' OR jobs.prowjob_job_name LIKE 'release-%%') + %s + GROUP BY %s, spot_check_component, spot_check_capability + HAVING spot_check_component IS NOT NULL AND spot_check_capability IS NOT NULL + `, selectVariants, spotCheckComponentFallback, spotCheckCapabilityFallback, + p.client.Dataset, joinVariants, variantFilters, + groupByVariants) + + params = append(params, + bigquery.QueryParameter{Name: "From", Value: start}, + bigquery.QueryParameter{Name: "To", Value: end}, + bigquery.QueryParameter{Name: "Release", Value: reqOptions.SampleRelease.Name}, + ) + + q := p.client.Query(ctx, bqlabel.CRSpotCheck, queryString) + q.Parameters = params + + it, err := q.Read(ctx) + if err != nil { + return nil, fmt.Errorf("error executing spot check query: %w", err) + } + + results, err := parseSpotCheckRows(it, groupByVariantSet) + if err != nil { + return nil, err + } + + log.Infof("spot check query returned %d groups", len(results)) + return results, nil +} + +// parseSpotCheckRows reads spot-check aggregation results from a BigQuery iterator +// and returns them as SpotCheckGroup slices. +func parseSpotCheckRows(it *bigquery.RowIterator, groupByVariantSet sets.String) ([]dataprovider.SpotCheckGroup, error) { + var results []dataprovider.SpotCheckGroup + for { + var rawRow map[string]bigquery.Value + err := it.Next(&rawRow) + if err == iterator.Done { + break + } + if err != nil { + return nil, fmt.Errorf("error reading spot check row: %w", err) + } + + group := dataprovider.SpotCheckGroup{ + Variants: map[string]string{}, + } + if val, ok := rawRow["spot_check_component"]; ok && val != nil { + group.Component = val.(string) + } + if val, ok := rawRow["spot_check_capability"]; ok && val != nil { + group.Capability = val.(string) + } + for _, v := range groupByVariantSet.List() { + cleanV := param.Cleanse(v) + if val, ok := rawRow["variant_"+cleanV]; ok && val != nil { + group.Variants[v] = val.(string) + } + } + if val, ok := rawRow["total_runs"]; ok && val != nil { + group.TotalRuns = int(val.(int64)) + } + if val, ok := rawRow["successful_runs"]; ok && val != nil { + group.SuccessfulRuns = int(val.(int64)) + } + if val, ok := rawRow["job_names"]; ok && val != nil { + for _, jn := range val.([]bigquery.Value) { + group.JobNames = append(group.JobNames, jn.(string)) + } + } + if val, ok := rawRow["last_failure"]; ok && val != nil { + group.LastFailure = val.(time.Time) + } + results = append(results, group) + } + return results, nil +} + +// QuerySpotCheckJobRunDetails returns individual job runs matching the given variant +// and component/capability filters, used to populate the test details drill-down page +// for a specific spot-check synthetic test. +// includeVariants specifies variant filters (ANDed) from the spot-check sample config. +func (p *BigQueryProvider) QuerySpotCheckJobRunDetails(ctx context.Context, reqOptions reqopts.RequestOptions, + allJobVariants crtest.JobVariants, + spotCheckIncludeVariants map[string][]string, + variants map[string]string, + component, capability string, + start, end time.Time) ([]dataprovider.JobRunDetail, error) { + + joinVariants := fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_Release ON jobs.prowjob_job_name = jv_Release.job_name AND jv_Release.variant_name = 'Release'\n", + p.client.Dataset) + joinVariants += fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_SpotCheckComponent ON jobs.prowjob_job_name = jv_SpotCheckComponent.job_name AND jv_SpotCheckComponent.variant_name = 'SpotCheckComponent'\n", + p.client.Dataset) + joinVariants += fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_SpotCheckCapability ON jobs.prowjob_job_name = jv_SpotCheckCapability.job_name AND jv_SpotCheckCapability.variant_name = 'SpotCheckCapability'\n", + p.client.Dataset) + + joinedVariants := sets.NewString("Release", "SpotCheckComponent", "SpotCheckCapability") + + variantFilters := "" + var params []bigquery.QueryParameter + + // Join and filter by the requested variant dimensions (e.g. Platform, Architecture) + for k, v := range variants { + cleanK := param.Cleanse(k) + if !joinedVariants.Has(k) { + joinVariants += fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_%s ON jobs.prowjob_job_name = jv_%s.job_name AND jv_%s.variant_name = '%s'\n", + p.client.Dataset, cleanK, cleanK, cleanK, cleanK) + joinedVariants.Insert(k) + } + paramName := fmt.Sprintf("variant_%s", cleanK) + variantFilters += fmt.Sprintf(" AND jv_%s.variant_value = @%s", cleanK, paramName) + params = append(params, bigquery.QueryParameter{ + Name: paramName, + Value: v, + }) + } + + // Apply spot-check sample-level include variant filters (e.g. JobTier: spotcheck-30d). + // During the transition period, also include 'rare' in the JobTier filter. + for _, group := range sortedKeys(spotCheckIncludeVariants) { + cleanGroup := param.Cleanse(group) + if !joinedVariants.Has(group) { + joinVariants += fmt.Sprintf( + "LEFT JOIN %s.job_variants jv_%s ON jobs.prowjob_job_name = jv_%s.job_name AND jv_%s.variant_name = '%s'\n", + p.client.Dataset, cleanGroup, cleanGroup, cleanGroup, cleanGroup) + joinedVariants.Insert(group) + } + values := spotCheckIncludeVariants[group] + if group == "JobTier" { + values = append(values, "rare") + } + paramName := fmt.Sprintf("spotCheck_%s", cleanGroup) + variantFilters += fmt.Sprintf(" AND (jv_%s.variant_value IN UNNEST(@%s))", cleanGroup, paramName) + params = append(params, bigquery.QueryParameter{ + Name: paramName, + Value: values, + }) + } + + queryString := fmt.Sprintf(` + SELECT + jobs.prowjob_job_name AS job_name, + jobs.prowjob_build_id AS run_id, + jobs.prowjob_url AS url, + TIMESTAMP(jobs.prowjob_start) AS start_time, + (jobs.prowjob_state = 'success') AS success + FROM %s.jobs jobs + %s + WHERE jobs.prowjob_start >= DATETIME(@From) + AND jobs.prowjob_start < DATETIME(@To) + AND jv_Release.variant_value = @Release + AND (jobs.prowjob_job_name LIKE 'periodic-%%' OR jobs.prowjob_job_name LIKE 'release-%%') + AND LOWER(%s) = LOWER(@SpotCheckComponent) + AND LOWER(%s) = LOWER(@SpotCheckCapability) + %s + ORDER BY jobs.prowjob_start ASC + `, p.client.Dataset, joinVariants, + spotCheckComponentFallback, spotCheckCapabilityFallback, + variantFilters) + + params = append(params, + bigquery.QueryParameter{Name: "From", Value: start}, + bigquery.QueryParameter{Name: "To", Value: end}, + bigquery.QueryParameter{Name: "Release", Value: reqOptions.SampleRelease.Name}, + bigquery.QueryParameter{Name: "SpotCheckComponent", Value: component}, + bigquery.QueryParameter{Name: "SpotCheckCapability", Value: capability}, + ) + + q := p.client.Query(ctx, bqlabel.CRSpotCheckDetails, queryString) + q.Parameters = params + + it, err := q.Read(ctx) + if err != nil { + return nil, fmt.Errorf("error executing spot check details query: %w", err) + } + + type detailRow struct { + JobName string `bigquery:"job_name"` + RunID string `bigquery:"run_id"` + URL string `bigquery:"url"` + StartTime time.Time `bigquery:"start_time"` + Success bool `bigquery:"success"` + } + + var results []dataprovider.JobRunDetail + for { + var row detailRow + err := it.Next(&row) + if err == iterator.Done { + break + } + if err != nil { + return nil, fmt.Errorf("error reading spot check detail row: %w", err) + } + results = append(results, dataprovider.JobRunDetail{ + JobName: row.JobName, + RunID: row.RunID, + URL: row.URL, + StartTime: row.StartTime, + Success: row.Success, + }) + } + + return results, nil +} + // --- Helpers --- func getSingleColumnResultToSlice(ctx context.Context, q *bigquery.Query) ([]string, error) { diff --git a/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go b/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go index 5d22938277..62b1ef4904 100644 --- a/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go +++ b/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go @@ -396,8 +396,9 @@ func BuildComponentReportQuery( joinVariants := "" groupByVariants := "" for _, v := range sortedKeys(allJobVariants.Variants) { + cleanV := param.Cleanse(v) joinVariants += fmt.Sprintf("LEFT JOIN %s.job_variants jv_%s ON junit_data.variant_registry_job_name = jv_%s.job_name AND jv_%s.variant_name = '%s'\n", - client.Dataset, v, v, v, v) + client.Dataset, cleanV, cleanV, cleanV, cleanV) } for _, v := range reqOptions.VariantOption.DBGroupBy.List() { v = param.Cleanse(v) diff --git a/pkg/api/componentreadiness/dataprovider/interface.go b/pkg/api/componentreadiness/dataprovider/interface.go index 37b40f91e2..78081d8678 100644 --- a/pkg/api/componentreadiness/dataprovider/interface.go +++ b/pkg/api/componentreadiness/dataprovider/interface.go @@ -66,12 +66,34 @@ type JobQuerier interface { LookupJobVariants(ctx context.Context, jobName string) (map[string]string, error) } +// SpotCheckQuerier fetches job-level pass/fail data for spot-check analysis. +type SpotCheckQuerier interface { + // QuerySpotCheckJobRuns returns aggregated pass/fail per spot-check group, + // grouped by SpotCheckComponent, SpotCheckCapability, and the DB group-by variants. + // includeVariants specifies variant filters (ANDed) from the spot-check sample config. + QuerySpotCheckJobRuns(ctx context.Context, reqOptions reqopts.RequestOptions, + allJobVariants crtest.JobVariants, + includeVariants map[string][]string, + start, end time.Time) ([]SpotCheckGroup, error) + + // QuerySpotCheckJobRunDetails returns individual job runs for a specific + // spot-check group, used for test details drill-down. + // includeVariants specifies variant filters (ANDed) from the spot-check sample config. + QuerySpotCheckJobRunDetails(ctx context.Context, reqOptions reqopts.RequestOptions, + allJobVariants crtest.JobVariants, + includeVariants map[string][]string, + variants map[string]string, + component, capability string, + start, end time.Time) ([]JobRunDetail, error) +} + // DataProvider combines all query capabilities needed by Component Readiness. type DataProvider interface { TestStatusQuerier TestDetailsQuerier MetadataQuerier JobQuerier + SpotCheckQuerier // Cache returns the cache implementation for storing/retrieving computed results. Cache() cache.Cache @@ -85,3 +107,24 @@ type JobRunStats struct { SuccessfulRuns int `json:"successful_runs"` PassRate float64 `json:"pass_rate"` } + +// SpotCheckGroup contains aggregated pass/fail for a set of spot-check jobs +// sharing the same component, capability, and variant column values. +type SpotCheckGroup struct { + Component string `json:"component"` + Capability string `json:"capability"` + Variants map[string]string `json:"variants"` + TotalRuns int `json:"total_runs"` + SuccessfulRuns int `json:"successful_runs"` + JobNames []string `json:"job_names"` + LastFailure time.Time `json:"last_failure"` +} + +// JobRunDetail contains data for a single job run, used in test details drill-down. +type JobRunDetail struct { + JobName string `json:"job_name"` + RunID string `json:"run_id"` + URL string `json:"url"` + StartTime time.Time `json:"start_time"` + Success bool `json:"success"` +} diff --git a/pkg/api/componentreadiness/dataprovider/postgres/provider.go b/pkg/api/componentreadiness/dataprovider/postgres/provider.go index 206f0dac4b..be1b6fbcc2 100644 --- a/pkg/api/componentreadiness/dataprovider/postgres/provider.go +++ b/pkg/api/componentreadiness/dataprovider/postgres/provider.go @@ -796,3 +796,16 @@ func (p *PostgresProvider) LookupJobVariants(ctx context.Context, jobName string } return parseVariants(row.Variants), nil } + +// --- SpotCheckQuerier --- +// Postgres does not support spot-check queries; these are BigQuery-only. + +func (p *PostgresProvider) QuerySpotCheckJobRuns(_ context.Context, _ reqopts.RequestOptions, + _ crtest.JobVariants, _ map[string][]string, _, _ time.Time) ([]dataprovider.SpotCheckGroup, error) { + return nil, nil +} + +func (p *PostgresProvider) QuerySpotCheckJobRunDetails(_ context.Context, _ reqopts.RequestOptions, + _ crtest.JobVariants, _ map[string][]string, _ map[string]string, _, _ string, _, _ time.Time) ([]dataprovider.JobRunDetail, error) { + return nil, nil +} diff --git a/pkg/api/componentreadiness/middleware/alltestspassrate/alltestspassrate.go b/pkg/api/componentreadiness/middleware/alltestspassrate/alltestspassrate.go new file mode 100644 index 0000000000..10018bc8bd --- /dev/null +++ b/pkg/api/componentreadiness/middleware/alltestspassrate/alltestspassrate.go @@ -0,0 +1,51 @@ +package alltestspassrate + +import ( + "context" + "sync" + + "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/analysis" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crstatus" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" +) + +type AllTestsPassRate struct { + reqOptions reqopts.RequestOptions +} + +func NewAllTestsPassRateMiddleware(reqOptions reqopts.RequestOptions) *AllTestsPassRate { + return &AllTestsPassRate{reqOptions: reqOptions} +} + +func (a *AllTestsPassRate) Query(_ context.Context, _ *sync.WaitGroup, _ crtest.JobVariants, + _, _ chan map[string]crstatus.TestStatus, _ chan error) { +} + +func (a *AllTestsPassRate) QueryTestDetails(_ context.Context, _ *sync.WaitGroup, _ chan error, _ crtest.JobVariants) { +} + +func (a *AllTestsPassRate) PreAnalysis(_ crtest.Identification, _ *testdetails.TestComparison) error { + return nil +} + +func (a *AllTestsPassRate) PostAnalysis(_ crtest.Identification, _ *testdetails.TestComparison) error { + return nil +} + +func (a *AllTestsPassRate) PreTestDetailsAnalysis(_ crtest.KeyWithVariants, _ *crstatus.TestJobRunStatuses) error { + return nil +} + +// Analyze claims all tests when PassRateRequiredAllTests is configured, applying a raw +// pass rate comparison instead of Fisher's exact test. +func (a *AllTestsPassRate) Analyze(_ crtest.Identification, testStats *testdetails.TestComparison) (bool, error) { + opts := a.reqOptions.AdvancedOption + if opts.PassRateRequiredAllTests == 0 { + return false, nil + } + + analysis.BuildPassRateTestStats(testStats, float64(opts.PassRateRequiredAllTests), opts) + return true, nil +} diff --git a/pkg/api/componentreadiness/middleware/alltestspassrate/alltestspassrate_test.go b/pkg/api/componentreadiness/middleware/alltestspassrate/alltestspassrate_test.go new file mode 100644 index 0000000000..58eecb9bf7 --- /dev/null +++ b/pkg/api/componentreadiness/middleware/alltestspassrate/alltestspassrate_test.go @@ -0,0 +1,87 @@ +package alltestspassrate + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/analysis" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" +) + +func TestAllTestsPassRateAnalyze(t *testing.T) { + tests := []struct { + name string + passRateRequiredAll int + minimumFailure int + successCount int + failureCount int + expectHandled bool + expectRegressionStatus bool + }{ + { + name: "PassRateRequiredAllTests=0 returns not handled, testStats unchanged", + passRateRequiredAll: 0, + successCount: 3, + failureCount: 3, + expectHandled: false, + }, + { + name: "PassRateRequiredAllTests>0 with passing run returns handled and NotSignificant", + passRateRequiredAll: 95, + successCount: 10, + failureCount: 0, + expectHandled: true, + expectRegressionStatus: false, + }, + { + name: "PassRateRequiredAllTests>0 with failing run returns handled and regression", + passRateRequiredAll: 95, + minimumFailure: 1, + successCount: 93, + failureCount: 7, + expectHandled: true, + expectRegressionStatus: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mw := NewAllTestsPassRateMiddleware(reqopts.RequestOptions{ + AdvancedOption: reqopts.Advanced{ + PassRateRequiredAllTests: tt.passRateRequiredAll, + MinimumFailure: tt.minimumFailure, + }, + }) + + testStats := &testdetails.TestComparison{ + SampleStats: testdetails.ReleaseStats{ + Stats: crtest.NewTestStats(tt.successCount, tt.failureCount, 0, false), + }, + } + + handled, err := mw.Analyze(crtest.Identification{}, testStats) + require.NoError(t, err) + assert.Equal(t, tt.expectHandled, handled) + + if !tt.expectHandled { + // testStats must not be modified + assert.Equal(t, crtest.Status(0), testStats.ReportStatus) + assert.Empty(t, testStats.Explanations) + return + } + + // analysis.BuildPassRateTestStats must have been applied + if tt.expectRegressionStatus { + assert.Less(t, int(testStats.ReportStatus), 0, "expected a regression status (negative)") + assert.Equal(t, crtest.PassRate, testStats.Comparison) + } else { + assert.Equal(t, crtest.NotSignificant, testStats.ReportStatus) + assert.Contains(t, testStats.Explanations, analysis.ExplanationNoRegression) + } + }) + } +} diff --git a/pkg/api/componentreadiness/middleware/analysis/passrate.go b/pkg/api/componentreadiness/middleware/analysis/passrate.go new file mode 100644 index 0000000000..764e508b68 --- /dev/null +++ b/pkg/api/componentreadiness/middleware/analysis/passrate.go @@ -0,0 +1,39 @@ +package analysis + +import ( + "fmt" + + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" +) + +const ExplanationNoRegression = "No significant regressions found" + +// BuildPassRateTestStats evaluates a test's sample pass rate against a required success rate +// and sets the report status accordingly. +func BuildPassRateTestStats(testStats *testdetails.TestComparison, requiredSuccessRate float64, opts reqopts.Advanced) { + effectiveSuccessReq := requiredSuccessRate + testStats.RequiredPassRateAdjustment + + // Assume 2x our allowed failure rate = an extreme regression. + severeRegressionSuccessRate := effectiveSuccessReq - (100 - requiredSuccessRate) + + sufficientRuns := testStats.SampleStats.Total() >= 6 + + successRate := testStats.SampleStats.PassRate(opts.FlakeAsFailure) + if sufficientRuns && successRate*100 < effectiveSuccessReq && testStats.SampleStats.FailureCount >= opts.MinimumFailure { + rStatus := crtest.SignificantRegression + if successRate*100 < severeRegressionSuccessRate { + rStatus = crtest.ExtremeRegression + } + testStats.ReportStatus = rStatus + testStats.Explanations = append(testStats.Explanations, + fmt.Sprintf("Test has a %.2f%% pass rate, but %.2f%% is required.", successRate*100, effectiveSuccessReq)) + testStats.Comparison = crtest.PassRate + testStats.SampleStats.SuccessRate = successRate + return + } + + testStats.ReportStatus = crtest.NotSignificant + testStats.Explanations = append(testStats.Explanations, ExplanationNoRegression) +} diff --git a/pkg/api/componentreadiness/middleware/fisherexact/fisherexact.go b/pkg/api/componentreadiness/middleware/fisherexact/fisherexact.go new file mode 100644 index 0000000000..f000dd107b --- /dev/null +++ b/pkg/api/componentreadiness/middleware/fisherexact/fisherexact.go @@ -0,0 +1,153 @@ +package fisherexact + +import ( + "context" + "fmt" + "sync" + + "github.com/apache/thrift/lib/go/thrift" + fischer "github.com/glycerine/golang-fisher-exact" + log "github.com/sirupsen/logrus" + + "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/analysis" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crstatus" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" +) + +type FisherExact struct { + reqOptions reqopts.RequestOptions +} + +func NewFisherExactMiddleware(reqOptions reqopts.RequestOptions) *FisherExact { + return &FisherExact{reqOptions: reqOptions} +} + +func (f *FisherExact) Query(_ context.Context, _ *sync.WaitGroup, _ crtest.JobVariants, + _, _ chan map[string]crstatus.TestStatus, _ chan error) { +} + +func (f *FisherExact) QueryTestDetails(_ context.Context, _ *sync.WaitGroup, _ chan error, _ crtest.JobVariants) { +} + +func (f *FisherExact) PreAnalysis(_ crtest.Identification, _ *testdetails.TestComparison) error { + return nil +} + +func (f *FisherExact) PostAnalysis(_ crtest.Identification, _ *testdetails.TestComparison) error { + return nil +} + +func (f *FisherExact) PreTestDetailsAnalysis(_ crtest.KeyWithVariants, _ *crstatus.TestJobRunStatuses) error { + return nil +} + +// Analyze is the catch-all analysis middleware. It always claims the test and performs +// Fisher's exact test to determine regression significance. +func (f *FisherExact) Analyze(_ crtest.Identification, testStats *testdetails.TestComparison) (bool, error) { + logger := log.WithField("middleware", "FisherExact") + opts := f.reqOptions.AdvancedOption + + if testStats.RequiredConfidence == 0 { + testStats.RequiredConfidence = opts.Confidence + } + + fisherExactVal := 0.0 + testStats.Comparison = crtest.FisherExact + + status := crtest.MissingBasis + if testStats.SampleStats.Total() == 0 { + if opts.IgnoreMissing { + status = crtest.NotSignificant + } else { + status = crtest.MissingSample + } + testStats.ReportStatus = status + testStats.FisherExact = thrift.Float64Ptr(0.0) + testStats.Explanations = append(testStats.Explanations, analysis.ExplanationNoRegression) + return true, nil + } else if testStats.BaseStats != nil && testStats.BaseStats.Total() != 0 { + samplePass := testStats.SampleStats.Passes(opts.FlakeAsFailure) + basePass := testStats.BaseStats.Passes(opts.FlakeAsFailure) + basisPassPercentage := float64(basePass) / float64(testStats.BaseStats.Total()) + effectivePityFactor := float64(opts.PityFactor) + testStats.PityAdjustment + effectiveMinimumFailure := opts.MinimumFailure + + status = crtest.NotSignificant + + samplePassPercentage := float64(samplePass) / float64(testStats.SampleStats.Total()) + + if effectiveMinimumFailure != 0 && + (testStats.SampleStats.Total()-samplePass) < effectiveMinimumFailure { + if status <= crtest.SignificantTriagedRegression { + testStats.Explanations = append(testStats.Explanations, + fmt.Sprintf("%s regression detected.", crtest.StringForStatus(status))) + } + testStats.ReportStatus = status + testStats.FisherExact = thrift.Float64Ptr(0.0) + return true, nil + } + significant := false + improved := samplePassPercentage >= basisPassPercentage + + if improved { + significant, fisherExactVal = fisherExactTest(testStats.RequiredConfidence, testStats.BaseStats.Total()-basePass, basePass, testStats.SampleStats.Total()-samplePass, samplePass) + } else if basisPassPercentage-samplePassPercentage > effectivePityFactor/100 { + significant, fisherExactVal = fisherExactTest(testStats.RequiredConfidence, testStats.SampleStats.Total()-samplePass, samplePass, testStats.BaseStats.Total()-basePass, basePass) + } + logger.Debugf("computed Fisher info: signifcant: %v, fisherExact: %v", significant, fisherExactVal) + if significant { + if improved { + status = crtest.SignificantImprovement + } else { + status = getRegressionStatus(basisPassPercentage, samplePassPercentage) + } + } + } + logger.Debugf("computed status: %d", int(status)) + testStats.ReportStatus = status + testStats.FisherExact = thrift.Float64Ptr(fisherExactVal) + + baseRelease := "no basis" + if testStats.BaseStats != nil { + baseRelease = testStats.BaseStats.Release + } + if testStats.ReportStatus <= crtest.SignificantTriagedRegression { + logger.Debugf("regression detected against: %s", baseRelease) + + if testStats.ReportStatus <= crtest.SignificantRegression { + testStats.Explanations = append(testStats.Explanations, + fmt.Sprintf("%s regression detected.", crtest.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))) + } else { + testStats.Explanations = append(testStats.Explanations, + fmt.Sprintf("%s regression detected.", crtest.StringForStatus(testStats.ReportStatus))) + } + } else { + logger.Debugf("NO regression detected against: %s", baseRelease) + } + + return true, nil +} + +func fisherExactTest(confidenceRequired, sampleFailure, sampleSuccess, baseFailure, baseSuccess int) (bool, float64) { + _, _, r, _ := fischer.FisherExactTest(sampleFailure, + sampleSuccess, + baseFailure, + baseSuccess) + return r < 1-float64(confidenceRequired)/100, r +} + +func getRegressionStatus(basisPassPercentage, samplePassPercentage float64) crtest.Status { + if (basisPassPercentage - samplePassPercentage) > 0.15 { + return crtest.ExtremeRegression + } + + return crtest.SignificantRegression +} diff --git a/pkg/api/componentreadiness/middleware/fisherexact/fisherexact_test.go b/pkg/api/componentreadiness/middleware/fisherexact/fisherexact_test.go new file mode 100644 index 0000000000..1e30beb769 --- /dev/null +++ b/pkg/api/componentreadiness/middleware/fisherexact/fisherexact_test.go @@ -0,0 +1,206 @@ +package fisherexact + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/analysis" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" +) + +// baseStats constructs a *testdetails.ReleaseStats with the given pass/fail counts. +func baseStats(success, failure int) *testdetails.ReleaseStats { + return &testdetails.ReleaseStats{ + Stats: crtest.NewTestStats(success, failure, 0, false), + } +} + +// sampleStats constructs a testdetails.ReleaseStats with the given pass/fail counts. +func sampleStats(success, failure int) testdetails.ReleaseStats { + return testdetails.ReleaseStats{ + Stats: crtest.NewTestStats(success, failure, 0, false), + } +} + +func TestFisherExactAnalyze(t *testing.T) { + tests := []struct { + name string + opts reqopts.Advanced + inputStats *testdetails.TestComparison + wantHandled bool + wantStatus crtest.Status + wantFisherZero bool // true: assert FisherExact == 0.0 + wantExplanations int // expected len(Explanations); -1 means skip check + explanationContains string + wantConfidence int // if non-zero, assert testStats.RequiredConfidence == this after call + }{ + { + name: "sample total=0, IgnoreMissing=false → MissingSample", + opts: reqopts.Advanced{IgnoreMissing: false, Confidence: 95}, + inputStats: &testdetails.TestComparison{ + SampleStats: sampleStats(0, 0), + BaseStats: baseStats(900, 100), + }, + wantHandled: true, + wantStatus: crtest.MissingSample, + wantFisherZero: true, + wantExplanations: 1, + explanationContains: analysis.ExplanationNoRegression, + }, + { + name: "sample total=0, IgnoreMissing=true → NotSignificant", + opts: reqopts.Advanced{IgnoreMissing: true, Confidence: 95}, + inputStats: &testdetails.TestComparison{ + SampleStats: sampleStats(0, 0), + BaseStats: baseStats(900, 100), + }, + wantHandled: true, + wantStatus: crtest.NotSignificant, + wantFisherZero: true, + wantExplanations: 1, + explanationContains: analysis.ExplanationNoRegression, + }, + { + name: "minimum-failure triage: failures below threshold → NotSignificant, no Fisher", + // sample has 5 failures, MinimumFailure=10 → early return + opts: reqopts.Advanced{Confidence: 95, MinimumFailure: 10, PityFactor: 5}, + inputStats: &testdetails.TestComparison{ + RequiredConfidence: 95, + SampleStats: sampleStats(95, 5), + BaseStats: baseStats(80, 20), + }, + wantHandled: true, + wantStatus: crtest.NotSignificant, + wantFisherZero: true, + wantExplanations: 0, + }, + { + name: "improved performance, Fisher significant → SignificantImprovement", + // base 50%, sample 90%: massive improvement, Fisher will be significant at 95% + opts: reqopts.Advanced{Confidence: 95}, + inputStats: &testdetails.TestComparison{ + RequiredConfidence: 95, + SampleStats: sampleStats(900, 100), + BaseStats: baseStats(500, 500), + }, + wantHandled: true, + wantStatus: crtest.SignificantImprovement, + wantFisherZero: false, + wantExplanations: 0, + }, + { + name: "degraded within pity → NotSignificant, no Fisher call", + // base 95%, sample 93%: drop 2% < PityFactor 5% + PityAdjustment 1% = 6% + opts: reqopts.Advanced{Confidence: 95, PityFactor: 5}, + inputStats: &testdetails.TestComparison{ + RequiredConfidence: 95, + PityAdjustment: 1.0, + SampleStats: sampleStats(93, 7), + BaseStats: baseStats(95, 5), + }, + wantHandled: true, + wantStatus: crtest.NotSignificant, + wantFisherZero: false, // FisherExact set to the computed (zero) val at end + wantExplanations: 0, + }, + { + name: "degraded beyond pity, drop 13% → SignificantRegression", + // base 95%, sample 82%: drop 13% < 15%, Fisher significant at 95% + opts: reqopts.Advanced{Confidence: 95, PityFactor: 5}, + inputStats: &testdetails.TestComparison{ + RequiredConfidence: 95, + SampleStats: sampleStats(820, 180), + BaseStats: baseStats(950, 50), + }, + wantHandled: true, + wantStatus: crtest.SignificantRegression, + wantFisherZero: false, + wantExplanations: 3, + explanationContains: "Significant", + }, + { + name: "degraded beyond pity, drop 25% → ExtremeRegression", + // base 95%, sample 70%: drop 25% > 15%, Fisher significant at 95% + opts: reqopts.Advanced{Confidence: 95, PityFactor: 5}, + inputStats: &testdetails.TestComparison{ + RequiredConfidence: 95, + SampleStats: sampleStats(700, 300), + BaseStats: baseStats(950, 50), + }, + wantHandled: true, + wantStatus: crtest.ExtremeRegression, + wantFisherZero: false, + wantExplanations: 3, + explanationContains: "Extreme", + }, + { + name: "RequiredConfidence=0 inherits opts.Confidence", + // Use missing-sample to keep the outcome predictable; focus is the confidence field. + opts: reqopts.Advanced{IgnoreMissing: true, Confidence: 97}, + inputStats: &testdetails.TestComparison{ + RequiredConfidence: 0, + SampleStats: sampleStats(0, 0), + }, + wantHandled: true, + wantStatus: crtest.NotSignificant, + wantFisherZero: true, + wantExplanations: 1, // missing-sample path always appends ExplanationNoRegression + wantConfidence: 97, + }, + { + name: "RequiredConfidence pre-set is not overwritten by opts.Confidence", + opts: reqopts.Advanced{IgnoreMissing: true, Confidence: 95}, + inputStats: &testdetails.TestComparison{ + RequiredConfidence: 90, + SampleStats: sampleStats(0, 0), + }, + wantHandled: true, + wantStatus: crtest.NotSignificant, + wantFisherZero: true, + wantExplanations: 1, + wantConfidence: 90, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mw := NewFisherExactMiddleware(reqopts.RequestOptions{ + AdvancedOption: tt.opts, + }) + + handled, err := mw.Analyze(crtest.Identification{}, tt.inputStats) + require.NoError(t, err) + assert.Equal(t, tt.wantHandled, handled) + assert.Equal(t, tt.wantStatus, tt.inputStats.ReportStatus) + + require.NotNil(t, tt.inputStats.FisherExact) + if tt.wantFisherZero { + assert.Equal(t, 0.0, *tt.inputStats.FisherExact) + } else { + assert.GreaterOrEqual(t, *tt.inputStats.FisherExact, 0.0) + } + + if tt.wantExplanations >= 0 { + assert.Len(t, tt.inputStats.Explanations, tt.wantExplanations) + } + if tt.explanationContains != "" { + assert.True(t, func() bool { + for _, e := range tt.inputStats.Explanations { + if strings.Contains(e, tt.explanationContains) { + return true + } + } + return false + }(), "expected an explanation containing %q, got: %v", tt.explanationContains, tt.inputStats.Explanations) + } + if tt.wantConfidence != 0 { + assert.Equal(t, tt.wantConfidence, tt.inputStats.RequiredConfidence) + } + }) + } +} diff --git a/pkg/api/componentreadiness/middleware/interface.go b/pkg/api/componentreadiness/middleware/interface.go index db846a7c8d..103da54be6 100644 --- a/pkg/api/componentreadiness/middleware/interface.go +++ b/pkg/api/componentreadiness/middleware/interface.go @@ -34,6 +34,12 @@ type Middleware interface { // This allows for cheap reloads with fresh triage data without having to do an expensive report recalculation. PostAnalysis(testKey crtest.Identification, testStats *testdetails.TestComparison) error + // Analyze gives middleware the opportunity to perform statistical analysis for a test, + // replacing the default Fisher's exact / pass-rate logic. Return true if this middleware + // handled the analysis (first responder wins, subsequent middleware will not be called). + // Return false to defer to the next middleware or the default analysis. + Analyze(testKey crtest.Identification, testStats *testdetails.TestComparison) (bool, error) + // PreTestDetailsAnalysis gives middleware the opportunity to adjust inputs to the report status // prior to analysis. PreTestDetailsAnalysis(testKey crtest.KeyWithVariants, status *crstatus.TestJobRunStatuses) error diff --git a/pkg/api/componentreadiness/middleware/linkinjector/linkinjector.go b/pkg/api/componentreadiness/middleware/linkinjector/linkinjector.go index 84bb66d4cd..d644789f37 100644 --- a/pkg/api/componentreadiness/middleware/linkinjector/linkinjector.go +++ b/pkg/api/componentreadiness/middleware/linkinjector/linkinjector.go @@ -45,6 +45,10 @@ func (l *LinkInjector) PreAnalysis(testKey crtest.Identification, testStats *tes return nil } +func (l *LinkInjector) Analyze(_ crtest.Identification, _ *testdetails.TestComparison) (bool, error) { + return false, nil +} + // PostAnalysis injects HATEOAS links into test analysis results func (l *LinkInjector) PostAnalysis(testKey crtest.Identification, testStats *testdetails.TestComparison) error { // Early return if status is above FixedRegression (i.e. regression has not yet rolled off) diff --git a/pkg/api/componentreadiness/middleware/list.go b/pkg/api/componentreadiness/middleware/list.go index 5205677e33..95ee5e0b9a 100644 --- a/pkg/api/componentreadiness/middleware/list.go +++ b/pkg/api/componentreadiness/middleware/list.go @@ -34,6 +34,19 @@ func (l List) PreAnalysis(testKey crtest.Identification, testStats *testdetails. return nil } +func (l List) Analyze(testKey crtest.Identification, testStats *testdetails.TestComparison) (bool, error) { + for _, mw := range l { + handled, err := mw.Analyze(testKey, testStats) + if err != nil { + return false, err + } + if handled { + return true, nil + } + } + return false, nil +} + func (l List) PostAnalysis(testKey crtest.Identification, testStats *testdetails.TestComparison) error { for _, mw := range l { if err := mw.PostAnalysis(testKey, testStats); err != nil { diff --git a/pkg/api/componentreadiness/middleware/newtestpassrate/newtestpassrate.go b/pkg/api/componentreadiness/middleware/newtestpassrate/newtestpassrate.go new file mode 100644 index 0000000000..749fe8c664 --- /dev/null +++ b/pkg/api/componentreadiness/middleware/newtestpassrate/newtestpassrate.go @@ -0,0 +1,56 @@ +package newtestpassrate + +import ( + "context" + "sync" + + "github.com/openshift/sippy/pkg/api/componentreadiness/middleware/analysis" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crstatus" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" +) + +type NewTestPassRate struct { + reqOptions reqopts.RequestOptions +} + +func NewNewTestPassRateMiddleware(reqOptions reqopts.RequestOptions) *NewTestPassRate { + return &NewTestPassRate{reqOptions: reqOptions} +} + +func (n *NewTestPassRate) Query(_ context.Context, _ *sync.WaitGroup, _ crtest.JobVariants, + _, _ chan map[string]crstatus.TestStatus, _ chan error) { +} + +func (n *NewTestPassRate) QueryTestDetails(_ context.Context, _ *sync.WaitGroup, _ chan error, _ crtest.JobVariants) { +} + +func (n *NewTestPassRate) PreAnalysis(_ crtest.Identification, _ *testdetails.TestComparison) error { + return nil +} + +func (n *NewTestPassRate) PostAnalysis(_ crtest.Identification, _ *testdetails.TestComparison) error { + return nil +} + +func (n *NewTestPassRate) PreTestDetailsAnalysis(_ crtest.KeyWithVariants, _ *crstatus.TestJobRunStatuses) error { + return nil +} + +// Analyze claims tests that have no base stats (new tests) when PassRateRequiredNewTests is configured. +func (n *NewTestPassRate) Analyze(_ crtest.Identification, testStats *testdetails.TestComparison) (bool, error) { + opts := n.reqOptions.AdvancedOption + if opts.PassRateRequiredNewTests == 0 { + return false, nil + } + if testStats.BaseStats != nil && testStats.BaseStats.Total() > 0 { + return false, nil + } + + analysis.BuildPassRateTestStats(testStats, float64(opts.PassRateRequiredNewTests), opts) + if testStats.ReportStatus == crtest.NotSignificant && opts.PassRateRequiredAllTests == 0 { + testStats.ReportStatus = crtest.MissingBasis + } + return true, nil +} diff --git a/pkg/api/componentreadiness/middleware/newtestpassrate/newtestpassrate_test.go b/pkg/api/componentreadiness/middleware/newtestpassrate/newtestpassrate_test.go new file mode 100644 index 0000000000..354420062f --- /dev/null +++ b/pkg/api/componentreadiness/middleware/newtestpassrate/newtestpassrate_test.go @@ -0,0 +1,106 @@ +package newtestpassrate + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" +) + +func TestNewTestPassRateAnalyze(t *testing.T) { + existingBase := &testdetails.ReleaseStats{ + Stats: crtest.NewTestStats(80, 20, 0, false), + } + + tests := []struct { + name string + passRateNewTests int + passRateAllTests int + baseStats *testdetails.ReleaseStats + sampleSuccess int + sampleFailure int + wantHandled bool + wantStatus crtest.Status + }{ + { + name: "PassRateRequiredNewTests=0 → not handled", + passRateNewTests: 0, + baseStats: nil, + sampleSuccess: 5, + sampleFailure: 5, + wantHandled: false, + wantStatus: 0, // unchanged zero value + }, + { + name: "BaseStats non-nil with runs → not handled", + passRateNewTests: 95, + baseStats: existingBase, + sampleSuccess: 8, + sampleFailure: 2, + wantHandled: false, + wantStatus: 0, + }, + { + name: "new test, passing sample, PassRateRequiredAllTests=0 → MissingBasis", + // 10 successes, 0 failures → 100% pass rate > 95% required → NotSignificant from + // BuildPassRateTestStats, then overridden to MissingBasis because PassRateRequiredAllTests==0. + passRateNewTests: 95, + passRateAllTests: 0, + baseStats: nil, + sampleSuccess: 10, + sampleFailure: 0, + wantHandled: true, + wantStatus: crtest.MissingBasis, + }, + { + name: "new test, passing sample, PassRateRequiredAllTests>0 → NotSignificant", + // Same passing stats but PassRateRequiredAllTests is set, so MissingBasis override is skipped. + passRateNewTests: 95, + passRateAllTests: 90, + baseStats: nil, + sampleSuccess: 10, + sampleFailure: 0, + wantHandled: true, + wantStatus: crtest.NotSignificant, + }, + { + name: "new test, failing sample → regression status", + // 3 successes, 4 failures: Total=7 (≥6), pass rate ≈42.8% < 95% required, + // drop > allowed → ExtremeRegression from BuildPassRateTestStats. + passRateNewTests: 95, + passRateAllTests: 0, + baseStats: nil, + sampleSuccess: 3, + sampleFailure: 4, + wantHandled: true, + wantStatus: crtest.ExtremeRegression, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mw := NewNewTestPassRateMiddleware(reqopts.RequestOptions{ + AdvancedOption: reqopts.Advanced{ + PassRateRequiredNewTests: tt.passRateNewTests, + PassRateRequiredAllTests: tt.passRateAllTests, + }, + }) + + testStats := &testdetails.TestComparison{ + BaseStats: tt.baseStats, + SampleStats: testdetails.ReleaseStats{ + Stats: crtest.NewTestStats(tt.sampleSuccess, tt.sampleFailure, 0, false), + }, + } + + handled, err := mw.Analyze(crtest.Identification{}, testStats) + require.NoError(t, err) + assert.Equal(t, tt.wantHandled, handled) + assert.Equal(t, tt.wantStatus, testStats.ReportStatus) + }) + } +} diff --git a/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances.go b/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances.go index 7e79cdb435..5e7cc69ce9 100644 --- a/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances.go +++ b/pkg/api/componentreadiness/middleware/regressionallowances/regressionallowances.go @@ -61,6 +61,10 @@ func (r *RegressionAllowances) PreAnalysis(testKey crtest.Identification, testSt return nil } +func (r *RegressionAllowances) Analyze(_ crtest.Identification, _ *testdetails.TestComparison) (bool, error) { + return false, nil +} + func (r *RegressionAllowances) PostAnalysis(testKey crtest.Identification, testStats *testdetails.TestComparison) error { return nil } diff --git a/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go b/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go index 124336d251..c5fc661ebe 100644 --- a/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go +++ b/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go @@ -104,6 +104,10 @@ func (r *RegressionTracker) PreAnalysis(testKey crtest.Identification, testStats return nil } +func (r *RegressionTracker) Analyze(_ crtest.Identification, _ *testdetails.TestComparison) (bool, error) { + return false, nil +} + // PostAnalysis adjusts triages and status code (and thus icons) based on the triaged state of open regressions. func (r *RegressionTracker) PostAnalysis(testKey crtest.Identification, testStats *testdetails.TestComparison) error { if testStats.ReportStatus > crtest.SignificantTriagedRegression { diff --git a/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go b/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go index 25dc1d83bf..3e7ee3cf91 100644 --- a/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go +++ b/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go @@ -67,8 +67,8 @@ type ReleaseFallback struct { releaseConfigs []v1.Release } -func (r *ReleaseFallback) Analyze(testID string, variants map[string]string, report *testdetails.TestComparison) error { - return nil +func (r *ReleaseFallback) Analyze(_ crtest.Identification, _ *testdetails.TestComparison) (bool, error) { + return false, nil } func (r *ReleaseFallback) Query(ctx context.Context, wg *sync.WaitGroup, allJobVariants crtest.JobVariants, diff --git a/pkg/api/componentreadiness/middleware/spotcheckjobs/spotcheckjobs.go b/pkg/api/componentreadiness/middleware/spotcheckjobs/spotcheckjobs.go new file mode 100644 index 0000000000..36962b1620 --- /dev/null +++ b/pkg/api/componentreadiness/middleware/spotcheckjobs/spotcheckjobs.go @@ -0,0 +1,351 @@ +package spotcheckjobs + +import ( + "context" + "fmt" + "strings" + "sync" + + "github.com/openshift/sippy/pkg/api/componentreadiness/dataprovider" + "github.com/openshift/sippy/pkg/api/componentreadiness/middleware" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crstatus" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" + log "github.com/sirupsen/logrus" +) + +var _ middleware.Middleware = &SpotCheckJobs{} + +// NewSpotCheckJobsMiddleware creates middleware that injects synthetic test results for +// spot-check jobs (e.g. cpu-partitioning, etcd-scaling) that don't run in the standard +// junit-based test pipeline. These jobs are evaluated on a simple pass/fail basis: +// at least one successful run in the sample window means healthy. +// +// Jobs are identified by the SpotCheckComponent and SpotCheckCapability variants in the +// variant registry. The component/capability values control where the synthetic test +// results appear in the component readiness report. +// +// Multiple spot-check samples can be configured (e.g. spotcheck-30d, spotcheck-90d), +// each with its own time window and variant filters. Each sample produces a separate +// set of synthetic test results. +func NewSpotCheckJobsMiddleware( + provider dataprovider.DataProvider, + reqOptions reqopts.RequestOptions, +) *SpotCheckJobs { + return &SpotCheckJobs{ + dataProvider: provider, + reqOptions: reqOptions, + log: log.WithField("middleware", "SpotCheckJobs"), + } +} + +type SpotCheckJobs struct { + dataProvider dataprovider.DataProvider + reqOptions reqopts.RequestOptions + log log.FieldLogger + + // sampleJobDetails is populated during QueryTestDetails and consumed by PreTestDetailsAnalysis. + sampleJobDetails map[string][]dataprovider.JobRunDetail + sampleJobDetailsMutex sync.Mutex +} + +// Query fetches aggregated spot-check job results from BigQuery for each configured +// sample, creates synthetic test statuses (one per component/capability/variant group +// per sample), and injects them into the sample status channel. +func (s *SpotCheckJobs) Query(ctx context.Context, wg *sync.WaitGroup, + allJobVariants crtest.JobVariants, + _, sampleStatusCh chan map[string]crstatus.TestStatus, errCh chan error) { + + if len(s.reqOptions.SpotCheckJobSamples) == 0 { + return + } + + for _, sample := range s.reqOptions.SpotCheckJobSamples { + wg.Go(func() { + select { + case <-ctx.Done(): + s.log.Info("context canceled during spot check query") + return + default: + } + + groups, err := s.dataProvider.QuerySpotCheckJobRuns(ctx, + s.reqOptions, allJobVariants, sample.IncludeVariants, + sample.Start, sample.End) + if err != nil { + errCh <- fmt.Errorf("spot check query failed for %s: %w", sample.Name, err) + return + } + + sampleStatus := map[string]crstatus.TestStatus{} + requestedVariants := map[string]string{} + if len(s.reqOptions.TestIDOptions) > 0 { + requestedVariants = s.reqOptions.TestIDOptions[0].RequestedVariants + } + for _, group := range groups { + if group.Component == "" || group.Capability == "" { + s.log.Warnf("skipping spot-check group with empty component/capability: %+v", group) + continue + } + + if !variantsMatch(group.Variants, requestedVariants) { + continue + } + + testKey := crtest.KeyWithVariants{ + TestID: syntheticTestID(sample.Name, group.Component, group.Capability), + Variants: group.Variants, + } + keyStr := testKey.KeyOrDie() + + sampleStatus[keyStr] = crstatus.TestStatus{ + TestName: syntheticTestName(group.Component, group.Capability), + Component: group.Component, + Capabilities: []string{group.Capability}, + Variants: variantMapToSlice(group.Variants), + Count: crtest.Count{ + TotalCount: group.TotalRuns, + SuccessCount: group.SuccessfulRuns, + }, + LastFailure: group.LastFailure, + } + } + + s.log.Infof("injecting %d spot-check synthetic test results for sample %s", len(sampleStatus), sample.Name) + sampleStatusCh <- sampleStatus + }) + } +} + +// QueryTestDetails fetches individual job run details for spot-check synthetic tests, +// storing them for later use by PreTestDetailsAnalysis to populate the drill-down view. +func (s *SpotCheckJobs) QueryTestDetails(ctx context.Context, wg *sync.WaitGroup, + errCh chan error, allJobVariants crtest.JobVariants) { + + if len(s.reqOptions.SpotCheckJobSamples) == 0 { + return + } + + for _, testIDOpt := range s.reqOptions.TestIDOptions { + if !isSpotCheckTestID(testIDOpt.TestID) { + continue + } + + sampleName, component, capability := componentCapabilityFromTestID(testIDOpt.TestID) + if component == "" || capability == "" { + s.log.Warnf("could not parse component/capability from spot-check test ID %s", testIDOpt.TestID) + continue + } + + sample := s.findSample(sampleName) + if sample == nil { + s.log.Warnf("no spot-check sample found for tier %s in test ID %s", sampleName, testIDOpt.TestID) + continue + } + + wg.Go(func() { + select { + case <-ctx.Done(): + return + default: + } + + details, err := s.dataProvider.QuerySpotCheckJobRunDetails(ctx, + s.reqOptions, allJobVariants, sample.IncludeVariants, + testIDOpt.RequestedVariants, component, capability, + sample.Start, sample.End) + if err != nil { + errCh <- fmt.Errorf("spot check details query failed: %w", err) + return + } + + testKey := crtest.KeyWithVariants{ + TestID: testIDOpt.TestID, + Variants: testIDOpt.RequestedVariants, + } + + s.sampleJobDetailsMutex.Lock() + defer s.sampleJobDetailsMutex.Unlock() + if s.sampleJobDetails == nil { + s.sampleJobDetails = map[string][]dataprovider.JobRunDetail{} + } + s.sampleJobDetails[testKey.KeyOrDie()] = details + s.log.WithField("variants", testIDOpt.RequestedVariants).Infof("loaded %d spot-check job run details for %s", len(details), testIDOpt.TestID) + }) + } +} + +func (s *SpotCheckJobs) PreAnalysis(_ crtest.Identification, + _ *testdetails.TestComparison) error { + return nil +} + +// Analyze claims spot-check tests and determines their status. The heuristic is: +// - Any successful run in the sample window = healthy (NotSignificant) +// - A single failed run with no successes = pending retry (MissingSample), since an +// external component will trigger a retry for failed spot-check jobs +// - Two or more failed runs with no successes = confirmed regression (ExtremeRegression) +// - No runs at all = no data (MissingSample) +// +// Returns false for non-spot-check tests to defer to other analyzers. +func (s *SpotCheckJobs) Analyze(testKey crtest.Identification, + testStats *testdetails.TestComparison) (bool, error) { + + if !isSpotCheckTestID(testKey.TestID) { + return false, nil + } + + sampleName, _, _ := componentCapabilityFromTestID(testKey.TestID) + sample := s.findSample(sampleName) + sampleDays := 0 + if sample != nil { + sampleDays = int(sample.End.Sub(sample.Start).Hours() / 24) + } + + totalRuns := testStats.SampleStats.Total() + successfulRuns := testStats.SampleStats.SuccessCount + failedRuns := totalRuns - successfulRuns + + switch { + case successfulRuns > 0: + testStats.ReportStatus = crtest.NotSignificant + testStats.Explanations = append(testStats.Explanations, + fmt.Sprintf("Spot-check job passed %d out of %d runs in the %d-day sample window", + successfulRuns, totalRuns, sampleDays)) + case failedRuns >= 3: + testStats.ReportStatus = crtest.ExtremeRegression + testStats.Explanations = append(testStats.Explanations, + fmt.Sprintf("Spot-check job did not pass in the %d-day sample window (%d runs, 0 successes)", + sampleDays, totalRuns)) + case failedRuns == 2: + testStats.ReportStatus = crtest.SignificantRegression + testStats.Explanations = append(testStats.Explanations, + fmt.Sprintf("Spot-check job failed %d times in the %d-day sample window with no successes", + failedRuns, sampleDays)) + case failedRuns == 1: + testStats.ReportStatus = crtest.MissingSample + testStats.Explanations = append(testStats.Explanations, + fmt.Sprintf("Spot-check job failed once in the %d-day sample window; awaiting retry before flagging regression", + sampleDays)) + default: + // No runs at all + testStats.ReportStatus = crtest.MissingSample + testStats.Explanations = append(testStats.Explanations, + fmt.Sprintf("No spot-check job runs found in the %d-day sample window", sampleDays)) + } + + testStats.Comparison = crtest.SpotCheck + testStats.BaseStats = nil + + return true, nil +} + +func (s *SpotCheckJobs) PostAnalysis(_ crtest.Identification, _ *testdetails.TestComparison) error { + return nil +} + +// PreTestDetailsAnalysis populates the test details drill-down view for spot-check tests +// by converting the cached job run details into TestJobRunRows for the sample side. +// There is no base side since spot-check tests have no historical baseline comparison. +func (s *SpotCheckJobs) PreTestDetailsAnalysis(testKey crtest.KeyWithVariants, + status *crstatus.TestJobRunStatuses) error { + + if !isSpotCheckTestID(testKey.TestID) { + return nil + } + + s.sampleJobDetailsMutex.Lock() + details := s.sampleJobDetails[testKey.KeyOrDie()] + s.sampleJobDetailsMutex.Unlock() + + if status.SampleStatus == nil { + status.SampleStatus = map[string][]crstatus.TestJobRunRows{} + } + + for _, run := range details { + successCount := 0 + if run.Success { + successCount = 1 + } + row := crstatus.TestJobRunRows{ + TestKey: testKey, + TestKeyStr: testKey.KeyOrDie(), + TestName: syntheticTestNameFromID(testKey.TestID), + ProwJob: run.JobName, + ProwJobRunID: run.RunID, + ProwJobURL: run.URL, + StartTime: run.StartTime, + Count: crtest.Count{ + TotalCount: 1, + SuccessCount: successCount, + }, + } + status.SampleStatus[run.JobName] = append(status.SampleStatus[run.JobName], row) + } + + return nil +} + +func (s *SpotCheckJobs) findSample(name string) *reqopts.SpotCheckJobSampleOpts { + for i := range s.reqOptions.SpotCheckJobSamples { + if s.reqOptions.SpotCheckJobSamples[i].Name == name { + return &s.reqOptions.SpotCheckJobSamples[i] + } + } + return nil +} + +func isSpotCheckTestID(testID string) bool { + return strings.HasPrefix(testID, "spotcheck-") +} + +// componentCapabilityFromTestID extracts the sample name, component and capability from +// a synthetic spot-check test ID. The format is "spotcheck-30d:component:capability". +func componentCapabilityFromTestID(testID string) (string, string, string) { + parts := strings.SplitN(testID, ":", 3) + if len(parts) != 3 { + return "", "", "" + } + component := parts[1] + capability := strings.ReplaceAll(parts[2], "-", " ") + return parts[0], component, capability +} + +func syntheticTestID(sampleName, component, capability string) string { + return fmt.Sprintf("%s:%s:%s", + sampleName, + strings.ToLower(component), + strings.ToLower(strings.ReplaceAll(capability, " ", "-"))) +} + +func syntheticTestName(component, capability string) string { + return fmt.Sprintf("[spot-check] %s / %s job must pass at least once per sample window", + component, capability) +} + +func syntheticTestNameFromID(testID string) string { + parts := strings.SplitN(testID, ":", 3) + if len(parts) == 3 { + return fmt.Sprintf("[spot-check] %s / %s job must pass at least once per sample window", + parts[1], strings.ReplaceAll(parts[2], "-", " ")) + } + return testID +} + +func variantsMatch(groupVariants, requestedVariants map[string]string) bool { + for k, v := range requestedVariants { + if gv, ok := groupVariants[k]; !ok || gv != v { + return false + } + } + return true +} + +func variantMapToSlice(m map[string]string) []string { + result := make([]string, 0, len(m)) + for k, v := range m { + result = append(result, k+":"+v) + } + return result +} diff --git a/pkg/api/componentreadiness/middleware/spotcheckjobs/spotcheckjobs_test.go b/pkg/api/componentreadiness/middleware/spotcheckjobs/spotcheckjobs_test.go new file mode 100644 index 0000000000..91d53b8464 --- /dev/null +++ b/pkg/api/componentreadiness/middleware/spotcheckjobs/spotcheckjobs_test.go @@ -0,0 +1,340 @@ +package spotcheckjobs + +import ( + "context" + "strings" + "sync" + "testing" + "time" + + log "github.com/sirupsen/logrus" + + "github.com/openshift/sippy/pkg/api/componentreadiness/dataprovider" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crstatus" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// stubProvider returns a fixed set of SpotCheckGroups; all other methods panic. +type stubProvider struct { + dataprovider.DataProvider // embed to satisfy the interface; only spot-check methods are implemented + groups []dataprovider.SpotCheckGroup +} + +func (s *stubProvider) QuerySpotCheckJobRuns(_ context.Context, _ reqopts.RequestOptions, + _ crtest.JobVariants, _ map[string][]string, _, _ time.Time) ([]dataprovider.SpotCheckGroup, error) { + return s.groups, nil +} + +func (s *stubProvider) QuerySpotCheckJobRunDetails(_ context.Context, _ reqopts.RequestOptions, + _ crtest.JobVariants, _ map[string][]string, _ map[string]string, _, _ string, _, _ time.Time) ([]dataprovider.JobRunDetail, error) { + return nil, nil +} + +// runQuery is a test helper that runs the middleware Query and collects injected sample statuses. +func runQuery(t *testing.T, mw *SpotCheckJobs) map[string]crstatus.TestStatus { + t.Helper() + wg := &sync.WaitGroup{} + baseCh := make(chan map[string]crstatus.TestStatus, 1) + sampleCh := make(chan map[string]crstatus.TestStatus, 10) + errCh := make(chan error, 10) + + mw.Query(context.Background(), wg, crtest.JobVariants{}, baseCh, sampleCh, errCh) + wg.Wait() + close(sampleCh) + close(errCh) + + for err := range errCh { + t.Fatalf("unexpected error from Query: %v", err) + } + + merged := map[string]crstatus.TestStatus{} + for batch := range sampleCh { + for k, v := range batch { + merged[k] = v + } + } + return merged +} + +func TestVariantsMatch(t *testing.T) { + tests := []struct { + name string + groupVariants map[string]string + requestedVariants map[string]string + expected bool + }{ + { + name: "empty requested matches everything", + groupVariants: map[string]string{"Network": "ovn", "Platform": "aws"}, + requestedVariants: map[string]string{}, + expected: true, + }, + { + name: "nil requested matches everything", + groupVariants: map[string]string{"Network": "ovn", "Platform": "aws"}, + requestedVariants: nil, + expected: true, + }, + { + name: "single match", + groupVariants: map[string]string{"Network": "ovn", "Platform": "aws", "Topology": "ha"}, + requestedVariants: map[string]string{"Platform": "aws"}, + expected: true, + }, + { + name: "all match", + groupVariants: map[string]string{"Network": "ovn", "Platform": "aws", "Topology": "ha"}, + requestedVariants: map[string]string{"Network": "ovn", "Platform": "aws", "Topology": "ha"}, + expected: true, + }, + { + name: "one mismatch", + groupVariants: map[string]string{"Network": "ovn", "Platform": "aws", "Topology": "ha"}, + requestedVariants: map[string]string{"Network": "ovn", "Platform": "gcp"}, + expected: false, + }, + { + name: "key missing from group", + groupVariants: map[string]string{"Network": "ovn"}, + requestedVariants: map[string]string{"Platform": "aws"}, + expected: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, variantsMatch(tt.groupVariants, tt.requestedVariants)) + }) + } +} + +func newSpotCheckSamples() []reqopts.SpotCheckJobSampleOpts { + return []reqopts.SpotCheckJobSampleOpts{ + { + Name: "spotcheck-30d", + Release: reqopts.Release{ + Start: time.Now().Add(-30 * 24 * time.Hour), + End: time.Now(), + }, + IncludeVariants: map[string][]string{"JobTier": {"spotcheck-30d"}}, + }, + } +} + +func TestQueryFiltering(t *testing.T) { + allGroups := []dataprovider.SpotCheckGroup{ + {Component: "Etcd", Capability: "Scaling", Variants: map[string]string{"Network": "ovn", "Platform": "aws", "Topology": "ha"}, TotalRuns: 3, SuccessfulRuns: 2}, + {Component: "Etcd", Capability: "Scaling", Variants: map[string]string{"Network": "ovn", "Platform": "gcp", "Topology": "ha"}, TotalRuns: 3, SuccessfulRuns: 3}, + {Component: "Node / Kubelet", Capability: "CPU Partitioning", Variants: map[string]string{"Network": "ovn", "Platform": "aws", "Topology": "ha"}, TotalRuns: 2, SuccessfulRuns: 1}, + } + + tests := []struct { + name string + requestedVariants map[string]string + requestedComp string + expectedKeys []string // syntheticTestID substrings to expect + notExpectedKeys []string + }{ + { + name: "no filter - all groups injected", + requestedVariants: map[string]string{}, + expectedKeys: []string{"spotcheck-30d:etcd:scaling", "spotcheck-30d:node / kubelet:cpu-partitioning"}, + }, + { + name: "environment filter Platform=aws - excludes gcp", + requestedVariants: map[string]string{"Platform": "aws"}, + expectedKeys: []string{"spotcheck-30d:etcd:scaling", "spotcheck-30d:node / kubelet:cpu-partitioning"}, + notExpectedKeys: []string{}, + }, + { + name: "environment filter Platform=gcp - only gcp etcd", + requestedVariants: map[string]string{"Platform": "gcp"}, + notExpectedKeys: []string{"spotcheck-30d:node / kubelet:cpu-partitioning"}, + }, + { + name: "environment filter Platform=metal - nothing matches", + requestedVariants: map[string]string{"Platform": "metal"}, + expectedKeys: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mw := &SpotCheckJobs{ + dataProvider: &stubProvider{groups: allGroups}, + reqOptions: reqopts.RequestOptions{ + SpotCheckJobSamples: newSpotCheckSamples(), + TestIDOptions: []reqopts.TestIdentification{ + {Component: tt.requestedComp, RequestedVariants: tt.requestedVariants}, + }, + }, + log: log.WithField("test", tt.name), + } + + result := runQuery(t, mw) + + for _, key := range tt.expectedKeys { + found := false + for k := range result { + if strings.Contains(k, key) { + found = true + break + } + } + assert.True(t, found, "expected key containing %q in results", key) + } + for _, key := range tt.notExpectedKeys { + for k := range result { + assert.False(t, strings.Contains(k, key), "did not expect key containing %q in results", key) + } + } + }) + } +} + +func TestAnalyze(t *testing.T) { + now := time.Now() + sampleStart := now.Add(-30 * 24 * time.Hour) + sampleEnd := now + + mw := &SpotCheckJobs{ + reqOptions: reqopts.RequestOptions{ + SpotCheckJobSamples: []reqopts.SpotCheckJobSampleOpts{ + { + Name: "spotcheck-30d", + Release: reqopts.Release{Start: sampleStart, End: sampleEnd}, + }, + }, + }, + } + + spotCheckTestKey := crtest.Identification{ + RowIdentification: crtest.RowIdentification{ + TestID: "spotcheck-30d:etcd:scaling", + }, + } + + nonSpotCheckTestKey := crtest.Identification{ + RowIdentification: crtest.RowIdentification{ + TestID: "openshift-tests:some-real-test", + }, + } + + tests := []struct { + name string + testKey crtest.Identification + successCount int + failureCount int + expectedStatus crtest.Status + expectHandled bool + explanationContains string + }{ + { + name: "non-spot-check test is not handled", + testKey: nonSpotCheckTestKey, + successCount: 0, + failureCount: 3, + expectedStatus: 0, // unchanged + expectHandled: false, + }, + { + name: "no runs - MissingSample", + testKey: spotCheckTestKey, + successCount: 0, + failureCount: 0, + expectedStatus: crtest.MissingSample, + expectHandled: true, + explanationContains: "No spot-check job runs found", + }, + { + name: "1 failure 0 successes - MissingSample awaiting retry", + testKey: spotCheckTestKey, + successCount: 0, + failureCount: 1, + expectedStatus: crtest.MissingSample, + expectHandled: true, + explanationContains: "awaiting retry", + }, + { + name: "2 failures 0 successes - SignificantRegression", + testKey: spotCheckTestKey, + successCount: 0, + failureCount: 2, + expectedStatus: crtest.SignificantRegression, + expectHandled: true, + explanationContains: "failed 2 times", + }, + { + name: "3 failures 0 successes - ExtremeRegression", + testKey: spotCheckTestKey, + successCount: 0, + failureCount: 3, + expectedStatus: crtest.ExtremeRegression, + expectHandled: true, + explanationContains: "did not pass", + }, + { + name: "5 failures 0 successes - ExtremeRegression", + testKey: spotCheckTestKey, + successCount: 0, + failureCount: 5, + expectedStatus: crtest.ExtremeRegression, + expectHandled: true, + explanationContains: "5 runs, 0 successes", + }, + { + name: "1 success 0 failures - NotSignificant", + testKey: spotCheckTestKey, + successCount: 1, + failureCount: 0, + expectedStatus: crtest.NotSignificant, + expectHandled: true, + explanationContains: "passed 1 out of 1", + }, + { + name: "1 success 3 failures - NotSignificant", + testKey: spotCheckTestKey, + successCount: 1, + failureCount: 3, + expectedStatus: crtest.NotSignificant, + expectHandled: true, + explanationContains: "passed 1 out of 4", + }, + { + name: "4 successes 0 failures - NotSignificant", + testKey: spotCheckTestKey, + successCount: 4, + failureCount: 0, + expectedStatus: crtest.NotSignificant, + expectHandled: true, + explanationContains: "passed 4 out of 4", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testStats := &testdetails.TestComparison{ + SampleStats: testdetails.ReleaseStats{ + Stats: crtest.NewTestStats(tt.successCount, tt.failureCount, 0, false), + }, + } + + handled, err := mw.Analyze(tt.testKey, testStats) + require.NoError(t, err) + assert.Equal(t, tt.expectHandled, handled) + + if !tt.expectHandled { + return + } + + assert.Equal(t, tt.expectedStatus, testStats.ReportStatus, "unexpected status") + assert.Equal(t, crtest.SpotCheck, testStats.Comparison) + assert.Nil(t, testStats.BaseStats) + require.Len(t, testStats.Explanations, 1) + assert.Contains(t, testStats.Explanations[0], tt.explanationContains) + }) + } +} diff --git a/pkg/api/componentreadiness/test_details.go b/pkg/api/componentreadiness/test_details.go index 16eded289a..98bd912d27 100644 --- a/pkg/api/componentreadiness/test_details.go +++ b/pkg/api/componentreadiness/test_details.go @@ -7,6 +7,7 @@ import ( "os" "slices" "sort" + "strings" "sync" "time" @@ -161,18 +162,24 @@ func (c *ComponentReportGenerator) GenerateTestDetailsReportMultiTest(ctx contex Variants: tOpt.RequestedVariants, } testKeyStr := testKey.KeyOrDie() - if statuses, ok := testKeyTestJobRunStatuses[testKeyStr]; ok { - report, generateReportErrs := c.GenerateDetailsReportForTest(ctx, tOpt, statuses, false) - if len(generateReportErrs) > 0 { - errs = append(errs, generateReportErrs...) - continue + statuses, ok := testKeyTestJobRunStatuses[testKeyStr] + if !ok { + // Spot-check synthetic tests won't appear in junit query results. + // Create an empty status so GenerateDetailsReportForTest can still run; + // the middleware will populate it via PreTestDetailsAnalysis. + statuses = crstatus.TestJobRunStatuses{ + BaseStatus: map[string][]crstatus.TestJobRunRows{}, + BaseOverrideStatus: map[string][]crstatus.TestJobRunRows{}, + SampleStatus: map[string][]crstatus.TestJobRunRows{}, + GeneratedAt: allTestsJobRunStatuses.GeneratedAt, } - reports = append(reports, report) - } else { - logrus.Errorf("missing test key in results: %v", testKeyStr) - } - + report, generateReportErrs := c.GenerateDetailsReportForTest(ctx, tOpt, statuses, false) + if len(generateReportErrs) > 0 { + errs = append(errs, generateReportErrs...) + continue + } + reports = append(reports, report) } return reports, errs } @@ -188,11 +195,14 @@ func (c *ComponentReportGenerator) GenerateDetailsReportForTest( if testIDOption.TestID == "" { return testdetails.Report{}, []error{fmt.Errorf("test_id has to be defined for test details")} } - for _, v := range c.ReqOptions.VariantOption.DBGroupBy.List() { - if _, ok := testIDOption.RequestedVariants[v]; !ok { - return testdetails.Report{}, []error{ - fmt.Errorf("all dbGroupBy variants have to be defined for test details: %s is missing in %v", - v, testIDOption.RequestedVariants), + isSpotCheck := strings.HasPrefix(testIDOption.TestID, "spotcheck-") + if !isSpotCheck { + for _, v := range c.ReqOptions.VariantOption.DBGroupBy.List() { + if _, ok := testIDOption.RequestedVariants[v]; !ok { + return testdetails.Report{}, []error{ + fmt.Errorf("all dbGroupBy variants have to be defined for test details: %s is missing in %v", + v, testIDOption.RequestedVariants), + } } } } @@ -205,6 +215,15 @@ func (c *ComponentReportGenerator) GenerateDetailsReportForTest( now := time.Now() componentJobRunTestReportStatus.GeneratedAt = &now + // Let middleware inject data (e.g. spot-check job runs) before generating reports. + testKey := crtest.KeyWithVariants{ + TestID: testIDOption.TestID, + Variants: testIDOption.RequestedVariants, + } + if err := c.middlewares.PreTestDetailsAnalysis(testKey, &componentJobRunTestReportStatus); err != nil { + return testdetails.Report{}, []error{err} + } + // Generate the report for the main release that was originally requested: report := c.internalGenerateTestDetailsReport( c.ReqOptions.BaseRelease.Name, @@ -224,14 +243,6 @@ func (c *ComponentReportGenerator) GenerateDetailsReportForTest( if testIDOption.BaseOverrideRelease != "" && testIDOption.BaseOverrideRelease != c.ReqOptions.BaseRelease.Name { - testKey := crtest.KeyWithVariants{ - TestID: testIDOption.TestID, - Variants: testIDOption.RequestedVariants, - } - if err := c.middlewares.PreTestDetailsAnalysis(testKey, &componentJobRunTestReportStatus); err != nil { - return testdetails.Report{}, []error{err} - } - start, end, err := utils.FindStartEndTimesForRelease(timeRanges, testIDOption.BaseOverrideRelease) if err != nil { return testdetails.Report{}, []error{err} @@ -467,7 +478,9 @@ func (c *ComponentReportGenerator) internalGenerateTestDetailsReport( logrus.WithError(err).Error("Failure from middleware analysis") } - c.assessComponentStatus(&testStats, log) + if _, err := c.middlewares.Analyze(testKey, &testStats); err != nil { + logrus.WithError(err).Error("Failure from middleware Analyze") + } report.TestComparison = testStats result.Analyses = []testdetails.Analysis{report} diff --git a/pkg/api/componentreadiness/utils/queryparamparser.go b/pkg/api/componentreadiness/utils/queryparamparser.go index d63465a418..69c164424f 100644 --- a/pkg/api/componentreadiness/utils/queryparamparser.go +++ b/pkg/api/componentreadiness/utils/queryparamparser.go @@ -172,6 +172,28 @@ func ParseComponentReportRequest( } } + // Resolve spot-check samples after all URL overrides (sampleRelease, date ranges) + // so they use the final release name and time context. + if view != nil { + for name, sample := range view.SpotCheckJobSamples { + spotCheckRelative := reqopts.RelativeRelease{ + Release: reqopts.Release{Name: opts.SampleRelease.Name}, + RelativeStart: sample.RelativeStart, + RelativeEnd: sample.RelativeEnd, + } + resolved, resolveErr := GetViewReleaseOptions(releases, "spot_check_"+name, spotCheckRelative, crTimeRoundingFactor, crTimeRoundingOffset) + if resolveErr != nil { + err = resolveErr + return + } + opts.SpotCheckJobSamples = append(opts.SpotCheckJobSamples, reqopts.SpotCheckJobSampleOpts{ + Name: name, + Release: resolved, + IncludeVariants: sample.IncludeVariants, + }) + } + } + // Params below this point can be used with and without views: // TODO: leave nil for safer cache keys if params not set, sync with metrics and primecache.go // TODO: unit test that metrics and primecache cache keys match a request object here @@ -210,6 +232,24 @@ func ParseComponentReportRequest( } } + // If no view provided SpotCheckJobSamples, default from the sample release dates + // so spot-check middleware runs on drill-down requests too. + if len(opts.SpotCheckJobSamples) == 0 && !opts.SampleRelease.Start.IsZero() && !opts.SampleRelease.End.IsZero() { + opts.SpotCheckJobSamples = []reqopts.SpotCheckJobSampleOpts{ + { + Name: "spotcheck-30d", + Release: reqopts.Release{ + Name: opts.SampleRelease.Name, + Start: opts.SampleRelease.Start, + End: opts.SampleRelease.End, + }, + IncludeVariants: map[string][]string{ + "JobTier": {"spotcheck-30d"}, + }, + }, + } + } + opts.CacheOption = cache.NewStandardCROptions(crTimeRoundingFactor, crTimeRoundingOffset) opts.CacheOption.ForceRefresh, err = ParseBoolArg(req, "forceRefresh", false) if err != nil { diff --git a/pkg/apis/api/componentreport/crtest/types.go b/pkg/apis/api/componentreport/crtest/types.go index 95900a8526..15d67a98e0 100644 --- a/pkg/apis/api/componentreport/crtest/types.go +++ b/pkg/apis/api/componentreport/crtest/types.go @@ -35,6 +35,7 @@ type Comparison string const ( PassRate Comparison = "pass_rate" FisherExact Comparison = "fisher_exact" + SpotCheck Comparison = "spot_check" ) const ( diff --git a/pkg/apis/api/componentreport/crview/types.go b/pkg/apis/api/componentreport/crview/types.go index ed2e2fa077..a6b81e9e69 100644 --- a/pkg/apis/api/componentreport/crview/types.go +++ b/pkg/apis/api/componentreport/crview/types.go @@ -13,6 +13,11 @@ type View struct { VariantOptions reqopts.Variants `json:"variant_options" yaml:"variant_options"` AdvancedOptions reqopts.Advanced `json:"advanced_options" yaml:"advanced_options"` + // SpotCheckJobSamples defines sample windows for spot-check job analysis, keyed by tier name + // (e.g. "spotcheck-30d"). Each entry specifies a time window and variant filters (ANDed) + // used to select which jobs to query. If empty, spot-check analysis is disabled for this view. + SpotCheckJobSamples map[string]SpotCheckJobSample `json:"spot_check_job_samples,omitempty" yaml:"spot_check_job_samples,omitempty"` + Metrics Metrics `json:"metrics" yaml:"metrics"` RegressionTracking RegressionTracking `json:"regression_tracking" yaml:"regression_tracking"` AutomateJira AutomateJira `json:"automate_jira" yaml:"automate_jira"` @@ -33,3 +38,11 @@ type PrimeCache struct { type AutomateJira struct { Enabled bool `json:"enabled" yaml:"enabled"` } + +// SpotCheckJobSample defines a spot-check sample window and variant filters for a tier. +// The release is always inherited from SampleRelease. +type SpotCheckJobSample struct { + RelativeStart string `json:"relative_start,omitempty" yaml:"relative_start,omitempty"` + RelativeEnd string `json:"relative_end,omitempty" yaml:"relative_end,omitempty"` + IncludeVariants map[string][]string `json:"include_variants,omitempty" yaml:"include_variants,omitempty"` +} diff --git a/pkg/apis/api/componentreport/reqopts/types.go b/pkg/apis/api/componentreport/reqopts/types.go index bba5202d74..9985044add 100644 --- a/pkg/apis/api/componentreport/reqopts/types.go +++ b/pkg/apis/api/componentreport/reqopts/types.go @@ -19,6 +19,9 @@ type RequestOptions struct { CacheOption cache.RequestOptions TestFilters TestIDOptions []TestIdentification + // SpotCheckJobSamples defines resolved sample windows for spot-check job analysis. + // Each entry specifies a tier name, time window, and variant filters to select jobs. + SpotCheckJobSamples []SpotCheckJobSampleOpts `json:"spot_check_job_samples,omitempty" yaml:"spot_check_job_samples,omitempty"` // ViewName is the name of the view used for this request, if any. // When generating test details URLs, if a view is present, we include just the view parameter // plus test-specific overrides, rather than expanding all view parameters into the URL. @@ -117,3 +120,10 @@ type Advanced struct { // When multiple key tests fail in the same job, only the highest priority (earliest in list) test is included. KeyTestNames []string `json:"key_test_names,omitempty" yaml:"key_test_names,omitempty"` } + +// SpotCheckJobSampleOpts is a resolved spot-check sample entry with absolute times. +type SpotCheckJobSampleOpts struct { + Name string `json:"name"` + Release `json:",inline" yaml:",inline"` //nolint:revive + IncludeVariants map[string][]string `json:"include_variants,omitempty" yaml:"include_variants,omitempty"` +} diff --git a/pkg/bigquery/bqlabel/labels.go b/pkg/bigquery/bqlabel/labels.go index f9d4c04261..34791400e8 100644 --- a/pkg/bigquery/bqlabel/labels.go +++ b/pkg/bigquery/bqlabel/labels.go @@ -73,6 +73,8 @@ const ( CRJunitSample QueryValue = "component-readiness-junit-sample" CRJunitFallback QueryValue = "component-readiness-junit-fallback" CRViewJobs QueryValue = "component-readiness-view-jobs" + CRSpotCheck QueryValue = "component-readiness-spot-check" + CRSpotCheckDetails QueryValue = "component-readiness-spot-check-details" TDJunitBase QueryValue = "test-details-junit-base" TDJunitSample QueryValue = "test-details-junit-sample" DisruptionDelta QueryValue = "disruption-delta" diff --git a/pkg/dataloader/regressioncacheloader/regressioncacheloader.go b/pkg/dataloader/regressioncacheloader/regressioncacheloader.go index 43e5f56f52..c691a27003 100644 --- a/pkg/dataloader/regressioncacheloader/regressioncacheloader.go +++ b/pkg/dataloader/regressioncacheloader/regressioncacheloader.go @@ -505,6 +505,24 @@ func (l *RegressionCacheLoader) buildGenerator( TestFilters: view.TestFilters, } + for name, sample := range view.SpotCheckJobSamples { + spotCheckRelative := reqopts.RelativeRelease{ + Release: reqopts.Release{Name: view.SampleRelease.Name}, + RelativeStart: sample.RelativeStart, + RelativeEnd: sample.RelativeEnd, + } + resolved, err := utils.GetViewReleaseOptions( + l.releases, "spot_check_"+name, spotCheckRelative, cacheOpts.CRTimeRoundingFactor, cacheOpts.CRTimeRoundingOffset) + if err != nil { + return nil, err + } + reqOpts.SpotCheckJobSamples = append(reqOpts.SpotCheckJobSamples, reqopts.SpotCheckJobSampleOpts{ + Name: name, + Release: resolved, + IncludeVariants: sample.IncludeVariants, + }) + } + generator := componentreadiness.NewComponentReportGenerator( bqprovider.NewBigQueryProvider(l.bqClient), reqOpts, l.dbc, diff --git a/sippy-ng/src/component_readiness/CompReadyTestDetailRow.js b/sippy-ng/src/component_readiness/CompReadyTestDetailRow.js index c320650958..50eddfc12b 100644 --- a/sippy-ng/src/component_readiness/CompReadyTestDetailRow.js +++ b/sippy-ng/src/component_readiness/CompReadyTestDetailRow.js @@ -35,6 +35,7 @@ export default function CompReadyTestDetailRow(props) { const { element, idx, + isSpotCheck, showOnlyFailures, searchJobArtifacts, searchJobRunIds, @@ -198,27 +199,29 @@ export default function CompReadyTestDetailRow(props) { return ( - - - {element.base_job_name ? ( - - {element.base_job_name} - {searchJobArtifacts && - getSelectingCheckBox(element.base_job_run_stats)} - - ) : ( - '(No basis equivalent)' - )} - - + {!isSpotCheck && ( + + + {element.base_job_name ? ( + + {element.base_job_name} + {searchJobArtifacts && + getSelectingCheckBox(element.base_job_run_stats)} + + ) : ( + '(No basis equivalent)' + )} + + + )} {element.sample_job_name ? ( @@ -234,20 +237,24 @@ export default function CompReadyTestDetailRow(props) { - - {element.base_job_name ? infoCell(element.base_stats) : ''} - - {testJobDetailCell(element, 'base')} - + {!isSpotCheck && ( + + {element.base_job_name ? infoCell(element.base_stats) : ''} + + )} + {!isSpotCheck && testJobDetailCell(element, 'base')} + {!isSpotCheck && } {element.sample_job_name ? infoCell(element.sample_stats) : ''} {testJobDetailCell(element, 'sample')} - - - {element.significant ? 'True' : 'False'} - - + {!isSpotCheck && ( + + + {element.significant ? 'True' : 'False'} + + + )} ) @@ -256,6 +263,7 @@ export default function CompReadyTestDetailRow(props) { CompReadyTestDetailRow.propTypes = { element: PropTypes.object.isRequired, idx: PropTypes.number.isRequired, + isSpotCheck: PropTypes.bool, showOnlyFailures: PropTypes.bool.isRequired, searchJobArtifacts: PropTypes.bool.isRequired, searchJobRunIds: PropTypes.object.isRequired, diff --git a/sippy-ng/src/component_readiness/CompReadyTestPanel.js b/sippy-ng/src/component_readiness/CompReadyTestPanel.js index a665a15728..c068353664 100644 --- a/sippy-ng/src/component_readiness/CompReadyTestPanel.js +++ b/sippy-ng/src/component_readiness/CompReadyTestPanel.js @@ -24,8 +24,16 @@ import PropTypes from 'prop-types' import React, { Fragment, useContext } from 'react' export default function CompReadyTestPanel(props) { - const { data, versions, loadedParams, testName, environment, component } = - props + const { + data, + comparison, + versions, + loadedParams, + testName, + environment, + component, + } = props + const isSpotCheck = comparison === 'spot_check' const classes = useContext(ComponentReadinessStyleContext) const significanceTitle = `Test results for individual Prow Jobs may not be statistically @@ -224,7 +232,7 @@ export default function CompReadyTestPanel(props) { return ( - {data.base_stats && ( + {!isSpotCheck && data.base_stats && ( {printParamsAndStats( 'Basis (historical)', @@ -237,9 +245,9 @@ export default function CompReadyTestPanel(props) { )} {data.sample_stats && data.sample_stats.Start && data.sample_stats.End && ( - + {printParamsAndStats( - 'Sample (being evaluated)', + isSpotCheck ? 'Spot-Check Window' : 'Sample (being evaluated)', data.sample_stats, data.sample_stats.Start.toString(), data.sample_stats.End.toString(), @@ -288,16 +296,17 @@ export default function CompReadyTestPanel(props) { - {tableCell('Basis Job', 0)} - {tableCell('Basis Runs', 1)} - {tableCell('', 2)} - {tableCell('Sample Job', 3)} - {tableCell('Sample Runs', 4)} - {tableTooltipCell( - 'Statistically Significant', - 5, - significanceTitle - )} + {!isSpotCheck && tableCell('Basis Job', 0)} + {!isSpotCheck && tableCell('Basis Runs', 1)} + {!isSpotCheck && tableCell('', 2)} + {tableCell(isSpotCheck ? 'Job' : 'Sample Job', 3)} + {tableCell(isSpotCheck ? 'Runs' : 'Sample Runs', 4)} + {!isSpotCheck && + tableTooltipCell( + 'Statistically Significant', + 5, + significanceTitle + )} @@ -320,6 +329,7 @@ export default function CompReadyTestPanel(props) { key={idx} element={element} idx={idx} + isSpotCheck={isSpotCheck} showOnlyFailures={showOnlyFailures} searchJobArtifacts={searchJobArtifacts} searchJobRunIds={searchJobRunIds} @@ -402,6 +412,7 @@ export default function CompReadyTestPanel(props) { CompReadyTestPanel.propTypes = { data: PropTypes.object.isRequired, + comparison: PropTypes.string, versions: PropTypes.object.isRequired, loadedParams: PropTypes.object.isRequired, testName: PropTypes.string.isRequired, diff --git a/sippy-ng/src/component_readiness/CompReadyUtils.js b/sippy-ng/src/component_readiness/CompReadyUtils.js index 88a5ee14f8..cfeb6610f9 100644 --- a/sippy-ng/src/component_readiness/CompReadyUtils.js +++ b/sippy-ng/src/component_readiness/CompReadyUtils.js @@ -386,6 +386,10 @@ export function getUpdatedUrlParts(vars) { //component: vars.component, } + if (vars.view) { + valuesMap.view = vars.view + } + if (vars.samplePROrg && vars.samplePRRepo && vars.samplePRNumber) { valuesMap.samplePROrg = vars.samplePROrg valuesMap.samplePRRepo = vars.samplePRRepo diff --git a/sippy-ng/src/component_readiness/TestDetailsReport.js b/sippy-ng/src/component_readiness/TestDetailsReport.js index 8265b3b015..2dad0b711b 100644 --- a/sippy-ng/src/component_readiness/TestDetailsReport.js +++ b/sippy-ng/src/component_readiness/TestDetailsReport.js @@ -365,6 +365,8 @@ export default function TestDetailsReport(props) { 0, accessibilityModeOn ) + const isSpotCheck = data.analyses[0].comparison === 'spot_check' + const significanceTitle = `Test results for individual Prow Jobs may not be statistically significant, but when taken in aggregate, there may be a statistically significant difference compared to the historical basis @@ -621,7 +623,7 @@ View the [test details report|${document.location.href}] for additional context. Environment: {environment} - {isBaseOverride && ( + {isBaseOverride && !isSpotCheck && ( {baseRelease} Override: Earlier release had a higher threshold @@ -667,6 +669,7 @@ View the [test details report|${document.location.href}] for additional context.