From 51b437ff66158ef1cd8150ff6d349bc2658db50b Mon Sep 17 00:00:00 2001 From: Ayoub Mrini Date: Thu, 27 Aug 2026 13:06:36 +0200 Subject: [PATCH] Fix presubmit risk analysis comparing against wrong release Since 3f0ab35ed ("Add OCPMCP mcp-0.5 release to customizations"), JobRunRiskAnalysis for Presubmits was picking mcp-0.5 as the comparison release (first result from GetReleasesFromDB ordered by development_start_date DESC). This caused all presubmit risk results to show "Unknown" since there's no test history overlap. Filter for the most recent OCP release from the mainline chain (has PreviousRelease set), skipping synthetic releases and non-OCP products. Until non-OCP presubmits are properly supported, this ensures risk analysis works correctly for OCP presubmits. --- pkg/api/job_runs.go | 20 +++++++++--- pkg/api/job_runs_test.go | 68 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/pkg/api/job_runs.go b/pkg/api/job_runs.go index a968b554ae..f71d3002c2 100644 --- a/pkg/api/job_runs.go +++ b/pkg/api/job_runs.go @@ -25,6 +25,7 @@ import ( apitype "github.com/openshift/sippy/pkg/apis/api" "github.com/openshift/sippy/pkg/apis/cache" "github.com/openshift/sippy/pkg/apis/openshift" + sippyv1 "github.com/openshift/sippy/pkg/apis/sippy/v1" sippyprocessingv1 "github.com/openshift/sippy/pkg/apis/sippyprocessing/v1" "github.com/openshift/sippy/pkg/bigquery" "github.com/openshift/sippy/pkg/dataloader/prowloader" @@ -612,6 +613,15 @@ func joinSegments(segments []string, start int, separator string) string { return strings.Join(segments[start:], separator) } +func latestReleaseForProduct(releases []sippyv1.Release, product string) string { + for _, r := range releases { + if r.Product == product && r.PreviousRelease != "" && r.Release != models.ReleasePresubmits { + return r.Release + } + } + return "" +} + // JobRunRiskAnalysis checks the test failures and linked bugs for a job run, and reports back an estimated // risk level for each failed test, and the job run overall. func JobRunRiskAnalysis( @@ -623,7 +633,7 @@ func JobRunRiskAnalysis( logger = logger.WithField("func", "JobRunRiskAnalysis") // If this job is a Presubmit, compare to test results from master, not presubmits, which may perform // worse due to dev code that hasn't merged. We do not presently track presubmits on branches other than - // master, so it should be safe to assume the latest compareRelease in the db. + // master, so we use the latest OCP release from the db. compareRelease := jobRun.ProwJob.Release neverStableJob := false if compareRelease == models.ReleasePresubmits { @@ -631,11 +641,11 @@ func JobRunRiskAnalysis( if err != nil { return apitype.ProwJobRunRiskAnalysis{}, err } - if len(ar) == 0 { - return apitype.ProwJobRunRiskAnalysis{}, fmt.Errorf("no releases found in db") + // TODO: Non-OCP are not supported yet. At least ensure adding new releases doesn't break OCP. + compareRelease = latestReleaseForProduct(ar, "OCP") + if compareRelease == "" { + return apitype.ProwJobRunRiskAnalysis{}, fmt.Errorf("no suitable OCP release found") } - - compareRelease = ar[0].Release } historicalCount, err := query.ProwJobHistoricalTestCounts(dbc, jobRun.ProwJob.ID, compareRelease) diff --git a/pkg/api/job_runs_test.go b/pkg/api/job_runs_test.go index b22603a98e..2edbd6a873 100644 --- a/pkg/api/job_runs_test.go +++ b/pkg/api/job_runs_test.go @@ -6,6 +6,7 @@ import ( "testing" apitype "github.com/openshift/sippy/pkg/apis/api" + sippyv1 "github.com/openshift/sippy/pkg/apis/sippy/v1" "github.com/openshift/sippy/pkg/db/models" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -488,3 +489,70 @@ func TestSelectRiskAnalysisResult(t *testing.T) { }) } } + +func TestLatestReleaseForProduct(t *testing.T) { + tests := []struct { + name string + releases []sippyv1.Release + expected string + }{ + { + name: "empty list", + expected: "", + }, + { + name: "skips non-OCP products", + releases: []sippyv1.Release{ + {Release: "mcp-0.5", Product: "OCPMCP"}, + {Release: "5.1", Product: "OCP", PreviousRelease: "5.0"}, + }, + expected: "5.1", + }, + { + name: "skips releases without PreviousRelease", + releases: []sippyv1.Release{ + {Release: "automation", Product: "OCP"}, + {Release: "Presubmits", Product: "OCP"}, + {Release: "4.23", Product: "OCP", PreviousRelease: "4.22"}, + }, + expected: "4.23", + }, + { + name: "skips Presubmits even if it had PreviousRelease", + releases: []sippyv1.Release{ + {Release: models.ReleasePresubmits, Product: "OCP", PreviousRelease: "something"}, + {Release: "5.0", Product: "OCP", PreviousRelease: "4.22"}, + }, + expected: "5.0", + }, + { + name: "realistic production ordering", + releases: []sippyv1.Release{ + {Release: "mcp-0.5", Product: "OCPMCP"}, + {Release: "5.1", Product: "OCP", PreviousRelease: "5.0"}, + {Release: "4.23", Product: "OCP", PreviousRelease: "4.22"}, + {Release: "5.0", Product: "OCP", PreviousRelease: "4.22"}, + {Release: "5.0-okd", Product: "OKD", PreviousRelease: "4.22-okd"}, + {Release: "4.22", Product: "OCP", PreviousRelease: "4.21"}, + {Release: "aro-integration", Product: "HCM"}, + {Release: "rosa-stage", Product: "ROSA"}, + }, + expected: "5.1", + }, + { + name: "no OCP releases", + releases: []sippyv1.Release{ + {Release: "mcp-0.5", Product: "OCPMCP"}, + {Release: "aro-integration", Product: "HCM"}, + {Release: "rosa-stage", Product: "ROSA"}, + }, + expected: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, latestReleaseForProduct(tc.releases, "OCP")) + }) + } +}