test: dashboard max query range limit #6813

ktx-akshay · 2025-05-15T05:34:38Z

PR Type

Tests, Enhancement

Description

Add dashboard max query range UI test
Introduce StreamSettingsPage page object
Enhance ManagementPage with streaming toggles
Include maxquery test in CI workflow

Diagram Walkthrough

flowchart LR
  spec["Add maxquery Playwright spec"] -- uses --> pm["PageManager utilities"]
  spec -- updates --> streamPage["StreamSettingsPage (new)"]
  spec -- verifies --> uiWarn["Max duration warning visible"]
  pm -- ensures --> streamingDisabled["Streaming disabled via ManagementPage"]
  ci["GitHub Actions workflow"] -- includes --> spec

File Walkthrough

Relevant files

Enhancement

streams.js `New StreamSettingsPage for stream max query updates` tests/ui-testing/pages/dashboardPages/streams.js Add StreamSettingsPage class Implement max query range update flow Add stable locators and waits Handle modal close after save	+57/-0
managementPage.js `ManagementPage refactor and streaming toggle helpers` tests/ui-testing/pages/generalPages/managementPage.js Refactor to modern style and quotes Add navigation helpers with validation Add enable/disable streaming utilities Improve selectors and waits	+162/-89

Tests

maxquery.spec.js `Playwright spec for dashboard max query warning` tests/ui-testing/playwright-tests/dashboards/maxquery.spec.js Add end-to-end test for max query limit Use PageManager and StreamSettingsPage Validate warning visibility and API response Restore stream setting after test	+107/-0

Configuration changes

playwright.yml `CI includes max query dashboard test` .github/workflows/playwright.yml Add maxquery.spec.js to Dashboards-Core matrix Minor YAML formatting adjustments Preserve existing test matrices	+75/-70

greptile-apps

Greptile Summary

This PR adds comprehensive UI test coverage for the dashboard max query range limit functionality. The changes include:

New Test File: maxquery.spec.js is added to validate that dashboards properly display warning messages when query time ranges exceed stream-configured limits (e.g., warning appears for 6-week queries when stream limit is 4 days, and disappears when reduced to 2 hours).
Page Object Pattern Enhancement: A new StreamSettingsPage class is created to encapsulate stream configuration UI interactions, following established testing patterns in the codebase. This provides reusable methods for updating stream max query range settings.
Management Page Extension: The ManagementPage class gains streaming control methods (checkStreaming() and ensureStreamingDisabled()) to ensure consistent test environment setup by managing the streaming feature state.
CI Integration: The new test is integrated into the GitHub Actions Playwright workflow under the Dashboards-Core test suite, with YAML formatting standardized across all test configurations.

The implementation follows the existing test architecture using PageManager for UI interactions and includes proper test lifecycle management with setup/teardown procedures. The functionality being tested appears to be a performance safeguard that prevents users from executing resource-intensive queries that exceed configured time limits.

Confidence score: 3/5

This PR has moderate risk due to test implementation quality issues that could affect reliability
Score reflects concerns about hardcoded timeouts, duplicate code, debugging artifacts, and potential test flakiness from timing dependencies
Pay close attention to maxquery.spec.js for code quality improvements and managementPage.js for timeout handling

_{4 files reviewed, 5 comments}

_{Edit Code Review Bot Settings | Greptile}

.github/workflows/playwright.yml

tests/ui-testing/pages/dashboardPages/streams.js

tests/ui-testing/pages/generalPages/managementPage.js

tests/ui-testing/playwright-tests/dashboards/maxquery.spec.js

