Skip to content

PF5: Tests: Fix goToPodDetails helper flakiness#2082

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:pattern-fly-5from
kyoto:pf5-fix-goToPodDetails
Jul 23, 2026
Merged

PF5: Tests: Fix goToPodDetails helper flakiness#2082
openshift-merge-bot[bot] merged 1 commit into
openshift:pattern-fly-5from
kyoto:pf5-fix-goToPodDetails

Conversation

@kyoto

@kyoto kyoto commented Jun 16, 2026

Copy link
Copy Markdown
Member

Manual backport of #2076

Summary by CodeRabbit

  • Tests
    • Improved the pod navigation test helper by selecting the pod row link that matches the provided pod name, with an explicit wait for the link to become visible (up to 30s) before clicking—making the test more reliable.

@kyoto kyoto added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 16, 2026
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8b964ad1-15ae-4969-8f32-561630678120

📥 Commits

Reviewing files that changed from the base of the PR and between e0b4d82 and e73a95a.

📒 Files selected for processing (1)
  • tests/tests/lightspeed.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/tests/lightspeed.spec.ts

📝 Walkthrough

Walkthrough

The goToPodDetails helper now filters pod rows by podName, waits up to 30 seconds for the matching link to become visible, and clicks it instead of using a generic first-link selection.

Changes

Pod navigation targeting

Layer / File(s) Summary
Targeted pod link selection
tests/tests/lightspeed.spec.ts
Filters resourceRows by pod name, waits for the matching link, and clicks it. Removes the previous row-count assertion and generic first-link click.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

Suggested reviewers: joaofula, syedriko

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: fixing flakiness in the goToPodDetails test helper.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci
openshift-ci Bot requested review from JoaoFula and syedriko June 16, 2026 05:59
@openshift-ci

openshift-ci Bot commented Jun 16, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

1 similar comment
@openshift-ci

openshift-ci Bot commented Jun 16, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/tests/lightspeed.spec.ts (1)

37-39: ⚡ Quick win

Consider storing .first() result and improving variable naming.

The variable link is somewhat misleading as it contains a row locator rather than a link element. Additionally, calling .first() twice (lines 38 and 39) re-evaluates the locator each time. While Playwright's auto-waiting should handle this safely, storing the result would improve clarity and ensure the exact same element is validated and clicked.

♻️ Suggested refactor
- const link = page.locator(resourceRows).filter({ hasText: podName });
- await expect(link.first()).toBeVisible({ timeout: 30_000 });
- await link.first().click();
+ const matchingRows = page.locator(resourceRows).filter({ hasText: podName });
+ const firstRow = matchingRows.first();
+ await expect(firstRow).toBeVisible({ timeout: 30_000 });
+ await firstRow.click();
🤖 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 `@tests/tests/lightspeed.spec.ts` around lines 37 - 39, The variable name
`link` is misleading because it holds a row locator, not a link element, and
calling `.first()` twice (in the expect statement and click statement)
re-evaluates the locator unnecessarily. Rename the `link` variable to something
more descriptive like `resourceRow` to better reflect that it contains a row
element, then store the result of `.first()` in a separate variable (e.g.,
`firstRow`) so that the exact same element is validated with expect and clicked
without re-evaluation.
🤖 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.

Nitpick comments:
In `@tests/tests/lightspeed.spec.ts`:
- Around line 37-39: The variable name `link` is misleading because it holds a
row locator, not a link element, and calling `.first()` twice (in the expect
statement and click statement) re-evaluates the locator unnecessarily. Rename
the `link` variable to something more descriptive like `resourceRow` to better
reflect that it contains a row element, then store the result of `.first()` in a
separate variable (e.g., `firstRow`) so that the exact same element is validated
with expect and clicked without re-evaluation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 482c5eb8-5956-4dd4-9818-9456b7b0348b

📥 Commits

Reviewing files that changed from the base of the PR and between 65d00db and e0b4d82.

📒 Files selected for processing (1)
  • tests/tests/lightspeed.spec.ts

@kyoto
kyoto force-pushed the pf5-fix-goToPodDetails branch from e0b4d82 to 7edd15e Compare June 17, 2026 07:28
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2026
@openshift-ci

openshift-ci Bot commented Jun 17, 2026

Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@kyoto

kyoto commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

/retest

@kyoto
kyoto force-pushed the pf5-fix-goToPodDetails branch from 7edd15e to e73a95a Compare July 23, 2026 06:00
@kyoto kyoto added the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit 4652274 into openshift:pattern-fly-5 Jul 23, 2026
5 checks passed
@kyoto
kyoto deleted the pf5-fix-goToPodDetails branch July 23, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant