Tests: Replace all console-image flags with the PR image#1868
Tests: Replace all console-image flags with the PR image#1868openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
Conversation
When running tests in CI, replace all console images with the image for the PR being tested. This ensures the PR image is always used regardless of the OCP version being used. Made-with: Cursor
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughThe test file's CSV patching logic has been updated to detect the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/tests/lightspeed-install.cy.ts`:
- Around line 176-177: The current conditional using
arg.startsWith('--console-image') with arg.replace(/=.*/,
`=${Cypress.env('CONSOLE_IMAGE')}`) misses the case where arg is exactly
'--console-image'; update the logic in tests/tests/lightspeed-install.cy.ts (the
code handling each arg) to detect if arg === '--console-image' and return
`--console-image=${Cypress.env('CONSOLE_IMAGE')}`, otherwise perform the
existing replace to swap any existing value; this ensures the PR console image
is always injected whether the arg has an '=' or not.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7ec034a2-bf22-476e-98e0-a886025525aa
📒 Files selected for processing (1)
tests/tests/lightspeed-install.cy.ts
| arg.startsWith('--console-image') | ||
| ? arg.replace(/=.*/, `=${Cypress.env('CONSOLE_IMAGE')}`) |
There was a problem hiding this comment.
Handle --console-image without =; current replacement can be a no-op.
When the arg is exactly --console-image, arg.replace(/=.*/, ...) does not inject the PR image. That means this path can still skip forcing the PR image.
Proposed fix
- (arg) =>
- arg.startsWith('--console-image')
- ? arg.replace(/=.*/, `=${Cypress.env('CONSOLE_IMAGE')}`)
- : arg,
+ (arg) =>
+ arg === '--console-image' || arg.startsWith('--console-image=')
+ ? `--console-image=${Cypress.env('CONSOLE_IMAGE')}`
+ : arg,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| arg.startsWith('--console-image') | |
| ? arg.replace(/=.*/, `=${Cypress.env('CONSOLE_IMAGE')}`) | |
| (arg) => | |
| arg === '--console-image' || arg.startsWith('--console-image=') | |
| ? `--console-image=${Cypress.env('CONSOLE_IMAGE')}` | |
| : arg, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/tests/lightspeed-install.cy.ts` around lines 176 - 177, The current
conditional using arg.startsWith('--console-image') with arg.replace(/=.*/,
`=${Cypress.env('CONSOLE_IMAGE')}`) misses the case where arg is exactly
'--console-image'; update the logic in tests/tests/lightspeed-install.cy.ts (the
code handling each arg) to detect if arg === '--console-image' and return
`--console-image=${Cypress.env('CONSOLE_IMAGE')}`, otherwise perform the
existing replace to swap any existing value; this ensures the PR console image
is always injected whether the arg has an '=' or not.
There was a problem hiding this comment.
We know that they all have the format --console-image.*=
There was a problem hiding this comment.
@kyoto, understood — since all --console-image args in the CSV are guaranteed to have an = (i.e., the format is always --console-image.*=), the arg.replace(/=.*/, ...) will always find a match and correctly substitute the PR image. My concern doesn't apply here. Thanks for the clarification!
📝 Learnings were identified and not saved (knowledge base disabled). Enable
|
/lgtm |
|
/cherry-pick release-4.19 |
|
@kyoto: #1868 failed to apply on top of branch "pattern-fly-5": DetailsIn response to this:
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. |
|
@kyoto: new pull request created: #1869 DetailsIn response to this:
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. |
When running tests in CI, replace all console images with the image for the PR being tested. This ensures the PR image is always used regardless of the OCP version being used.
Made-with: Cursor
Summary by CodeRabbit