github-actions · 2025-09-03T09:43:51Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Flaky Waits The method relies on fixed timeouts (e.g., waitForTimeout 3000) instead of waiting for specific network or UI conditions, which can cause flakiness. Prefer waiting on targeted responses or element states already hinted by the commented waitForResponse. await this.searchInput.click(); await this.searchInput.fill(streamName); await this.page.waitForTimeout(3000); // Wait for the expected response // await this.page.waitForResponse( // (response) => // response // .url() // .includes( // "/api/default/streams?type=logs&offset=0&limit=20&keyword=e2e_automate" // ) && response.status() === 200 // ); // Wait for stream details to appear and click await this.streamDetailButton.waitFor({ state: "visible", timeout: 5000 }); await this.streamDetailButton.click(); Locator Specificity Using first() on the 'Stream Detail' button may click the wrong stream if multiple results are present. Consider scoping the locator to the searched stream row to ensure the correct detail modal is opened. this.streamDetailButton = page .getByRole("button", { name: "Stream Detail" }) .first(); this.maxQueryInput = page.locator( '[data-test="stream-details-max-query-range-input"]' ); Save After Toggle In ensureStreamingDisabled the code clicks the submit button after toggling but does not re-verify the persisted state post-save. Add a post-save check (e.g., reload and assert aria-checked) to ensure the change is committed. // If Streaming is enabled, click to disable it if (isChecked) { console.log("Disabling Streaming..."); await this.page.click(toggleSelector); await this.page.waitForTimeout(500); // allow UI to update // Verify toggle is now disabled const newCheckedState = await this.page.$eval( toggleSelector, (toggle) => { return toggle.getAttribute("aria-checked") === "true"; } ); await this.page.locator('[data-test="dashboard-add-submit"]').click(); await this.page.waitForTimeout(5000); if (!newCheckedState) { console.log("Streaming has been successfully disabled."); } else { console.log("Failed to disable Streaming."); } } else { console.log("Streaming is already disabled."); } }

github-actions · 2025-09-03T09:44:24Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Use locator getAttribute for stability Accessing attributes with `$eval` can throw if the element re-renders; use a locator and `getAttribute` to reduce detached element errors. This improves stability under UI updates. tests/ui-testing/pages/generalPages/managementPage.js [86-88] -const isChecked = await this.page.$eval(toggleSelector, (toggle) => { - return toggle.getAttribute("aria-checked") === "true"; -}); +const toggle = this.page.locator(toggleSelector); +await toggle.waitFor({ state: "visible" }); +const isChecked = (await toggle.getAttribute("aria-checked")) === "true"; Suggestion importance[1-10]: 7 __ Why: Switching from $eval to locator.getAttribute reduces detached element errors during re-renders and matches Playwright best practices. It's contextually correct and enhances stability without changing behavior.	Medium
Possible issue	Avoid fixed sleeps with conditional wait Replace the fixed timeout with a condition-based wait to avoid flakiness and race conditions. Wait for the specific `streamName` row/button to appear before proceeding. tests/ui-testing/pages/dashboardPages/streams.js [23-25] await this.searchInput.click(); await this.searchInput.fill(streamName); -await this.page.waitForTimeout(3000); +await this.streamDetailButton.filter({ hasText: "Stream Detail" }).waitFor({ state: "visible", timeout: 15000 }); Suggestion importance[1-10]: 6 __ Why: Replacing a hardcoded timeout with a condition-based wait reduces flakiness and aligns with best practices. However, using `this.streamDetailButton.filter({ hasText: "Stream Detail" })` is redundant since the locator already targets that button, so impact is moderate.	Low
General	Guarantee input is cleared before fill Ensure the input is truly cleared before typing to prevent concatenated values causing wrong settings. Use a clear action or select-all-and-delete before filling. tests/ui-testing/pages/dashboardPages/streams.js [45] +await this.maxQueryInput.click(); +await this.maxQueryInput.press('Control+A'); +await this.maxQueryInput.press('Delete'); await this.maxQueryInput.fill(newValue); Suggestion importance[1-10]: 7 __ Why: Ensuring the input is cleared before filling prevents concatenated values and potential test instability. The approach is accurate and improves reliability with minimal risk.	Medium

…ort path

…wait timeouts and improve locator usage

…ate test case status

…pdate test case status

…ecks for streaming toggle

…Base and remove unnecessary wait timeouts

… and removing redundant wait timeouts

… WebSocket toggle logic with improved logging

… with enhanced logging and streamlined methods

…tion

github-actions bot added the 🧹 Updates label May 15, 2025

