test: migrate support case tests from IQE (RHCLOUD-44391)#304
Open
catastrophe-brandon wants to merge 11 commits intoRedHatInsights:masterfrom
Open
test: migrate support case tests from IQE (RHCLOUD-44391)#304catastrophe-brandon wants to merge 11 commits intoRedHatInsights:masterfrom
catastrophe-brandon wants to merge 11 commits intoRedHatInsights:masterfrom
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughAdds a new Playwright E2E test suite that opens the Help Panel, selects "My support cases", waits up to 15s for either an empty state or a cases table, conditionally asserts the "Open a support case" CTA or table/pagination, and verifies external navigation to access.redhat.com. ChangesSupport Case E2E Tests
Sequence Diagram(s)sequenceDiagram
rect rgba(220,240,255,0.5)
participant Tester as Test Runner
end
rect rgba(205,255,220,0.5)
participant Browser
end
rect rgba(255,235,205,0.5)
participant App as Web App
end
rect rgba(255,220,220,0.5)
participant External as access.redhat.com
end
Tester->>Browser: launch, disable cookie prompts, navigate to app
Browser->>App: load UI
App-->>Browser: render "Hi," header
Tester->>Browser: open Help Panel -> select "My support cases"
alt Empty state visible
App-->>Browser: show empty state
Tester->>Browser: verify "Open a support case" visible
else Cases table visible
App-->>Browser: render cases table + pagination
Tester->>Browser: assert table rows >= 1
end
Tester->>Browser: click "Open a support case"
Browser->>Browser: open new tab -> navigate to external
External-->>Browser: respond (access.redhat.com)
Tester->>Browser: close new tab
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
catastrophe-brandon
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 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 `@playwright/support-case.spec.ts`:
- Around line 90-95: The assertion reads the popup URL too early—after awaiting
pagePromise (newPage) the page may still be at about:blank; call
newPage.waitForURL(...) (or newPage.waitForURL('**/*') to wait for any
navigation) before creating the URL object and asserting hostname so the
navigation has completed; update the sequence around newPage, add the waitForURL
call, then compute new URL via newPage.url() and keep the
expect(url.hostname).toBe('access.redhat.com') check.
- Around line 117-131: The current try/catch around await
expect(supportTable.or(emptyState)).toBeVisible({ timeout:
SUPPORT_API_LOAD_TIMEOUT }) is converting readiness timeouts into test.skip()
and hiding real failures; remove the catch that calls test.skip() (or rethrow
the caught error) so readiness timeouts fail the test, and after the successful
wait check specifically if emptyState.isVisible() (or const emptyVisible = await
emptyState.isVisible().catch(() => false)) and only call test.skip() when
emptyState is actually visible (indicating zero cases); use supportTable and
emptyState checks rather than skipping on any wait failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 2e2961fe-1543-47ab-870e-51f253c8bf8a
📒 Files selected for processing (1)
playwright/support-case.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
playwright/support-case.spec.ts (1)
92-97:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a specific popup URL wait instead of wildcard matcher.
Line 93 uses
waitForURL('**/*'), which is too broad for proving the popup reached the expected destination before Line 97 hostname assertion. This can still be flaky.Suggested fix
- await newPage.waitForURL('**/*'); + await newPage.waitForURL(/access\.redhat\.com/, { + timeout: SUPPORT_API_LOAD_TIMEOUT, + });In Playwright, does page.waitForURL('**/*') reliably ensure navigation away from about:blank, and is waiting for a specific host pattern (e.g., /access\.redhat\.com/) the recommended approach for popup navigation assertions?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playwright/support-case.spec.ts` around lines 92 - 97, The test uses a wildcard waitForURL('**/*') which is too broad and can be flaky; replace that call (newPage.waitForURL) with a specific host pattern or regex that targets access.redhat.com (e.g., a glob or RegExp matching access\.redhat\.com) so the popup navigation is awaited precisely before creating the URL object and asserting expect(url.hostname).toBe('access.redhat.com'); update the wait to use the specific matcher rather than '**/*'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@playwright/support-case.spec.ts`:
- Around line 56-61: The test currently expects the "Open a support case"
control unconditionally after awaiting emptyState.or(supportTable); instead,
detect which branch rendered and only assert empty-state-only elements when
emptyState is visible: use the emptyState locator (emptyState.isVisible() or an
awaited check) to conditionally expect the "Open a support case" control,
otherwise assert supportTable-specific expectations; update both occurrences
(around the emptyState/supportTable check and the similar block at lines 79–88)
so branch-specific assertions are guarded by emptyState visibility rather than
run unconditionally.
---
Duplicate comments:
In `@playwright/support-case.spec.ts`:
- Around line 92-97: The test uses a wildcard waitForURL('**/*') which is too
broad and can be flaky; replace that call (newPage.waitForURL) with a specific
host pattern or regex that targets access.redhat.com (e.g., a glob or RegExp
matching access\.redhat\.com) so the popup navigation is awaited precisely
before creating the URL object and asserting
expect(url.hostname).toBe('access.redhat.com'); update the wait to use the
specific matcher rather than '**/*'.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 208e021f-f51f-4dc3-a9b9-cb12e172d9f3
📒 Files selected for processing (1)
playwright/support-case.spec.ts
Migrates support case help panel tests from iqe-platform-ui-plugin test_support_case.py to Playwright. Tests verify: - "Open a support case" button appears in help panel support tab - Clicking button opens Red Hat Customer Portal in new tab - Support cases table displays when user has open cases The tests handle both empty state (no cases) and populated state (with cases) gracefully using conditional skipping based on what's rendered. Note: The complex test_support_case_from_apps (testing pre-filled support case data from different apps) was not migrated as it requires cross-domain testing and is better suited for the insights-chrome repository. Requirements: - PLATFORM_UI-INSIGHTS_CHROME - PLATFORM_UI-SUPPORT_CASES Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Define timeout constants at module level for better maintainability - Replace page.waitForTimeout() with Playwright's expect().toPass() pattern - Use auto-retry for waiting on API-loaded content - Explicitly specify timeouts on assertions for clarity Timeout constants: - SUPPORT_API_LOAD_TIMEOUT (15s) - Support cases API load time - ELEMENT_VISIBLE_TIMEOUT (10s) - Element visibility wait - EXTERNAL_PAGE_LOAD_TIMEOUT (30s) - External page navigation Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Simplifies all three tests to follow the actual user interaction flow:
1. Click help button to open help panel
2. Wait for help panel to load
3. Click on support tab
4. Verify expected content is visible
Key improvements:
- Removed complex conditional logic and state checking
- Tests now follow clear step-by-step user flow with numbered comments
- Uses getByRole('link') to find 'Open a support case' link (works in both states)
- Removed unnecessary empty state vs table differentiation in first two tests
- Third test uses .or() locator for cleaner either/or waiting
The 'Open a support case' link is always visible:
- In empty state: as a primary action button
- With cases: as a link in the description text
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The 'Open a support case' element is rendered differently depending on state:
- Empty state (no cases): PatternFly Button with OUIA ID
- Populated state (has cases): Actual link in description text
Updated selectors to use .or() locator to find either:
- Button: [data-ouia-component-id="help-panel-open-support-case-button"]
- Link: getByRole('link', { name: /open a support case/i })
This makes tests work regardless of whether user has support cases.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…/link The SupportPanel shows a SkeletonTable while fetching support cases from the API. We need to wait for this loading state to complete before looking for the 'Open a support case' button/link. Changes: - Added step to wait for either empty state or table to appear (skeleton gone) - Only then check for the button/link visibility - This ensures we're not checking while the loading skeleton is still showing - Removed timeout from final visibility check since we already waited for loading Step flow now: 1. Click help button 2. Wait for help panel 3. Click support tab 4. Wait for loading to complete (empty state or table appears) 5. Verify button/link is visible 6-8. (Test 2 only) Click and verify new tab Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace dual locator approach (OUIA ID + role) with simple text search. The text 'Open a support case' is the same whether rendered as a Button or Link, so searching by text is cleaner and more maintainable.
Change URL validation to check only the hostname (access.redhat.com) rather than the full path, since we cannot validate page content due to authentication requirements on the external Customer Portal. Also removed unnecessary waitForLoadState and unused timeout constants.
Clarify that test_support_case_from_apps requires complex setup: - API-created support cases - Cross-domain interaction - External portal authentication - Pre-filled data validation May require separate E2E suite or insights-chrome repository.
1. Add waitForURL before asserting hostname to ensure navigation completes (newPage starts at about:blank after pagePromise resolves) 2. Remove try/catch that hides real failures by converting timeouts to skips - Let API load failures fail the test naturally - Only skip when empty state is actually visible (user has no cases) - Removes false negatives from timeout/network issues
Replace broad wildcard waitForURL('**/*') with specific RegExp
/access\.redhat\.com/ to await popup navigation precisely and
fail faster if navigation goes to wrong domain.
catastrophe-brandon
force-pushed
the
btweed/iqe-support-case-tests
branch
from
c43951b to
9b6e3bb
Compare
Apply CodeRabbit review feedback to improve test determinism: 1. Conditionally assert "Open a support case" CTA based on state - Test 1: Assert CTA only when empty state is visible - When cases exist, assert table visibility instead - Prevents false failures when user has actual support cases 2. Skip popup test when user has cases - Test 2: "Open a support case" CTA only appears in empty state - Skip test when user has actual cases in the table - Makes test deterministic across different user accounts 3. Add explicit timeout to waitForURL - Use SUPPORT_API_LOAD_TIMEOUT for consistency - Prevents indefinite waits on slow navigation These changes ensure tests work correctly for both empty-state and populated-cases scenarios, addressing CodeRabbit's concerns about branch-specific assertions. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates support case help panel tests from
iqe-platform-ui-plugin/tests/test_support_case.pyto Playwright.Changes
playwright/support-case.spec.tsTest Coverage (3 test cases)
Tests verify support case functionality in the help panel:
access.redhat.com/supportin new tabSmart Conditional Testing
The tests handle both UI states gracefully:
Tests use conditional skipping based on what's actually rendered, making them resilient to different user account states.
Notes
test_support_case_from_apps) not migrated - complex cross-app test better suited for insights-chrome repoOriginal IQE Tests
The original IQE tests were all marked as
@pytest.mark.skip(reason="skip all failing tests")and required updates for current UI.Requirements
Related
iqe_platform_ui/tests/test_support_case.py🤖 Generated with Claude Code