NO-JIRA: perses automation testing fixes after project selector fixes#833
Open
etmurasaki wants to merge 1 commit intoopenshift:mainfrom
Open
NO-JIRA: perses automation testing fixes after project selector fixes#833etmurasaki wants to merge 1 commit intoopenshift:mainfrom
etmurasaki wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@etmurasaki: This pull request explicitly references no jira issue. 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. |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: etmurasaki 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 |
openshift-ci
bot
added
the
approved
📝 WalkthroughWalkthroughMultiple Cypress test files are updated to add preliminary navigation steps before reaching Perses dashboards, test page objects are refactored to scope DOM selectors within dialog contexts, error validation messages in constants are revised, and fixture tag values are normalized to lowercase. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 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)
Comment |
![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 (2)
web/cypress/e2e/perses/99.coo_rbac_perses_user1.cy.ts (1)
66-67: Extract the two-step Perses navigation into one helper.This exact sequence now appears across all
99.coo_rbac_perses_user{1..6}.cy.tsspecs. Wrapping it in something likenav.sidenav.openPersesDashboards()would keep future route fixes in one place.♻️ Suggested cleanup
- nav.sidenav.clickNavLink(['Observe', 'Dashboards']); - nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']); + nav.sidenav.openPersesDashboards();// web/cypress/views/nav.ts openPersesDashboards: () => { nav.sidenav.clickNavLink(['Observe', 'Dashboards']); nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/e2e/perses/99.coo_rbac_perses_user1.cy.ts` around lines 66 - 67, Extract the repeated two-step navigation into a single helper method on the nav view: create a function named openPersesDashboards (e.g., nav.sidenav.openPersesDashboards) that calls nav.sidenav.clickNavLink(['Observe', 'Dashboards']) then nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']), and replace the two-line sequences in all 99.coo_rbac_perses_user{1..6}.cy.ts specs with a single call to nav.sidenav.openPersesDashboards() so future route changes are centralized.web/cypress/views/perses-dashboards-create-dashboard.ts (1)
65-69: Drop the new fixed pause before clicking Cancel.The button is already gated by
should('be.visible'), so the extracy.wait(2000)only adds suite latency and still leaves the interaction time-based instead of state-based.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/views/perses-dashboards-create-dashboard.ts` around lines 65 - 69, In createDashboardDialogCancelButton remove the pre-click fixed pause (the cy.wait(2000) immediately before the .byPFRole('dialog')...click call) and rely on the existing should('be.visible') assertion for a state-based interaction; leave the rest of the method unchanged so the click uses the visibility guard instead of a time-based wait.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/cypress/support/perses/99.coo_rbac_perses_user1.cy.ts`:
- Around line 479-494: The test is filtering/deleting using a hard-coded prefix
('Testing Dashboard - UP') which can match multiple dashboards; change the flow
to use the exact generated dashboard name stored in suite scope (e.g. a variable
created when the dashboard is made) and pass that variable to
listPersesDashboardsPage.filter.byName(generatedName) and to the delete action;
instead of listPersesDashboardsPage.clickKebabIcon() use a row-scoped action
(e.g. clickKebabForDashboard(generatedName) or locate the row by name before
clicking) so the kebab targets the exact row, and update countDashboards
assertions to expect '1'/'0' against that exact name. Ensure the generatedName
is accessible where the delete steps run (store in outer describe or
create+delete in the same it).
In `@web/cypress/views/perses-dashboards-import-dashboard.ts`:
- Around line 101-108: The assertions assertFailedToMigrateGrafanaDashboard and
assertDuplicatedDashboardError currently use cy.get('h4') which is too broad;
scope them to the alert component instead (e.g., use cy.get('[role="alert"]') or
your alert class) and then check the heading text inside that alert (for
example, cy.get('[role="alert"]').contains('h4',
persesDashboardsImportDashboard.DIALOG_FAILED_TO_MIGRATE_GRAFANA_DASHBOARD').should('be.visible')
and similarly for DIALOG_DUPLICATED_DASHBOARD_ERROR) so the match is constrained
to the alert component.
In `@web/src/components/data-test.ts`:
- Line 221: The selector PersesCreateDashboardProjectDropdown uses an exact
class-match string which is brittle; replace it with a standard class selector
(e.g., button.pf-v6-c-menu-toggle__button or just .pf-v6-c-menu-toggle__button)
wherever PersesCreateDashboardProjectDropdown is defined/used so it tolerates
additional classes and order changes—update the definition in
web/src/components/data-test.ts and replace all 13 usages across the Cypress
helpers (create, duplicate, import) to reference the new class-style selector.
---
Nitpick comments:
In `@web/cypress/e2e/perses/99.coo_rbac_perses_user1.cy.ts`:
- Around line 66-67: Extract the repeated two-step navigation into a single
helper method on the nav view: create a function named openPersesDashboards
(e.g., nav.sidenav.openPersesDashboards) that calls
nav.sidenav.clickNavLink(['Observe', 'Dashboards']) then
nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']), and replace the
two-line sequences in all 99.coo_rbac_perses_user{1..6}.cy.ts specs with a
single call to nav.sidenav.openPersesDashboards() so future route changes are
centralized.
In `@web/cypress/views/perses-dashboards-create-dashboard.ts`:
- Around line 65-69: In createDashboardDialogCancelButton remove the pre-click
fixed pause (the cy.wait(2000) immediately before the
.byPFRole('dialog')...click call) and rely on the existing should('be.visible')
assertion for a state-based interaction; leave the rest of the method unchanged
so the click uses the visibility guard instead of a time-based wait.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f57fe68d-cda1-40c6-a1d1-6480b2a0fdc9
📒 Files selected for processing (16)
web/cypress/e2e/perses/99.coo_rbac_perses_user1.cy.tsweb/cypress/e2e/perses/99.coo_rbac_perses_user2.cy.tsweb/cypress/e2e/perses/99.coo_rbac_perses_user3.cy.tsweb/cypress/e2e/perses/99.coo_rbac_perses_user4.cy.tsweb/cypress/e2e/perses/99.coo_rbac_perses_user5.cy.tsweb/cypress/e2e/perses/99.coo_rbac_perses_user6.cy.tsweb/cypress/fixtures/coo/coo141_perses/import/acm-vm-status.jsonweb/cypress/fixtures/perses/constants.tsweb/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user1.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user3.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user5.cy.tsweb/cypress/views/perses-dashboards-create-dashboard.tsweb/cypress/views/perses-dashboards-import-dashboard.tsweb/cypress/views/perses-dashboards-list-dashboards.tsweb/src/components/data-test.ts
Contributor
|
@etmurasaki: all tests passed! 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. |
jgbernalp
reviewed
Mar 12, 2026
| }); | ||
|
|
||
| beforeEach(() => { | ||
| nav.sidenav.clickNavLink(['Observe', 'Dashboards']); |
Contributor
There was a problem hiding this comment.
Not sure why we need to click on legacy dashboards to test perses. Is it some reset step missing?
jgbernalp
reviewed
Mar 12, 2026
| cy.log(`5.5. Click on the Kebab icon - Delete`); | ||
| nav.sidenav.clickNavLink(['Observe', 'Alerting']); | ||
| nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']);s | ||
| nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']); |
Contributor
There was a problem hiding this comment.
Are the testing files being excluded from typescript and lint checks? This should have been caught by the ts compiler.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit