Skip to content

TRT-2932: Fix presubmit risk analysis comparing against wrong release #3956

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
machine424 wants to merge 1 commit into
base: main
Choose a base branch
from
+83 −5
Open

TRT-2932: Fix presubmit risk analysis comparing against wrong release #3956

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions pkg/api/job_runs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand All @@ -623,19 +633,19 @@ 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 {
ar, err := GetReleasesFromDB(ctx, dbc)

@neisw neisw Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's try using https://github.com/machine424/openshift-sippy/blob/51b437ff66158ef1cd8150ff6d349bc2658db50b/pkg/db/query/release_queries.go#L14 instead

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)
Expand Down
68 changes: 68 additions & 0 deletions pkg/api/job_runs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"))
})
}
}