ktx-akshay force-pushed the test--dashboard-max-query-range-limit branch 3 times, most recently from 80b9c93 to c2425b2 Compare May 22, 2025 09:28

ktx-akshay force-pushed the test--dashboard-max-query-range-limit branch 3 times, most recently from e0ff378 to 230ad8e Compare July 17, 2025 07:27

ktx-akshay force-pushed the test--dashboard-max-query-range-limit branch 2 times, most recently from 651be45 to 4850684 Compare August 5, 2025 05:04

ktx-akshay force-pushed the test--dashboard-max-query-range-limit branch from dafd257 to 586e496 Compare September 3, 2025 08:51

ktx-akshay marked this pull request as ready for review September 3, 2025 09:42

greptile-apps bot reviewed Sep 3, 2025

View reviewed changes

github-actions bot added the Review effort 2/5 label Sep 3, 2025

ktx-akshay force-pushed the test--dashboard-max-query-range-limit branch 4 times, most recently from 38511d7 to b33be23 Compare September 4, 2025 12:58

ktx-kirtan approved these changes Sep 4, 2025

View reviewed changes

ktx-akshay added 10 commits September 5, 2025 10:33

added the test case

ea2c294

added the test file

6ec0a56

fix the failed test case

74387a8

fix the failed test case

df419a7

added the test case and update the method

594dc1e

remove console

af362d6

update the test case

81bde89

Removed hard wait

75b6a62

add the wait for api response

cd8133f

skip the test case

99da1c7

ktx-akshay added 18 commits September 5, 2025 10:33

added the wait

1e0185e

test :fixed the failed test case

6ff592f

update the test case steps

7c3e85c

clear the commented code

a6f36c7

remove Extra added envourment variable

6ffe64e

refactor maxquery test to use PageManager

0837876

add wait timeout before closing modal in StreamSettingsPage

239707b

add maxquery.spec.js to Playwright test suite and update log data imp…

7a5ae95

…ort path

remove unnecessary console logs for

0a5f890

refactor StreamSettingsPage and maxquery tests to remove unnecessary …

b6bcda6

…wait timeouts and improve locator usage

refactor dashboard filter logic to improve dropdown selection and upd…

2e8df55

…ate test case status

refactor dashboard filter logic to improve suggestion selection and u…

a2bb339

…pdate test case status

refactor ManagementPage to remove console logs and redundant state ch…

4222595

…ecks for streaming toggle

refactor: dashboard test files to replace login calls with navigateTo…

805fa04

…Base and remove unnecessary wait timeouts

refactor: enhance maxquery test by adding wait for date-time controls…

115b3b4

… and removing redundant wait timeouts

refactor ManagementPage: streamline goToManagement method and enhance…

0bfc1d1

… WebSocket toggle logic with improved logging

refactor ManagementPage: improve WebSocket and streaming toggle logic…

a109a15

… with enhanced logging and streamlined methods

refactor: move StreamSettingsPage to generalPages for better organiza…

2a243c9

…tion

ktx-akshay force-pushed the test--dashboard-max-query-range-limit branch from d424afc to 2a243c9 Compare September 5, 2025 05:05

refactor maxquery test: comment

3a41999

ktx-akshay merged commit 62d5f2d into main Sep 5, 2025
34 of 40 checks passed

ktx-akshay deleted the test--dashboard-max-query-range-limit branch September 5, 2025 06:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: dashboard max query range limit #6813

test: dashboard max query range limit #6813

Uh oh!

ktx-akshay commented May 15, 2025 •

edited by github-actions bot

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 3, 2025

Uh oh!

github-actions bot commented Sep 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

test: dashboard max query range limit #6813

test: dashboard max query range limit #6813

Uh oh!

Conversation

ktx-akshay commented May 15, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Summary

Confidence score: 3/5

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 3, 2025

PR Reviewer Guide 🔍

Uh oh!

github-actions bot commented Sep 3, 2025

PR Code Suggestions ✨

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ktx-akshay commented May 15, 2025 •

edited by github-actions bot

Loading