OBSINTA-1290: fix incident detection test selectors

[DavidRajnoha]

Summary

• Make the Incidents tab selector case-insensitive ([data-test-id="horizontal-link-incidents" i]) — the Console SDK has shipped both "incidents" and "Incidents" across versions
• Replace cy.byTestID lookups for detail table cells (alert rule, namespace, severity, state, start, end) with data-label attribute selectors — the data-test IDs weren't being rendered on
  these cells, while data-label is a built-in PatternFly structural attribute
• Fix getIncidentIds to extract IDs from data-test attributes instead of button text content, which was picking up the description suffix (e.g. "1 component") and corrupting the ID used
  for filter selection
• Use container existence check instead of visibility assertion for the filter dropdown (renders with 0 height)
• Add scrollIntoView() and .first() across tests to fix viewport clipping and multi-element ambiguity failures

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability of the Incidents page tab selection so it works consistently across different text casing.
• Tests
  • Strengthened Incidents filtering and incident details verification by locating the correct expanded-row content more precisely and scrolling relevant UI elements into view before assertions.
  • Adjusted Incidents regression setup order and refined chip-visibility checks to be more consistent when filters are applied or removed.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: DavidRajnoha
Once this PR has been reviewed and has the lgtm label, please assign jan--f for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Walkthrough

Cypress incidents coverage was updated with case-insensitive tab detection, row-based incident detail selectors, data-attribute-based incident ID extraction, reordered walkthrough setup, and more targeted scrolling and chip visibility assertions.

Changes

Incidents Cypress coverage

