Add QAN URL state persistence tests - #1139
Open
kiranvuyurru wants to merge 2 commits into
Open
Conversation
Add comprehensive tests for URL state persistence across RTA overview, sessions, and stored metrics pages. Includes new page object methods and locators for filters, pagination, and rows-per-page controls. Adds mock helper for RTA sessions and new storedMetrics URL state test suite (PMM-T2265, PMM-T2266, PMM-T2267, PMM-T2268).
kiranvuyurru
marked this pull request as ready for review
August 5, 2026 17:06
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughChangesURL State Coverage
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
e2e_tests/tests/qan/storedMetrics/urlState.test.ts (1)
17-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit the scenario into
test.step()phases.Separate filter changes, pagination changes, URL assertions, and shared-URL restoration into named steps. This makes a failed URL-state assertion identifiable in CI output.
As per coding guidelines, “Use Playwright's
test.step()for readable test structure.”🤖 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 `@e2e_tests/tests/qan/storedMetrics/urlState.test.ts` around lines 17 - 68, Split the scenario in the test callback into named Playwright test.step phases covering filter changes, pagination changes, URL assertions, and shared-URL restoration. Move the corresponding existing actions and assertions into these steps without changing their order or behavior, and keep console-error collection and cleanup correctly scoped across the full scenario.Source: Coding guidelines
🤖 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 `@e2e_tests/tests/qan/rta/overview.test.ts`:
- Around line 262-274: Select a known service in the test before capturing
serviceIds, using the existing service-selection control or helper. Assert that
the pre-reload URL contains the selected service ID, then preserve the existing
reload flow and verify the post-reload serviceIds matches the captured value.
- Around line 245-275: Add readable pmmTest.step() blocks to structure the
URL-state tests: in e2e_tests/tests/qan/rta/overview.test.ts lines 245-275,
separate state setup, URL verification, reload, and restored-state checks; in
lines 278-331, separate duration setup, filtered-results verification, URL
verification, and restoration checks; in e2e_tests/tests/qan/rta/session.test.ts
lines 55-72, separate mock setup, page-size selection, URL verification, and
reload checks.
In `@e2e_tests/tests/qan/storedMetrics/urlState.test.ts`:
- Line 16: Add the `@qan` tag to the test title for “PMM-T2268 Verify QAN shared
URL restores filters and pagination,” while retaining the existing `@rta` tag.
- Line 26: Wrap the test body after the page setup and console listener
registration in a try/finally block. Keep the existing assertions and test flow
in try, and move page.off('console', collectErrors) plus sharedPage.close() into
finally so both cleanup operations run on success and failure.
---
Nitpick comments:
In `@e2e_tests/tests/qan/storedMetrics/urlState.test.ts`:
- Around line 17-68: Split the scenario in the test callback into named
Playwright test.step phases covering filter changes, pagination changes, URL
assertions, and shared-URL restoration. Move the corresponding existing actions
and assertions into these steps without changing their order or behavior, and
keep console-error collection and cleanup correctly scoped across the full
scenario.
🪄 Autofix
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: Pro Plus
Run ID: 31dd7321-b48b-4220-a07f-0ac1c7967a38
📒 Files selected for processing (7)
e2e_tests/helpers/apiEndpoints.tse2e_tests/helpers/mocks.helper.tse2e_tests/pages/qan/rta/realTimeAnalytics.page.tse2e_tests/pages/qan/storedMetrics/storedMetrics.page.tse2e_tests/tests/qan/rta/overview.test.tse2e_tests/tests/qan/rta/session.test.tse2e_tests/tests/qan/storedMetrics/urlState.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
percona/pmm-qa(manual)percona/pmm(manual)
Comment on lines
+245
to
+275
| pmmTest( | ||
| 'PMM-T2265 Verify RTA overview table state is stored in the URL and restored after refresh @rta', | ||
| async ({ page, queryAnalytics }) => { | ||
| const { rta } = queryAnalytics; | ||
|
|
||
| await rta.buttons.pauseRealTimeAnalytics.click(); | ||
| await rta.filterQueriesByText('db.runCommand'); | ||
| await rta.inputs.rowsLimit.click(); | ||
| await rta.builders.rowsPerPageOption('10').click(); | ||
| await rta.clickElapsedTimeHeader(); | ||
|
|
||
| await expect | ||
| .poll(() => new URL(page.url()).searchParams.get('overview.f.queryText')) | ||
| .toBe('db.runCommand'); | ||
| await expect.poll(() => new URL(page.url()).searchParams.get('overview.pageSize')).toBe('10'); | ||
| await expect.poll(() => new URL(page.url()).searchParams.get('overview.sort')).not.toBeNull(); | ||
|
|
||
| const urlBeforeReload = new URL(page.url()); | ||
| const serviceIds = urlBeforeReload.searchParams.getAll('serviceIds'); | ||
|
|
||
| await page.reload(); | ||
| await rta.elements.realTimeTable.waitFor({ state: 'visible' }); | ||
| await rta.openFiltersIfHidden(); | ||
|
|
||
| await expect(rta.inputs.filterByQueryText).toHaveValue('db.runCommand'); | ||
| await expect(rta.inputs.rowsLimit).toHaveText('10'); | ||
| await expect(rta.elements.elapsedTimeColumnHeader).toHaveAccessibleName( | ||
| /Elapsed time Sorted by Elapsed time descending/, | ||
| ); | ||
| expect(new URL(page.url()).searchParams.getAll('serviceIds')).toEqual(serviceIds); | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add readable pmmTest.step() blocks to the new URL-state tests.
e2e_tests/tests/qan/rta/overview.test.ts#L245-L275: Separate state setup, URL verification, reload, and restored-state checks.e2e_tests/tests/qan/rta/overview.test.ts#L278-L331: Separate duration setup, filtered-results verification, URL verification, and restoration checks.e2e_tests/tests/qan/rta/session.test.ts#L55-L72: Separate mock setup, page-size selection, URL verification, and reload checks.
As per coding guidelines, “Use Playwright's test.step() for readable test structure.”
📍 Affects 2 files
e2e_tests/tests/qan/rta/overview.test.ts#L245-L275(this comment)e2e_tests/tests/qan/rta/overview.test.ts#L278-L331e2e_tests/tests/qan/rta/session.test.ts#L55-L72
🤖 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 `@e2e_tests/tests/qan/rta/overview.test.ts` around lines 245 - 275, Add
readable pmmTest.step() blocks to structure the URL-state tests: in
e2e_tests/tests/qan/rta/overview.test.ts lines 245-275, separate state setup,
URL verification, reload, and restored-state checks; in lines 278-331, separate
duration setup, filtered-results verification, URL verification, and restoration
checks; in e2e_tests/tests/qan/rta/session.test.ts lines 55-72, separate mock
setup, page-size selection, URL verification, and reload checks.
Source: Coding guidelines
Comment on lines
+262
to
+274
| const urlBeforeReload = new URL(page.url()); | ||
| const serviceIds = urlBeforeReload.searchParams.getAll('serviceIds'); | ||
|
|
||
| await page.reload(); | ||
| await rta.elements.realTimeTable.waitFor({ state: 'visible' }); | ||
| await rta.openFiltersIfHidden(); | ||
|
|
||
| await expect(rta.inputs.filterByQueryText).toHaveValue('db.runCommand'); | ||
| await expect(rta.inputs.rowsLimit).toHaveText('10'); | ||
| await expect(rta.elements.elapsedTimeColumnHeader).toHaveAccessibleName( | ||
| /Elapsed time Sorted by Elapsed time descending/, | ||
| ); | ||
| expect(new URL(page.url()).searchParams.getAll('serviceIds')).toEqual(serviceIds); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Select a service before verifying serviceIds restoration.
This test does not change the selected service state. It can pass with an empty serviceIds list. Select a known service, then assert its URL value before and after reload.
🤖 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 `@e2e_tests/tests/qan/rta/overview.test.ts` around lines 262 - 274, Select a
known service in the test before capturing serviceIds, using the existing
service-selection control or helper. Assert that the pre-reload URL contains the
selected service ID, then preserve the existing reload flow and verify the
post-reload serviceIds matches the captured value.
| }); | ||
|
|
||
| pmmTest( | ||
| 'PMM-T2268 Verify QAN shared URL restores filters and pagination @rta', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the @qan tag.
This QAN test declares only @rta. A CI selection for @qan cannot include this test. Retain @rta if that selection is also required.
As per coding guidelines, “Tag tests for CI filtering, using applicable tags such as @inventory, @dashboards, and @qan.”
🤖 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 `@e2e_tests/tests/qan/storedMetrics/urlState.test.ts` at line 16, Add the `@qan`
tag to the test title for “PMM-T2268 Verify QAN shared URL restores filters and
pagination,” while retaining the existing `@rta` tag.
Source: Coding guidelines
| if (message.type() === 'error') errors.push(message.text()); | ||
| }; | ||
|
|
||
| page.on('console', collectErrors); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Make test cleanup unconditional.
If an assertion fails after Line 26, page.off('console', collectErrors) and sharedPage.close() do not run. Use try/finally so the listener is removed and the created page is closed on every path.
As per coding guidelines, “Make tests idempotent and clean up resources created during tests.”
Also applies to: 55-68
🤖 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 `@e2e_tests/tests/qan/storedMetrics/urlState.test.ts` at line 26, Wrap the test
body after the page setup and console listener registration in a try/finally
block. Keep the existing assertions and test flow in try, and move
page.off('console', collectErrors) plus sharedPage.close() into finally so both
cleanup operations run on success and failure.
Source: Coding guidelines
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.
Add comprehensive tests for URL state persistence across RTA overview, sessions, and stored metrics pages. Includes new page object methods and locators for filters, pagination, and rows-per-page controls. Adds mock helper for RTA sessions and new storedMetrics URL state test suite (PMM-T2265, PMM-T2266, PMM-T2267, PMM-T2268).
https://github.com/percona/pmm-qa/actions/runs/31022412293/job/92362695351