Skip to content

OPRUN-4436: fix(test): update PolarionID:87224 for 4.23/5.0 upgrade boundary#766

Open
tmshort wants to merge 1 commit into
openshift:mainfrom
tmshort:oprun-4432-polarion-87224
Open

OPRUN-4436: fix(test): update PolarionID:87224 for 4.23/5.0 upgrade boundary#766
tmshort wants to merge 1 commit into
openshift:mainfrom
tmshort:oprun-4432-polarion-87224

Conversation

@tmshort

@tmshort tmshort commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Fix GetNextMinorVersion to return "5.1" for 4.23 clusters instead of "4.24": OCP 4.23 and 5.0 are co-released equivalents whose only upgrade target is 5.1.

Remove the redundant && strings.Contains(message, "5") guard from the Upgradeable message poll — the expectedPattern built from GetNextMinorVersion now encodes the full version string and is sufficient on its own.

Summary by CodeRabbit

  • Bug Fixes

    • Improved upgradeability status checks so valid status messages are recognized without relying on an unrelated numeric value.
    • Added correct next-version handling for the OpenShift 4.23 co-release scenario, returning version 5.1 as expected.
    • Preserved existing validation and error handling for invalid or unavailable version information.
  • Tests

    • Updated test behavior to better reflect supported upgrade paths and status reporting.

…grade boundary

Fix GetNextMinorVersion to return "5.1" for 4.23 clusters instead of
"4.24": OCP 4.23 and 5.0 are co-released equivalents whose only upgrade
target is 5.1.

Remove the redundant `&& strings.Contains(message, "5")` guard from the
Upgradeable message poll — the expectedPattern built from
GetNextMinorVersion now encodes the full version string and is
sufficient on its own.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@openshift-ci-robot

openshift-ci-robot commented Jul 16, 2026

Copy link
Copy Markdown

@tmshort: This pull request references OPRUN-4436 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 story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Fix GetNextMinorVersion to return "5.1" for 4.23 clusters instead of "4.24": OCP 4.23 and 5.0 are co-released equivalents whose only upgrade target is 5.1.

Remove the redundant && strings.Contains(message, "5") guard from the Upgradeable message poll — the expectedPattern built from GetNextMinorVersion now encodes the full version string and is sufficient on its own.

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-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 16, 2026
@tmshort

tmshort commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

/hold
This needs to be manually tested against openshift/cluster-olm-operator#216 (either post merge, or as a clusrter-bot+pr)

@openshift-ci openshift-ci Bot requested review from dtfranz and pedjak July 16, 2026 19:16
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Walkthrough

The changes special-case OCP 4.23 to return version 5.1 as the next minor target and broaden the Upgradeable status polling condition in an OLMv1 QE test.

Changes

OpenShift upgrade test adjustments

Layer / File(s) Summary
Handle the 4.23 to 5.1 upgrade target
openshift/tests-extension/test/qe/util/tools.go
GetNextMinorVersion documents and returns 5.1 for OCP 4.23 while retaining the general minor-version increment for other inputs.
Broaden Upgradeable status matching
openshift/tests-extension/test/qe/specs/olmv1_ce.go
The Polarion 87224 polling condition now succeeds when the expected Upgradeable message pattern is present without also requiring the substring 5.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the 4.23/5.0 upgrade-boundary test update, including the affected Polarion test.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Stable And Deterministic Test Names ✅ Passed The PR only changes upgrade-version logic and a poll condition; the Ginkgo titles in olmv1_ce.go are static string literals, with no interpolated names.
Test Structure And Quality ✅ Passed PASS: the changed Ginkgo test stays focused on one upgradeability scenario, uses deferred cleanup, and the poll has a 60s timeout; helper change is a version-case tweak.
Microshift Test Compatibility ✅ Passed No new Ginkgo test was added; the PR only tweaks an existing OLM serial test and a helper. No new MicroShift-unsupported API use was introduced.
Single Node Openshift (Sno) Test Compatibility ✅ Passed Only a message check and version helper changed; no new Ginkgo tests or node-count/HA assumptions were added, and the test body has no SNO-sensitive logic.
Topology-Aware Scheduling Compatibility ✅ Passed PR only changes a test utility and a test assertion; it introduces no scheduling constraints, manifests, controllers, or topology-dependent pod placement.
Ote Binary Stdout Contract ✅ Passed The patch only changes version calculation and a test predicate; no main/init/suite-level stdout writes or logging redirection changes were introduced.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new e2e test or network code was added; changes are version-string logic and a poll condition only, with no IPv4 or external connectivity assumptions.
No-Weak-Crypto ✅ Passed Changed code only adjusts version parsing and a string-contains check; no weak crypto, custom crypto, or secret comparisons were added.
Container-Privileges ✅ Passed Only test logic and a version helper changed; no privileged, hostPID/Network/IPC, SYS_ADMIN, or allowPrivilegeEscalation settings were added.
No-Sensitive-Data-In-Logs ✅ Passed The patch only adjusts version logic and a poll predicate; it adds no new logs, and existing logs emit version/status strings, not secrets or PII.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 16, 2026
@openshift-ci

openshift-ci Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tmshort

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 16, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
openshift/tests-extension/test/qe/util/tools.go (1)

593-598: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add regression coverage for the 4.23 boundary.

Please cover 4.23 → 5.1, alongside adjacent cases such as 4.22 → 4.23 and 5.0 → 5.1, so this release-specific mapping cannot regress.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openshift/tests-extension/test/qe/util/tools.go` around lines 593 - 598, Add
regression test coverage for the release-target calculation function containing
the 4.23 special case. Include assertions for 4.22→4.23, 4.23→5.1, and 5.0→5.1,
preserving the expected error behavior for valid inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@openshift/tests-extension/test/qe/util/tools.go`:
- Around line 593-598: Add regression test coverage for the release-target
calculation function containing the 4.23 special case. Include assertions for
4.22→4.23, 4.23→5.1, and 5.0→5.1, preserving the expected error behavior for
valid inputs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 91a3c3be-dd91-420f-a6a8-797b7e056f80

📥 Commits

Reviewing files that changed from the base of the PR and between 93894cf and fc300e0.

📒 Files selected for processing (2)
  • openshift/tests-extension/test/qe/specs/olmv1_ce.go
  • openshift/tests-extension/test/qe/util/tools.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants