OU-1184: lint enabled against cypress files and changes post coo#862
Open
etmurasaki wants to merge 1 commit intoopenshift:mainfrom
Open
OU-1184: lint enabled against cypress files and changes post coo#862etmurasaki wants to merge 1 commit intoopenshift:mainfrom
etmurasaki wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
@etmurasaki: This pull request references OU-1184 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 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. |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
needs-rebase
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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughFormatting and structural refactors across Cypress E2E specs and support code, plus a few behavioral/signature edits: Perses runner rename and one spec deletion, fixture path/tag updates, Prometheus mock shape change, reduced helper signatures, incident command branching refactor, and added ESLint disables/trailing-newlines. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/cypress/support/commands/selector-commands.ts (1)
8-44:⚠️ Potential issue | 🟠 MajorConsolidate the shared selector commands into one support module.
The Cypress support bootstrap file imports both
web/cypress/support/selectors.tsandweb/cypress/support/commands/selector-commands.ts, causing duplicateCypress.Commands.add()registrations forbyOUIAID,byClass,byDataID, andbySemanticElement. This creates a runtime risk of command override or failure and introduces multiple sources of truth for the same selectors. Consolidate all selector command implementations into a single module to eliminate duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/selector-commands.ts` around lines 8 - 44, There are duplicated Cypress.Commands.add registrations for byOUIAID, byClass, byDataID, and bySemanticElement across the two selector modules; consolidate by moving the canonical implementations into a single support module (the selector-commands implementation should be the source of truth or vice versa), remove the duplicate Cypress.Commands.add calls from the other module, export any shared helper functions instead of re-registering commands, and update the support bootstrap to import only the consolidated module so each command (byOUIAID, byClass, byDataID, bySemanticElement, etc.) is registered exactly once.
🧹 Nitpick comments (16)
web/cypress/views/operator-hub-page.ts (1)
6-7: UseURLSearchParamsand stop hardcodingtargetNamespace=undefined.Line 6–7 manually concatenates query params and injects raw values; this is brittle for names containing reserved characters and obscures namespace intent. Build the URL via
URLSearchParamsand derivetargetNamespacefrominstallNamespace(or omit it when unset).Proposed refactor
- cy.visit( - `/operatorhub/subscribe?pkg=${operatorName}&catalog=${csName}` + - `&catalogNamespace=openshift-marketplace&targetNamespace=undefined`, - ); + const params = new URLSearchParams({ + pkg: operatorName, + catalog: csName, + catalogNamespace: 'openshift-marketplace', + ...(installNamespace ? { targetNamespace: installNamespace } : {}), + }); + cy.visit(`/operatorhub/subscribe?${params.toString()}`);As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/views/operator-hub-page.ts` around lines 6 - 7, Replace the manual query-string concatenation that builds `/operatorhub/subscribe?pkg=${operatorName}&catalog=${csName}&catalogNamespace=openshift-marketplace&targetNamespace=undefined` with a URLSearchParams-based builder: create a new URLSearchParams, append pkg (operatorName), catalog (csName) and catalogNamespace, then only append targetNamespace when installNamespace is set (use installNamespace as the value); finally build the full path as '/operatorhub/subscribe?' + params.toString() so values are properly encoded. Target the code that references operatorName, csName and the current hardcoded targetNamespace value.web/cypress/support/perses/99.coo_rbac_perses_user6.cy.ts (1)
17-20: Consider using a template literal for improved readability.The test description uses string concatenation across multiple lines, which is less readable than a single template literal. A template literal would make the test name easier to scan and maintain.
📝 Refactor to use template literal
- it( - `1.${perspective.name} perspective - List Dashboards - Namespace validation and ` + - `Dashboard search`, - () => { + it(`1.${perspective.name} perspective - List Dashboards - Namespace validation and Dashboard search`, () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/99.coo_rbac_perses_user6.cy.ts` around lines 17 - 20, Replace the concatenated multi-line test title in the it(...) call with a single template literal using the perspective.name variable for readability; locate the it invocation that begins with `it(` and change the string concatenation (`'1.' + perspective.name + ' perspective - List Dashboards - Namespace validation and ' + 'Dashboard search'`) into one template literal (e.g., `` `1.${perspective.name} perspective - List Dashboards - Namespace validation and Dashboard search` ``) so the test name is a single, readable string.web/cypress/views/alerting-rule-details-page.ts (1)
26-34: Consider removing ineffective try/catch blocks.Cypress commands are asynchronous and queued, so
try/catchblocks cannot catch Cypress assertion failures or command errors. Thecatchblocks inclickAlertingRulesBreadcrumbandassertNoKebabwill never execute for Cypress failures. The error handling is ineffective, though it doesn't break functionality.Consider removing these try/catch blocks or using Cypress's built-in error handling mechanisms (e.g.,
.should()with custom assertions or.then()callbacks).Do you want me to generate updated implementations without the try/catch blocks?
Also applies to: 36-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/views/alerting-rule-details-page.ts` around lines 26 - 34, Remove the ineffective try/catch blocks around Cypress commands (they cannot catch Cypress failures) in the clickAlertingRulesBreadcrumb and assertNoKebab helpers: delete the surrounding try { ... } catch (error) { ... } blocks, keep the cy.log calls and the Cypress chain (e.g., cy.byTestID(DataTestIDs.Breadcrumb).contains('Alerting rules').click()), and if you need assertions convert them to proper Cypress assertions using .should() or .then() rather than a try/catch; apply the same change to the other occurrence referenced (lines 36-45) so all Cypress command helpers return the Cypress chain and rely on Cypress error handling.web/cypress/support/monitoring/03.reg_legacy_dashboards.cy.ts (1)
115-124: Consider replacing fixed waits with assertions.Multiple
cy.wait(2000)calls make tests slower and potentially flaky. Cypress best practice is to use assertions that implicitly wait for conditions, e.g., chaining.should('not.exist')or.should('be.visible')after actions.However, if this pattern is established in the codebase, it can be addressed in a follow-up.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/monitoring/03.reg_legacy_dashboards.cy.ts` around lines 115 - 124, Replace the fixed cy.wait(2000) calls with Cypress assertions that implicitly wait: after alertingRuleDetailsPage.clickShowGraphButton() chain cy.byTestID(DataTestIDs.MetricGraph).should('be.visible') (instead of a preceding wait), and after alertingRuleDetailsPage.clickHideGraphButton() chain cy.byTestID(DataTestIDs.MetricGraph).should('not.exist'); remove the intermediate cy.wait(...) lines and any extra waits before those initial/final assertions so the test relies on implicit retries rather than fixed delays.web/cypress/support/commands/dashboards-commands.ts (2)
36-110: Theif/elsebranches apply identical fixture paths — consider consolidating.Both the
COO_UI_INSTALLand non-COO_UI_INSTALLbranches now apply the samecoo140_perses/dashboards/*fixtures. Unless future divergence is planned, this duplication could be simplified to a single code path.The TODO on line 33-34 suggests this may be intentional WIP. If so, consider adding a comment clarifying when the branches will differ.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/dashboards-commands.ts` around lines 36 - 110, The if/else controlled by Cypress.env('COO_UI_INSTALL') duplicates the same cy.exec apply calls for the five dashboard fixtures (openshift-cluster-sample-dashboard.yaml, perses-dashboard-sample.yaml, prometheus-overview-variables.yaml, thanos-compact-overview-1var.yaml, thanos-querier-datasource.yaml); consolidate by removing the duplicate branch and executing the cy.exec sequence once (keep the cy.log messages and use the same KUBECONFIG interpolation), or if branching is intentionally WIP (see TODO around COO_UI_INSTALL), replace the duplicate branch with a short comment explaining when the behavior will diverge to avoid confusion.
210-283: Same duplication incleanupDashboards— both branches are identical.The cleanup logic mirrors the setup logic with the same duplicated branches. If you consolidate
setupDashboardsAndPlugins, apply the same simplification here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/dashboards-commands.ts` around lines 210 - 283, cleanupDashboards duplicates the same oc delete calls in both branches; consolidate by removing the duplicated blocks and running the delete sequence once. Update the cleanupDashboards function to compute the kubeconfig and any differing log message (use Cypress.env('COO_UI_INSTALL')) if needed, then iterate over the dashboard filenames (e.g., openshift-cluster-sample-dashboard.yaml, perses-dashboard-sample.yaml, prometheus-overview-variables.yaml, thanos-compact-overview-1var.yaml, thanos-querier-datasource.yaml) and call cy.executeAndDelete with the same `oc delete -f ./cypress/fixtures/coo/coo140_perses/dashboards/<filename> --ignore-not-found --kubeconfig ${Cypress.env('KUBECONFIG_PATH')}` template; keep function name cleanupDashboards and mirror the simplification you applied to setupDashboardsAndPlugins.web/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.ts (1)
115-117: Navigation workaround pattern noted.The sidebar navigation via Alerting and back to Dashboards (Perses) appears to be a workaround for clearing state. This pattern is consistent with similar changes in this PR. Consider adding a brief comment explaining why direct navigation is needed here if it's non-obvious.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.ts` around lines 115 - 117, The test uses a navigation workaround calling nav.sidenav.clickNavLink(['Observe', 'Alerting']) then nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']) to clear/refresh sidebar state; add a concise inline comment above these two calls explaining the reason (e.g., “force sidebar state reset to avoid stale selection between tests”) and why direct navigation is required, referencing the nav.sidenav.clickNavLink usage so future readers understand this non-obvious workaround.web/cypress/support/commands/perses-commands.ts (1)
131-136: Inconsistent kubeconfig variable usage.The
kcvariable is defined at line 94 but not used here. For consistency with the rest ofcleanupExtraDashboards, use the cached variable.Suggested fix
cy.log('Remove observ-test namespace'); cy.exec( - `oc delete namespace observ-test --ignore-not-found --kubeconfig ${Cypress.env( - 'KUBECONFIG_PATH', - )}`, + `oc delete namespace observ-test --ignore-not-found --kubeconfig ${kc}`, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/perses-commands.ts` around lines 131 - 136, The cleanupExtraDashboards routine uses a cached kubeconfig variable kc (defined earlier) but this cy.exec call still inlines Cypress.env('KUBECONFIG_PATH'); update the cy.exec invocation inside cleanupExtraDashboards to reference the existing kc variable instead of calling Cypress.env again (replace the template usage of Cypress.env('KUBECONFIG_PATH') with kc) so the command `oc delete namespace observ-test --ignore-not-found --kubeconfig ...` consistently uses the cached kubeconfig value.web/cypress/e2e/perses/04.coo_import_perses_admin.cy.ts (1)
20-23: Remove stale TODO comment.The tag has been changed to
@dashboards, so the TODO on line 20 is no longer relevant.Suggested fix
-//TODO: change tag to `@dashboards` when customizable-dashboards gets merged describe( 'COO - Dashboards (Perses) - Import perses dashboard', { tags: ['@perses', '@dashboards'] },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/e2e/perses/04.coo_import_perses_admin.cy.ts` around lines 20 - 23, Remove the stale TODO comment on the line above the describe block in the test file: delete the "//TODO: change tag to `@dashboards` when customizable-dashboards gets merged" comment so the describe('COO - Dashboards (Perses) - Import perses dashboard', { tags: ['@perses', '@dashboards'] }, ...) block no longer contains outdated TODO text and the file only retains the relevant describe and tags.web/cypress/e2e/perses/99.coo_rbac_perses_user3.cy.ts (1)
20-23: TODO comment may be stale.Similar to the other file, line 20 mentions changing tags when
customizable-dashboardsgets merged, but the tags have already been updated to['@perses', '@perses-dev']. Consider removing or updating the TODO.🤖 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_user3.cy.ts` around lines 20 - 23, Remove or update the stale TODO comment that precedes the top-level describe('RBAC User3: COO - Dashboards (Perses) - Administrator perspective', { tags: ['@perses', '@perses-dev'] }) block: either delete the line "//TODO: change tag to `@smoke`, `@dashboards`, `@perses` when customizable-dashboards gets merged" or replace it with an accurate note reflecting the current tags (['@perses', '@perses-dev']) so the comment no longer suggests outdated work.web/cypress/e2e/perses/02.coo_edit_perses_admin.cy.ts (1)
21-24: TODO comment may be stale.The comment on line 21 mentions changing the tag to
@dashboardswhencustomizable-dashboardsgets merged, but line 24 already shows the tag as@dashboards. Consider removing or updating the TODO if this is now resolved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/e2e/perses/02.coo_edit_perses_admin.cy.ts` around lines 21 - 24, The TODO comment about changing the tag to `@dashboards` is stale—remove or update the TODO above the describe block for "COO - Dashboards (Perses) - Edit perses dashboard"; specifically edit the comment that references customizable-dashboards so it either is deleted or changed to reflect that the tags array already includes '@dashboards' in the describe(...) declaration (check the tags: ['@perses', '@dashboards'] and the describe title to ensure consistency).web/cypress/e2e/perses/99.coo_rbac_perses_user6.cy.ts (1)
67-70: Verify intentional double navigation.The
beforeEachhook navigates to both'Dashboards'and'Dashboards (Perses)'sequentially. If this is a workaround for a navigation issue, consider adding a brief comment explaining why both clicks are needed to help future maintainers.🤖 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_user6.cy.ts` around lines 67 - 70, The beforeEach hook currently calls nav.sidenav.clickNavLink(['Observe', 'Dashboards']) followed immediately by nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']); if the double navigation is intentional (e.g., to work around a flaky route or ensure correct menu state), add a short explanatory comment above the calls describing the reason and any relevant ticket/issue ID; otherwise remove the first click so only nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']) remains. Ensure the comment references beforeEach and nav.sidenav.clickNavLink so future maintainers understand why both navigations are present.web/cypress/support/perses/02.coo_edit_perses_admin.cy.ts (1)
45-46: Consider replacing hardcoded waits with explicit condition waits.Multiple
cy.wait()calls with hardcoded durations (15000ms, 10000ms, 2000ms) throughout this file can lead to flaky tests and unnecessarily slow execution. Cypress best practices recommend waiting for specific conditions instead.For example, instead of
cy.wait(15000), consider waiting for a specific element or network request that indicates the page is ready.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/02.coo_edit_perses_admin.cy.ts` around lines 45 - 46, Replace hardcoded cy.wait calls (e.g., the cy.wait(15000) before persesDashboardsPage.clickEditButton() and other 10000/2000ms waits in this spec) with explicit condition waits: intercept the relevant network request(s) using cy.intercept(...) and wait on an alias (cy.wait('@alias')) or wait for a UI element to be ready (cy.get('<selector>').should('be.visible') or .should('exist') or .should('not.be.disabled')) before calling persesDashboardsPage.clickEditButton() or other actions; update each occurrence to target the specific request/DOM selector that signals readiness rather than a fixed timeout.web/cypress/support/perses/00.coo_bvt_perses_admin.cy.ts (1)
202-204: Unusual base for random suffix generation.
Math.random().toString(5)produces a string with only digits 0-4, resulting in a less unique suffix. Consider usingtoString(36)for alphanumeric characters orDate.now()for timestamp-based uniqueness.- const randomSuffix = Math.random().toString(5); + const randomSuffix = Math.random().toString(36).substring(2, 8);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/00.coo_bvt_perses_admin.cy.ts` around lines 202 - 204, The random suffix generation for dashboardName currently uses Math.random().toString(5) which yields only digits 0-4; update the generation of randomSuffix to produce a more unique alphanumeric string (e.g., use Math.random().toString(36) and slice off the leading "0." or combine with Date.now() for extra uniqueness) and append that to dashboardName; change references around the randomSuffix variable to use the new generation strategy (look for the dashboardName and randomSuffix variables in the test).web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts (1)
82-83: Hard-coded waits may cause test flakiness.Multiple
cy.wait(2000)calls (lines 82, 167, 199, 278) can lead to flaky tests - either failing on slower environments or unnecessarily slowing execution. Consider replacing with explicit waits for UI elements or API responses when possible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts` around lines 82 - 83, Replace the hard-coded cy.wait(2000) calls with deterministic waits: remove the cy.wait(2000) before persesDashboardsPage.clickEditButton() and instead wait for the target UI element or relevant network call—e.g., add a cy.intercept(...) and cy.wait('@yourAlias') for the dashboard load or assert the edit button is visible/enabled (e.g., persesDashboardsPage.getEditButton().should('be.visible').and('not.be.disabled')) before calling persesDashboardsPage.clickEditButton(); apply the same change for the other cy.wait(2000) occurrences so all sleeps are replaced by explicit element or API waits.web/cypress/support/incidents_prometheus_query_mocks/mock-generators.ts (1)
187-198: Debug logging in mock generators - consider making conditional.The
console.logstatements with eslint-disable directives are useful for debugging but will output to console during every test run. For cleaner test output, consider wrapping these in a debug flag or using Cypress's built-in logging (cy.log) if applicable from this context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/incidents_prometheus_query_mocks/mock-generators.ts` around lines 187 - 198, The console.log debug lines around the mock generator loop (the logs referencing alert.name, incident.id and results.length around the results.push) should be made conditional: add a debug flag (e.g., a DEBUG_MOCKS constant or read from Cypress.env) or replace console.log with Cypress.log, then wrap each log (the three inside the loop and the final "Final results array length" log) in an if (DEBUG) check or call Cypress.log so they only appear when debugging is enabled; update usages around results.push, alert, and incident to use the conditional logger and remove the eslint-disable-next-line no-console comments.
🤖 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/e2e/incidents/regression/03.reg_api_calls.cy.ts`:
- Around line 87-93: Two identical step labels and out-of-order numbering are
present around the hover/tooltip checks: the cy.log calls and verifyAlertTooltip
invocations (verifyAlertTooltip(0, ['SyntheticSharedFiring002'], true)) are
duplicated and misnumbered as 1.4 then 1.3; update the cy.log strings so the
steps are unique and in logical order (e.g., 1.3 then 1.4) or remove the
duplicate block if unintended, ensuring the verifyAlertTooltip call remains once
per intended step and the accompanying cy.log text matches the corrected step
number.
In `@web/cypress/e2e/perses/01.coo_list_perses_admin.cy.ts`:
- Around line 51-54: The describe block titled 'COO - Dashboards (Perses) - List
perses dashboards - Namespace' has an incorrect tag `@dashboards-`; update the
tags object in that describe call to use `@dashboards` (so it matches the other
Perses suites and line 27) by replacing `@dashboards-` with `@dashboards` in the
tags array for this describe declaration.
In `@web/cypress/e2e/perses/03.coo_create_perses_admin.cy.ts`:
- Around line 31-35: Replace the hard-coded cy.wait(5000) inside the beforeEach
that calls nav.sidenav.clickNavLink and cy.changeNamespace with a deterministic
visibility check: remove cy.wait(5000) and instead wait for the dashboards UI
root element to be visible using
cy.get('<dashboards-root-selector>').should('be.visible', { timeout: 15000 });
use the same selector that web/cypress/support/commands/perses-commands.ts uses
for post-navigation checks so beforeEach (nav.sidenav.clickNavLink,
cy.changeNamespace) only proceeds when the dashboards UI is actually loaded.
In `@web/cypress/support/incidents_prometheus_query_mocks/utils.ts`:
- Around line 71-74: The parseDuration utility currently calls cy.log, which
will throw when used outside Cypress; update parseDuration in
incidents_prometheus_query_mocks/utils.ts to remove reliance on the Cypress
global by replacing cy.log with a non-test runtime logger (e.g.,
console.log/console.warn) or simply remove the debug log since the function
throws with the diagnostic message immediately after; ensure the thrown error
still contains the original diagnostic text so callers receive the same
information.
In `@web/cypress/support/monitoring/01.reg_alerts.cy.ts`:
- Around line 213-214: The test contains a duplicate consecutive call to
silencesListPage.shouldBeLoaded(); remove the redundant second invocation so the
page load assertion is only performed once; update the test to leave a single
silencesListPage.shouldBeLoaded() call (keeping the first) and run the spec to
ensure no behavioral change.
In `@web/cypress/support/monitoring/04.reg_alerts_namespace.cy.ts`:
- Around line 213-214: Remove the duplicated assertion call: the test invokes
silencesListPage.shouldBeLoaded() twice in succession (the repeated call on the
same page is redundant); locate the duplicate invocation of
silencesListPage.shouldBeLoaded() and delete the second occurrence so the test
only calls it once.
In `@web/cypress/support/perses/01.coo_list_perses_admin.cy.ts`:
- Around line 287-299: The test uses hardcoded cy.wait(2000) calls which cause
flakiness; remove those waits and replace them with deterministic UI assertions
that wait for the expected state before proceeding — for example after
navigating with nav.sidenav.clickNavLink(...) assert the dashboards list is
visible, after listPersesDashboardsPage.filter.byProject('perses-dev') assert
the project filter chip or results are visible/updated, and after
listPersesDashboardsPage.filter.byName(...) assert the specific dashboard item
(e.g., the value from
persesDashboardsDashboardDropdownPersesDev.THANOS_COMPACT_OVERVIEW[0]) is
visible or the countDashboards('1') condition becomes true; update the test flow
to use these explicit should(...) or byTestID checks instead of cy.wait().
In `@web/cypress/support/perses/02.coo_edit_perses_admin.cy.ts`:
- Around line 264-275: Update the duplicated step numbers in the test's log
messages so they read sequentially: change the cy.log text for the calls around
persesDashboardsPage.clickEditActionButton('EditVariables'),
persesDashboardsEditVariables.toggleVariableVisibility(1, true),
persesDashboardsEditVariables.clickDeleteVariableButton(0), and
persesDashboardsEditVariables.clickButton('Apply') (and the subsequent
persesDashboardsPage.clickEditActionButton('Save') call if it logs a step) from
the repeated 4.16/4.17 labels to 4.16, 4.17, 4.18, 4.19 respectively so the step
numbering is unique and in order.
In `@web/cypress/views/perses-dashboards-create-dashboard.ts`:
- Around line 73-77: The test helper assertDuplicatedNameValidation was changed
to a zero-arg signature but is still invoked with one argument at call sites;
restore a single-parameter signature so callers compile: change
assertDuplicatedNameValidation to accept one parameter (e.g., expectedText) and
inside the function use that parameter in the .should('have.text', ...)
assertion instead of the hardcoded
persesCreateDashboard.DIALOG_DUPLICATED_NAME_BKD_VALIDATION; keep the existing
call sites that pass the expected text.
- Around line 82-87: Remove the unconditional cy.wait(2000) call and rely on
Cypress's retryable assertions: delete the cy.wait line and keep the existing
chain starting from cy.byPFRole('dialog') (the locator used) and its
.should('be.visible') check before clicking the Cancel button; if an extra guard
is needed use a retryable assertion on cy.byPFRole('dialog') (e.g.,
.should('exist') or .should('be.visible')) rather than a fixed sleep so the
Cancel button click only proceeds when the dialog is ready.
---
Outside diff comments:
In `@web/cypress/support/commands/selector-commands.ts`:
- Around line 8-44: There are duplicated Cypress.Commands.add registrations for
byOUIAID, byClass, byDataID, and bySemanticElement across the two selector
modules; consolidate by moving the canonical implementations into a single
support module (the selector-commands implementation should be the source of
truth or vice versa), remove the duplicate Cypress.Commands.add calls from the
other module, export any shared helper functions instead of re-registering
commands, and update the support bootstrap to import only the consolidated
module so each command (byOUIAID, byClass, byDataID, bySemanticElement, etc.) is
registered exactly once.
---
Nitpick comments:
In `@web/cypress/e2e/perses/02.coo_edit_perses_admin.cy.ts`:
- Around line 21-24: The TODO comment about changing the tag to `@dashboards` is
stale—remove or update the TODO above the describe block for "COO - Dashboards
(Perses) - Edit perses dashboard"; specifically edit the comment that references
customizable-dashboards so it either is deleted or changed to reflect that the
tags array already includes '@dashboards' in the describe(...) declaration
(check the tags: ['@perses', '@dashboards'] and the describe title to ensure
consistency).
In `@web/cypress/e2e/perses/04.coo_import_perses_admin.cy.ts`:
- Around line 20-23: Remove the stale TODO comment on the line above the
describe block in the test file: delete the "//TODO: change tag to `@dashboards`
when customizable-dashboards gets merged" comment so the describe('COO -
Dashboards (Perses) - Import perses dashboard', { tags: ['@perses',
'@dashboards'] }, ...) block no longer contains outdated TODO text and the file
only retains the relevant describe and tags.
In `@web/cypress/e2e/perses/99.coo_rbac_perses_user3.cy.ts`:
- Around line 20-23: Remove or update the stale TODO comment that precedes the
top-level describe('RBAC User3: COO - Dashboards (Perses) - Administrator
perspective', { tags: ['@perses', '@perses-dev'] }) block: either delete the
line "//TODO: change tag to `@smoke`, `@dashboards`, `@perses` when
customizable-dashboards gets merged" or replace it with an accurate note
reflecting the current tags (['@perses', '@perses-dev']) so the comment no
longer suggests outdated work.
In `@web/cypress/e2e/perses/99.coo_rbac_perses_user6.cy.ts`:
- Around line 67-70: The beforeEach hook currently calls
nav.sidenav.clickNavLink(['Observe', 'Dashboards']) followed immediately by
nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']); if the double
navigation is intentional (e.g., to work around a flaky route or ensure correct
menu state), add a short explanatory comment above the calls describing the
reason and any relevant ticket/issue ID; otherwise remove the first click so
only nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']) remains.
Ensure the comment references beforeEach and nav.sidenav.clickNavLink so future
maintainers understand why both navigations are present.
In `@web/cypress/support/commands/dashboards-commands.ts`:
- Around line 36-110: The if/else controlled by Cypress.env('COO_UI_INSTALL')
duplicates the same cy.exec apply calls for the five dashboard fixtures
(openshift-cluster-sample-dashboard.yaml, perses-dashboard-sample.yaml,
prometheus-overview-variables.yaml, thanos-compact-overview-1var.yaml,
thanos-querier-datasource.yaml); consolidate by removing the duplicate branch
and executing the cy.exec sequence once (keep the cy.log messages and use the
same KUBECONFIG interpolation), or if branching is intentionally WIP (see TODO
around COO_UI_INSTALL), replace the duplicate branch with a short comment
explaining when the behavior will diverge to avoid confusion.
- Around line 210-283: cleanupDashboards duplicates the same oc delete calls in
both branches; consolidate by removing the duplicated blocks and running the
delete sequence once. Update the cleanupDashboards function to compute the
kubeconfig and any differing log message (use Cypress.env('COO_UI_INSTALL')) if
needed, then iterate over the dashboard filenames (e.g.,
openshift-cluster-sample-dashboard.yaml, perses-dashboard-sample.yaml,
prometheus-overview-variables.yaml, thanos-compact-overview-1var.yaml,
thanos-querier-datasource.yaml) and call cy.executeAndDelete with the same `oc
delete -f ./cypress/fixtures/coo/coo140_perses/dashboards/<filename>
--ignore-not-found --kubeconfig ${Cypress.env('KUBECONFIG_PATH')}` template;
keep function name cleanupDashboards and mirror the simplification you applied
to setupDashboardsAndPlugins.
In `@web/cypress/support/commands/perses-commands.ts`:
- Around line 131-136: The cleanupExtraDashboards routine uses a cached
kubeconfig variable kc (defined earlier) but this cy.exec call still inlines
Cypress.env('KUBECONFIG_PATH'); update the cy.exec invocation inside
cleanupExtraDashboards to reference the existing kc variable instead of calling
Cypress.env again (replace the template usage of Cypress.env('KUBECONFIG_PATH')
with kc) so the command `oc delete namespace observ-test --ignore-not-found
--kubeconfig ...` consistently uses the cached kubeconfig value.
In `@web/cypress/support/incidents_prometheus_query_mocks/mock-generators.ts`:
- Around line 187-198: The console.log debug lines around the mock generator
loop (the logs referencing alert.name, incident.id and results.length around the
results.push) should be made conditional: add a debug flag (e.g., a DEBUG_MOCKS
constant or read from Cypress.env) or replace console.log with Cypress.log, then
wrap each log (the three inside the loop and the final "Final results array
length" log) in an if (DEBUG) check or call Cypress.log so they only appear when
debugging is enabled; update usages around results.push, alert, and incident to
use the conditional logger and remove the eslint-disable-next-line no-console
comments.
In `@web/cypress/support/monitoring/03.reg_legacy_dashboards.cy.ts`:
- Around line 115-124: Replace the fixed cy.wait(2000) calls with Cypress
assertions that implicitly wait: after
alertingRuleDetailsPage.clickShowGraphButton() chain
cy.byTestID(DataTestIDs.MetricGraph).should('be.visible') (instead of a
preceding wait), and after alertingRuleDetailsPage.clickHideGraphButton() chain
cy.byTestID(DataTestIDs.MetricGraph).should('not.exist'); remove the
intermediate cy.wait(...) lines and any extra waits before those initial/final
assertions so the test relies on implicit retries rather than fixed delays.
In `@web/cypress/support/perses/00.coo_bvt_perses_admin.cy.ts`:
- Around line 202-204: The random suffix generation for dashboardName currently
uses Math.random().toString(5) which yields only digits 0-4; update the
generation of randomSuffix to produce a more unique alphanumeric string (e.g.,
use Math.random().toString(36) and slice off the leading "0." or combine with
Date.now() for extra uniqueness) and append that to dashboardName; change
references around the randomSuffix variable to use the new generation strategy
(look for the dashboardName and randomSuffix variables in the test).
In `@web/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.ts`:
- Around line 115-117: The test uses a navigation workaround calling
nav.sidenav.clickNavLink(['Observe', 'Alerting']) then
nav.sidenav.clickNavLink(['Observe', 'Dashboards (Perses)']) to clear/refresh
sidebar state; add a concise inline comment above these two calls explaining the
reason (e.g., “force sidebar state reset to avoid stale selection between
tests”) and why direct navigation is required, referencing the
nav.sidenav.clickNavLink usage so future readers understand this non-obvious
workaround.
In `@web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts`:
- Around line 82-83: Replace the hard-coded cy.wait(2000) calls with
deterministic waits: remove the cy.wait(2000) before
persesDashboardsPage.clickEditButton() and instead wait for the target UI
element or relevant network call—e.g., add a cy.intercept(...) and
cy.wait('@yourAlias') for the dashboard load or assert the edit button is
visible/enabled (e.g.,
persesDashboardsPage.getEditButton().should('be.visible').and('not.be.disabled'))
before calling persesDashboardsPage.clickEditButton(); apply the same change for
the other cy.wait(2000) occurrences so all sleeps are replaced by explicit
element or API waits.
In `@web/cypress/support/perses/02.coo_edit_perses_admin.cy.ts`:
- Around line 45-46: Replace hardcoded cy.wait calls (e.g., the cy.wait(15000)
before persesDashboardsPage.clickEditButton() and other 10000/2000ms waits in
this spec) with explicit condition waits: intercept the relevant network
request(s) using cy.intercept(...) and wait on an alias (cy.wait('@alias')) or
wait for a UI element to be ready (cy.get('<selector>').should('be.visible') or
.should('exist') or .should('not.be.disabled')) before calling
persesDashboardsPage.clickEditButton() or other actions; update each occurrence
to target the specific request/DOM selector that signals readiness rather than a
fixed timeout.
In `@web/cypress/support/perses/99.coo_rbac_perses_user6.cy.ts`:
- Around line 17-20: Replace the concatenated multi-line test title in the
it(...) call with a single template literal using the perspective.name variable
for readability; locate the it invocation that begins with `it(` and change the
string concatenation (`'1.' + perspective.name + ' perspective - List Dashboards
- Namespace validation and ' + 'Dashboard search'`) into one template literal
(e.g., `` `1.${perspective.name} perspective - List Dashboards - Namespace
validation and Dashboard search` ``) so the test name is a single, readable
string.
In `@web/cypress/views/alerting-rule-details-page.ts`:
- Around line 26-34: Remove the ineffective try/catch blocks around Cypress
commands (they cannot catch Cypress failures) in the
clickAlertingRulesBreadcrumb and assertNoKebab helpers: delete the surrounding
try { ... } catch (error) { ... } blocks, keep the cy.log calls and the Cypress
chain (e.g., cy.byTestID(DataTestIDs.Breadcrumb).contains('Alerting
rules').click()), and if you need assertions convert them to proper Cypress
assertions using .should() or .then() rather than a try/catch; apply the same
change to the other occurrence referenced (lines 36-45) so all Cypress command
helpers return the Cypress chain and rely on Cypress error handling.
In `@web/cypress/views/operator-hub-page.ts`:
- Around line 6-7: Replace the manual query-string concatenation that builds
`/operatorhub/subscribe?pkg=${operatorName}&catalog=${csName}&catalogNamespace=openshift-marketplace&targetNamespace=undefined`
with a URLSearchParams-based builder: create a new URLSearchParams, append pkg
(operatorName), catalog (csName) and catalogNamespace, then only append
targetNamespace when installNamespace is set (use installNamespace as the
value); finally build the full path as '/operatorhub/subscribe?' +
params.toString() so values are properly encoded. Target the code that
references operatorName, csName and the current hardcoded targetNamespace value.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 76a590dc-b1c6-410b-9a28-04da08bb7597
📒 Files selected for processing (126)
web/cypress/dummy.tsxweb/cypress/e2e/coo/01.coo_bvt.cy.tsweb/cypress/e2e/coo/01.coo_ivt.cy.tsweb/cypress/e2e/coo/02.acm_alerting_ui.cy.tsweb/cypress/e2e/incidents/00.coo_incidents_e2e.cy.tsweb/cypress/e2e/incidents/01.incidents.cy.tsweb/cypress/e2e/incidents/02.incidents-mocking-example.cy.tsweb/cypress/e2e/incidents/regression/01.reg_filtering.cy.tsweb/cypress/e2e/incidents/regression/02.reg_ui_charts_comprehensive.cy.tsweb/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.tsweb/cypress/e2e/incidents/regression/03.reg_api_calls.cy.tsweb/cypress/e2e/incidents/regression/04.reg_redux_effects.cy.tsweb/cypress/e2e/monitoring/00.bvt_admin.cy.tsweb/cypress/e2e/monitoring/00.bvt_dev.cy.tsweb/cypress/e2e/monitoring/regression/01.reg_alerts_admin.cy.tsweb/cypress/e2e/monitoring/regression/01.reg_alerts_dev.cy.tsweb/cypress/e2e/monitoring/regression/02.reg_metrics_admin_1.cy.tsweb/cypress/e2e/monitoring/regression/02.reg_metrics_admin_2.cy.tsweb/cypress/e2e/monitoring/regression/03.reg_legacy_dashboards_admin.cy.tsweb/cypress/e2e/perses/00.coo_bvt_perses_admin.cy.tsweb/cypress/e2e/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/e2e/perses/01.coo_list_perses_admin.cy.tsweb/cypress/e2e/perses/02.coo_edit_perses_admin.cy.tsweb/cypress/e2e/perses/03.coo_create_perses_admin.cy.tsweb/cypress/e2e/perses/04.coo_import_perses_admin.cy.tsweb/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/e2e/virtualization/00.coo_ivt.cy.tsweb/cypress/e2e/virtualization/01.coo_ivt_alerts.cy.tsweb/cypress/e2e/virtualization/02.coo_ivt_metrics_1.cy.tsweb/cypress/e2e/virtualization/02.coo_ivt_metrics_2.cy.tsweb/cypress/e2e/virtualization/03.coo_ivt_legacy_dashboards.cy.tsweb/cypress/e2e/virtualization/04.coo_ivt_perses.cy.tsweb/cypress/fixtures/coo/coo140_perses/dashboards/openshift-cluster-sample-dashboard.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/perses-dashboard-sample.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/prometheus-overview-variables.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/thanos-compact-overview-1var.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/thanos-querier-datasource.yamlweb/cypress/fixtures/coo/coo140_perses/import/accelerators-dashboard-cr-v1alpha1.yamlweb/cypress/fixtures/coo/coo140_perses/import/accelerators-dashboard-cr-v1alpha2.yamlweb/cypress/fixtures/coo/coo140_perses/import/acm-vm-status.jsonweb/cypress/fixtures/coo/coo140_perses/import/grafana_to_check_errors.jsonweb/cypress/fixtures/coo/coo140_perses/import/testing-perses-dashboard.jsonweb/cypress/fixtures/coo/coo140_perses/import/testing-perses-dashboard.yamlweb/cypress/fixtures/coo/coo140_perses/rbac/rbac_perses_e2e_ci_users.shweb/cypress/fixtures/monitoring/alert.tsweb/cypress/fixtures/monitoring/constants.tsweb/cypress/fixtures/perses/constants.tsweb/cypress/support/commands/auth-commands.tsweb/cypress/support/commands/coo-install-commands.tsweb/cypress/support/commands/dashboards-commands.tsweb/cypress/support/commands/image-patch-commands.tsweb/cypress/support/commands/incident-commands.tsweb/cypress/support/commands/operator-commands.tsweb/cypress/support/commands/perses-commands.tsweb/cypress/support/commands/selector-commands.tsweb/cypress/support/commands/utility-commands.tsweb/cypress/support/commands/virtualization-commands.tsweb/cypress/support/commands/wait-utils.tsweb/cypress/support/e2e.jsweb/cypress/support/incidents_prometheus_query_mocks/mock-generators.tsweb/cypress/support/incidents_prometheus_query_mocks/prometheus-mocks.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/fixture-converter.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/schema-validator.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/validate-fixtures.tsweb/cypress/support/incidents_prometheus_query_mocks/types.tsweb/cypress/support/incidents_prometheus_query_mocks/utils.tsweb/cypress/support/index.tsweb/cypress/support/monitoring/00.bvt_monitoring.cy.tsweb/cypress/support/monitoring/00.bvt_monitoring_namespace.cy.tsweb/cypress/support/monitoring/01.reg_alerts.cy.tsweb/cypress/support/monitoring/02.reg_metrics_1.cy.tsweb/cypress/support/monitoring/02.reg_metrics_2.cy.tsweb/cypress/support/monitoring/03.reg_legacy_dashboards.cy.tsweb/cypress/support/monitoring/04.reg_alerts_namespace.cy.tsweb/cypress/support/monitoring/05.reg_metrics_namespace_1.cy.tsweb/cypress/support/monitoring/05.reg_metrics_namespace_2.cy.tsweb/cypress/support/monitoring/06.reg_legacy_dashboards_namespace.cy.tsweb/cypress/support/perses/00.coo_bvt_perses_admin.cy.tsweb/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/support/perses/01.coo_list_perses_admin.cy.tsweb/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.tsweb/cypress/support/perses/02.coo_edit_perses_admin.cy.tsweb/cypress/support/perses/02.coo_edit_perses_admin_1.cy.tsweb/cypress/support/perses/03.coo_create_perses_admin.cy.tsweb/cypress/support/perses/04.coo_import_perses_admin.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user1.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user2.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user3.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user4.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user5.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user6.cy.tsweb/cypress/support/selectors.tsweb/cypress/support/test-tags.d.tsweb/cypress/views/acm-alerting-page.tsweb/cypress/views/alerting-rule-details-page.tsweb/cypress/views/alerting-rule-list-page.tsweb/cypress/views/common.tsweb/cypress/views/details-page.tsweb/cypress/views/incidents-page.tsweb/cypress/views/legacy-dashboards.tsweb/cypress/views/list-page.tsweb/cypress/views/metrics.tsweb/cypress/views/nav.tsweb/cypress/views/operator-hub-page.tsweb/cypress/views/overview-page.tsweb/cypress/views/perses-dashboards-create-dashboard.tsweb/cypress/views/perses-dashboards-edit-datasources.tsweb/cypress/views/perses-dashboards-edit-variables.tsweb/cypress/views/perses-dashboards-import-dashboard.tsweb/cypress/views/perses-dashboards-list-dashboards.tsweb/cypress/views/perses-dashboards-panel.tsweb/cypress/views/perses-dashboards-panelgroup.tsweb/cypress/views/perses-dashboards.tsweb/cypress/views/silence-alert-page.tsweb/cypress/views/silence-details-page.tsweb/cypress/views/silences-list-page.tsweb/cypress/views/tour.tsweb/cypress/views/troubleshooting-panel.tsweb/cypress/views/utils.tsweb/package.jsonweb/src/components/data-test.ts
💤 Files with no reviewable changes (3)
- web/cypress/e2e/coo/01.coo_bvt.cy.ts
- web/cypress/e2e/perses/00.coo_bvt_perses_admin_1.cy.ts
- web/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.ts
etmurasaki
force-pushed
the
etmura-post-coo140
branch
from
bbef11f to
62c7b43
Compare
openshift-ci
bot
removed
the
needs-rebase
etmurasaki
force-pushed
the
etmura-post-coo140
branch
from
62c7b43 to
82ac6e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
web/cypress/support/perses/04.coo_import_perses_admin.cy.ts (1)
25-27: Extract a shared import-fixture base path constant.The same base path is repeated across many uploads. Centralizing it reduces future migration churn and typo risk.
Refactor sketch
+const IMPORT_FIXTURE_BASE = './cypress/fixtures/coo/coo140_perses/import'; - persesImportDashboardsPage.uploadFile( - './cypress/fixtures/coo/coo140_perses/import/acm-vm-status.json', - ); + persesImportDashboardsPage.uploadFile(`${IMPORT_FIXTURE_BASE}/acm-vm-status.json`);Also applies to: 34-36, 43-45, 71-73, 101-103, 121-123, 148-150, 167-169, 194-196
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/04.coo_import_perses_admin.cy.ts` around lines 25 - 27, Extract a shared base path constant (e.g., const IMPORT_FIXTURE_BASE = './cypress/fixtures/coo/coo140_perses/import') and replace each literal path passed to persesImportDashboardsPage.uploadFile(...) with a join/concatenation using that constant (e.g., `${IMPORT_FIXTURE_BASE}/accelerators-dashboard-cr-v1alpha1.yaml`); update all occurrences that match the repeated patterns (the uploadFile calls around the noted blocks) so imports reference the constant to avoid duplication and typos.web/cypress/support/commands/incident-commands.ts (1)
55-83: Reuse-mode branches execute in parallel—verify this is intentional.The PrometheusRule existence check (lines 55-71) and the pod existence check (lines 73-83) are two separate
cy.exec(...).then(...)chains that Cypress will execute concurrently. IfcreateOrUpdatePrometheusRule()must complete beforecreatePod()for the alert to fire correctly, this could introduce a race condition.If ordering matters, chain the pod check inside the
.then()callback of the PrometheusRule block. If both resources are truly independent, this is fine as-is, but consider adding a brief comment clarifying the intent.Note: Per the context snippets, no current callers use the reuse mode (
testNameparameter), so this is low-risk today but could surface if the feature is adopted later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/incident-commands.ts` around lines 55 - 83, The two async checks under shouldReuseResources (the oc get for PrometheusRule that calls createOrUpdatePrometheusRule() and the oc get for the pod that calls createPod()) currently run in parallel and can race; if createOrUpdatePrometheusRule() must finish before createPod() to ensure the alert fires, move the second cy.exec(...) call (the pod existence check that invokes createPod()) into the .then(...) callback of the PrometheusRule check so it runs after createOrUpdatePrometheusRule() completes; if there is no ordering dependency, leave the code but add a clear comment next to shouldReuseResources explaining that PrometheusRule and pod creation are independent and intentionally executed concurrently to avoid future regressions.web/cypress/e2e/perses/99.coo_rbac_perses_user1.cy.ts (1)
26-43: Reduce stale commented setup/teardown blocksThe commented role-grant/remove block is large and already drifting (
LOGIN_USERNAMEvs activeLOGIN_USERNAME1usage). Please move this into a helper behind a flag, or remove it and keep only a short Jira reference.As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 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 26 - 43, Remove or relocate the large commented grant/remove cluster-admin block: either delete the commented oc adm commands and leave a single-line JIRA reference, or move them into a test helper (e.g., a new helper function like grantTemporaryClusterAdminForDev) and guard it with a boolean flag (e.g., USE_TEMP_ADMIN) that the test reads; update references to use the correct env var (LOGIN_USERNAME1 vs LOGIN_USERNAME) to avoid drift, and keep the main test using cy.beforeBlockCOO and cy.setupPersesRBACandExtraDashboards unchanged so the setup/teardown is short and maintainable.web/cypress/support/commands/virtualization-commands.ts (1)
241-246: Duplicate CRD deletion commands with same label selector.Lines 236-239 and 242-246 both attempt to delete CRDs with the same label selector. If the dry-run flag is removed from the first command, these become redundant operations. Clarify the intent or consolidate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/virtualization-commands.ts` around lines 241 - 246, The code issues duplicate CRD deletion commands using the same label selector via cy.executeAndDelete; either consolidate them into a single deletion call or make their intent explicit (e.g., first is dry-run, second is actual delete). Locate the two invocations of cy.executeAndDelete that construct the oc delete crd ... -l operators.coreos.com/kubevirt-hyperconverged.openshift-cnv command and remove the redundant one or change the first to include --dry-run=client (or similar) while keeping the second as the real delete so you only run the actual removal once.web/cypress/support/commands/utility-commands.ts (1)
59-59: Consider replacing hard-coded waits with proper assertions.Multiple
cy.wait()calls with fixed durations (2000ms, 5000ms) are scattered throughout the commands. These can lead to flaky tests and unnecessarily slow execution. Cypress best practice is to use assertions that wait for specific conditions instead.Also applies to: 143-143, 147-147, 184-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/utility-commands.ts` at line 59, Replace hard-coded cy.wait(...) calls in utility-commands.ts with deterministic waits: remove the cy.wait(2000) (and other fixed waits at the other occurrences) and instead wait for specific conditions such as network responses via cy.intercept() and cy.wait('@alias'), or DOM/state assertions like cy.get(selector).should('exist') / .should('be.visible') / .contains(...). For each place where cy.wait(2000) is used, identify the element or request the test is actually waiting for and change the code to assert that condition (use actionable selectors or route aliases) in the corresponding custom command or helper function so the test becomes reliable and faster.
🤖 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/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.ts`:
- Around line 294-311: The test references the alias '@initialLoadTime' but
never defines it; fix by capturing the initial load timestamp at the start of
the same test (or in its before/beforeEach) and register it as an alias named
'initialLoadTime' (e.g. call cy.wrap(Date.now()).as('initialLoadTime') or wrap
the existing response value and .as('initialLoadTime')), ensuring the stored
value is a millisecond epoch number (since the code later divides by 1000) so
cy.get('@initialLoadTime') in the block that does the time assertions resolves
correctly.
- Around line 42-377: The before() hook currently encloses the three it() tests
(it blocks) causing Mocha/Cypress to ignore them; move the closing brace and
parenthesis that end the before() callback to immediately after the alert setup
(after cy.createKubePodCrashLoopingAlert(...).then(...) where currentAlertName
is assigned) so that before() contains only setup code and the it(...) blocks
(the three test cases that reference currentAlertName) become direct children of
the describe scope; ensure you update the surrounding braces/indentation so
before(), the it() blocks, and any remaining hooks are correctly balanced.
In `@web/cypress/e2e/perses/99.coo_rbac_perses_user1.cy.ts`:
- Around line 47-49: The synchronous call
Cypress.session.clearAllSavedSessions() is being invoked outside the Cypress
command queue and can run out of order; wrap that call inside a cy.then() after
cy.log('Clearing Cypress session cache...') so it executes as part of the
Cypress command chain, e.g., use cy.log(...).then(() =>
Cypress.session.clearAllSavedSessions()), ensuring the session-clear runs in
sequence before subsequent commands; update every RBAC user test file that uses
Cypress.session.clearAllSavedSessions() similarly.
In `@web/cypress/support/commands/utility-commands.ts`:
- Around line 169-170: The code currently clicks the pod link twice (first via
cy.get(...).as('podLink').click() and again with
cy.get('@podLink').should('be.visible').click()), causing unintended double
navigation; update the sequence in utility-commands.ts to assign the alias
'podLink' without clicking, assert visibility with
cy.get('@podLink').should('be.visible'), then perform a single
cy.get('@podLink').click() (or alternatively assign and click once, then assert
navigation) so that 'podLink' is only clicked one time.
In `@web/cypress/support/commands/virtualization-commands.ts`:
- Around line 234-239: The logged message "Delete Hyperconverged CRD instance."
is misleading because the cy.executeAndDelete call uses `--dry-run=client` which
prevents real deletion; update the command invocation in
virtualization-commands.ts (the cy.executeAndDelete call that builds the `oc
delete crd --dry-run=client -l
operators.coreos.com/kubevirt-hyperconverged.openshift-cnv --ignore-not-found
--kubeconfig ${Cypress.env('KUBECONFIG_PATH')}` string) to either remove
`--dry-run=client` if you intend to perform the deletion, or change the
log/message to clarify this is a dry-run if only validation was intended.
In `@web/cypress/support/perses/04.coo_import_perses_admin.cy.ts`:
- Around line 56-57: The generic persesDashboardsPage.closeAlert() can close the
wrong element because its implementation targets any visible h4; update the test
to use an alert-scoped helper (e.g., persesDashboardsPage.closeSuccessAlert() or
persesDashboardsPage.closeErrorAlert()) or call a scoped version like
persesDashboardsPage.closeAlert(selector) to target the specific alert container
for each occurrence (the calls at the current test: the one after
persesDashboardsPage.closeAlert() and the other occurrences flagged). Modify the
helpers in the perses-dashboards module (or add new
closeSuccessAlert/closeErrorAlert methods) so they query the specific alert
selector instead of any visible h4, and replace the four generic closeAlert()
calls with the appropriate scoped helper.
---
Nitpick comments:
In `@web/cypress/e2e/perses/99.coo_rbac_perses_user1.cy.ts`:
- Around line 26-43: Remove or relocate the large commented grant/remove
cluster-admin block: either delete the commented oc adm commands and leave a
single-line JIRA reference, or move them into a test helper (e.g., a new helper
function like grantTemporaryClusterAdminForDev) and guard it with a boolean flag
(e.g., USE_TEMP_ADMIN) that the test reads; update references to use the correct
env var (LOGIN_USERNAME1 vs LOGIN_USERNAME) to avoid drift, and keep the main
test using cy.beforeBlockCOO and cy.setupPersesRBACandExtraDashboards unchanged
so the setup/teardown is short and maintainable.
In `@web/cypress/support/commands/incident-commands.ts`:
- Around line 55-83: The two async checks under shouldReuseResources (the oc get
for PrometheusRule that calls createOrUpdatePrometheusRule() and the oc get for
the pod that calls createPod()) currently run in parallel and can race; if
createOrUpdatePrometheusRule() must finish before createPod() to ensure the
alert fires, move the second cy.exec(...) call (the pod existence check that
invokes createPod()) into the .then(...) callback of the PrometheusRule check so
it runs after createOrUpdatePrometheusRule() completes; if there is no ordering
dependency, leave the code but add a clear comment next to shouldReuseResources
explaining that PrometheusRule and pod creation are independent and
intentionally executed concurrently to avoid future regressions.
In `@web/cypress/support/commands/utility-commands.ts`:
- Line 59: Replace hard-coded cy.wait(...) calls in utility-commands.ts with
deterministic waits: remove the cy.wait(2000) (and other fixed waits at the
other occurrences) and instead wait for specific conditions such as network
responses via cy.intercept() and cy.wait('@alias'), or DOM/state assertions like
cy.get(selector).should('exist') / .should('be.visible') / .contains(...). For
each place where cy.wait(2000) is used, identify the element or request the test
is actually waiting for and change the code to assert that condition (use
actionable selectors or route aliases) in the corresponding custom command or
helper function so the test becomes reliable and faster.
In `@web/cypress/support/commands/virtualization-commands.ts`:
- Around line 241-246: The code issues duplicate CRD deletion commands using the
same label selector via cy.executeAndDelete; either consolidate them into a
single deletion call or make their intent explicit (e.g., first is dry-run,
second is actual delete). Locate the two invocations of cy.executeAndDelete that
construct the oc delete crd ... -l
operators.coreos.com/kubevirt-hyperconverged.openshift-cnv command and remove
the redundant one or change the first to include --dry-run=client (or similar)
while keeping the second as the real delete so you only run the actual removal
once.
In `@web/cypress/support/perses/04.coo_import_perses_admin.cy.ts`:
- Around line 25-27: Extract a shared base path constant (e.g., const
IMPORT_FIXTURE_BASE = './cypress/fixtures/coo/coo140_perses/import') and replace
each literal path passed to persesImportDashboardsPage.uploadFile(...) with a
join/concatenation using that constant (e.g.,
`${IMPORT_FIXTURE_BASE}/accelerators-dashboard-cr-v1alpha1.yaml`); update all
occurrences that match the repeated patterns (the uploadFile calls around the
noted blocks) so imports reference the constant to avoid duplication and typos.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4095afef-1706-472a-93b0-8f07bd1801c0
📒 Files selected for processing (128)
web/cypress/dummy.tsxweb/cypress/e2e/coo/01.coo_bvt.cy.tsweb/cypress/e2e/coo/01.coo_ivt.cy.tsweb/cypress/e2e/coo/02.acm_alerting_ui.cy.tsweb/cypress/e2e/incidents/00.coo_incidents_e2e.cy.tsweb/cypress/e2e/incidents/01.incidents.cy.tsweb/cypress/e2e/incidents/02.incidents-mocking-example.cy.tsweb/cypress/e2e/incidents/regression/01.reg_filtering.cy.tsweb/cypress/e2e/incidents/regression/02.reg_ui_charts_comprehensive.cy.tsweb/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.tsweb/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.tsweb/cypress/e2e/incidents/regression/03.reg_api_calls.cy.tsweb/cypress/e2e/incidents/regression/04.reg_redux_effects.cy.tsweb/cypress/e2e/incidents/regression/05.reg_stress_testing_ui.cy.tsweb/cypress/e2e/monitoring/00.bvt_admin.cy.tsweb/cypress/e2e/monitoring/00.bvt_dev.cy.tsweb/cypress/e2e/monitoring/regression/01.reg_alerts_admin.cy.tsweb/cypress/e2e/monitoring/regression/01.reg_alerts_dev.cy.tsweb/cypress/e2e/monitoring/regression/02.reg_metrics_admin_1.cy.tsweb/cypress/e2e/monitoring/regression/02.reg_metrics_admin_2.cy.tsweb/cypress/e2e/monitoring/regression/03.reg_legacy_dashboards_admin.cy.tsweb/cypress/e2e/perses/00.coo_bvt_perses_admin.cy.tsweb/cypress/e2e/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/e2e/perses/01.coo_list_perses_admin.cy.tsweb/cypress/e2e/perses/02.coo_edit_perses_admin.cy.tsweb/cypress/e2e/perses/03.coo_create_perses_admin.cy.tsweb/cypress/e2e/perses/04.coo_import_perses_admin.cy.tsweb/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/e2e/virtualization/00.coo_ivt.cy.tsweb/cypress/e2e/virtualization/01.coo_ivt_alerts.cy.tsweb/cypress/e2e/virtualization/02.coo_ivt_metrics_1.cy.tsweb/cypress/e2e/virtualization/02.coo_ivt_metrics_2.cy.tsweb/cypress/e2e/virtualization/03.coo_ivt_legacy_dashboards.cy.tsweb/cypress/e2e/virtualization/04.coo_ivt_perses.cy.tsweb/cypress/fixtures/coo/coo140_perses/dashboards/openshift-cluster-sample-dashboard.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/perses-dashboard-sample.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/prometheus-overview-variables.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/thanos-compact-overview-1var.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/thanos-querier-datasource.yamlweb/cypress/fixtures/coo/coo140_perses/import/accelerators-dashboard-cr-v1alpha1.yamlweb/cypress/fixtures/coo/coo140_perses/import/accelerators-dashboard-cr-v1alpha2.yamlweb/cypress/fixtures/coo/coo140_perses/import/acm-vm-status.jsonweb/cypress/fixtures/coo/coo140_perses/import/grafana_to_check_errors.jsonweb/cypress/fixtures/coo/coo140_perses/import/testing-perses-dashboard.jsonweb/cypress/fixtures/coo/coo140_perses/import/testing-perses-dashboard.yamlweb/cypress/fixtures/coo/coo140_perses/rbac/rbac_perses_e2e_ci_users.shweb/cypress/fixtures/monitoring/alert.tsweb/cypress/fixtures/monitoring/constants.tsweb/cypress/fixtures/perses/constants.tsweb/cypress/support/commands/auth-commands.tsweb/cypress/support/commands/coo-install-commands.tsweb/cypress/support/commands/dashboards-commands.tsweb/cypress/support/commands/image-patch-commands.tsweb/cypress/support/commands/incident-commands.tsweb/cypress/support/commands/operator-commands.tsweb/cypress/support/commands/perses-commands.tsweb/cypress/support/commands/selector-commands.tsweb/cypress/support/commands/utility-commands.tsweb/cypress/support/commands/virtualization-commands.tsweb/cypress/support/commands/wait-utils.tsweb/cypress/support/e2e.jsweb/cypress/support/incidents_prometheus_query_mocks/mock-generators.tsweb/cypress/support/incidents_prometheus_query_mocks/prometheus-mocks.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/fixture-converter.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/schema-validator.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/validate-fixtures.tsweb/cypress/support/incidents_prometheus_query_mocks/types.tsweb/cypress/support/incidents_prometheus_query_mocks/utils.tsweb/cypress/support/index.tsweb/cypress/support/monitoring/00.bvt_monitoring.cy.tsweb/cypress/support/monitoring/00.bvt_monitoring_namespace.cy.tsweb/cypress/support/monitoring/01.reg_alerts.cy.tsweb/cypress/support/monitoring/02.reg_metrics_1.cy.tsweb/cypress/support/monitoring/02.reg_metrics_2.cy.tsweb/cypress/support/monitoring/03.reg_legacy_dashboards.cy.tsweb/cypress/support/monitoring/04.reg_alerts_namespace.cy.tsweb/cypress/support/monitoring/05.reg_metrics_namespace_1.cy.tsweb/cypress/support/monitoring/05.reg_metrics_namespace_2.cy.tsweb/cypress/support/monitoring/06.reg_legacy_dashboards_namespace.cy.tsweb/cypress/support/perses/00.coo_bvt_perses_admin.cy.tsweb/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/support/perses/01.coo_list_perses_admin.cy.tsweb/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.tsweb/cypress/support/perses/02.coo_edit_perses_admin.cy.tsweb/cypress/support/perses/02.coo_edit_perses_admin_1.cy.tsweb/cypress/support/perses/03.coo_create_perses_admin.cy.tsweb/cypress/support/perses/04.coo_import_perses_admin.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user1.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user2.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user3.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user4.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user5.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user6.cy.tsweb/cypress/support/selectors.tsweb/cypress/support/test-tags.d.tsweb/cypress/views/acm-alerting-page.tsweb/cypress/views/alerting-rule-details-page.tsweb/cypress/views/alerting-rule-list-page.tsweb/cypress/views/common.tsweb/cypress/views/details-page.tsweb/cypress/views/incidents-page.tsweb/cypress/views/legacy-dashboards.tsweb/cypress/views/list-page.tsweb/cypress/views/metrics.tsweb/cypress/views/nav.tsweb/cypress/views/operator-hub-page.tsweb/cypress/views/overview-page.tsweb/cypress/views/perses-dashboards-create-dashboard.tsweb/cypress/views/perses-dashboards-edit-datasources.tsweb/cypress/views/perses-dashboards-edit-variables.tsweb/cypress/views/perses-dashboards-import-dashboard.tsweb/cypress/views/perses-dashboards-list-dashboards.tsweb/cypress/views/perses-dashboards-panel.tsweb/cypress/views/perses-dashboards-panelgroup.tsweb/cypress/views/perses-dashboards.tsweb/cypress/views/silence-alert-page.tsweb/cypress/views/silence-details-page.tsweb/cypress/views/silences-list-page.tsweb/cypress/views/tour.tsweb/cypress/views/troubleshooting-panel.tsweb/cypress/views/utils.tsweb/package.jsonweb/src/components/data-test.ts
💤 Files with no reviewable changes (3)
- web/cypress/e2e/coo/01.coo_bvt.cy.ts
- web/cypress/e2e/perses/00.coo_bvt_perses_admin_1.cy.ts
- web/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.ts
✅ Files skipped from review due to trivial changes (53)
- web/cypress/support/e2e.js
- web/cypress/e2e/incidents/01.incidents.cy.ts
- web/cypress/support/perses/99.coo_rbac_perses_user6.cy.ts
- web/cypress/views/acm-alerting-page.ts
- web/cypress/e2e/monitoring/00.bvt_dev.cy.ts
- web/cypress/e2e/incidents/02.incidents-mocking-example.cy.ts
- web/cypress/e2e/incidents/regression/01.reg_filtering.cy.ts
- web/cypress/e2e/incidents/00.coo_incidents_e2e.cy.ts
- web/cypress/fixtures/coo/coo140_perses/import/acm-vm-status.json
- web/cypress/e2e/monitoring/regression/01.reg_alerts_dev.cy.ts
- web/cypress/e2e/coo/01.coo_ivt.cy.ts
- web/cypress/support/index.ts
- web/cypress/support/monitoring/03.reg_legacy_dashboards.cy.ts
- web/cypress/e2e/virtualization/03.coo_ivt_legacy_dashboards.cy.ts
- web/cypress/e2e/monitoring/regression/01.reg_alerts_admin.cy.ts
- web/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.ts
- web/cypress/support/test-tags.d.ts
- web/cypress/support/commands/wait-utils.ts
- web/cypress/e2e/coo/02.acm_alerting_ui.cy.ts
- web/cypress/e2e/monitoring/regression/02.reg_metrics_admin_1.cy.ts
- web/cypress/e2e/virtualization/01.coo_ivt_alerts.cy.ts
- web/cypress/support/incidents_prometheus_query_mocks/types.ts
- web/cypress/support/incidents_prometheus_query_mocks/schema/validate-fixtures.ts
- web/cypress/e2e/incidents/regression/04.reg_redux_effects.cy.ts
- web/cypress/views/alerting-rule-list-page.ts
- web/cypress/e2e/incidents/regression/05.reg_stress_testing_ui.cy.ts
- web/cypress/support/monitoring/02.reg_metrics_2.cy.ts
- web/cypress/support/incidents_prometheus_query_mocks/schema/fixture-converter.ts
- web/cypress/e2e/monitoring/regression/03.reg_legacy_dashboards_admin.cy.ts
- web/cypress/e2e/perses/03.coo_create_perses_admin.cy.ts
- web/cypress/e2e/monitoring/regression/02.reg_metrics_admin_2.cy.ts
- web/cypress/support/monitoring/05.reg_metrics_namespace_2.cy.ts
- web/cypress/support/incidents_prometheus_query_mocks/utils.ts
- web/cypress/support/perses/02.coo_edit_perses_admin.cy.ts
- web/cypress/support/perses/99.coo_rbac_perses_user2.cy.ts
- web/cypress/support/perses/99.coo_rbac_perses_user4.cy.ts
- web/cypress/support/perses/03.coo_create_perses_admin.cy.ts
- web/cypress/support/monitoring/05.reg_metrics_namespace_1.cy.ts
- web/cypress/support/commands/coo-install-commands.ts
- web/cypress/support/commands/operator-commands.ts
- web/cypress/views/alerting-rule-details-page.ts
- web/cypress/e2e/virtualization/00.coo_ivt.cy.ts
- web/cypress/support/monitoring/02.reg_metrics_1.cy.ts
- web/cypress/dummy.tsx
- web/cypress/support/incidents_prometheus_query_mocks/mock-generators.ts
- web/cypress/e2e/virtualization/04.coo_ivt_perses.cy.ts
- web/cypress/e2e/monitoring/00.bvt_admin.cy.ts
- web/cypress/e2e/incidents/regression/03.reg_api_calls.cy.ts
- web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts
- web/cypress/support/perses/00.coo_bvt_perses_admin.cy.ts
- web/cypress/support/commands/image-patch-commands.ts
- web/cypress/views/incidents-page.ts
- web/cypress/support/perses/01.coo_list_perses_admin.cy.ts
🚧 Files skipped from review as they are similar to previous changes (25)
- web/cypress/e2e/virtualization/02.coo_ivt_metrics_2.cy.ts
- web/cypress/e2e/perses/02.coo_edit_perses_admin.cy.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user6.cy.ts
- web/cypress/views/common.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user3.cy.ts
- web/cypress/support/selectors.ts
- web/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.ts
- web/cypress/support/monitoring/06.reg_legacy_dashboards_namespace.cy.ts
- web/cypress/e2e/perses/01.coo_list_perses_admin.cy.ts
- web/cypress/support/commands/dashboards-commands.ts
- web/cypress/support/commands/auth-commands.ts
- web/cypress/support/monitoring/00.bvt_monitoring.cy.ts
- web/cypress/e2e/incidents/regression/02.reg_ui_charts_comprehensive.cy.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user4.cy.ts
- web/cypress/e2e/perses/00.coo_bvt_perses_admin.cy.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user5.cy.ts
- web/cypress/e2e/virtualization/02.coo_ivt_metrics_1.cy.ts
- web/cypress/support/perses/99.coo_rbac_perses_user1.cy.ts
- web/cypress/views/legacy-dashboards.ts
- web/cypress/fixtures/monitoring/alert.ts
- web/cypress/support/perses/99.coo_rbac_perses_user5.cy.ts
- web/cypress/support/commands/perses-commands.ts
- web/cypress/support/commands/selector-commands.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user2.cy.ts
- web/cypress/support/perses/99.coo_rbac_perses_user3.cy.ts
web/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/cypress/support/commands/utility-commands.ts (1)
22-40:⚠️ Potential issue | 🟠 MajorRegister the timeout handler scope to only this operation, not the entire test.
The
cy.on('fail', ...)handler at line 28 persists for the entire test duration without cleanup. IfwaitUntilWithCustomTimeoutis called multiple times in a test, handlers accumulate. A later unrelated timeout can then be caught by an earlier handler with a staletimeoutMessage. Usecy.off('fail', handlerReference)aftercy.waitUntil()completes, or temporarily register the handler for only this command's duration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/utility-commands.ts` around lines 22 - 40, The failure handler registered with cy.on('fail', ...) inside waitUntilWithCustomTimeout persists across the whole test and can accumulate; update waitUntilWithCustomTimeout to store the handler in a local const (e.g., failHandler), register it with cy.on('fail', failHandler) before calling cy.waitUntil(fn, waitOptions), and ensure you remove it afterward with cy.off('fail', failHandler) in both success and failure paths (use a finally-style cleanup around the cy.waitUntil promise chain) so the handler only applies to this invocation.
♻️ Duplicate comments (2)
web/cypress/support/perses/04.coo_import_perses_admin.cy.ts (1)
56-56:⚠️ Potential issue | 🟠 MajorUse alert-scoped dismissal helpers here.
closeAlert()is still implemented against any visibleh4, so these calls can dismiss the wrong element once the import modal and toast overlap. Use the success/error-specific closer or scope the selector to the alert container.Also applies to: 109-109, 131-131, 177-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/04.coo_import_perses_admin.cy.ts` at line 56, The call to persesDashboardsPage.closeAlert() is too generic and can close the wrong h4 when import modal and toast overlap; update each usage (the calls to persesDashboardsPage.closeAlert() at the shown location and the other occurrences) to use the alert-scoped dismissal helpers (e.g., the success/error-specific closer method) or call the closer with a selector scoped to the alert container so it targets only the intended toast/modal alert element; ensure you replace the generic closeAlert() with the appropriate closeSuccessAlert()/closeErrorAlert() or pass the containing alert selector to avoid dismissing unrelated alerts.web/cypress/support/commands/utility-commands.ts (1)
169-170:⚠️ Potential issue | 🟠 MajorClick the pod link only once.
Line 169 already triggers navigation. Clicking
@podLinkagain on Line 170 can race the route change and makepodImage()flaky.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/utility-commands.ts` around lines 169 - 170, Duplicate clicks on the pod link are causing flaky navigation; keep a single click and only assert visibility afterwards. Remove the second .click() on the alias '@podLink' so the sequence becomes cy.get(...).eq(0).as('podLink').click() followed by cy.get('@podLink').should('be.visible'), ensuring the click (in the cy.get(...).click()) triggers navigation and the subsequent assertion only verifies visibility; references: cy.get, alias 'podLink', and helper podImage().
🧹 Nitpick comments (6)
web/cypress/support/incidents_prometheus_query_mocks/schema/validate-fixtures.ts (1)
25-26: Consider a file-level ESLint disable forno-consolein this CLI tool.This script has 15+
// eslint-disable-next-line no-consolecomments scattered throughout. Since this is a CLI tool whereconsole.logis the intended output mechanism, a single file-level disable would be cleaner and more maintainable.♻️ Proposed refactor
Add at the top of the file (after the shebang and doc comment):
*/ +/* eslint-disable no-console */ + import * as fs from 'fs';Then remove all the individual
// eslint-disable-next-line no-consolecomments throughout the file.Also applies to: 36-41, 44-49, 54-57, 66-75, 86-87, 96-97, 101-104, 110-111, 117-118, 125-126
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/incidents_prometheus_query_mocks/schema/validate-fixtures.ts` around lines 25 - 26, This file uses many inline "// eslint-disable-next-line no-console" comments; instead add a single file-level ESLint disable (/* eslint-disable no-console */) at the top of the file (immediately after any shebang and doc comment) and remove all individual "// eslint-disable-next-line no-console" occurrences (they appear around console.log calls such as the one logging `Validating: ${filePath}`) so console usage is allowed consistently across the CLI script.web/cypress/support/monitoring/01.reg_alerts.cy.ts (1)
73-236: Split this scenario into smalleritblocks to improve maintainability and failure triage.This single test currently chains many independent behaviors (silence create/assert/edit/recreate/expire/cancel). One mid-flow failure masks downstream checks and increases flake investigation time.
As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/monitoring/01.reg_alerts.cy.ts` around lines 73 - 236, The test currently contains one large it(...) that chains many independent behaviors; split it into multiple smaller it blocks (e.g., "silence creation", "silence appears in Silences tab", "expire silence", "recreate silence", "edit silence", "cancel silence creation from Alert Details") so failures are isolated. For each new it, move the relevant steps using the existing page objects (listPage, listPage.ARRows, silenceAlertPage, silencesListPage, silenceDetailsPage, detailsPage, nav, commonPages) into its own test, ensure shared setup (navigating to Observe > Alerting and filtering by WatchdogAlert.ALERTNAME) is factored into a beforeEach or helper, and keep assertions local to that it; maintain calls like listPage.filter.byName, listPage.ARRows.expandRow(), silenceAlertPage.clickSubmit(), silencesListPage.rows.assertSilencedAlertKebab(), silenceDetailsPage.expireSilence(), detailsPage.clickSilenceAlertButton(), etc., to preserve intent.web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts (2)
85-90: Avoid binding test behavior to object key order.Line 88 (
panelTypeKeys[panelTypeKeys.length - 1]) makes this scenario sensitive to constant declaration order, which is brittle for maintenance. Use an explicit panel type (or explicit selection rule) instead.Proposed refactor
- const panelTypeKeys = Object.keys( - persesDashboardsAddListPanelType, - ) as (keyof typeof persesDashboardsAddListPanelType)[]; - const lastKey = panelTypeKeys[panelTypeKeys.length - 1]; // Get the last KEY from the array - const lastPanelName = persesDashboardsAddListPanelType[lastKey]; // Use the KEY to get the VALUE + const targetPanelName = persesDashboardsAddListPanelType.BAR_CHART;Then replace
lastPanelNameusages withtargetPanelName(or another explicitly chosen fixture value for round-trip edit coverage).As per coding guidelines, "**: Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts` around lines 85 - 90, The test is brittle because it relies on object key order via panelTypeKeys and lastKey to derive lastPanelName; instead pick an explicit panel type value from persesDashboardsAddListPanelType (e.g., define targetPanelName) and replace uses of lastPanelName with that explicit value so the test no longer depends on declaration order; update any assertions or interactions that reference lastPanelName to use targetPanelName and remove the panelTypeKeys/lastKey logic.
173-178: End scenario 13 with an explicit Save/Cancel to avoid cross-test state leakage.This test duplicates a panel and asserts, but does not explicitly close edit mode. Please verify isolation hooks guarantee cleanup; otherwise add
SaveorCancelbefore exit.Minimal stabilization option
cy.log(`13.7. Assert duplicated panel`); persesDashboardsPage.assertDuplicatedPanel('Legend Example', 2); + cy.log(`13.8. Close edit session`); + persesDashboardsPage.clickEditActionButton('Cancel'); });As per coding guidelines, "**: Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts` around lines 173 - 178, The test ends after duplicating a panel but never exits edit mode, risking cross-test state leakage; after persesDashboardsPage.assertDuplicatedPanel('Legend Example', 2) call, explicitly close edit mode by invoking the appropriate page action (e.g., persesDashboardsPage.saveDashboard() or persesDashboardsPage.cancelEdit()) to persist or discard changes, and if unsure, prefer calling persesDashboardsPage.cancelEdit() to avoid modifying shared state; also ensure test-level isolation hooks still run by verifying afterAdd/afterEach cleanup remains intact.web/cypress/support/commands/selector-commands.ts (2)
49-97: Return the Cypress chain explicitly in selector commands.On Line 49 onward, several command callbacks call
cy.get(...)but don’t explicitly return it, while later commands (for examplebyAriaLabelandbyPFRole) do. Making returns explicit keeps subject propagation predictable and improves maintainability.Suggested diff
Cypress.Commands.add( 'byTestID', (selector: string, options?: Partial<Loggable & Timeoutable & Withinable & Shadow>) => { - cy.get(`[data-test="${selector}"]`, options); + return cy.get(`[data-test="${selector}"]`, options); }, ); @@ Cypress.Commands.add( 'byLegacyTestID', (selector: string, options?: Partial<Loggable & Timeoutable & Withinable & Shadow>) => { - cy.get(`[data-test-id="${selector}"]`, options); + return cy.get(`[data-test-id="${selector}"]`, options); }, ); @@ Cypress.Commands.add('byButtonText', (selector: string) => { - cy.get('button[type="button"]').contains(`${selector}`); + return cy.get('button[type="button"]').contains(`${selector}`); }); @@ Cypress.Commands.add('byDataID', (selector: string) => { - cy.get(`[data-id="${selector}"]`); + return cy.get(`[data-id="${selector}"]`); }); @@ Cypress.Commands.add( 'byTestSelector', (selector: string, options?: Partial<Loggable & Timeoutable & Withinable & Shadow>) => { - cy.get(`[data-test-selector="${selector}"]`, options); + return cy.get(`[data-test-selector="${selector}"]`, options); }, ); @@ Cypress.Commands.add('byTestDropDownMenu', (selector: string) => { - cy.get(`[data-test-dropdown-menu="${selector}"]`); + return cy.get(`[data-test-dropdown-menu="${selector}"]`); });As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/selector-commands.ts` around lines 49 - 97, Several custom Cypress commands (byTestID, byLegacyTestID, byButtonText, byDataID, byTestSelector, byTestDropDownMenu, byTestOperatorRow, byTestSectionHeading, byTestOperandLink) call cy.get(...) inside a block but do not explicitly return the Cypress chain, breaking subject propagation; update each of these command callbacks to return the cy.get(...) call (preserving any options parameters) so the command yields the chain like byAriaLabel and byPFRole do—look for the named functions in the diff and add an explicit return statement before the cy.get(...) expression in each.
87-89: Use the same options type as theChainabledeclaration forbyTestOperatorRow.Line 87 uses
options?: object, which is looser than the declared command type and reduces type safety.Suggested diff
-Cypress.Commands.add('byTestOperatorRow', (selector: string, options?: object) => { +Cypress.Commands.add( + 'byTestOperatorRow', + (selector: string, options?: Partial<Loggable & Timeoutable & Withinable & Shadow>) => { cy.get(`[data-test-operator-row="${selector}"]`, options); -}); + }, +);As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/selector-commands.ts` around lines 87 - 89, The command byTestOperatorRow uses a loose options?: object type; update its signature to use the exact same options type as declared for the Chainable command overload so types align (replace options?: object with the exact type used in the Chainable declaration for custom commands). Locate the Chainable declaration in your types (the custom commands.d.ts / support type file) and import/reference that same options type in the byTestOperatorRow definition so callers get correct compile-time checking.
🤖 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/commands/perses-commands.ts`:
- Around line 44-57: The conditional is incorrectly stringifying environment
variables so a missing LOGIN_USERNAME2 becomes the literal "undefined" and the
branch still runs; change the check to use raw values from Cypress.env (e.g.
Cypress.env('LOGIN_USERNAME1') !== 'kubeadmin' && Cypress.env('LOGIN_USERNAME2')
!== undefined) or test truthiness (Boolean(Cypress.env('LOGIN_USERNAME2')))
before calling cy.exec, and ensure the env map passed to cy.exec uses the raw
Cypress.env values (USER1: Cypress.env('LOGIN_USERNAME1'), USER2:
Cypress.env('LOGIN_USERNAME2'), etc.) so undefined values are not sent to the
RBAC script; update the conditional around cy.exec and the env assignment in
perses-commands.ts accordingly.
In `@web/cypress/support/commands/virtualization-commands.ts`:
- Around line 76-82: The cy.exec invocation uses a misspelled fixture path
'virtulization/virtualization_stage.sh' which causes file-not-found failures;
update the path in the cy.exec call (the call in virtualization-commands.ts that
passes env FBC_STAGE_KBV_IMAGE and KUBECONFIG) to use the correct directory name
'virtualization/virtualization_stage.sh' so the script is located correctly when
running tests.
In `@web/cypress/support/monitoring/00.bvt_monitoring.cy.ts`:
- Around line 65-67: The current assertion on timeIntervalValue is weak because
expect(value).not.to.equal('') will pass for null/undefined; update the test
that uses getValFromElement (which .invoke('val')s the element) to assert the
resolved value is a non-empty string and not null/undefined — e.g., for the
timeIntervalValue Chainable assert the value is of type string and not empty (or
explicitly check value !== null && value !== undefined && value.trim() !== '')
so the test fails on missing values.
In `@web/cypress/support/monitoring/01.reg_alerts.cy.ts`:
- Around line 128-130: The test leaves SilenceState.EXPIRED selected alongside
other states which can make rows.editSilence() and rows.expireSilence(true)
target the wrong row; update the test to explicitly reset silence-state filters
before any edit/expire action by calling listPage.filter.selectFilterOption for
all SilenceState values (EXPIRED, ACTIVE, PENDING) to a known state (false) and
then select only the intended state (true) immediately before invoking
rows.editSilence() or rows.expireSilence(true), e.g., clear
EXPIRED/ACTIVE/PENDING first via listPage.filter.selectFilterOption(...) and
then select the single state you want so the row targeting is deterministic.
In `@web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts`:
- Around line 82-83: The tests use hard-coded cy.wait(2000) before calling
persesDashboardsPage.clickEditButton(), which makes them flaky and slow; replace
these sleeps with deterministic waits such as intercepting the relevant network
request and using cy.wait('@alias') or waiting for a specific DOM condition
(e.g., cy.get(selector).should('be.visible') or .should('exist')) before
invoking persesDashboardsPage.clickEditButton(), and apply the same change where
other fixed waits occur so the click happens only after the UI/data is ready.
In `@web/cypress/views/common.ts`:
- Around line 30-32: The CSS selector passed to cy.get is missing the class dot
for PF6; update the selector string used in the cy.get call so the PF6 class is
prefixed with a dot (change pf-v6-c-notification-drawer__list-item-header to
.pf-v6-c-notification-drawer__list-item-header) so both PF6 and PF5 notification
list item headers are matched correctly.
---
Outside diff comments:
In `@web/cypress/support/commands/utility-commands.ts`:
- Around line 22-40: The failure handler registered with cy.on('fail', ...)
inside waitUntilWithCustomTimeout persists across the whole test and can
accumulate; update waitUntilWithCustomTimeout to store the handler in a local
const (e.g., failHandler), register it with cy.on('fail', failHandler) before
calling cy.waitUntil(fn, waitOptions), and ensure you remove it afterward with
cy.off('fail', failHandler) in both success and failure paths (use a
finally-style cleanup around the cy.waitUntil promise chain) so the handler only
applies to this invocation.
---
Duplicate comments:
In `@web/cypress/support/commands/utility-commands.ts`:
- Around line 169-170: Duplicate clicks on the pod link are causing flaky
navigation; keep a single click and only assert visibility afterwards. Remove
the second .click() on the alias '@podLink' so the sequence becomes
cy.get(...).eq(0).as('podLink').click() followed by
cy.get('@podLink').should('be.visible'), ensuring the click (in the
cy.get(...).click()) triggers navigation and the subsequent assertion only
verifies visibility; references: cy.get, alias 'podLink', and helper podImage().
In `@web/cypress/support/perses/04.coo_import_perses_admin.cy.ts`:
- Line 56: The call to persesDashboardsPage.closeAlert() is too generic and can
close the wrong h4 when import modal and toast overlap; update each usage (the
calls to persesDashboardsPage.closeAlert() at the shown location and the other
occurrences) to use the alert-scoped dismissal helpers (e.g., the
success/error-specific closer method) or call the closer with a selector scoped
to the alert container so it targets only the intended toast/modal alert
element; ensure you replace the generic closeAlert() with the appropriate
closeSuccessAlert()/closeErrorAlert() or pass the containing alert selector to
avoid dismissing unrelated alerts.
---
Nitpick comments:
In `@web/cypress/support/commands/selector-commands.ts`:
- Around line 49-97: Several custom Cypress commands (byTestID, byLegacyTestID,
byButtonText, byDataID, byTestSelector, byTestDropDownMenu, byTestOperatorRow,
byTestSectionHeading, byTestOperandLink) call cy.get(...) inside a block but do
not explicitly return the Cypress chain, breaking subject propagation; update
each of these command callbacks to return the cy.get(...) call (preserving any
options parameters) so the command yields the chain like byAriaLabel and
byPFRole do—look for the named functions in the diff and add an explicit return
statement before the cy.get(...) expression in each.
- Around line 87-89: The command byTestOperatorRow uses a loose options?: object
type; update its signature to use the exact same options type as declared for
the Chainable command overload so types align (replace options?: object with the
exact type used in the Chainable declaration for custom commands). Locate the
Chainable declaration in your types (the custom commands.d.ts / support type
file) and import/reference that same options type in the byTestOperatorRow
definition so callers get correct compile-time checking.
In
`@web/cypress/support/incidents_prometheus_query_mocks/schema/validate-fixtures.ts`:
- Around line 25-26: This file uses many inline "// eslint-disable-next-line
no-console" comments; instead add a single file-level ESLint disable (/*
eslint-disable no-console */) at the top of the file (immediately after any
shebang and doc comment) and remove all individual "// eslint-disable-next-line
no-console" occurrences (they appear around console.log calls such as the one
logging `Validating: ${filePath}`) so console usage is allowed consistently
across the CLI script.
In `@web/cypress/support/monitoring/01.reg_alerts.cy.ts`:
- Around line 73-236: The test currently contains one large it(...) that chains
many independent behaviors; split it into multiple smaller it blocks (e.g.,
"silence creation", "silence appears in Silences tab", "expire silence",
"recreate silence", "edit silence", "cancel silence creation from Alert
Details") so failures are isolated. For each new it, move the relevant steps
using the existing page objects (listPage, listPage.ARRows, silenceAlertPage,
silencesListPage, silenceDetailsPage, detailsPage, nav, commonPages) into its
own test, ensure shared setup (navigating to Observe > Alerting and filtering by
WatchdogAlert.ALERTNAME) is factored into a beforeEach or helper, and keep
assertions local to that it; maintain calls like listPage.filter.byName,
listPage.ARRows.expandRow(), silenceAlertPage.clickSubmit(),
silencesListPage.rows.assertSilencedAlertKebab(),
silenceDetailsPage.expireSilence(), detailsPage.clickSilenceAlertButton(), etc.,
to preserve intent.
In `@web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts`:
- Around line 85-90: The test is brittle because it relies on object key order
via panelTypeKeys and lastKey to derive lastPanelName; instead pick an explicit
panel type value from persesDashboardsAddListPanelType (e.g., define
targetPanelName) and replace uses of lastPanelName with that explicit value so
the test no longer depends on declaration order; update any assertions or
interactions that reference lastPanelName to use targetPanelName and remove the
panelTypeKeys/lastKey logic.
- Around line 173-178: The test ends after duplicating a panel but never exits
edit mode, risking cross-test state leakage; after
persesDashboardsPage.assertDuplicatedPanel('Legend Example', 2) call, explicitly
close edit mode by invoking the appropriate page action (e.g.,
persesDashboardsPage.saveDashboard() or persesDashboardsPage.cancelEdit()) to
persist or discard changes, and if unsure, prefer calling
persesDashboardsPage.cancelEdit() to avoid modifying shared state; also ensure
test-level isolation hooks still run by verifying afterAdd/afterEach cleanup
remains intact.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 623829f9-e3ea-4cd8-ba5e-57a4e3aeea98
📒 Files selected for processing (128)
web/cypress/dummy.tsxweb/cypress/e2e/coo/01.coo_bvt.cy.tsweb/cypress/e2e/coo/01.coo_ivt.cy.tsweb/cypress/e2e/coo/02.acm_alerting_ui.cy.tsweb/cypress/e2e/incidents/00.coo_incidents_e2e.cy.tsweb/cypress/e2e/incidents/01.incidents.cy.tsweb/cypress/e2e/incidents/02.incidents-mocking-example.cy.tsweb/cypress/e2e/incidents/regression/01.reg_filtering.cy.tsweb/cypress/e2e/incidents/regression/02.reg_ui_charts_comprehensive.cy.tsweb/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.tsweb/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.tsweb/cypress/e2e/incidents/regression/03.reg_api_calls.cy.tsweb/cypress/e2e/incidents/regression/04.reg_redux_effects.cy.tsweb/cypress/e2e/incidents/regression/05.reg_stress_testing_ui.cy.tsweb/cypress/e2e/monitoring/00.bvt_admin.cy.tsweb/cypress/e2e/monitoring/00.bvt_dev.cy.tsweb/cypress/e2e/monitoring/regression/01.reg_alerts_admin.cy.tsweb/cypress/e2e/monitoring/regression/01.reg_alerts_dev.cy.tsweb/cypress/e2e/monitoring/regression/02.reg_metrics_admin_1.cy.tsweb/cypress/e2e/monitoring/regression/02.reg_metrics_admin_2.cy.tsweb/cypress/e2e/monitoring/regression/03.reg_legacy_dashboards_admin.cy.tsweb/cypress/e2e/perses/00.coo_bvt_perses_admin.cy.tsweb/cypress/e2e/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/e2e/perses/01.coo_list_perses_admin.cy.tsweb/cypress/e2e/perses/02.coo_edit_perses_admin.cy.tsweb/cypress/e2e/perses/03.coo_create_perses_admin.cy.tsweb/cypress/e2e/perses/04.coo_import_perses_admin.cy.tsweb/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/e2e/virtualization/00.coo_ivt.cy.tsweb/cypress/e2e/virtualization/01.coo_ivt_alerts.cy.tsweb/cypress/e2e/virtualization/02.coo_ivt_metrics_1.cy.tsweb/cypress/e2e/virtualization/02.coo_ivt_metrics_2.cy.tsweb/cypress/e2e/virtualization/03.coo_ivt_legacy_dashboards.cy.tsweb/cypress/e2e/virtualization/04.coo_ivt_perses.cy.tsweb/cypress/fixtures/coo/coo140_perses/dashboards/openshift-cluster-sample-dashboard.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/perses-dashboard-sample.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/prometheus-overview-variables.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/thanos-compact-overview-1var.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/thanos-querier-datasource.yamlweb/cypress/fixtures/coo/coo140_perses/import/accelerators-dashboard-cr-v1alpha1.yamlweb/cypress/fixtures/coo/coo140_perses/import/accelerators-dashboard-cr-v1alpha2.yamlweb/cypress/fixtures/coo/coo140_perses/import/acm-vm-status.jsonweb/cypress/fixtures/coo/coo140_perses/import/grafana_to_check_errors.jsonweb/cypress/fixtures/coo/coo140_perses/import/testing-perses-dashboard.jsonweb/cypress/fixtures/coo/coo140_perses/import/testing-perses-dashboard.yamlweb/cypress/fixtures/coo/coo140_perses/rbac/rbac_perses_e2e_ci_users.shweb/cypress/fixtures/monitoring/alert.tsweb/cypress/fixtures/monitoring/constants.tsweb/cypress/fixtures/perses/constants.tsweb/cypress/support/commands/auth-commands.tsweb/cypress/support/commands/coo-install-commands.tsweb/cypress/support/commands/dashboards-commands.tsweb/cypress/support/commands/image-patch-commands.tsweb/cypress/support/commands/incident-commands.tsweb/cypress/support/commands/operator-commands.tsweb/cypress/support/commands/perses-commands.tsweb/cypress/support/commands/selector-commands.tsweb/cypress/support/commands/utility-commands.tsweb/cypress/support/commands/virtualization-commands.tsweb/cypress/support/commands/wait-utils.tsweb/cypress/support/e2e.jsweb/cypress/support/incidents_prometheus_query_mocks/mock-generators.tsweb/cypress/support/incidents_prometheus_query_mocks/prometheus-mocks.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/fixture-converter.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/schema-validator.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/validate-fixtures.tsweb/cypress/support/incidents_prometheus_query_mocks/types.tsweb/cypress/support/incidents_prometheus_query_mocks/utils.tsweb/cypress/support/index.tsweb/cypress/support/monitoring/00.bvt_monitoring.cy.tsweb/cypress/support/monitoring/00.bvt_monitoring_namespace.cy.tsweb/cypress/support/monitoring/01.reg_alerts.cy.tsweb/cypress/support/monitoring/02.reg_metrics_1.cy.tsweb/cypress/support/monitoring/02.reg_metrics_2.cy.tsweb/cypress/support/monitoring/03.reg_legacy_dashboards.cy.tsweb/cypress/support/monitoring/04.reg_alerts_namespace.cy.tsweb/cypress/support/monitoring/05.reg_metrics_namespace_1.cy.tsweb/cypress/support/monitoring/05.reg_metrics_namespace_2.cy.tsweb/cypress/support/monitoring/06.reg_legacy_dashboards_namespace.cy.tsweb/cypress/support/perses/00.coo_bvt_perses_admin.cy.tsweb/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/support/perses/01.coo_list_perses_admin.cy.tsweb/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.tsweb/cypress/support/perses/02.coo_edit_perses_admin.cy.tsweb/cypress/support/perses/02.coo_edit_perses_admin_1.cy.tsweb/cypress/support/perses/03.coo_create_perses_admin.cy.tsweb/cypress/support/perses/04.coo_import_perses_admin.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user1.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user2.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user3.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user4.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user5.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user6.cy.tsweb/cypress/support/selectors.tsweb/cypress/support/test-tags.d.tsweb/cypress/views/acm-alerting-page.tsweb/cypress/views/alerting-rule-details-page.tsweb/cypress/views/alerting-rule-list-page.tsweb/cypress/views/common.tsweb/cypress/views/details-page.tsweb/cypress/views/incidents-page.tsweb/cypress/views/legacy-dashboards.tsweb/cypress/views/list-page.tsweb/cypress/views/metrics.tsweb/cypress/views/nav.tsweb/cypress/views/operator-hub-page.tsweb/cypress/views/overview-page.tsweb/cypress/views/perses-dashboards-create-dashboard.tsweb/cypress/views/perses-dashboards-edit-datasources.tsweb/cypress/views/perses-dashboards-edit-variables.tsweb/cypress/views/perses-dashboards-import-dashboard.tsweb/cypress/views/perses-dashboards-list-dashboards.tsweb/cypress/views/perses-dashboards-panel.tsweb/cypress/views/perses-dashboards-panelgroup.tsweb/cypress/views/perses-dashboards.tsweb/cypress/views/silence-alert-page.tsweb/cypress/views/silence-details-page.tsweb/cypress/views/silences-list-page.tsweb/cypress/views/tour.tsweb/cypress/views/troubleshooting-panel.tsweb/cypress/views/utils.tsweb/package.jsonweb/src/components/data-test.ts
💤 Files with no reviewable changes (3)
- web/cypress/e2e/coo/01.coo_bvt.cy.ts
- web/cypress/e2e/perses/00.coo_bvt_perses_admin_1.cy.ts
- web/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.ts
✅ Files skipped from review due to trivial changes (46)
- web/cypress/support/e2e.js
- web/cypress/support/perses/99.coo_rbac_perses_user6.cy.ts
- web/cypress/e2e/coo/01.coo_ivt.cy.ts
- web/cypress/e2e/incidents/02.incidents-mocking-example.cy.ts
- web/cypress/e2e/incidents/01.incidents.cy.ts
- web/cypress/e2e/monitoring/00.bvt_dev.cy.ts
- web/cypress/e2e/virtualization/03.coo_ivt_legacy_dashboards.cy.ts
- web/cypress/fixtures/coo/coo140_perses/import/acm-vm-status.json
- web/cypress/support/incidents_prometheus_query_mocks/types.ts
- web/cypress/e2e/monitoring/regression/01.reg_alerts_admin.cy.ts
- web/cypress/e2e/monitoring/regression/01.reg_alerts_dev.cy.ts
- web/cypress/e2e/monitoring/00.bvt_admin.cy.ts
- web/cypress/e2e/virtualization/00.coo_ivt.cy.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user4.cy.ts
- web/cypress/support/index.ts
- web/cypress/e2e/monitoring/regression/02.reg_metrics_admin_1.cy.ts
- web/cypress/e2e/virtualization/01.coo_ivt_alerts.cy.ts
- web/cypress/e2e/incidents/regression/01.reg_filtering.cy.ts
- web/cypress/support/incidents_prometheus_query_mocks/schema/fixture-converter.ts
- web/cypress/e2e/monitoring/regression/03.reg_legacy_dashboards_admin.cy.ts
- web/cypress/support/commands/wait-utils.ts
- web/cypress/support/incidents_prometheus_query_mocks/mock-generators.ts
- web/cypress/e2e/monitoring/regression/02.reg_metrics_admin_2.cy.ts
- web/cypress/e2e/incidents/regression/05.reg_stress_testing_ui.cy.ts
- web/cypress/e2e/incidents/regression/04.reg_redux_effects.cy.ts
- web/cypress/dummy.tsx
- web/cypress/support/monitoring/03.reg_legacy_dashboards.cy.ts
- web/cypress/views/acm-alerting-page.ts
- web/cypress/views/alerting-rule-list-page.ts
- web/cypress/views/incidents-page.ts
- web/cypress/support/perses/99.coo_rbac_perses_user2.cy.ts
- web/cypress/e2e/virtualization/02.coo_ivt_metrics_1.cy.ts
- web/cypress/support/selectors.ts
- web/cypress/support/perses/99.coo_rbac_perses_user4.cy.ts
- web/cypress/e2e/coo/02.acm_alerting_ui.cy.ts
- web/cypress/e2e/perses/02.coo_edit_perses_admin.cy.ts
- web/cypress/support/incidents_prometheus_query_mocks/schema/schema-validator.ts
- web/cypress/e2e/incidents/regression/03.reg_api_calls.cy.ts
- web/cypress/support/monitoring/02.reg_metrics_1.cy.ts
- web/cypress/support/perses/01.coo_list_perses_admin.cy.ts
- web/cypress/e2e/incidents/00.coo_incidents_e2e.cy.ts
- web/cypress/support/monitoring/02.reg_metrics_2.cy.ts
- web/cypress/support/monitoring/00.bvt_monitoring_namespace.cy.ts
- web/cypress/support/test-tags.d.ts
- web/cypress/fixtures/monitoring/constants.ts
- web/cypress/support/commands/image-patch-commands.ts
🚧 Files skipped from review as they are similar to previous changes (29)
- web/cypress/views/alerting-rule-details-page.ts
- web/cypress/support/incidents_prometheus_query_mocks/utils.ts
- web/cypress/e2e/virtualization/02.coo_ivt_metrics_2.cy.ts
- web/cypress/support/monitoring/05.reg_metrics_namespace_2.cy.ts
- web/cypress/views/details-page.ts
- web/cypress/fixtures/monitoring/alert.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user3.cy.ts
- web/cypress/support/commands/coo-install-commands.ts
- web/cypress/support/commands/operator-commands.ts
- web/cypress/e2e/perses/01.coo_list_perses_admin.cy.ts
- web/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.ts
- web/cypress/e2e/virtualization/04.coo_ivt_perses.cy.ts
- web/cypress/support/commands/auth-commands.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user6.cy.ts
- web/cypress/support/monitoring/04.reg_alerts_namespace.cy.ts
- web/cypress/e2e/perses/00.coo_bvt_perses_admin.cy.ts
- web/cypress/views/legacy-dashboards.ts
- web/cypress/support/perses/99.coo_rbac_perses_user1.cy.ts
- web/cypress/e2e/perses/03.coo_create_perses_admin.cy.ts
- web/cypress/e2e/incidents/regression/02.reg_ui_charts_comprehensive.cy.ts
- web/cypress/views/list-page.ts
- web/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.ts
- web/cypress/e2e/perses/04.coo_import_perses_admin.cy.ts
- web/cypress/support/monitoring/05.reg_metrics_namespace_1.cy.ts
- web/cypress/support/perses/02.coo_edit_perses_admin.cy.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user1.cy.ts
- web/cypress/support/commands/dashboards-commands.ts
- web/cypress/fixtures/perses/constants.ts
- web/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.ts
etmurasaki
force-pushed
the
etmura-post-coo140
branch
2 times, most recently
from
7bb7719 to
351078d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
web/cypress/support/commands/virtualization-commands.ts (1)
234-246:⚠️ Potential issue | 🟠 Major
--dry-run=clientstill prevents CRD deletion at line 236.The log states "Delete Hyperconverged CRD instance" but
--dry-run=clientmakes this a no-op. The actual deletion occurs in the subsequent command (lines 242-246) which targets the same CRDs. Either remove--dry-run=clientor update the log to clarify this is a validation step.Proposed fix (if deletion was intended)
cy.log('Delete Hyperconverged CRD instance.'); cy.executeAndDelete( - `oc delete crd --dry-run=client ` + + `oc delete crd ` + `-l operators.coreos.com/kubevirt-hyperconverged.openshift-cnv ` + `--ignore-not-found --kubeconfig ${Cypress.env('KUBECONFIG_PATH')}`, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/virtualization-commands.ts` around lines 234 - 246, The first oc command call is using --dry-run=client so it doesn't actually delete the Hyperconverged CRD even though the preceding cy.log says "Delete Hyperconverged CRD instance."; update the code in virtualization-commands.ts around the cy.executeAndDelete invocation (the call to oc with --dry-run=client and the surrounding cy.log) to either remove the --dry-run=client flag so the command performs deletion, or change the cy.log text to indicate this is a dry-run/validation step; ensure you adjust the call to cy.executeAndDelete (the same helper used for the second oc delete) and the log message consistently so intent and behavior match.web/cypress/support/incidents_prometheus_query_mocks/utils.ts (1)
71-74:⚠️ Potential issue | 🟠 MajorRemove Cypress runtime dependency from
parseDuration.Line 71 uses
cy.log(...)inside a shared utility. This keepsparseDurationcoupled to Cypress globals and can throw outside Cypress contexts instead of surfacing the intendedError.Suggested fix
if (!matchedAny) { - cy.log( - `Invalid duration format: ${duration}. ` + - `Use formats like "30m", "2h", "7d", "1h30m", "7d2h30m"`, - ); throw new Error( `Invalid duration format: ${duration}. Use formats like "30m", "2h", "7d", "1h30m", "7d2h30m", ${normalized}, ${componentRegex.lastIndex}, ${matchedAny}, ${totalSeconds}`, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/incidents_prometheus_query_mocks/utils.ts` around lines 71 - 74, The parseDuration utility currently calls cy.log(...) which couples it to Cypress and prevents the intended Error from surfacing; remove the Cypress runtime call from parseDuration and instead throw a descriptive Error (e.g., new Error(`Invalid duration format: ${duration}. Use formats like "30m", "2h", ...`)) when the format is invalid so callers can handle it outside of Cypress; ensure the function signature and return behavior remain the same and no other Cypress globals are referenced in parseDuration.
🧹 Nitpick comments (5)
web/cypress/support/commands/dashboards-commands.ts (2)
210-283: Same redundant conditional pattern in cleanup.Similar to
setupDashboardsAndPlugins, both branches ofcleanupDashboardsexecute identical deletion commands fromcoo140_perses. Consolidating would halve the code and reduce maintenance burden.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/dashboards-commands.ts` around lines 210 - 283, The cleanupDashboards function duplicates the same oc delete calls in both branches of the if (Cypress.env('COO_UI_INSTALL')) conditional; refactor by extracting the repeated cy.executeAndDelete(...) calls (for openshift-cluster-sample-dashboard.yaml, perses-dashboard-sample.yaml, prometheus-overview-variables.yaml, thanos-compact-overview-1var.yaml, thanos-querier-datasource.yaml) into a single shared block after handling any branch-specific logging, keeping the conditional only for the different cy.log messages (or removing it entirely if logs are unnecessary) so that cleanupDashboards contains one set of deletion commands referenced by the same resource filenames.
32-110: Redundant conditional: both branches execute identical code.The
if (COO_UI_INSTALL)andelseblocks (lines 36–110) apply the exact same dashboard fixtures fromcoo140_perses. The TODO comment on lines 32–35 appears stale since the migration to COO 1.4.0 has already been completed.Consider removing the conditional and the stale TODO, keeping only the common logic:
♻️ Suggested consolidation
- /** - * TODO: When COO1.4.0 is released, points COO_UI_INSTALL to install dashboards on - * COO1.4.0 folder - */ - if (Cypress.env('COO_UI_INSTALL')) { - cy.log('COO_UI_INSTALL is set. Installing dashboards on COO1.4.0 folder'); - // ... identical dashboard setup ... - } else { - cy.log('COO_UI_INSTALL is not set. Installing dashboards on COO1.4.0 folder'); - // ... identical dashboard setup ... - } + cy.log('Installing dashboards from COO1.4.0 folder'); + + cy.log('Create openshift-cluster-sample-dashboard instance.'); + cy.exec( + `oc apply -f ./cypress/fixtures/coo/coo140_perses/dashboards/` + + `openshift-cluster-sample-dashboard.yaml ` + + `--kubeconfig ${Cypress.env('KUBECONFIG_PATH')}`, + ); + // ... remaining dashboard setup (single copy) ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/dashboards-commands.ts` around lines 32 - 110, The conditional on Cypress.env('COO_UI_INSTALL') and the preceding TODO are redundant because both branches run identical cy.log/cy.exec sequences; remove the TODO comment and the entire if/else structure, and keep a single consolidated block that runs the existing cy.log and cy.exec calls for openshift-cluster-sample-dashboard, perses-dashboard-sample, prometheus-overview-variables, thanos-compact-overview-1var, and thanos-querier-datasource (preserving the use of Cypress.env('KUBECONFIG_PATH') in each cy.exec); ensure no duplicate calls remain and that the sequence and messages are unchanged.web/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.ts (1)
66-68: Multi-line string will embed newline and spaces in error message.The template literal spans lines and will include the literal newline plus leading whitespace in the rendered message.
💡 Suggested fix
- errorMsg: `Incident with alert ${currentAlertName} should appear within ${maxMinutes} - minutes`, + errorMsg: `Incident with alert ${currentAlertName} should appear within ${maxMinutes} minutes`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.ts` around lines 66 - 68, The errorMsg template literal contains a newline and leading spaces which get embedded into the message; update the assignment for errorMsg (the object holding the assertion message) to produce a single-line string — e.g. put the entire template on one line or build it with concatenation (`'Incident with alert ' + currentAlertName + ' should appear within ' + maxMinutes + ' minutes'`) or call .trim() so no newline/leading spaces are included; change the errorMsg expression where it's defined to one of these approaches.web/cypress/fixtures/monitoring/alert.ts (1)
19-19: Remove dead commented code.This references
NAMESPACEwhich isn't defined. The correct constantWatchdogAlert.NAMESPACEis already used on line 21.Proposed fix
labels: { - // namespace: `${NAMESPACE}`, prometheus: 'openshift-monitoring/k8s',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/fixtures/monitoring/alert.ts` at line 19, Remove the dead commented line that references the undefined NAMESPACE constant; instead rely on the existing WatchdogAlert.NAMESPACE used on line 21. Locate the commented-out snippet containing "// namespace: `${NAMESPACE}`,", delete that comment so there is no reference to the undefined NAMESPACE and the fixture consistently uses WatchdogAlert.NAMESPACE.web/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.ts (1)
85-90: Strengthen the segment-count precondition to match later index access.Line 85 only checks
segments.length > 1, but Line 102 and Line 103 always access segment index2. Make this guard require at least 3 segments so failures are explicit at setup time.Suggested change
- expect( - segments.length, - 'Multi-severity bar should have at least 2 visible segments', - ).to.be.greaterThan(1); + expect( + segments.length, + 'Multi-severity bar should have at least 3 visible segments', + ).to.be.greaterThan(2);As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.ts` around lines 85 - 90, The current precondition uses incidentsPage.getIncidentBarVisibleSegments(0) and expects segments.length > 1, but later code accesses segments[2]; change the assertion to require at least 3 segments (e.g., use .to.be.at.least(3) or .to.be.greaterThan(2)) and update the assertion message accordingly so the guard matches the later index access to segments[2].
🤖 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/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.ts`:
- Line 116: Remove all committed interactive debug pauses by deleting the
cy.pause() calls in the regression spec (the three occurrences of cy.pause() in
02.reg_ui_tooltip_boundary_times.cy.ts). Locate each cy.pause() invocation and
remove it so the tests run headless/CI-friendly; if a non-interactive wait is
necessary replace with an appropriate deterministic assertion or
cy.wait()/cy.get(...).should(...) in the surrounding test code to preserve
stability.
In `@web/cypress/fixtures/monitoring/alert.ts`:
- Line 5: The intercept pattern currently uses '/api/prometheus/api/v1/rules?'
which treats '?' as a literal and won't match requests with query parameters;
update the cy.intercept call (the line beginning with cy.intercept('GET',
'/api/prometheus/api/v1/rules?') ) to use a glob or regex that matches queries
such as '**/api/prometheus/api/v1/rules*' or a regex like
/\/api\/prometheus\/api\/v1\/rules.*?/ so the mock intercepts requests with
query parameters.
In `@web/cypress/support/incidents_prometheus_query_mocks/mock-generators.ts`:
- Line 252: Guard the use of alert.silenced before calling toString(): replace
the direct call at the silenced assignment with a null/undefined check (e.g.,
use alert.silenced != null ? alert.silenced.toString() : '' or the same
defensive pattern used earlier in this file) so that the property is only
stringified when present; reference the alert.silenced access and the silenced
assignment to locate and update the code.
In `@web/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.ts`:
- Around line 171-176: Summary: the cy.log message "Click on the Kebab icon -
Delete" is misleading because the block uses
listPersesDashboardsPage.clearAllFilters() and
listPersesDashboardsPage.filter.byName(...) to filter the renamed dashboard;
update the log to reflect filtering (e.g., "Filter for renamed dashboard before
delete"). Locate the block that calls
listPersesDashboardsPage.clearAllFilters(),
listPersesDashboardsPage.filter.byName(...), and
listPersesDashboardsPage.countDashboards('1') and change the cy.log text
accordingly (refer to symbols listPersesDashboardsPage, filter.byName, and
persesDashboardsDashboardDropdownPersesDev.PERSES_DASHBOARD_SAMPLE to find the
exact spot).
---
Duplicate comments:
In `@web/cypress/support/commands/virtualization-commands.ts`:
- Around line 234-246: The first oc command call is using --dry-run=client so it
doesn't actually delete the Hyperconverged CRD even though the preceding cy.log
says "Delete Hyperconverged CRD instance."; update the code in
virtualization-commands.ts around the cy.executeAndDelete invocation (the call
to oc with --dry-run=client and the surrounding cy.log) to either remove the
--dry-run=client flag so the command performs deletion, or change the cy.log
text to indicate this is a dry-run/validation step; ensure you adjust the call
to cy.executeAndDelete (the same helper used for the second oc delete) and the
log message consistently so intent and behavior match.
In `@web/cypress/support/incidents_prometheus_query_mocks/utils.ts`:
- Around line 71-74: The parseDuration utility currently calls cy.log(...) which
couples it to Cypress and prevents the intended Error from surfacing; remove the
Cypress runtime call from parseDuration and instead throw a descriptive Error
(e.g., new Error(`Invalid duration format: ${duration}. Use formats like "30m",
"2h", ...`)) when the format is invalid so callers can handle it outside of
Cypress; ensure the function signature and return behavior remain the same and
no other Cypress globals are referenced in parseDuration.
---
Nitpick comments:
In `@web/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.ts`:
- Around line 85-90: The current precondition uses
incidentsPage.getIncidentBarVisibleSegments(0) and expects segments.length > 1,
but later code accesses segments[2]; change the assertion to require at least 3
segments (e.g., use .to.be.at.least(3) or .to.be.greaterThan(2)) and update the
assertion message accordingly so the guard matches the later index access to
segments[2].
In `@web/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.ts`:
- Around line 66-68: The errorMsg template literal contains a newline and
leading spaces which get embedded into the message; update the assignment for
errorMsg (the object holding the assertion message) to produce a single-line
string — e.g. put the entire template on one line or build it with concatenation
(`'Incident with alert ' + currentAlertName + ' should appear within ' +
maxMinutes + ' minutes'`) or call .trim() so no newline/leading spaces are
included; change the errorMsg expression where it's defined to one of these
approaches.
In `@web/cypress/fixtures/monitoring/alert.ts`:
- Line 19: Remove the dead commented line that references the undefined
NAMESPACE constant; instead rely on the existing WatchdogAlert.NAMESPACE used on
line 21. Locate the commented-out snippet containing "// namespace:
`${NAMESPACE}`,", delete that comment so there is no reference to the undefined
NAMESPACE and the fixture consistently uses WatchdogAlert.NAMESPACE.
In `@web/cypress/support/commands/dashboards-commands.ts`:
- Around line 210-283: The cleanupDashboards function duplicates the same oc
delete calls in both branches of the if (Cypress.env('COO_UI_INSTALL'))
conditional; refactor by extracting the repeated cy.executeAndDelete(...) calls
(for openshift-cluster-sample-dashboard.yaml, perses-dashboard-sample.yaml,
prometheus-overview-variables.yaml, thanos-compact-overview-1var.yaml,
thanos-querier-datasource.yaml) into a single shared block after handling any
branch-specific logging, keeping the conditional only for the different cy.log
messages (or removing it entirely if logs are unnecessary) so that
cleanupDashboards contains one set of deletion commands referenced by the same
resource filenames.
- Around line 32-110: The conditional on Cypress.env('COO_UI_INSTALL') and the
preceding TODO are redundant because both branches run identical cy.log/cy.exec
sequences; remove the TODO comment and the entire if/else structure, and keep a
single consolidated block that runs the existing cy.log and cy.exec calls for
openshift-cluster-sample-dashboard, perses-dashboard-sample,
prometheus-overview-variables, thanos-compact-overview-1var, and
thanos-querier-datasource (preserving the use of Cypress.env('KUBECONFIG_PATH')
in each cy.exec); ensure no duplicate calls remain and that the sequence and
messages are unchanged.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 77bfd730-307f-4ca9-a396-8d40fa36fbb2
📒 Files selected for processing (128)
web/cypress/dummy.tsxweb/cypress/e2e/coo/01.coo_bvt.cy.tsweb/cypress/e2e/coo/01.coo_ivt.cy.tsweb/cypress/e2e/coo/02.acm_alerting_ui.cy.tsweb/cypress/e2e/incidents/00.coo_incidents_e2e.cy.tsweb/cypress/e2e/incidents/01.incidents.cy.tsweb/cypress/e2e/incidents/02.incidents-mocking-example.cy.tsweb/cypress/e2e/incidents/regression/01.reg_filtering.cy.tsweb/cypress/e2e/incidents/regression/02.reg_ui_charts_comprehensive.cy.tsweb/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.tsweb/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.tsweb/cypress/e2e/incidents/regression/03.reg_api_calls.cy.tsweb/cypress/e2e/incidents/regression/04.reg_redux_effects.cy.tsweb/cypress/e2e/incidents/regression/05.reg_stress_testing_ui.cy.tsweb/cypress/e2e/monitoring/00.bvt_admin.cy.tsweb/cypress/e2e/monitoring/00.bvt_dev.cy.tsweb/cypress/e2e/monitoring/regression/01.reg_alerts_admin.cy.tsweb/cypress/e2e/monitoring/regression/01.reg_alerts_dev.cy.tsweb/cypress/e2e/monitoring/regression/02.reg_metrics_admin_1.cy.tsweb/cypress/e2e/monitoring/regression/02.reg_metrics_admin_2.cy.tsweb/cypress/e2e/monitoring/regression/03.reg_legacy_dashboards_admin.cy.tsweb/cypress/e2e/perses/00.coo_bvt_perses_admin.cy.tsweb/cypress/e2e/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/e2e/perses/01.coo_list_perses_admin.cy.tsweb/cypress/e2e/perses/02.coo_edit_perses_admin.cy.tsweb/cypress/e2e/perses/03.coo_create_perses_admin.cy.tsweb/cypress/e2e/perses/04.coo_import_perses_admin.cy.tsweb/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/e2e/virtualization/00.coo_ivt.cy.tsweb/cypress/e2e/virtualization/01.coo_ivt_alerts.cy.tsweb/cypress/e2e/virtualization/02.coo_ivt_metrics_1.cy.tsweb/cypress/e2e/virtualization/02.coo_ivt_metrics_2.cy.tsweb/cypress/e2e/virtualization/03.coo_ivt_legacy_dashboards.cy.tsweb/cypress/e2e/virtualization/04.coo_ivt_perses.cy.tsweb/cypress/fixtures/coo/coo140_perses/dashboards/openshift-cluster-sample-dashboard.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/perses-dashboard-sample.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/prometheus-overview-variables.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/thanos-compact-overview-1var.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/thanos-querier-datasource.yamlweb/cypress/fixtures/coo/coo140_perses/import/accelerators-dashboard-cr-v1alpha1.yamlweb/cypress/fixtures/coo/coo140_perses/import/accelerators-dashboard-cr-v1alpha2.yamlweb/cypress/fixtures/coo/coo140_perses/import/acm-vm-status.jsonweb/cypress/fixtures/coo/coo140_perses/import/grafana_to_check_errors.jsonweb/cypress/fixtures/coo/coo140_perses/import/testing-perses-dashboard.jsonweb/cypress/fixtures/coo/coo140_perses/import/testing-perses-dashboard.yamlweb/cypress/fixtures/coo/coo140_perses/rbac/rbac_perses_e2e_ci_users.shweb/cypress/fixtures/monitoring/alert.tsweb/cypress/fixtures/monitoring/constants.tsweb/cypress/fixtures/perses/constants.tsweb/cypress/support/commands/auth-commands.tsweb/cypress/support/commands/coo-install-commands.tsweb/cypress/support/commands/dashboards-commands.tsweb/cypress/support/commands/image-patch-commands.tsweb/cypress/support/commands/incident-commands.tsweb/cypress/support/commands/operator-commands.tsweb/cypress/support/commands/perses-commands.tsweb/cypress/support/commands/selector-commands.tsweb/cypress/support/commands/utility-commands.tsweb/cypress/support/commands/virtualization-commands.tsweb/cypress/support/commands/wait-utils.tsweb/cypress/support/e2e.jsweb/cypress/support/incidents_prometheus_query_mocks/mock-generators.tsweb/cypress/support/incidents_prometheus_query_mocks/prometheus-mocks.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/fixture-converter.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/schema-validator.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/validate-fixtures.tsweb/cypress/support/incidents_prometheus_query_mocks/types.tsweb/cypress/support/incidents_prometheus_query_mocks/utils.tsweb/cypress/support/index.tsweb/cypress/support/monitoring/00.bvt_monitoring.cy.tsweb/cypress/support/monitoring/00.bvt_monitoring_namespace.cy.tsweb/cypress/support/monitoring/01.reg_alerts.cy.tsweb/cypress/support/monitoring/02.reg_metrics_1.cy.tsweb/cypress/support/monitoring/02.reg_metrics_2.cy.tsweb/cypress/support/monitoring/03.reg_legacy_dashboards.cy.tsweb/cypress/support/monitoring/04.reg_alerts_namespace.cy.tsweb/cypress/support/monitoring/05.reg_metrics_namespace_1.cy.tsweb/cypress/support/monitoring/05.reg_metrics_namespace_2.cy.tsweb/cypress/support/monitoring/06.reg_legacy_dashboards_namespace.cy.tsweb/cypress/support/perses/00.coo_bvt_perses_admin.cy.tsweb/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/support/perses/01.coo_list_perses_admin.cy.tsweb/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.tsweb/cypress/support/perses/02.coo_edit_perses_admin.cy.tsweb/cypress/support/perses/02.coo_edit_perses_admin_1.cy.tsweb/cypress/support/perses/03.coo_create_perses_admin.cy.tsweb/cypress/support/perses/04.coo_import_perses_admin.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user1.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user2.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user3.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user4.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user5.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user6.cy.tsweb/cypress/support/selectors.tsweb/cypress/support/test-tags.d.tsweb/cypress/views/acm-alerting-page.tsweb/cypress/views/alerting-rule-details-page.tsweb/cypress/views/alerting-rule-list-page.tsweb/cypress/views/common.tsweb/cypress/views/details-page.tsweb/cypress/views/incidents-page.tsweb/cypress/views/legacy-dashboards.tsweb/cypress/views/list-page.tsweb/cypress/views/metrics.tsweb/cypress/views/nav.tsweb/cypress/views/operator-hub-page.tsweb/cypress/views/overview-page.tsweb/cypress/views/perses-dashboards-create-dashboard.tsweb/cypress/views/perses-dashboards-edit-datasources.tsweb/cypress/views/perses-dashboards-edit-variables.tsweb/cypress/views/perses-dashboards-import-dashboard.tsweb/cypress/views/perses-dashboards-list-dashboards.tsweb/cypress/views/perses-dashboards-panel.tsweb/cypress/views/perses-dashboards-panelgroup.tsweb/cypress/views/perses-dashboards.tsweb/cypress/views/silence-alert-page.tsweb/cypress/views/silence-details-page.tsweb/cypress/views/silences-list-page.tsweb/cypress/views/tour.tsweb/cypress/views/troubleshooting-panel.tsweb/cypress/views/utils.tsweb/package.jsonweb/src/components/data-test.ts
💤 Files with no reviewable changes (3)
- web/cypress/e2e/coo/01.coo_bvt.cy.ts
- web/cypress/e2e/perses/00.coo_bvt_perses_admin_1.cy.ts
- web/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.ts
✅ Files skipped from review due to trivial changes (53)
- web/cypress/support/e2e.js
- web/cypress/views/acm-alerting-page.ts
- web/cypress/e2e/monitoring/00.bvt_dev.cy.ts
- web/cypress/e2e/monitoring/00.bvt_admin.cy.ts
- web/cypress/fixtures/coo/coo140_perses/import/acm-vm-status.json
- web/cypress/support/perses/99.coo_rbac_perses_user6.cy.ts
- web/cypress/support/test-tags.d.ts
- web/cypress/support/incidents_prometheus_query_mocks/types.ts
- web/cypress/support/index.ts
- web/cypress/e2e/monitoring/regression/01.reg_alerts_dev.cy.ts
- web/cypress/e2e/virtualization/01.coo_ivt_alerts.cy.ts
- web/cypress/e2e/incidents/01.incidents.cy.ts
- web/cypress/e2e/coo/01.coo_ivt.cy.ts
- web/cypress/e2e/virtualization/03.coo_ivt_legacy_dashboards.cy.ts
- web/cypress/support/commands/image-patch-commands.ts
- web/cypress/e2e/virtualization/00.coo_ivt.cy.ts
- web/cypress/e2e/incidents/regression/05.reg_stress_testing_ui.cy.ts
- web/cypress/e2e/perses/02.coo_edit_perses_admin.cy.ts
- web/cypress/e2e/incidents/02.incidents-mocking-example.cy.ts
- web/cypress/support/monitoring/03.reg_legacy_dashboards.cy.ts
- web/cypress/e2e/monitoring/regression/03.reg_legacy_dashboards_admin.cy.ts
- web/cypress/e2e/virtualization/02.coo_ivt_metrics_2.cy.ts
- web/cypress/e2e/incidents/regression/04.reg_redux_effects.cy.ts
- web/cypress/e2e/perses/01.coo_list_perses_admin.cy.ts
- web/cypress/e2e/monitoring/regression/02.reg_metrics_admin_2.cy.ts
- web/cypress/support/commands/operator-commands.ts
- web/cypress/e2e/monitoring/regression/01.reg_alerts_admin.cy.ts
- web/cypress/e2e/incidents/regression/01.reg_filtering.cy.ts
- web/cypress/support/perses/99.coo_rbac_perses_user2.cy.ts
- web/cypress/support/incidents_prometheus_query_mocks/schema/schema-validator.ts
- web/cypress/views/alerting-rule-list-page.ts
- web/cypress/support/monitoring/02.reg_metrics_1.cy.ts
- web/cypress/e2e/coo/02.acm_alerting_ui.cy.ts
- web/cypress/support/selectors.ts
- web/cypress/e2e/incidents/regression/02.reg_ui_charts_comprehensive.cy.ts
- web/cypress/support/monitoring/02.reg_metrics_2.cy.ts
- web/cypress/support/incidents_prometheus_query_mocks/schema/fixture-converter.ts
- web/cypress/support/monitoring/06.reg_legacy_dashboards_namespace.cy.ts
- web/cypress/support/monitoring/05.reg_metrics_namespace_1.cy.ts
- web/cypress/support/commands/coo-install-commands.ts
- web/cypress/views/list-page.ts
- web/cypress/fixtures/monitoring/constants.ts
- web/cypress/e2e/incidents/00.coo_incidents_e2e.cy.ts
- web/cypress/support/perses/02.coo_edit_perses_admin.cy.ts
- web/cypress/e2e/perses/03.coo_create_perses_admin.cy.ts
- web/cypress/support/perses/01.coo_list_perses_admin.cy.ts
- web/cypress/support/commands/auth-commands.ts
- web/cypress/support/commands/utility-commands.ts
- web/cypress/e2e/incidents/regression/03.reg_api_calls.cy.ts
- web/cypress/views/incidents-page.ts
- web/cypress/support/commands/selector-commands.ts
- web/cypress/support/monitoring/05.reg_metrics_namespace_2.cy.ts
- web/cypress/support/perses/99.coo_rbac_perses_user4.cy.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- web/cypress/e2e/virtualization/02.coo_ivt_metrics_1.cy.ts
- web/cypress/support/commands/wait-utils.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user1.cy.ts
- web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user4.cy.ts
- web/cypress/e2e/perses/04.coo_import_perses_admin.cy.ts
- web/cypress/e2e/perses/00.coo_bvt_perses_admin.cy.ts
- web/cypress/views/legacy-dashboards.ts
- web/cypress/views/common.ts
- web/cypress/support/perses/03.coo_create_perses_admin.cy.ts
- web/cypress/dummy.tsx
- web/cypress/views/details-page.ts
- web/cypress/support/perses/99.coo_rbac_perses_user1.cy.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user2.cy.ts
- web/cypress/views/alerting-rule-details-page.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user6.cy.ts
- web/cypress/e2e/monitoring/regression/02.reg_metrics_admin_1.cy.ts
- web/cypress/support/perses/00.coo_bvt_perses_admin.cy.ts
- web/cypress/support/commands/perses-commands.ts
web/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.ts
Show resolved
Hide resolved
Contributor
Author
|
/test e2e-monitoring |
Contributor
Author
|
/test e2e-incidents |
etmurasaki
force-pushed
the
etmura-post-coo140
branch
2 times, most recently
from
65f55c2 to
ffa24ef
Compare
etmurasaki
force-pushed
the
etmura-post-coo140
branch
from
ffa24ef to
b4d68e8
Compare
etmurasaki
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
web/cypress/support/incidents_prometheus_query_mocks/mock-generators.ts (1)
252-252:⚠️ Potential issue | 🟠 MajorGuard
alert.silencedbefore string conversion.Line 252 can throw when
alert.silencedisundefined/null(toString()on missing value). This is inconsistent with the defensive pattern already used in the same file (Line 155).Proposed fix
- silenced: alert.silenced.toString(), + silenced: (alert.silenced === true).toString(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/incidents_prometheus_query_mocks/mock-generators.ts` at line 252, The code calls alert.silenced.toString() which can throw if alert.silenced is null/undefined; update the silenced assignment to guard against missing values (e.g., use a null-coalescing or conditional check like (alert.silenced ?? false).toString() or String(alert.silenced) to match the defensive pattern used elsewhere in the file) so that the mock generator always produces a safe string for the silenced field.web/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.ts (1)
116-191:⚠️ Potential issue | 🟠 MajorRemove committed
cy.pause()calls from this regression spec.
cy.pause()at Line 116, Line 144, and Line 191 introduces interactive breakpoints that can stall/slow non-interactive CI runs.#!/bin/bash # Verify committed interactive pauses in this spec rg -nP --type=ts '\bcy\.pause\s*\(' web/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.tsAs per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.ts` around lines 116 - 191, This spec contains interactive cy.pause() calls that will stall CI; remove the cy.pause() invocations found in the regression test (the ones between the checks for "Consecutive interval boundaries" and before "2.3 Incident tooltip..." and the final one after the alerts table checks). Search for and delete the three cy.pause() calls in the test (they appear immediately before and after the block that uses incidentsPage.hoverOverIncidentBarSegment, incidentsPage.selectIncidentById, and incidentsPage.getSelectedIncidentAlerts) and run the grep check used in the review to ensure no cy.pause() remain.web/cypress/support/perses/01.coo_list_perses_admin.cy.ts (1)
287-287:⚠️ Potential issue | 🟠 MajorReplace the fixed 2s sleep with a state-based wait.
Line 287 is still timing-dependent.
countDashboards('1')on Line 288 should be the deterministic wait here; the extracy.wait(2000)just adds flakiness and latency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/01.coo_list_perses_admin.cy.ts` at line 287, Remove the fixed cy.wait(2000) and replace it with a state-based wait by using the existing countDashboards('1') assertion to deterministically wait for the expected UI state; locate the cy.wait(2000) in the test in web/cypress/support/perses/01.coo_list_perses_admin.cy.ts and remove it, then call or await countDashboards('1') (the helper used on the following line) as the synchronization point so the test proceeds only after the dashboard count reaches 1.web/cypress/support/perses/04.coo_import_perses_admin.cy.ts (1)
54-56:⚠️ Potential issue | 🟠 MajorUse alert-scoped close helpers in these import paths.
persesDashboardsPage.closeAlert()inweb/cypress/views/perses-dashboards.ts:802-815waits 2s and closes the first visibleh4-backed alert. In these flows that can dismiss the wrong element and make the spec nondeterministic. Switch back to success/error-specific helpers, or pass a selector-scoped close method.Also applies to: 107-109, 131-131, 177-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/04.coo_import_perses_admin.cy.ts` around lines 54 - 56, The generic persesDashboardsPage.closeAlert() is being used in import flows and can close the wrong alert; replace it with an alert-scoped closer (e.g., use the success/error-specific helper such as persesDashboardsPage.closeErrorAlert() or persesDashboardsPage.closeSuccessAlert(), or call a scoped close like persesDashboardsPage.closeAlert(selector)) wherever used with persesImportDashboardsPage.assertFailedToMigrateGrafanaDashboard() (and the other occurrences noted around the spec) so the close targets the specific alert element and the test becomes deterministic.web/cypress/support/commands/perses-commands.ts (1)
44-57:⚠️ Potential issue | 🟠 MajorCheck the raw env vars before stringifying them.
`${Cypress.env('LOGIN_USERNAME2')}` !== undefinedis always true whenLOGIN_USERNAME2is missing, because the template literal becomes the string'undefined'. That makes this setup run with bogus usernames instead of skipping cleanly.Proposed fix
Cypress.Commands.add('setupPersesRBACandExtraDashboards', () => { + const loginUsername1 = Cypress.env('LOGIN_USERNAME1'); + const loginUsername2 = Cypress.env('LOGIN_USERNAME2'); + if ( - `${Cypress.env('LOGIN_USERNAME1')}` !== 'kubeadmin' && - `${Cypress.env('LOGIN_USERNAME2')}` !== undefined + typeof loginUsername1 === 'string' && + loginUsername1 !== 'kubeadmin' && + typeof loginUsername2 === 'string' ) { cy.exec('./cypress/fixtures/coo/coo140_perses/rbac/rbac_perses_e2e_ci_users.sh', { env: { - USER1: `${Cypress.env('LOGIN_USERNAME1')}`, - USER2: `${Cypress.env('LOGIN_USERNAME2')}`, - USER3: `${Cypress.env('LOGIN_USERNAME3')}`, - USER4: `${Cypress.env('LOGIN_USERNAME4')}`, - USER5: `${Cypress.env('LOGIN_USERNAME5')}`, - USER6: `${Cypress.env('LOGIN_USERNAME6')}`, + USER1: loginUsername1, + USER2: loginUsername2, + USER3: Cypress.env('LOGIN_USERNAME3'), + USER4: Cypress.env('LOGIN_USERNAME4'), + USER5: Cypress.env('LOGIN_USERNAME5'), + USER6: Cypress.env('LOGIN_USERNAME6'), }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/perses-commands.ts` around lines 44 - 57, The conditional is incorrectly stringifying env vars so `${Cypress.env('LOGIN_USERNAME2')}` becomes the literal "undefined" and the block runs with bad usernames; change the checks to use the raw env values (e.g., Cypress.env('LOGIN_USERNAME1') !== 'kubeadmin' && Cypress.env('LOGIN_USERNAME2') !== undefined / truthy) and when calling cy.exec pass the raw values (USER1: Cypress.env('LOGIN_USERNAME1'), USER2: Cypress.env('LOGIN_USERNAME2'), etc.) instead of template literals so the guard skips correctly and real undefined/missing values aren’t turned into the string "undefined" before invoking rbac_perses_e2e_ci_users.sh (adjust checks for any other LOGIN_USERNAME3..6 uses similarly).web/cypress/support/commands/utility-commands.ts (1)
169-170:⚠️ Potential issue | 🟡 MinorUnintentional double-click on pod link remains unaddressed.
The pod link is clicked twice in succession—first via
.click()on line 169, then again via.click()on line 170 after retrieving the alias. This can cause navigation issues if the first click already navigates away from the page.Proposed fix
- cy.get(`a[data-test^="${pod}"]`).eq(0).as('podLink').click(); - cy.get('@podLink').should('be.visible').click(); + cy.get(`a[data-test^="${pod}"]`).eq(0).should('be.visible').click();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/commands/utility-commands.ts` around lines 169 - 170, The test double-clicks the pod link: the element is aliased as 'podLink' and clicked immediately, then the alias is retrieved and clicked again; remove the redundant initial .click() so you only alias the element (cy.get(...).eq(0).as('podLink')), then assert visibility on '@podLink' and perform a single .click() on that alias (use the existing 'podLink' alias/selector to locate the element before clicking).
🧹 Nitpick comments (4)
web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts (1)
39-62: Save once per scenario instead of once per panel type.Lines 45-61 and Lines 138-145 reopen edit mode and persist after every single add/delete. That scales directly with the number of panel types, slows the spec, and increases the flake surface. Queue the changes in one edit session, then save once per scenario.
As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
Also applies to: 131-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts` around lines 39 - 62, The test currently opens edit mode and calls persesDashboardsPage.clickEditActionButton('Save') inside the panelTypeKeys.forEach loop (using persesDashboardsPage.clickEditButton, clickEditActionButton('AddGroup'/'AddPanel', addPanelGroup, addPanel, etc.), causing a save per panel type; change the flow to open edit mode once before iterating panelTypeKeys, perform all addPanelGroup and addPanel calls inside the loop without calling clickEditActionButton('Save') inside it, then call persesDashboardsPage.clickEditActionButton('Save') once after the loop to persist all changes in one edit session; apply the same pattern for the deletion block referenced (remove repeated clickEditButton and Save calls inside per-panel loops and save once per scenario).web/cypress/fixtures/perses/constants.ts (1)
159-160: Update to use double quotes in label matchers for standards compliance.The
job='$job'andinstance=~'$instance'label matchers use single quotes, which work in PromQL but deviate from the documented standard. Prometheus documentation examples exclusively use double quotes for label matcher values. Update these tojob="$job"andinstance=~"$instance"for consistency with PromQL conventions and to match other matchers in the same query (likemode!="nice").Proposed fix
export const persesDashboardSampleQueries = { CPU_LINE_MULTI_SERIES: - 'avg without (cpu)(rate(node_cpu_seconds_total{job=\'$job\',instance=~\'$instance\',mode!="nice",mode!="steal",mode!="irq"}[$interval]))', + 'avg without (cpu)(rate(node_cpu_seconds_total{job="$job",instance=~"$instance",mode!="nice",mode!="steal",mode!="irq"}[$interval]))',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/fixtures/perses/constants.ts` around lines 159 - 160, The CPU_LINE_MULTI_SERIES constant contains PromQL label matchers using single quotes (job='$job' and instance=~'$instance'); update those to use double quotes (job="$job" and instance=~"$instance") to follow Prometheus conventions and match other matchers in the same query—locate the CPU_LINE_MULTI_SERIES constant in web/cypress/fixtures/perses/constants.ts and replace the single-quoted label values with double-quoted ones.web/cypress/views/legacy-dashboards.ts (1)
181-216: Minor inconsistency:exportAsCSVsignature differs fromlist-page.ts.This function takes
fileNameExp: stringand uses.includes()(line 194), whilelist-page.tstakesfileNameExp: RegExpand uses.test(). This works but creates API inconsistency across similar helpers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/views/legacy-dashboards.ts` around lines 181 - 216, The exportAsCSV helper (exportAsCSV) is inconsistent with list-page.ts: it currently accepts fileNameExp: string and uses String.includes; change the parameter type to RegExp (fileNameExp: RegExp) and update the matching logic to use fileNameExp.test(file) (replace .includes usage) so the API and behavior match the list-page.ts helper; ensure any callers pass a RegExp or update their calls accordingly and keep the rest of the waitUntil logic and assertions the same.web/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.ts (1)
73-125: Consider using a synchronous counter pattern for clarity.The alias-based counter pattern (
@initialFiringCount) with repeatedcy.get/cy.wrapinside async loops can be hard to follow. This works correctly but is complex. For future maintainability, consider accumulating counts in a closure variable and wrapping once at the end.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.ts` around lines 73 - 125, Replace the alias-based counter (`@initialFiringCount`) with a simple closure variable: declare let initialFiringCount = 0 before calling incidentsPage.getSelectedIncidentAlerts(), increment initialFiringCount inside the isFiring branch where you currently call cy.get/cy.wrap, remove the intermediate cy.wrap/cy.get calls, and at the final .then() call wrap the single final value with cy.wrap(initialFiringCount) (or use cy.then(() => { expect(initialFiringCount)... })) to perform the assertion; update references in the block that use currentAlertName, alert.getAlertRuleCell(), and alert.getEndCell() accordingly so only the final wrap/assert is done once.
🤖 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/fixtures/perses/constants.ts`:
- Around line 26-30: The K8S_COMPUTE_RESOURCES_CLUSTER fixture mixes two
different dashboards: element [0] labels 'Kubernetes / Compute Resources /
Cluster' while element [2] points to 'openshift-cluster-sample-dashboard' (which
actually has display.name 'OpenShift Cluster Sample Dashboard'); update
K8S_COMPUTE_RESOURCES_CLUSTER so the name and the referenced dashboard are
consistent—either change the label at [0] to match 'OpenShift Cluster Sample
Dashboard' or replace the dashboard slug at [2] with the correct slug for
'Kubernetes / Compute Resources / Cluster' so list/search assertions and
download/view flows target the same dashboard.
In `@web/cypress/support/perses/00.coo_bvt_perses_admin.cy.ts`:
- Around line 46-50: Replace the hardcoded cy.wait(2000) after opening
dashboards with a state-based assertion: after calling
listPersesDashboardsPage.clickDashboard(...) remove cy.wait and call
listPersesDashboardsPage.shouldBeLoaded1() to wait for the page to be ready; do
the same for the second occurrence (the other cy.wait after a clickDashboard
call). This keeps the test deterministic and leverages existing assertions
instead of fixed delays.
In `@web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts`:
- Around line 23-147: Tests 10–12 depend on panels created in Test 10 which
makes them brittle; make each test self-contained by creating the panels/groups
it needs at the start and cleaning them up in an afterEach (or give each test
unique names and delete those in afterEach). Specifically, in the tests
referencing persesDashboardsPanel.addPanel,
persesDashboardsPanelGroup.addPanelGroup, persesDashboardsPanel.deletePanel,
persesDashboardsPanel.clickDeletePanelButton and
persesDashboardsPanelGroup.clickDeletePanelGroupButton, ensure you call
addPanelGroup/addPanel with a unique test-scoped name at the beginning of the
test (or in a beforeEach), perform the test actions, and then remove those same
named resources in an afterEach using deletePanel/clickDeletePanelButton and
clickDeletePanelGroupButton (or alternatively seed the exact required state at
test start), so each test can run in isolation and no mid-test failures leave
state behind.
---
Duplicate comments:
In `@web/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.ts`:
- Around line 116-191: This spec contains interactive cy.pause() calls that will
stall CI; remove the cy.pause() invocations found in the regression test (the
ones between the checks for "Consecutive interval boundaries" and before "2.3
Incident tooltip..." and the final one after the alerts table checks). Search
for and delete the three cy.pause() calls in the test (they appear immediately
before and after the block that uses incidentsPage.hoverOverIncidentBarSegment,
incidentsPage.selectIncidentById, and incidentsPage.getSelectedIncidentAlerts)
and run the grep check used in the review to ensure no cy.pause() remain.
In `@web/cypress/support/commands/perses-commands.ts`:
- Around line 44-57: The conditional is incorrectly stringifying env vars so
`${Cypress.env('LOGIN_USERNAME2')}` becomes the literal "undefined" and the
block runs with bad usernames; change the checks to use the raw env values
(e.g., Cypress.env('LOGIN_USERNAME1') !== 'kubeadmin' &&
Cypress.env('LOGIN_USERNAME2') !== undefined / truthy) and when calling cy.exec
pass the raw values (USER1: Cypress.env('LOGIN_USERNAME1'), USER2:
Cypress.env('LOGIN_USERNAME2'), etc.) instead of template literals so the guard
skips correctly and real undefined/missing values aren’t turned into the string
"undefined" before invoking rbac_perses_e2e_ci_users.sh (adjust checks for any
other LOGIN_USERNAME3..6 uses similarly).
In `@web/cypress/support/commands/utility-commands.ts`:
- Around line 169-170: The test double-clicks the pod link: the element is
aliased as 'podLink' and clicked immediately, then the alias is retrieved and
clicked again; remove the redundant initial .click() so you only alias the
element (cy.get(...).eq(0).as('podLink')), then assert visibility on '@podLink'
and perform a single .click() on that alias (use the existing 'podLink'
alias/selector to locate the element before clicking).
In `@web/cypress/support/incidents_prometheus_query_mocks/mock-generators.ts`:
- Line 252: The code calls alert.silenced.toString() which can throw if
alert.silenced is null/undefined; update the silenced assignment to guard
against missing values (e.g., use a null-coalescing or conditional check like
(alert.silenced ?? false).toString() or String(alert.silenced) to match the
defensive pattern used elsewhere in the file) so that the mock generator always
produces a safe string for the silenced field.
In `@web/cypress/support/perses/01.coo_list_perses_admin.cy.ts`:
- Line 287: Remove the fixed cy.wait(2000) and replace it with a state-based
wait by using the existing countDashboards('1') assertion to deterministically
wait for the expected UI state; locate the cy.wait(2000) in the test in
web/cypress/support/perses/01.coo_list_perses_admin.cy.ts and remove it, then
call or await countDashboards('1') (the helper used on the following line) as
the synchronization point so the test proceeds only after the dashboard count
reaches 1.
In `@web/cypress/support/perses/04.coo_import_perses_admin.cy.ts`:
- Around line 54-56: The generic persesDashboardsPage.closeAlert() is being used
in import flows and can close the wrong alert; replace it with an alert-scoped
closer (e.g., use the success/error-specific helper such as
persesDashboardsPage.closeErrorAlert() or
persesDashboardsPage.closeSuccessAlert(), or call a scoped close like
persesDashboardsPage.closeAlert(selector)) wherever used with
persesImportDashboardsPage.assertFailedToMigrateGrafanaDashboard() (and the
other occurrences noted around the spec) so the close targets the specific alert
element and the test becomes deterministic.
---
Nitpick comments:
In `@web/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.ts`:
- Around line 73-125: Replace the alias-based counter (`@initialFiringCount`) with
a simple closure variable: declare let initialFiringCount = 0 before calling
incidentsPage.getSelectedIncidentAlerts(), increment initialFiringCount inside
the isFiring branch where you currently call cy.get/cy.wrap, remove the
intermediate cy.wrap/cy.get calls, and at the final .then() call wrap the single
final value with cy.wrap(initialFiringCount) (or use cy.then(() => {
expect(initialFiringCount)... })) to perform the assertion; update references in
the block that use currentAlertName, alert.getAlertRuleCell(), and
alert.getEndCell() accordingly so only the final wrap/assert is done once.
In `@web/cypress/fixtures/perses/constants.ts`:
- Around line 159-160: The CPU_LINE_MULTI_SERIES constant contains PromQL label
matchers using single quotes (job='$job' and instance=~'$instance'); update
those to use double quotes (job="$job" and instance=~"$instance") to follow
Prometheus conventions and match other matchers in the same query—locate the
CPU_LINE_MULTI_SERIES constant in web/cypress/fixtures/perses/constants.ts and
replace the single-quoted label values with double-quoted ones.
In `@web/cypress/support/perses/02.coo_edit_perses_admin_1.cy.ts`:
- Around line 39-62: The test currently opens edit mode and calls
persesDashboardsPage.clickEditActionButton('Save') inside the
panelTypeKeys.forEach loop (using persesDashboardsPage.clickEditButton,
clickEditActionButton('AddGroup'/'AddPanel', addPanelGroup, addPanel, etc.),
causing a save per panel type; change the flow to open edit mode once before
iterating panelTypeKeys, perform all addPanelGroup and addPanel calls inside the
loop without calling clickEditActionButton('Save') inside it, then call
persesDashboardsPage.clickEditActionButton('Save') once after the loop to
persist all changes in one edit session; apply the same pattern for the deletion
block referenced (remove repeated clickEditButton and Save calls inside
per-panel loops and save once per scenario).
In `@web/cypress/views/legacy-dashboards.ts`:
- Around line 181-216: The exportAsCSV helper (exportAsCSV) is inconsistent with
list-page.ts: it currently accepts fileNameExp: string and uses String.includes;
change the parameter type to RegExp (fileNameExp: RegExp) and update the
matching logic to use fileNameExp.test(file) (replace .includes usage) so the
API and behavior match the list-page.ts helper; ensure any callers pass a RegExp
or update their calls accordingly and keep the rest of the waitUntil logic and
assertions the same.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ebf6dfd1-47e1-4f22-b068-e060b5a63024
📒 Files selected for processing (128)
web/cypress/dummy.tsxweb/cypress/e2e/coo/01.coo_bvt.cy.tsweb/cypress/e2e/coo/01.coo_ivt.cy.tsweb/cypress/e2e/coo/02.acm_alerting_ui.cy.tsweb/cypress/e2e/incidents/00.coo_incidents_e2e.cy.tsweb/cypress/e2e/incidents/01.incidents.cy.tsweb/cypress/e2e/incidents/02.incidents-mocking-example.cy.tsweb/cypress/e2e/incidents/regression/01.reg_filtering.cy.tsweb/cypress/e2e/incidents/regression/02.reg_ui_charts_comprehensive.cy.tsweb/cypress/e2e/incidents/regression/02.reg_ui_tooltip_boundary_times.cy.tsweb/cypress/e2e/incidents/regression/03-04.reg_e2e_firing_alerts.cy.tsweb/cypress/e2e/incidents/regression/03.reg_api_calls.cy.tsweb/cypress/e2e/incidents/regression/04.reg_redux_effects.cy.tsweb/cypress/e2e/incidents/regression/05.reg_stress_testing_ui.cy.tsweb/cypress/e2e/monitoring/00.bvt_admin.cy.tsweb/cypress/e2e/monitoring/00.bvt_dev.cy.tsweb/cypress/e2e/monitoring/regression/01.reg_alerts_admin.cy.tsweb/cypress/e2e/monitoring/regression/01.reg_alerts_dev.cy.tsweb/cypress/e2e/monitoring/regression/02.reg_metrics_admin_1.cy.tsweb/cypress/e2e/monitoring/regression/02.reg_metrics_admin_2.cy.tsweb/cypress/e2e/monitoring/regression/03.reg_legacy_dashboards_admin.cy.tsweb/cypress/e2e/perses/00.coo_bvt_perses_admin.cy.tsweb/cypress/e2e/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/e2e/perses/01.coo_list_perses_admin.cy.tsweb/cypress/e2e/perses/02.coo_edit_perses_admin.cy.tsweb/cypress/e2e/perses/03.coo_create_perses_admin.cy.tsweb/cypress/e2e/perses/04.coo_import_perses_admin.cy.tsweb/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/e2e/virtualization/00.coo_ivt.cy.tsweb/cypress/e2e/virtualization/01.coo_ivt_alerts.cy.tsweb/cypress/e2e/virtualization/02.coo_ivt_metrics_1.cy.tsweb/cypress/e2e/virtualization/02.coo_ivt_metrics_2.cy.tsweb/cypress/e2e/virtualization/03.coo_ivt_legacy_dashboards.cy.tsweb/cypress/e2e/virtualization/04.coo_ivt_perses.cy.tsweb/cypress/fixtures/coo/coo140_perses/dashboards/openshift-cluster-sample-dashboard.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/perses-dashboard-sample.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/prometheus-overview-variables.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/thanos-compact-overview-1var.yamlweb/cypress/fixtures/coo/coo140_perses/dashboards/thanos-querier-datasource.yamlweb/cypress/fixtures/coo/coo140_perses/import/accelerators-dashboard-cr-v1alpha1.yamlweb/cypress/fixtures/coo/coo140_perses/import/accelerators-dashboard-cr-v1alpha2.yamlweb/cypress/fixtures/coo/coo140_perses/import/acm-vm-status.jsonweb/cypress/fixtures/coo/coo140_perses/import/grafana_to_check_errors.jsonweb/cypress/fixtures/coo/coo140_perses/import/testing-perses-dashboard.jsonweb/cypress/fixtures/coo/coo140_perses/import/testing-perses-dashboard.yamlweb/cypress/fixtures/coo/coo140_perses/rbac/rbac_perses_e2e_ci_users.shweb/cypress/fixtures/monitoring/alert.tsweb/cypress/fixtures/monitoring/constants.tsweb/cypress/fixtures/perses/constants.tsweb/cypress/support/commands/auth-commands.tsweb/cypress/support/commands/coo-install-commands.tsweb/cypress/support/commands/dashboards-commands.tsweb/cypress/support/commands/image-patch-commands.tsweb/cypress/support/commands/incident-commands.tsweb/cypress/support/commands/operator-commands.tsweb/cypress/support/commands/perses-commands.tsweb/cypress/support/commands/selector-commands.tsweb/cypress/support/commands/utility-commands.tsweb/cypress/support/commands/virtualization-commands.tsweb/cypress/support/commands/wait-utils.tsweb/cypress/support/e2e.jsweb/cypress/support/incidents_prometheus_query_mocks/mock-generators.tsweb/cypress/support/incidents_prometheus_query_mocks/prometheus-mocks.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/fixture-converter.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/schema-validator.tsweb/cypress/support/incidents_prometheus_query_mocks/schema/validate-fixtures.tsweb/cypress/support/incidents_prometheus_query_mocks/types.tsweb/cypress/support/incidents_prometheus_query_mocks/utils.tsweb/cypress/support/index.tsweb/cypress/support/monitoring/00.bvt_monitoring.cy.tsweb/cypress/support/monitoring/00.bvt_monitoring_namespace.cy.tsweb/cypress/support/monitoring/01.reg_alerts.cy.tsweb/cypress/support/monitoring/02.reg_metrics_1.cy.tsweb/cypress/support/monitoring/02.reg_metrics_2.cy.tsweb/cypress/support/monitoring/03.reg_legacy_dashboards.cy.tsweb/cypress/support/monitoring/04.reg_alerts_namespace.cy.tsweb/cypress/support/monitoring/05.reg_metrics_namespace_1.cy.tsweb/cypress/support/monitoring/05.reg_metrics_namespace_2.cy.tsweb/cypress/support/monitoring/06.reg_legacy_dashboards_namespace.cy.tsweb/cypress/support/perses/00.coo_bvt_perses_admin.cy.tsweb/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.tsweb/cypress/support/perses/01.coo_list_perses_admin.cy.tsweb/cypress/support/perses/01.coo_list_perses_admin_namespace.cy.tsweb/cypress/support/perses/02.coo_edit_perses_admin.cy.tsweb/cypress/support/perses/02.coo_edit_perses_admin_1.cy.tsweb/cypress/support/perses/03.coo_create_perses_admin.cy.tsweb/cypress/support/perses/04.coo_import_perses_admin.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user1.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user2.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user3.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user4.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user5.cy.tsweb/cypress/support/perses/99.coo_rbac_perses_user6.cy.tsweb/cypress/support/selectors.tsweb/cypress/support/test-tags.d.tsweb/cypress/views/acm-alerting-page.tsweb/cypress/views/alerting-rule-details-page.tsweb/cypress/views/alerting-rule-list-page.tsweb/cypress/views/common.tsweb/cypress/views/details-page.tsweb/cypress/views/incidents-page.tsweb/cypress/views/legacy-dashboards.tsweb/cypress/views/list-page.tsweb/cypress/views/metrics.tsweb/cypress/views/nav.tsweb/cypress/views/operator-hub-page.tsweb/cypress/views/overview-page.tsweb/cypress/views/perses-dashboards-create-dashboard.tsweb/cypress/views/perses-dashboards-edit-datasources.tsweb/cypress/views/perses-dashboards-edit-variables.tsweb/cypress/views/perses-dashboards-import-dashboard.tsweb/cypress/views/perses-dashboards-list-dashboards.tsweb/cypress/views/perses-dashboards-panel.tsweb/cypress/views/perses-dashboards-panelgroup.tsweb/cypress/views/perses-dashboards.tsweb/cypress/views/silence-alert-page.tsweb/cypress/views/silence-details-page.tsweb/cypress/views/silences-list-page.tsweb/cypress/views/tour.tsweb/cypress/views/troubleshooting-panel.tsweb/cypress/views/utils.tsweb/package.jsonweb/src/components/data-test.ts
💤 Files with no reviewable changes (3)
- web/cypress/support/perses/00.coo_bvt_perses_admin_1.cy.ts
- web/cypress/e2e/coo/01.coo_bvt.cy.ts
- web/cypress/e2e/perses/00.coo_bvt_perses_admin_1.cy.ts
✅ Files skipped from review due to trivial changes (47)
- web/cypress/support/e2e.js
- web/cypress/support/perses/99.coo_rbac_perses_user6.cy.ts
- web/cypress/e2e/incidents/02.incidents-mocking-example.cy.ts
- web/cypress/e2e/coo/01.coo_ivt.cy.ts
- web/cypress/e2e/virtualization/02.coo_ivt_metrics_1.cy.ts
- web/cypress/e2e/incidents/regression/01.reg_filtering.cy.ts
- web/cypress/e2e/monitoring/regression/01.reg_alerts_dev.cy.ts
- web/cypress/fixtures/coo/coo140_perses/import/acm-vm-status.json
- web/cypress/support/commands/wait-utils.ts
- web/cypress/e2e/monitoring/00.bvt_dev.cy.ts
- web/cypress/e2e/virtualization/01.coo_ivt_alerts.cy.ts
- web/cypress/e2e/incidents/00.coo_incidents_e2e.cy.ts
- web/cypress/support/commands/image-patch-commands.ts
- web/cypress/e2e/perses/04.coo_import_perses_admin.cy.ts
- web/cypress/e2e/virtualization/02.coo_ivt_metrics_2.cy.ts
- web/cypress/e2e/monitoring/regression/01.reg_alerts_admin.cy.ts
- web/cypress/support/commands/coo-install-commands.ts
- web/cypress/support/incidents_prometheus_query_mocks/types.ts
- web/cypress/support/incidents_prometheus_query_mocks/schema/fixture-converter.ts
- web/cypress/e2e/incidents/01.incidents.cy.ts
- web/cypress/e2e/monitoring/00.bvt_admin.cy.ts
- web/cypress/support/test-tags.d.ts
- web/cypress/e2e/coo/02.acm_alerting_ui.cy.ts
- web/cypress/support/incidents_prometheus_query_mocks/schema/schema-validator.ts
- web/cypress/e2e/perses/02.coo_edit_perses_admin.cy.ts
- web/cypress/e2e/monitoring/regression/02.reg_metrics_admin_1.cy.ts
- web/cypress/e2e/monitoring/regression/03.reg_legacy_dashboards_admin.cy.ts
- web/cypress/support/commands/operator-commands.ts
- web/cypress/support/monitoring/02.reg_metrics_2.cy.ts
- web/cypress/support/monitoring/02.reg_metrics_1.cy.ts
- web/cypress/support/monitoring/00.bvt_monitoring.cy.ts
- web/cypress/e2e/incidents/regression/04.reg_redux_effects.cy.ts
- web/cypress/views/alerting-rule-list-page.ts
- web/cypress/e2e/virtualization/03.coo_ivt_legacy_dashboards.cy.ts
- web/cypress/e2e/perses/01.coo_list_perses_admin.cy.ts
- web/cypress/dummy.tsx
- web/cypress/views/incidents-page.ts
- web/cypress/fixtures/monitoring/constants.ts
- web/cypress/views/acm-alerting-page.ts
- web/cypress/support/monitoring/03.reg_legacy_dashboards.cy.ts
- web/cypress/support/commands/auth-commands.ts
- web/cypress/e2e/incidents/regression/05.reg_stress_testing_ui.cy.ts
- web/cypress/support/monitoring/05.reg_metrics_namespace_1.cy.ts
- web/cypress/e2e/monitoring/regression/02.reg_metrics_admin_2.cy.ts
- web/cypress/e2e/incidents/regression/02.reg_ui_charts_comprehensive.cy.ts
- web/cypress/e2e/incidents/regression/03.reg_api_calls.cy.ts
- web/cypress/e2e/virtualization/00.coo_ivt.cy.ts
🚧 Files skipped from review as they are similar to previous changes (20)
- web/cypress/support/monitoring/01.reg_alerts.cy.ts
- web/cypress/support/index.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user4.cy.ts
- web/cypress/views/common.ts
- web/cypress/support/incidents_prometheus_query_mocks/schema/validate-fixtures.ts
- web/cypress/e2e/perses/03.coo_create_perses_admin.cy.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user6.cy.ts
- web/cypress/views/details-page.ts
- web/cypress/support/monitoring/06.reg_legacy_dashboards_namespace.cy.ts
- web/cypress/support/monitoring/05.reg_metrics_namespace_2.cy.ts
- web/cypress/views/alerting-rule-details-page.ts
- web/cypress/e2e/virtualization/04.coo_ivt_perses.cy.ts
- web/cypress/support/perses/03.coo_create_perses_admin.cy.ts
- web/cypress/e2e/perses/99.coo_rbac_perses_user2.cy.ts
- web/cypress/support/selectors.ts
- web/cypress/e2e/perses/00.coo_bvt_perses_admin.cy.ts
- web/cypress/support/commands/selector-commands.ts
- web/cypress/support/perses/02.coo_edit_perses_admin.cy.ts
- web/cypress/support/commands/dashboards-commands.ts
- web/cypress/support/perses/99.coo_rbac_perses_user1.cy.ts
Contributor
Author
|
/test ? |
etmurasaki
force-pushed
the
etmura-post-coo140
branch
from
b4d68e8 to
6317a20
Compare
Contributor
Author
|
/test e2e-monitoring |
Contributor
Author
|
/test e2e-incidents |
|
@etmurasaki: This pull request references OU-1184 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 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. |
Contributor
Author
|
/test e2e-incidents |
Contributor
Author
|
/test ? |
Contributor
Author
|
/test e2e-coo |
Contributor
Author
|
/test e2e-monitoring-dev |
Contributor
|
@etmurasaki: The following tests 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. |
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
2 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.
Including #833