TRT-2932: Fix presubmit risk analysis comparing against wrong release by machine424 · Pull Request #3956 · openshift/sippy

machine424 · 2026-08-27T11:08:22Z

Since 3f0ab35 ("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.

Summary by CodeRabbit

Bug Fixes
- Presubmit job risk analysis now uses the latest eligible OCP release.
- Excludes presubmit-only releases and releases without a predecessor.
- Reports an error when no suitable OCP release is available instead of selecting an arbitrary result.
Tests
- Added coverage for product filtering, release ordering, exclusions, missing predecessors, empty results, and unavailable OCP releases.

Since 3f0ab35 ("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.

openshift-merge-bot · 2026-08-27T11:08:25Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

coderabbitai · 2026-08-27T11:08:42Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 5ab0c88c-778e-4fb0-9b33-0f5ba9f58e46

📥 Commits

Reviewing files that changed from the base of the PR and between 204c2fc and 51b437f.

📒 Files selected for processing (2)

pkg/api/job_runs.go
pkg/api/job_runs_test.go

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The API now selects the latest eligible OCP release for presubmit risk analysis. It excludes non-OCP releases, Presubmits releases, and releases without predecessors. The analysis returns an error when no suitable release exists. Table-driven tests cover these selection rules.

Changes

OCP release selection

Layer / File(s)	Summary
Eligible release filtering `pkg/api/job_runs.go`, `pkg/api/job_runs_test.go`	Adds `latestReleaseForProduct` and tests product filtering, predecessor requirements, Presubmits exclusion, ordering, empty input, and no-match cases.
Risk analysis integration `pkg/api/job_runs.go`	Updates presubmit risk analysis to use the selected OCP release and return an error when no suitable release exists.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 51b43

The PR narrows presubmit risk analysis to the appropriate OCP release and adds corresponding test coverage; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: neisw, mstaeble, dgoodwin

🚥 Pre-merge checks | ✅ 20 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (20 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Go Error Handling	✅ Passed	The pull request adds no custom-check failure. The new helper has no returned errors, ignored errors, panics, or pointer dereferences. The presubmit path checks `GetReleasesFromDB` and returns its err…
Sql Injection Prevention	✅ Passed	No SQL injection issue was introduced. The pull request adds only in-memory release filtering and changes the selected release value. The release lookup uses a static GORM `Order("development_start_da…
Excessive Css In React Should Use Styles	✅ Passed	PASS: The pull request changes only `pkg/api/job_runs.go` and `pkg/api/job_runs_test.go`, both Go files. The diff adds release-selection logic and Go tests. It adds no React components, JSX, inline st…
Test Coverage For New Features	✅ Passed	PASS: The pull request adds unit coverage for the new pure selector, `latestReleaseForProduct`. Table-driven cases cover empty input, product filtering, missing `PreviousRelease`, excluded `Presubmits…
Single Responsibility And Clear Naming	✅ Passed	The patch adds one focused helper, `latestReleaseForProduct`, that selects an eligible release for a specified product. Its name clearly describes its action and scope. `JobRunRiskAnalysis` delegates …
Feature Documentation	✅ Passed	PASS. The pull request changes presubmit risk-analysis release selection in `pkg/api/job_runs.go` and adds tests. The only file under `docs/features/` documents job-analysis symptoms and labels, not t…
Stable And Deterministic Test Names	✅ Passed	The pull request adds only static Go table-test names: "empty list", "skips non-OCP products", "skips releases without PreviousRelease", "skips Presubmits even if it had PreviousRelease", "realistic p…
Test Structure And Quality	✅ Passed	PASS: The changed tests use Go's `testing.T` with `testify`, not Ginkgo. Each table subtest checks one pure `latestReleaseForProduct` behavior. The tests create no cluster resources and perform no wai…
Microshift Test Compatibility	✅ Passed	The check is not applicable. The pull request changes only `pkg/api/job_runs.go` and `pkg/api/job_runs_test.go`. The added tests use Go `testing` with `TestLatestReleaseForProduct` and `t.Run`; they a…
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: The pull request adds only a standard Go `testing` unit test, `TestLatestReleaseForProduct`, in `pkg/api/job_runs_test.go`. The diff adds no Ginkgo `It()`, `Describe()`, `Context()`, or `When()`…
Topology-Aware Scheduling Compatibility	✅ Passed	PASS: The pull request changes only `pkg/api/job_runs.go` and `pkg/api/job_runs_test.go`. The code selects a database release for risk analysis and adds unit tests. The diff introduces no deployment m…
Ote Binary Stdout Contract	✅ Passed	PASS. The pull request changes only `pkg/api/job_runs.go` and `pkg/api/job_runs_test.go`. The added code selects releases and returns an error; it does not write to stdout or modify `main()`, `init()`…
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PASS: The pull request adds a standard Go table-driven unit test, `TestLatestReleaseForProduct`, not a Ginkgo e2e test. The added code contains no IPv4 assumptions or external connectivity requirement…
No-Weak-Crypto	✅ Passed	The pull request introduces no weak-crypto usage. The changed code adds release filtering and string comparisons for product, predecessor, and release names. It adds no MD5, SHA-1, DES, RC4, 3DES, Blo…
Container-Privileges	✅ Passed	PASS. The pull request changes only `pkg/api/job_runs.go` and `pkg/api/job_runs_test.go`. The diff contains no container or Kubernetes manifest files and no privilege settings such as `privileged`, `h…
No-Sensitive-Data-In-Logs	✅ Passed	The pull request adds no logging calls or sensitive fields. The implementation only filters release metadata and returns a fixed error. Existing logs may include release or job names, but the pull req…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: correcting presubmit risk analysis to use the correct comparison release.

Full details: Go Error Handling

Explanation

The pull request adds no custom-check failure. The new helper has no returned errors, ignored errors, panics, or pointer dereferences. The presubmit path checks GetReleasesFromDB and returns its error, then returns an explicit error when no suitable release exists. The direct return of the database error is unchanged from the base revision, so it is not pull-request-caused.

Full details: Sql Injection Prevention

Explanation

No SQL injection issue was introduced. The pull request adds only in-memory release filtering and changes the selected release value. The release lookup uses a static GORM Order("development_start_date DESC") clause. The selected release is passed to existing database queries through placeholders, including ProwJobSimilarName (release = ?) and ProwJobHistoricalTestCounts (prow_job_run_release = ?). No added SQL concatenation or formatting appears in the diff.

Full details: Excessive Css In React Should Use Styles

Explanation

PASS: The pull request changes only pkg/api/job_runs.go and pkg/api/job_runs_test.go, both Go files. The diff adds release-selection logic and Go tests. It adds no React components, JSX, inline style objects, or CSS. The custom check is therefore not applicable.

Full details: Test Coverage For New Features

Explanation

PASS: The pull request adds unit coverage for the new pure selector, latestReleaseForProduct. Table-driven cases cover empty input, product filtering, missing PreviousRelease, excluded Presubmits, production ordering with mcp-0.5 first, and no suitable OCP release. The production path uses this selector and handles its empty result. The realistic ordering case covers the reported regression scenario.

Full details: Single Responsibility And Clear Naming

Explanation

The patch adds one focused helper, latestReleaseForProduct, that selects an eligible release for a specified product. Its name clearly describes its action and scope. JobRunRiskAnalysis delegates release selection to this helper. The patch adds no structs, packages, or exported entities, and it introduces no excessive parameter lists or generic names. The existing broad pkg/api package and larger methods were not created by this change.

Full details: Feature Documentation

Explanation

PASS. The pull request changes presubmit risk-analysis release selection in pkg/api/job_runs.go and adds tests. The only file under docs/features/ documents job-analysis symptoms and labels, not this risk-analysis behavior. No feature documentation update is present, but the custom check states that relevant documentation updates are strongly encouraged and not strictly required. Therefore, no explicit failure condition is met.

Full details: Stable And Deterministic Test Names

Explanation

The pull request adds only static Go table-test names: "empty list", "skips non-OCP products", "skips releases without PreviousRelease", "skips Presubmits even if it had PreviousRelease", "realistic production ordering", and "no OCP releases". No Ginkgo title calls or dynamic values appear in the changed test code. Release names and products remain in test data, not test titles.

Full details: Test Structure And Quality

Explanation

PASS: The changed tests use Go's testing.T with testify, not Ginkgo. Each table subtest checks one pure latestReleaseForProduct behavior. The tests create no cluster resources and perform no waits, so Ginkgo lifecycle and timeout requirements do not apply. The subtest names provide failure context.

Full details: Microshift Test Compatibility

Explanation

The check is not applicable. The pull request changes only pkg/api/job_runs.go and pkg/api/job_runs_test.go. The added tests use Go testing with TestLatestReleaseForProduct and t.Run; they add no Ginkgo e2e tests and reference no MicroShift-incompatible APIs or features.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request adds only a standard Go testing unit test, TestLatestReleaseForProduct, in pkg/api/job_runs_test.go. The diff adds no Ginkgo It(), Describe(), Context(), or When() e2e test. The test only filters in-memory release structs and makes no cluster or node-topology assumptions.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes only pkg/api/job_runs.go and pkg/api/job_runs_test.go. The code selects a database release for risk analysis and adds unit tests. The diff introduces no deployment manifests, operators, controllers, pod specifications, replica settings, affinity, topology spread, node selectors, tolerations, or PDBs. Therefore it introduces no scheduling constraint covered by this check.

Full details: Ote Binary Stdout Contract

Explanation

PASS. The pull request changes only pkg/api/job_runs.go and pkg/api/job_runs_test.go. The added code selects releases and returns an error; it does not write to stdout or modify main(), init(), TestMain(), Ginkgo suite setup, or OTE process configuration. The changed files contain no fmt.Print*, println, klog, RunSpecs, or suite-setup stdout calls.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

PASS: The pull request adds a standard Go table-driven unit test, TestLatestReleaseForProduct, not a Ginkgo e2e test. The added code contains no IPv4 assumptions or external connectivity requirements. Existing example.com fixture URLs are unchanged.

Full details: No-Weak-Crypto

Explanation

The pull request introduces no weak-crypto usage. The changed code adds release filtering and string comparisons for product, predecessor, and release names. It adds no MD5, SHA-1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret/token comparison logic.

Full details: Container-Privileges

Explanation

PASS. The pull request changes only pkg/api/job_runs.go and pkg/api/job_runs_test.go. The diff contains no container or Kubernetes manifest files and no privilege settings such as privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation.

Full details: No-Sensitive-Data-In-Logs

Explanation

The pull request adds no logging calls or sensitive fields. The implementation only filters release metadata and returns a fixed error. Existing logs may include release or job names, but the pull request does not add sensitive data to those logs.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

openshift-merge-bot · 2026-08-27T11:26:21Z

Scheduling required tests:
/test e2e

neisw · 2026-08-27T11:48:43Z

/lgtm

neisw · 2026-08-27T12:31:48Z

/test e2e

machine424 · 2026-08-27T13:56:42Z

/retitle TRT-2932: Fix presubmit risk analysis comparing against wrong release

openshift-ci-robot · 2026-08-27T13:57:06Z

@machine424: This pull request references TRT-2932 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Since 3f0ab35 ("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.

Summary by CodeRabbit

Bug Fixes

Presubmit job risk analysis now uses the latest eligible OCP release.

Excludes presubmit-only releases and releases without a predecessor.

Reports an error when no suitable OCP release is available instead of selecting an arbitrary result.

Tests

Added coverage for product filtering, release ordering, exclusions, missing predecessors, empty results, and unavailable OCP releases.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-merge-bot · 2026-08-27T14:15:12Z

/retest-required

Remaining retests: 0 against base HEAD 204c2fc and 2 for PR HEAD 51b437f in total

machine424 · 2026-08-27T16:31:36Z

/retest-required

openshift-merge-bot · 2026-08-27T19:14:35Z

/retest-required

Remaining retests: 0 against base HEAD 4ba9539 and 1 for PR HEAD 51b437f in total

openshift-ci · 2026-08-27T20:22:44Z

@machine424: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e	`51b437f`	link	true	`/test e2e`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

neisw · 2026-08-27T20:23:51Z

/lgtm cancel

openshift-ci · 2026-08-27T20:24:06Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: machine424
Once this PR has been reviewed and has the lgtm label, please ask for approval from neisw. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

neisw · 2026-08-27T20:43:44Z

 	compareRelease := jobRun.ProwJob.Release
 	neverStableJob := false
 	if compareRelease == models.ReleasePresubmits {
 		ar, err := GetReleasesFromDB(ctx, dbc)


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

openshift-ci Bot requested review from petr-muller and stbenjam August 27, 2026 11:08

coderabbitai Bot approved these changes Aug 27, 2026

View reviewed changes

openshift-ci Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Aug 27, 2026

machine424 changed the title ~~Fix presubmit risk analysis comparing against wrong release~~ WIP: Fix presubmit risk analysis comparing against wrong release Aug 27, 2026

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 27, 2026

machine424 changed the title ~~WIP: Fix presubmit risk analysis comparing against wrong release~~ Fix presubmit risk analysis comparing against wrong release Aug 27, 2026

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 27, 2026

openshift-ci Bot assigned neisw Aug 27, 2026

openshift-ci Bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 27, 2026

openshift-ci Bot changed the title ~~Fix presubmit risk analysis comparing against wrong release~~ TRT-2932: Fix presubmit risk analysis comparing against wrong release Aug 27, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 27, 2026

machine424 mentioned this pull request Aug 27, 2026

Fix historical test count query using wrong release for presubmits #3961

Open

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2026

openshift-ci Bot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 27, 2026

neisw reviewed Aug 27, 2026

View reviewed changes

Uh oh!

Conversation

machine424 commented Aug 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

openshift-merge-bot Bot commented Aug 27, 2026

Uh oh!

coderabbitai Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

openshift-merge-bot Bot commented Aug 27, 2026

Uh oh!

neisw commented Aug 27, 2026

Uh oh!

neisw commented Aug 27, 2026

Uh oh!

machine424 commented Aug 27, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Aug 27, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

openshift-merge-bot Bot commented Aug 27, 2026

Uh oh!

machine424 commented Aug 27, 2026

Uh oh!

openshift-merge-bot Bot commented Aug 27, 2026

Uh oh!

openshift-ci Bot commented Aug 27, 2026

Uh oh!

neisw commented Aug 27, 2026

Uh oh!

openshift-ci Bot commented Aug 27, 2026

Uh oh!

neisw Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

machine424 commented Aug 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 27, 2026 •

edited

Loading

machine424 commented Aug 27, 2026 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Aug 27, 2026 •

edited by openshift-ci Bot

Loading