CONSOLE-5094: cypress test improvements#16046
CONSOLE-5094: cypress test improvements#16046logonoff wants to merge 7 commits intoopenshift:mainfrom
Conversation
|
@logonoff: This pull request references CONSOLE-5094 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.22.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@logonoff: This pull request references CONSOLE-5094 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.22.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThis pull request consolidates and restructures the frontend integration test infrastructure. The 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/packages/knative-plugin/integration-tests/features/serverless/create-knative-service-from-deployment-or-deployment-config.feature (1)
13-13:⚠️ Potential issue | 🟡 MinorPre-existing typo:
dep-worloadshould bedep-workload.Line 13 references
"dep-worload"(missing 'k'), but the workload is created as"dep-workload"on line 12 and referenced correctly elsewhere. This will cause a step definition mismatch and likely a test failure. While this is in unchanged code, it's worth fixing while you're in this file.- And workload "dep-worload" is present in topology page + And workload "dep-workload" is present in topology page🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/knative-plugin/integration-tests/features/serverless/create-knative-service-from-deployment-or-deployment-config.feature` at line 13, Fix the typo in the feature step text: replace the incorrect workload name "dep-worload" with the correct "dep-workload" so the step matches the created workload and existing step definitions; update the string in the scenario step (search for "dep-worload") to "dep-workload" to prevent the step mismatch.frontend/packages/knative-plugin/integration-tests/features/eventing/managed-services-kafka-connection.feature (1)
1-1:⚠️ Potential issue | 🔴 CriticalStep definitions missing for external service interactions—these tests will fail in CI.
The three scenarios on lines 14, 33, and 85 were changed from
@manualto@regression, but the underlying step definitions for their external service operations don't exist:
user clicks on link "https://cloud.redhat.com/openshift/token"user copies the API token from the linkuser pastes the token in the API TokenThese steps are not implemented in the codebase. Additionally, the cypress.config.js filter explicitly includes
@regressiontests in CI while excluding@manualtests. This means these scenarios will now run in CI and fail because the steps are missing.Either implement the step definitions (with proper mocking of the external RHOAS service), or restore the
@manualtags on lines 13, 32, and 84. For consistency with other feature files in the suite, if these tests require external service interaction, keeping@regression@manual`` tags is the appropriate approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/knative-plugin/integration-tests/features/eventing/managed-services-kafka-connection.feature` at line 1, The three scenarios in managed-services-kafka-connection.feature now marked `@regression` rely on missing step definitions for external interactions ("user clicks on link \"https://cloud.redhat.com/openshift/token\"", "user copies the API token from the link", "user pastes the token in the API Token") and will fail in CI because cypress.config.js includes `@regression` tests; either (A) revert those three scenarios to include `@manual` (i.e., change their tags back to "@regression `@manual`" to exclude them from CI) or (B) implement mocked step definitions in the Cypress step files to simulate the external RHOAS flow: add step handlers for the three steps that open a mocked token URL, extract a fake token, and programmatically populate the API Token input (ensure mocks avoid real network calls). Reference the feature step strings above and update cypress.config.js filters only if you choose to run real external tests.
🧹 Nitpick comments (4)
frontend/packages/knative-plugin/integration-tests/features/serverless/side-bar-of-knative-revision-and-service.feature (1)
62-63: Typo in scenario name: "Resoruce" → "Resource".While this line wasn't modified in this PR, since you're already touching this file for tag standardization, consider fixing the typo in the scenario title for consistency and clarity in test reports.
`@regression` - Scenario: Resoruce details of knative service in side bar: KN-06-TC04 + Scenario: Resource details of knative service in side bar: KN-06-TC04🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/knative-plugin/integration-tests/features/serverless/side-bar-of-knative-revision-and-service.feature` around lines 62 - 63, Fix the typo in the scenario title string: change "Resoruce details of knative service in side bar: KN-06-TC04" to "Resource details of knative service in side bar: KN-06-TC04" so the Scenario name is spelled correctly (look for the Scenario line in the feature file where the title appears).frontend/packages/knative-plugin/integration-tests/features/serverless/serverless-function.feature (1)
1-3: Minor: Pre-existing typo in Feature title.Line 2 has "fuctions" instead of "functions". While not part of this PR's changes, since you're already touching this file for lint compliance, it might be worth correcting the Feature title for consistency.
`@knative-serverless` `@knative` -Feature: Creation and Visualisation of serverless fuctions +Feature: Creation and Visualisation of serverless functions As a user, I want to create and verify a serverless function from Add Options and I should be able to differentiate between a plain Knative Service and a Serverless Function when looking at the Details page of the KSVC and the details tab of the side panel in Topology.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/knative-plugin/integration-tests/features/serverless/serverless-function.feature` around lines 1 - 3, Fix the typo in the Gherkin Feature title string "Creation and Visualisation of serverless fuctions" by correcting "fuctions" to "functions" so the line reads "Creation and Visualisation of serverless functions"; update the Feature header text (the line starting with Feature:) in the serverless-function.feature where that exact title appears and ensure spacing/formatting remain unchanged.frontend/tsconfig.json (1)
46-47: Remove duplicate exclude entry.The
"**/integration-tests"entry appears twice; keeping one is sufficient.♻️ Proposed cleanup
"exclude": [ ".yarn", "**/node_modules", "public/dist", "packages/console-dynamic-plugin-sdk/scripts", "**/integration-tests", - "**/integration-tests" + "**/integration-tests" ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/tsconfig.json` around lines 46 - 47, The tsconfig.json exclude array contains a duplicate entry "**/integration-tests"; remove one of the two identical entries so the exclude array only lists "**/integration-tests" once (locate the exclude array in tsconfig.json and update it accordingly).frontend/packages/knative-plugin/integration-tests/features/eventing/managed-services-kafka-connection.feature (1)
23-23: Minor: Typo in step definition — "buton" should be "button".This typo appears on lines 23, 43, 78, and 95. While pre-existing, since you're touching this file anyway, consider fixing for consistency. If there's a corresponding step definition matching "buton", both would need updating together.
📝 Suggested fix
- And user clicks on close buton + And user clicks on close buttonApply to lines 23, 43, 78, and 95.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/knative-plugin/integration-tests/features/eventing/managed-services-kafka-connection.feature` at line 23, Fix the typo in the Gherkin step text "And user clicks on close buton" by changing "buton" to "button" in all occurrences (the step at "And user clicks on close buton" and the other instances mentioned); also update any matching step definition/step regex or function name that currently expects "buton" so it matches the corrected step text (search for the exact step string or step definition regex that contains "buton" and replace it with "button" or adjust the regex accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/packages/knative-plugin/integration-tests/features/serverless/create-knative-service-from-deployment-or-deployment-config.feature`:
- Around line 60-61: The feature file has a feature-level `@broken-test` tag that
causes all scenarios to be excluded despite removing scenario-level tags; to
re-enable KN-04-TC05 and KN-04-TC06 either remove the `@broken-test` tag from the
top of this feature or move the two scenarios named "Edit knative workload
created from deployment: KN-04-TC05" and its KN-04-TC06 counterpart into a new
separate feature file without the `@broken-test` tag so the Cypress filter (which
excludes `@broken-test`) will allow those scenarios to run.
In `@README.md`:
- Around line 305-306: The link text "[**_More information on Console's Cypress
usage_**]" uses underscore-based emphasis causing markdownlint MD049; update the
inline emphasis to use asterisks instead (e.g., replace the underscores around
"More information on Console's Cypress usage" with asterisks so the text uses **
*...* ** or simply ***...*** as appropriate) to satisfy markdownlint; locate
this string in README.md and adjust the emphasis markers accordingly.
---
Outside diff comments:
In
`@frontend/packages/knative-plugin/integration-tests/features/eventing/managed-services-kafka-connection.feature`:
- Line 1: The three scenarios in managed-services-kafka-connection.feature now
marked `@regression` rely on missing step definitions for external interactions
("user clicks on link \"https://cloud.redhat.com/openshift/token\"", "user
copies the API token from the link", "user pastes the token in the API Token")
and will fail in CI because cypress.config.js includes `@regression` tests; either
(A) revert those three scenarios to include `@manual` (i.e., change their tags
back to "@regression `@manual`" to exclude them from CI) or (B) implement mocked
step definitions in the Cypress step files to simulate the external RHOAS flow:
add step handlers for the three steps that open a mocked token URL, extract a
fake token, and programmatically populate the API Token input (ensure mocks
avoid real network calls). Reference the feature step strings above and update
cypress.config.js filters only if you choose to run real external tests.
In
`@frontend/packages/knative-plugin/integration-tests/features/serverless/create-knative-service-from-deployment-or-deployment-config.feature`:
- Line 13: Fix the typo in the feature step text: replace the incorrect workload
name "dep-worload" with the correct "dep-workload" so the step matches the
created workload and existing step definitions; update the string in the
scenario step (search for "dep-worload") to "dep-workload" to prevent the step
mismatch.
---
Nitpick comments:
In
`@frontend/packages/knative-plugin/integration-tests/features/eventing/managed-services-kafka-connection.feature`:
- Line 23: Fix the typo in the Gherkin step text "And user clicks on close
buton" by changing "buton" to "button" in all occurrences (the step at "And user
clicks on close buton" and the other instances mentioned); also update any
matching step definition/step regex or function name that currently expects
"buton" so it matches the corrected step text (search for the exact step string
or step definition regex that contains "buton" and replace it with "button" or
adjust the regex accordingly).
In
`@frontend/packages/knative-plugin/integration-tests/features/serverless/serverless-function.feature`:
- Around line 1-3: Fix the typo in the Gherkin Feature title string "Creation
and Visualisation of serverless fuctions" by correcting "fuctions" to
"functions" so the line reads "Creation and Visualisation of serverless
functions"; update the Feature header text (the line starting with Feature:) in
the serverless-function.feature where that exact title appears and ensure
spacing/formatting remain unchanged.
In
`@frontend/packages/knative-plugin/integration-tests/features/serverless/side-bar-of-knative-revision-and-service.feature`:
- Around line 62-63: Fix the typo in the scenario title string: change "Resoruce
details of knative service in side bar: KN-06-TC04" to "Resource details of
knative service in side bar: KN-06-TC04" so the Scenario name is spelled
correctly (look for the Scenario line in the feature file where the title
appears).
In `@frontend/tsconfig.json`:
- Around line 46-47: The tsconfig.json exclude array contains a duplicate entry
"**/integration-tests"; remove one of the two identical entries so the exclude
array only lists "**/integration-tests" once (locate the exclude array in
tsconfig.json and update it accordingly).
...ests/features/serverless/create-knative-service-from-deployment-or-deployment-config.feature
Show resolved
Hide resolved
|
@logonoff: This pull request references CONSOLE-5094 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.22.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/label px-approved |
4afd41b to
71d0550
Compare
71d0550 to
374b3b5
Compare
- renamed all `integration-tests-cypress` folders to `integration-tests` - we only use cypress so this makes more sense - moved olm and console test scripts into their own package.json to be more consistent with other packages
notable changes: - 15.10.0 deprecates `Cypress.env()`. replacement is `cy.env()` - Minor performance improvements - electron/chromium version bumps - cy.press() supports more buttons now
374b3b5 to
abfa77a
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: logonoff, vojtechszocs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/label px-approved |
|
@logonoff: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
integration-tests-cypresstointegration-tests(the prevailing pattern)gherkin-lintissues and run it in CItest-cypressscriptsSummary by CodeRabbit