CONSOLE-4915: add auto scenario about custom icon in template to upstream #15771
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@yanpzhan: This pull request references CONSOLE-4915 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 sub-task to target the "4.21.0" version, but no target version was set. In 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 openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughAdds an OpenShift httpd application template, a Cypress integration test that exercises the template via the catalog, and a catalog view helper with keyword-filter and item-image verification methods. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
openshift-ci
bot
added
the
kind/cypress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/packages/integration-tests-cypress/views/catalogs.ts (1)
5-9: Consider adding type annotation forsrcTextparameter.The
checkItemImagemethod implementation is correct. However, adding a type annotation for thesrcTextparameter would improve type safety and consistency with thefilteByKeywordmethod.Apply this diff:
- checkItemImage: (srcText) => { + checkItemImage: (srcText: string) => { cy.get('img.catalog-item-header-pf-icon') .should('have.attr', 'src') .and('contain', `${srcText}`); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
frontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yaml(1 hunks)frontend/packages/integration-tests-cypress/tests/app/template.cy.ts(1 hunks)frontend/packages/integration-tests-cypress/views/catalogs.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/integration-tests-cypress/views/catalogs.tsfrontend/packages/integration-tests-cypress/tests/app/template.cy.tsfrontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yaml
🧬 Code graph analysis (1)
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts (1)
frontend/packages/integration-tests-cypress/views/catalogs.ts (1)
catalog(1-10)
🔇 Additional comments (3)
frontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yaml (1)
1-182: LGTM! Test fixture is well-structured.The OpenShift template fixture is properly configured for integration testing. The custom iconClass URL (line 14) and version string with test artifacts (line 22) are appropriate for test purposes.
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts (2)
5-16: LGTM! Setup and teardown logic is correct.The before/after hooks properly create and clean up the test template in the openshift namespace.
21-29: LGTM! Icon verification logic is correct.The test properly retrieves the iconClass annotation and verifies the catalog item image. The use of
failOnNonZeroExit: falseprovides graceful error handling.
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts
Outdated
Show resolved
Hide resolved
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts
Outdated
Show resolved
Hide resolved
yanpzhan
force-pushed
the
auto
branch
2 times, most recently
from
2abd6cc to
afc151b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
frontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yaml(1 hunks)frontend/packages/integration-tests-cypress/tests/app/template.cy.ts(1 hunks)frontend/packages/integration-tests-cypress/views/catalogs.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/packages/integration-tests-cypress/tests/app/template.cy.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yamlfrontend/packages/integration-tests-cypress/views/catalogs.ts
🔇 Additional comments (1)
frontend/packages/integration-tests-cypress/views/catalogs.ts (1)
1-10: Catalog helper looks correct and minimal for the current testsThe keyword filter and image checks are straightforward, readable, and align with Cypress best practices for these flows. No changes needed.
frontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts (1)
20-20: Correct the search keyword spelling for clarityThe search term
'test apach'is misspelled; while it likely still matches via substring, it’s confusing next to the template display name “Test Apache HTTP Server”. Consider fixing the typo:- catalog.filterByKeyword('test apach'); + catalog.filterByKeyword('test apache');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
frontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yaml(1 hunks)frontend/packages/integration-tests-cypress/tests/app/template.cy.ts(1 hunks)frontend/packages/integration-tests-cypress/views/catalogs.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/packages/integration-tests-cypress/views/catalogs.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/integration-tests-cypress/tests/app/template.cy.tsfrontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yaml
🔇 Additional comments (1)
frontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yaml (1)
1-182: Fixture structure and annotations look appropriateTemplate definition (objects, parameters, annotations) is consistent with a standard httpd example and suitable as an isolated test fixture.
iconClassnow uses a neutral example domain and webhook secrets are generated, so there are no obvious security or maintainability concerns here.
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts
Outdated
Show resolved
Hide resolved
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts
Outdated
Show resolved
Hide resolved
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts
Outdated
Show resolved
Hide resolved
yanpzhan
force-pushed
the
auto
branch
2 times, most recently
from
f5ffc9a to
a717ddc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts (1)
20-20: Fix typo in search keyword.The search term
'test apach'is missing the 'e' from 'apache'. The template's display name is "Test Apache HTTP Server", so the keyword should be'test apache'for correctness.Apply this diff:
- catalog.filterByKeyword('test apach'); + catalog.filterByKeyword('test apache');
🧹 Nitpick comments (1)
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts (1)
22-28: Consider trimming stdout for robustness.The
stdoutfrom theoc getcommand might include extra whitespace or surrounding quotes from the jsonpath expression. While the test may work as-is, trimming ensures cleaner output for the image verification.Apply this diff:
cy.exec( `oc get template httpd-example-test -n openshift -o jsonpath='{.metadata.annotations.iconClass}'`, ).then((output) => { - const iconClass = output.stdout; + const iconClass = output.stdout.trim(); cy.log(`1. icon url: ${iconClass}`); - catalog.checkItemImage(`${iconClass}`); + catalog.checkItemImage(iconClass); });Note: The template literal on line 27 is also unnecessary since
iconClassis already a string.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
frontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yaml(1 hunks)frontend/packages/integration-tests-cypress/tests/app/template.cy.ts(1 hunks)frontend/packages/integration-tests-cypress/views/catalogs.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/packages/integration-tests-cypress/views/catalogs.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yamlfrontend/packages/integration-tests-cypress/tests/app/template.cy.ts
🧬 Code graph analysis (1)
frontend/packages/integration-tests-cypress/tests/app/template.cy.ts (1)
frontend/packages/integration-tests-cypress/views/catalogs.ts (1)
catalog(1-10)
🔇 Additional comments (4)
frontend/packages/integration-tests-cypress/fixtures/httpd-example-template.yaml (1)
1-182: LGTM! Well-structured OpenShift template fixture.The template fixture is properly configured for testing purposes. The previous concern about the iconClass domain has been addressed by using
example.com, which is an IANA-reserved domain suitable for examples and testing.frontend/packages/integration-tests-cypress/tests/app/template.cy.ts (3)
1-3: LGTM! Clean imports.The imports are appropriate for the test functionality.
5-12: LGTM! Proper test setup.The before hook correctly creates the template, verifies creation, and initializes the test environment.
14-16: LGTM! Proper test cleanup.The after hook ensures the template is deleted, maintaining a clean test environment.
|
All comments have been addressed. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jhadvig, yanpzhan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/test e2e-gcp-console |
|
@yanpzhan: all tests passed! Full PR test history. Your PR dashboard. 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. |
|
/verified by CI |
|
@yapei: This PR has been marked as verified by In 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
verified
|
/label acknowledge-critical-fixes-only |
openshift-ci
bot
added
acknowledge-critical-fixes-only
4 participants
Tracked in https://issues.redhat.com/browse/CONSOLE-4915
@yapei pls help to review, thanks!