Layer / File(s)	Summary
Incident page selectors and ID extraction web/cypress/views/incidents-page.ts	The Incidents tab selector is case-insensitive; detail cells use row and data-label selectors; incident IDs are parsed from option data-test attributes.
Walkthrough setup ordering web/cypress/e2e/incidents/01.incidents.cy.ts	The days filter is set before fixture loading, and filters are cleared afterward.
Filtering regression assertions web/cypress/e2e/incidents/regression/*.cy.ts	Table and chip assertions scroll targets into view and select the first Incident ID chip where needed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: jgbernalp, etmurasaki

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	PASS: The PR only changes selectors/setup; the affected test titles are static strings and no dynamic/generated values were added.
Test Structure And Quality	✅ Passed	PASS: The PR only changes Cypress tests/utilities, not Ginkgo tests, so the Ginkgo-specific quality checklist is not applicable.
Microshift Test Compatibility	✅ Passed	No new Go/Ginkgo tests were added; the PR only changes Cypress JS tests and selectors, so MicroShift Ginkgo compatibility is not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Touched files are Cypress UI tests/helpers only; I found no node/topology/HA assumptions or SNO-specific gaps in the changed blocks.
Topology-Aware Scheduling Compatibility	✅ Passed	Only Cypress test/view code changed; no manifests, controllers, or scheduling logic were modified, so topology-aware scheduling is not implicated.
Ote Binary Stdout Contract	✅ Passed	PASS: The PR only touches Cypress TS files; no Go/main/TestMain/BeforeSuite hooks or stdout writes (fmt.Print/klog/console.log) appear in the changed files.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR only changes Cypress incident tests/helpers and adds no Ginkgo e2e tests or IPv4/external-network assumptions.
No-Weak-Crypto	✅ Passed	Touched files only change Cypress selectors/assertions; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret/token comparisons found.
Container-Privileges	✅ Passed	PR only changes Cypress TS files; no container/K8s manifests or privilege-related settings were added.
No-Sensitive-Data-In-Logs	✅ Passed	No new logging of secrets/PII/session IDs/internal hosts was added; the patch only changes selectors and generic Cypress test logs.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the changeset by describing the selector-focused incident test fix.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

web/cypress/views/incidents-page.ts (2)

282-287: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Fixed cy.wait(250) calls around setDays interaction.

Two arbitrary sleeps bracket the toggle click. The .should('be.visible') assertion already retries until the dropdown renders, so the trailing cy.wait(250) is redundant, and the leading one doesn't guarantee the toggle is ready — Cypress's built-in actionability retries already handle that. Fixed waits add flakiness risk without a real synchronization guarantee, and slow down every test calling setDays.

♻️ Proposed fix

   setDays: (value: '1 day' | '3 days' | '7 days' | '15 days') => {
     cy.log('incidentsPage.setDays');
-    cy.wait(250);
     incidentsPage.elements.daysSelectToggle().scrollIntoView().click({ force: true });
     incidentsPage.elements.daysSelectList().should('be.visible');
-    cy.wait(250);
     const dayKey = value.replace(' ', '-');

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/cypress/views/incidents-page.ts` around lines 282 - 287, Remove both
arbitrary cy.wait(250) calls from the setDays interaction. Keep the existing
scrollIntoView().click({ force: true }) action and
daysSelectList().should('be.visible') assertion so Cypress handles actionability
and dropdown readiness through its built-in retries.

---

186-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Inconsistent side-effect: scrollIntoView() only on the alert-rule cell getter.

incidentsDetailsAlertRuleCell scrolls into view as part of the selector, but the sibling incidentsDetailsAlertRuleLink and the other cell getters (Namespace/Severity/State/Start/End) don't. Baking an action (scroll) into a query getter is inconsistent with the rest of the elements object and could produce different assertion behavior depending on which sibling selector a caller picks.

Consider moving the scroll to the call site (e.g., in the test, before .should('be.visible')) or applying it uniformly across all detail cell getters.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/cypress/views/incidents-page.ts` around lines 186 - 229, Remove the
embedded scrollIntoView side effect from incidentsDetailsAlertRuleCell so all
incidents detail getters, including incidentsDetailsAlertRuleLink and the
Namespace/Severity/State/Start/End cell getters, remain selector-only. Move
scrolling to the relevant test call sites before visibility assertions,
preserving the existing visibility behavior without making getter selection
alter Cypress actions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/cypress/e2e/incidents/01.incidents.cy.ts`:
- Around line 96-102: Add an explicit incidentsPage.goTo() call at the start of
the “6. Admin perspective - Incidents page - Bar click selection walkthrough”
test before loading fixtures or applying filters, so it always begins on the
Incidents page independently of prior test state.

---

Nitpick comments:
In `@web/cypress/views/incidents-page.ts`:
- Around line 282-287: Remove both arbitrary cy.wait(250) calls from the setDays
interaction. Keep the existing scrollIntoView().click({ force: true }) action
and daysSelectList().should('be.visible') assertion so Cypress handles
actionability and dropdown readiness through its built-in retries.
- Around line 186-229: Remove the embedded scrollIntoView side effect from
incidentsDetailsAlertRuleCell so all incidents detail getters, including
incidentsDetailsAlertRuleLink and the Namespace/Severity/State/Start/End cell
getters, remain selector-only. Move scrolling to the relevant test call sites
before visibility assertions, preserving the existing visibility behavior
without making getter selection alter Cypress actions.

🪄 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), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: f83b6335-29a9-4837-b77e-b0c1c6c48e53

📥 Commits

Reviewing files that changed from the base of the PR and between fd2f1fd and 0a520a2.

📒 Files selected for processing (4)

• web/cypress/e2e/incidents/01.incidents.cy.ts
• web/cypress/e2e/incidents/regression/01.reg_filtering.cy.ts
• web/cypress/e2e/incidents/regression/04.reg_redux_effects.cy.ts
• web/cypress/views/incidents-page.ts

[openshift-ci-robot]

@DavidRajnoha: This pull request references OBSINTA-1290 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 "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

• The OpenShift console changed the data-test-id for the Incidents tab from horizontal-link-incidents to horizontal-link-Incidents
• This caused waitForIncidentsTab to time out in the before all hook, failing all Incidents tests
• Uses the CSS attribute selector i flag for case-insensitive matching, supporting both casings

Test plan

• Run incidents E2E suite against a cluster with the updated console
• Verify the Incidents tab is found during warmUpForPlugin

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
• Improved reliability of the Incidents page tab selection so it works consistently across different text casing.
• Tests
• Strengthened Incidents filtering and incident details verification by locating the correct expanded-row content more precisely and scrolling relevant UI elements into view before assertions.
• Adjusted Incidents regression setup order and refined chip-visibility checks to be more consistent when filters are applied or removed.

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]

@DavidRajnoha: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[DavidRajnoha]

/cc @etmurasaki @tremes @sthundat