test: migrate feedback and support case tests from IQE (RHCLOUD-44391)#305
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:
WalkthroughAdds two Playwright E2E test specs: one automates the Help panel Feedback workflow (open form, back navigation, submit, success UI), and one verifies Help panel Support Cases flows (empty-state vs table handling, external "Open a support case" tab, and table/pagination checks). ChangesFeedback - Help Panel
Support Cases - Help Panel
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
catastrophe-brandon
force-pushed
the
btweed/iqe-feedback-tests
branch
from
a5ae4f4 to
c6cd63f
Compare
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: 1
🤖 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 assumes the "Open a support case" CTA is always
present but the SupportPanel renders that CTA only in empty-state; update the
spec to branch on which locator is visible: use the existing emptyState and
supportTable locators (emptyState, supportTable) to await visibility, then if
emptyState.isVisible() assert page.getByText(/open a support
case/i).toBeVisible(), else assert the CTA is not present or instead assert the
supportTable has expected rows; reference the SupportPanel component behavior
(src/components/HelpPanel/HelpPanelTabs/SupportPanel) when adding the
conditional assertion so tests are deterministic for accounts with open cases.
🪄 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: 40cb5586-615c-4875-8d96-8b92e55b1701
📒 Files selected for processing (2)
playwright/feedback.spec.tsplaywright/support-case.spec.ts
catastrophe-brandon
force-pushed
the
btweed/iqe-feedback-tests
branch
from
b9399eb to
12a6311
Compare
catastrophe-brandon
force-pushed
the
btweed/iqe-feedback-tests
branch
from
12a6311 to
bd6ee75
Compare
catastrophe-brandon
force-pushed
the
btweed/iqe-feedback-tests
branch
from
ac74ed6 to
82b079d
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
playwright/support-case.spec.ts (1)
135-141:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftConditional skip hides the table scenario in CI.
When empty-state is shown, this test exits as skipped, so the “has open cases” path may never be validated. Prefer explicit fixture-gating (
fixmewith reason) or seeded data to avoid green builds with missing coverage.Suggested adjustment
- const emptyVisible = await emptyState.isVisible().catch(() => false); - if (emptyVisible) { - // User has no cases, skip this test - test.skip(); - return; - } + const emptyVisible = await emptyState.isVisible(); + test.fixme( + emptyVisible, + 'Requires a seeded account with at least one open support case' + );🤖 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 `@playwright/support-case.spec.ts` around lines 135 - 141, The current runtime conditional that sets emptyVisible via emptyState.isVisible() and then calls test.skip() hides the "has open cases" path in CI; remove this runtime skip logic (the emptyVisible variable and the test.skip() call) and instead gate the test explicitly using a fixture or Playwright's metadata helper (e.g., use test.fixme(condition, 'reason') at the top of the test) or ensure the test runs against seeded data so emptyState never appears; update references to emptyState and remove the test.skip() branch so the test either uses a deterministic seeded dataset or is marked with test.fixme with a clear reason.
🧹 Nitpick comments (1)
playwright/support-case.spec.ts (1)
43-57: ⚡ Quick winExtract repeated “open support tab” flow into a helper.
The same panel-open/subtab-navigation sequence appears in three tests. Centralizing it will reduce drift and future flake fixes.
Refactor sketch
+async function openMySupportCasesTab(page: Page) { + await page.getByLabel('Toggle help panel').click(); + await expect(page.locator('[data-ouia-component-id="help-panel-title"]')).toBeVisible(); + await expect(page.locator('[data-ouia-component-id="help-panel-subtabs"]')).toBeVisible({ + timeout: SUBTABS_LOAD_TIMEOUT, + }); + await page.locator('[data-ouia-component-id="help-panel-subtab-support"]').click(); +}- await page.getByLabel('Toggle help panel').click(); - ... - await supportTab.click(); + await openMySupportCasesTab(page);Also applies to: 70-84, 113-126
🤖 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 `@playwright/support-case.spec.ts` around lines 43 - 57, Extract the repeated sequence that opens the help panel and navigates to the "My support cases" subtab into a reusable helper (e.g., openSupportTab or gotoSupportSubtab) and replace the duplicated blocks found around the current locator usage (page.getByLabel('Toggle help panel'), help-panel-title, help-panel-subtabs, help-panel-subtab-support) with calls to that helper; the helper should click the help toggle, await visibility of '[data-ouia-component-id="help-panel-title"]', await visibility of '[data-ouia-component-id="help-panel-subtabs"]' with SUBTABS_LOAD_TIMEOUT, then click '[data-ouia-component-id="help-panel-subtab-support"]', and update the three test sites (the blocks at lines noted in the comment) to call this helper instead of repeating the steps.
🤖 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.
Duplicate comments:
In `@playwright/support-case.spec.ts`:
- Around line 135-141: The current runtime conditional that sets emptyVisible
via emptyState.isVisible() and then calls test.skip() hides the "has open cases"
path in CI; remove this runtime skip logic (the emptyVisible variable and the
test.skip() call) and instead gate the test explicitly using a fixture or
Playwright's metadata helper (e.g., use test.fixme(condition, 'reason') at the
top of the test) or ensure the test runs against seeded data so emptyState never
appears; update references to emptyState and remove the test.skip() branch so
the test either uses a deterministic seeded dataset or is marked with test.fixme
with a clear reason.
---
Nitpick comments:
In `@playwright/support-case.spec.ts`:
- Around line 43-57: Extract the repeated sequence that opens the help panel and
navigates to the "My support cases" subtab into a reusable helper (e.g.,
openSupportTab or gotoSupportSubtab) and replace the duplicated blocks found
around the current locator usage (page.getByLabel('Toggle help panel'),
help-panel-title, help-panel-subtabs, help-panel-subtab-support) with calls to
that helper; the helper should click the help toggle, await visibility of
'[data-ouia-component-id="help-panel-title"]', await visibility of
'[data-ouia-component-id="help-panel-subtabs"]' with SUBTABS_LOAD_TIMEOUT, then
click '[data-ouia-component-id="help-panel-subtab-support"]', and update the
three test sites (the blocks at lines noted in the comment) to call this helper
instead of repeating the steps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: db7bc7f6-378b-484f-a85c-3ae09c62dcba
📒 Files selected for processing (2)
playwright/feedback.spec.tsplaywright/support-case.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- playwright/feedback.spec.ts
|
@catastrophe-brandon some of the commits look unrelated to the PR. Looks like you have commits from the #304 PR in here as well. |
karelhala
left a comment
karelhala
left a comment
There was a problem hiding this comment.
Split the merged test back into two separate tests
The PR description lists two IQE tests being migrated, but the code combines them into a single test:
test('should open feedback form, test navigation, and submit successfully', async ({ page }) => {This reduces diagnostic granularity — if the submission step fails (e.g., API timeout), the navigation test is also reported as failed even though it would have passed on its own. Split back into two tests matching the original IQE structure:
test('should open and close feedback form', async ({ page }) => {
// Open help panel, navigate to feedback tab
// Click "Share general feedback", verify form visible
// Click Back, verify return to feedback home
});
test('should submit feedback successfully', async ({ page }) => {
// Open help panel, navigate to feedback form
// Fill in feedback text, click submit
// Verify success message and "Share more feedback" button
});This way each test passes or fails independently, and a CI failure points you directly to what broke.
|
@catastrophe-brandon looking at the test, it creates actual JIRA ticket. Do we really want it? It is creating tickets on stage so might not be an issue, just making sure. |
The merge was deliberate - one e2e test covering multiple touchpoints is often more useful than lots of granular, smaller tests. In fact, granular e2e tests are considered an anti-pattern, the remedy to which is fine-grained component/integration coverage. 
|
This is how the previous IQE test worked, so I chose to keep the same flow. There's no known way to test ticket submission functionality e2e without creating an issue. We could look into a periodic clean-up routine to remove the created issues if it's problematic from a book-keeping standpoint. |
catastrophe-brandon
force-pushed
the
btweed/iqe-feedback-tests
branch
3 times, most recently
from
3bc19c6 to
fe1040e
Compare
catastrophe-brandon
changed the title
|
@CodeRabbit-ai what's blocking approval? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me check the current state of my review comments on this PR. 🏁 Script executed: Length of output: 30299
From my side (CodeRabbit):
From human reviewers:
If you address the |
Migrates feedback and support case e2e tests from IQE to Playwright with comprehensive test utilities and helper functions. New test suites: - feedback.spec.ts: Tests feedback form submission flow including navigation, form interaction, and success message validation - support-case.spec.ts: Tests support case creation flows, including navigation to Customer Portal and handling of various support case entry points across different help panel states - support-case-helpers.ts: Page object helpers for support case tests (openSupportPanel, isEmptyState, getSupportPanelLocators) Test utilities refactoring: - Extracted duplicated help panel interaction patterns into reusable helper functions to reduce code duplication (~25% reduction) - openHelpPanel(): Opens help panel and waits for it to load - switchToHelpPanelTab(): Switches to a specific tab by name - waitForHelpPanelTabsLoaded(): Waits for tabs container to render - waitForSupportTabLoaded(): Waits for support content (empty or table) - navigateToFeedbackTab(): Navigates to feedback tab and waits Timeout constants centralized: - PAGE_LOAD_TIMEOUT (60s) - Initial page load - ELEMENT_VISIBLE_TIMEOUT (10s) - Element visibility - RESOURCE_COUNT_TIMEOUT (20s) - Resource count extraction - CHROME_HEADER_LOAD_TIMEOUT (30s) - Chrome shell header (for flaky first test) - SUPPORT_API_LOAD_TIMEOUT (15s) - Support API data - HELP_PANEL_TABS_LOAD_TIMEOUT (10s) - Tab rendering - FEEDBACK_SUBMISSION_TIMEOUT (10s) - Feedback submission Test improvements: - Replaced runtime conditional skip with test.fixme() for deterministic test execution (support cases table test requires test user to have open support cases in stage environment) All tests handle feature flag dependencies and environment-specific behavior appropriately. Incorporates changes from PR RedHatInsights#304 to consolidate test migration efforts. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
catastrophe-brandon
force-pushed
the
btweed/iqe-feedback-tests
branch
from
fe1040e to
7b4f04d
Compare
|
@CodeRabbit-ai Updates made. Do I have your blessing? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me check the current state of the file to confirm the fix. 🏁 Script executed: Length of output: 4565
The [approve] 🐇 ✨ |
2 participants
Summary
Migrates feedback and support case e2e tests from IQE to Playwright with comprehensive test utilities and helper functions. This PR now incorporates changes from #304 to consolidate test migration efforts and reduce review overhead.
Changes
New Test Suites
1. Feedback Tests (
playwright/feedback.spec.ts)Migrated from:
iqe_platform_ui/tests/test_feedback.pyTest: should open feedback form, test navigation, and submit successfully
Markers:
@pytest.mark.qa,@pytest.mark.coreNote: JIRA API verification of ticket creation is not automated. The test verifies form submission succeeds but does not query JIRA to confirm ticket details. Tickets are created in https://issues.redhat.com/projects/CRCFEEDBK/issues/ with labels
learning-resourcesandhelp-panel-feedback.2. Support Case Tests (
playwright/support-case.spec.ts)Migrated from:
iqe_platform_ui/tests/test_support_case.pyTest 1: should display "Open a support case" link in help panel
Test 2: should open Customer Portal when clicking "Open a support case" link
access.redhat.comURLTest 3: should display support cases table when user has open cases
TODO: Test #3 from IQE (
test_support_case_from_apps) - Not yet migrated. This test verifies support case data is pre-filled correctly when opened from different apps. May be better suited for insights-chrome repository.Test Infrastructure
New Helper Functions (
playwright/test-utils.ts)Extracted duplicated help panel interaction patterns into reusable helpers:
openHelpPanel()- Opens help panel and waits for it to loadswitchToHelpPanelTab(tabName)- Switches to a specific tab by namewaitForHelpPanelTabsLoaded()- Waits for tabs container to renderwaitForSupportTabLoaded()- Waits for support content (empty or table)navigateToFeedbackTab()- Navigates to feedback tab and waitsImpact: Reduces ~60 lines of duplicated code across test files (~25% reduction)
Support Case Page Objects (
playwright/support-case-helpers.ts)Convenience helpers for support case tests:
openSupportPanel()- Opens help panel and navigates to Support tabisEmptyState()- Checks if support panel shows empty stategetSupportPanelLocators()- Returns locators for common elementsCentralized Timeout Constants
All hard-coded timeout values replaced with symbolic constants:
PAGE_LOAD_TIMEOUT(60s) - Initial page loadELEMENT_VISIBLE_TIMEOUT(10s) - Element visibilityRESOURCE_COUNT_TIMEOUT(20s) - Resource count extractionCHROME_HEADER_LOAD_TIMEOUT(30s) - Chrome shell header (fixes flaky first test)SUPPORT_API_LOAD_TIMEOUT(15s) - Support API dataHELP_PANEL_TABS_LOAD_TIMEOUT(10s) - Tab renderingFEEDBACK_SUBMISSION_TIMEOUT(10s) - Feedback submissionRefactored Tests
playwright/help-panel.spec.ts- Now uses shared helper functions (42 lines, simplified)playwright/all-learning-resources.spec.ts- Uses symbolic timeout constantsRequirements
Related
🤖 Generated with Claude Code