test: add test cases for visualization using "Select *" queries #8589

ktx-akshay · 2025-09-23T06:29:45Z

User description

… query

PR Type

Tests

Description

Add E2E test for SELECT * visualization
Verify default line chart selection
Validate inspector shows transformed histogram query
Save panel to dashboard and assert success

Diagram Walkthrough

flowchart LR
  A["Open Logs"] -- "Enter SELECT *" --> B["Apply Query"]
  B -- "Open Visualise" --> C["Verify line chart rendered"]
  C -- "Run & Add Panel" --> D["Save to new dashboard"]
  D -- "Open Query Inspector" --> E["Assert histogram query present"]
  E -- "Close & Cleanup" --> F["Delete dashboard"]

File Walkthrough

Relevant files

Tests

visualize.spec.js `Add E2E test for SELECT * visualization and inspector` tests/ui-testing/playwright-tests/dashboards/visualize.spec.js Add test for SELECT * rendering as line chart Save panel to new dashboard and assert toast Open query inspector and validate generated histogram SQL Cleanup by navigating back and deleting dashboard	+55/-0

github-actions · 2025-09-23T06:30:32Z

PR Reviewer Guide 🔍

(Review updated until commit `eef523e`)

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 Selector The success toast assertion uses getByText("Panel added to dashboard"), which may be unstable if multiple toasts or similar text exist. Prefer a data-test selector or scoping within the toast container to reduce flakiness. const successMessage = page.getByText("Panel added to dashboard"); await expect(successMessage).toBeVisible({ timeout: 10000 }); Brittle String Match The inspector assertion matches the entire transformed SQL string including stream name, aliases, order, and spacing. Minor backend or formatting changes will break this test. Consider partial matching or regex for key fragments (histogram, count(), GROUP BY). await expect( page .getByRole("cell", { name: 'SELECT histogram(_timestamp) AS zo_sql_key, count() AS zo_sql_num FROM "e2e_automate" GROUP BY zo_sql_key ORDER BY zo_sql_key DESC', }) .first() ).toBeVisible(); Hardcoded Resource Names The assertion hardcodes stream name "e2e_automate" while the query uses STREAM_NAME variable earlier. Ensure consistency by building expected text from the same variable to avoid environment-specific failures. .getByRole("cell", { name: 'SELECT histogram(_timestamp) AS zo_sql_key, count(*) AS zo_sql_num FROM "e2e_automate" GROUP BY zo_sql_key ORDER BY zo_sql_key DESC', })

github-actions · 2025-09-23T06:30:48Z

PR Code Suggestions ✨

Latest suggestions up to eef523e
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Stabilize success assertion The success toast text can vary slightly and may disappear quickly, causing flakiness. Prefer waiting for a stable selector/state change indicating the panel was added, or match the toast with a role/aria and regex and await it to be hidden after visibility to ensure synchronization. tests/ui-testing/playwright-tests/dashboards/visualize.spec.js [550-558] await pm.logsVisualise.addPanelToNewDashboard( randomDashboardName, panelName ); -// Wait for and assert the success message -const successMessage = page.getByText("Panel added to dashboard"); -await expect(successMessage).toBeVisible({ timeout: 10000 }); +// Assert via a more stable signal and resilient toast check +await expect( + page.locator(`[data-test="dashboard-panel-${panelName}"]`) +).toBeVisible({ timeout: 15000 }); +const toast = page.getByRole('status', { name: /panel added to dashboard/i }); +await expect(toast).toBeVisible({ timeout: 10000 }); +await expect(toast).toBeHidden({ timeout: 10000 }); + Suggestion importance[1-10]: 7 __ Why: Using a stable panel selector and a role-based, case-insensitive toast check with visibility+hidden synchronization can significantly reduce flakiness; however, it’s still an improvement in test stability, not a functional bug fix.	Medium
General	Make inspector assertion resilient Avoid hardcoding the stream name and exact transformed SQL, which makes the test brittle across environments and timezones. Instead, derive the expected inspector SQL from `STREAM_NAME` and check for stable substrings like function/aliases, or assert the presence of both `histogram(_timestamp)` and `count()` with the dynamic stream. This reduces flakiness when the underlying stream or formatting changes. tests/ui-testing/playwright-tests/dashboards/visualize.spec.js [533-573] const selectAllQuery = `SELECT FROM "${STREAM_NAME}"`; ... await pm.logsVisualise.verifyChartRenders(page); ... -await expect( - page - .getByRole("cell", { - name: 'SELECT histogram(_timestamp) AS zo_sql_key, count() AS zo_sql_num FROM "e2e_automate" GROUP BY zo_sql_key ORDER BY zo_sql_key DESC', - }) - .first() -).toBeVisible(); +const expectedParts = [ + 'SELECT histogram(_timestamp) AS zo_sql_key', + 'count() AS zo_sql_num', + `FROM "${STREAM_NAME}"`, + 'GROUP BY zo_sql_key', + 'ORDER BY zo_sql_key DESC', +]; +for (const part of expectedParts) { + await expect( + page.getByRole("cell", { name: new RegExp(part.replace(/[.*+?^${}()\|[\]\\]/g, '\\$&')) }).first() + ).toBeVisible(); +} Suggestion importance[1-10]: 6 __ Why: Replacing a fully hardcoded inspector SQL with assertions on stable substrings tied to `STREAM_NAME` reduces brittleness and improves test reliability, but it’s a test robustness enhancement rather than a critical fix.	Low
Possible issue	Prevent visualization race condition Ensure the query completes before asserting visualization state to avoid race conditions. Move the "line chart selected" assertion after `runQueryAndWaitForCompletion()` or add an explicit wait for chart readiness. tests/ui-testing/playwright-tests/dashboards/visualize.spec.js [546-549] +await pm.logsVisualise.runQueryAndWaitForCompletion(); + // Expect line chart to be the selected/default visualization await pm.logsVisualise.verifyChartTypeSelected(page, "line", true); -await pm.logsVisualise.runQueryAndWaitForCompletion(); - Suggestion importance[1-10]: 5 __ Why: Moving the "line chart selected" assertion after waiting for query completion can avoid race conditions, but the current flow already verifies rendering and then runs the query; impact is moderate and context-dependent.	Low

Previous suggestions

Suggestions up to commit 928bf96

Category	Suggestion	Impact
Possible issue	Use dynamic stream name This assertion hard-codes the stream name to `"e2e_automate"`, which will fail if `STREAM_NAME` differs. Build the expected inspector query dynamically from `STREAM_NAME` to match the environment and keep the test robust. tests/ui-testing/playwright-tests/dashboards/visualize.spec.js [567-573] +const expectedInspectorQuery = + `SELECT histogram(_timestamp) AS zo_sql_key, count() AS zo_sql_num FROM "${STREAM_NAME}" GROUP BY zo_sql_key ORDER BY zo_sql_key DESC`; + await expect( page - .getByRole("cell", { - name: 'SELECT histogram(_timestamp) AS zo_sql_key, count() AS zo_sql_num FROM "e2e_automate" GROUP BY zo_sql_key ORDER BY zo_sql_key DESC', - }) + .getByRole("cell", { name: expectedInspectorQuery }) .first() ).toBeVisible(); Suggestion importance[1-10]: 8 __ Why: The test hardcodes `"e2e_automate"` while earlier it uses `STREAM_NAME`; making the expected query use `${STREAM_NAME}` fixes an environment-dependent bug.	Medium
General	Wait for stable UI state Relying on generic toasts can be flaky and slow if multiple messages appear. Instead, wait for a stable dashboard UI signal that the panel exists, e.g., presence of the panel by its name. This reduces timing issues and makes the test more deterministic. tests/ui-testing/playwright-tests/dashboards/visualize.spec.js [550-558] await pm.logsVisualise.addPanelToNewDashboard( randomDashboardName, panelName ); -// Wait for and assert the success message -const successMessage = page.getByText("Panel added to dashboard"); -await expect(successMessage).toBeVisible({ timeout: 10000 }); +// Wait for the panel to appear on the dashboard to ensure save success +await expect( + page.locator(`[data-test="dashboard-panel-title-${panelName}"]`) +).toBeVisible({ timeout: 10000 }); Suggestion importance[1-10]: 6 __ Why: Replacing a transient toast check with asserting the panel's presence by `panelName` is a solid stability improvement, though not critical to correctness.	Low
General	Avoid visualization race condition Verifying the chart type before ensuring the query has finished can cause false negatives. Run the query and wait for completion before asserting the selected visualization to avoid race conditions. tests/ui-testing/playwright-tests/dashboards/visualize.spec.js [546-549] +await pm.logsVisualise.runQueryAndWaitForCompletion(); + // Expect line chart to be the selected/default visualization await pm.logsVisualise.verifyChartTypeSelected(page, "line", true); -await pm.logsVisualise.runQueryAndWaitForCompletion(); - Suggestion importance[1-10]: 5 __ Why: Moving the chart-type assertion after waiting for query completion can reduce flakiness, but the current order may still work if selection is immediate.	Low

… query

github-actions · 2025-09-23T06:37:23Z

Persistent review updated to latest commit eef523e

greptile-apps

Greptile Overview

Summary

This PR adds a comprehensive end-to-end test for SELECT * query visualization functionality. The test validates that SELECT * queries default to line chart visualization, can be successfully saved to dashboards, and that the query inspector shows the correct transformed aggregation query.

Key test coverage includes:

Running SELECT * query and verifying line chart is selected as default visualization
Adding the visualization panel to a new dashboard with proper success confirmation
Opening query inspector and asserting the transformed query shows histogram aggregation with proper aliases
Clean dashboard cleanup to avoid test pollution

The test follows the existing pattern used in the test suite and maintains proper test isolation with dynamic dashboard/panel names and cleanup procedures.

Confidence Score: 4/5

This PR is safe to merge with minimal risk as it only adds test coverage without modifying production code
Score reflects well-structured test code that follows existing patterns, proper cleanup procedures, and comprehensive coverage of the SELECT * visualization flow. Minor deduction due to some hardcoded values and potential timing dependencies
No files require special attention - the single test file follows established patterns and includes proper cleanup

Important Files Changed

File Analysis

Filename	Score	Overview
tests/ui-testing/playwright-tests/dashboards/visualize.spec.js	4/5	Added comprehensive e2e test for SELECT * query visualization, verifying line chart rendering, dashboard saving, and query transformation inspection

Sequence Diagram

sequenceDiagram
    participant Test as Playwright Test
    participant Page as Browser Page
    participant PM as PageManager
    participant LogsUI as Logs UI
    participant VizUI as Visualization UI
    participant Dashboard as Dashboard
    participant Inspector as Query Inspector

    Test->>PM: Initialize PageManager
    Test->>PM: openLogs()
    PM->>LogsUI: Navigate to logs page
    
    Test->>PM: fillLogsQueryEditor("SELECT * FROM e2e_automate")
    PM->>LogsUI: Fill query editor with SELECT * query
    
    Test->>PM: setRelative("6", "w")
    PM->>LogsUI: Set time range to 6 weeks
    
    Test->>PM: logsApplyQueryButton()
    PM->>LogsUI: Execute SELECT * query
    
    Test->>PM: openVisualiseTab()
    PM->>VizUI: Switch to visualization tab
    
    Test->>PM: verifyChartRenders(page)
    PM->>VizUI: Verify chart renders successfully
    
    Test->>PM: verifyChartTypeSelected("line", true)
    PM->>VizUI: Verify line chart is selected as default
    
    Test->>PM: runQueryAndWaitForCompletion()
    PM->>VizUI: Wait for query execution completion
    
    Test->>PM: addPanelToNewDashboard(randomDashboardName, panelName)
    PM->>Dashboard: Create new dashboard and add panel
    Dashboard-->>Test: "Panel added to dashboard" success message
    
    Test->>Page: Click panel dropdown menu
    Test->>Page: Click Query Inspector option
    PM->>Inspector: Open query inspector
    
    Test->>Page: Assert transformed aggregation query
    Inspector-->>Test: Show "SELECT histogram(_timestamp) AS zo_sql_key, count(*) AS zo_sql_num..."
    
    Test->>Page: Close query inspector
    Test->>Page: Navigate back to dashboard list
    Test->>PM: deleteDashboard(randomDashboardName)
    PM->>Dashboard: Clean up test dashboard

_{1 file reviewed, no comments}

_{Edit Code Review Bot Settings | Greptile}

github-actions bot added ✏️ Feature 🧹 Updates Review effort 2/5 labels Sep 23, 2025

ktx-akshay changed the title ~~test: add test cases for visualization using "SELECT *" queries~~ test: add test cases for visualization using "Select *" queries Sep 23, 2025

ktx-akshay marked this pull request as ready for review September 23, 2025 06:36

test: add line chart rendering and dashboard saving test for SELECT *…

eef523e

… query

ktx-akshay force-pushed the e2e-select---query-test branch from 928bf96 to eef523e Compare September 23, 2025 06:37

greptile-apps bot reviewed Sep 23, 2025

View reviewed changes

ktx-kirtan approved these changes Sep 23, 2025

View reviewed changes

ktx-akshay merged commit 83b2951 into main Sep 23, 2025
29 checks passed

ktx-akshay deleted the e2e-select---query-test branch September 23, 2025 07:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: add test cases for visualization using "Select *" queries #8589

test: add test cases for visualization using "Select *" queries #8589

Uh oh!

ktx-akshay commented Sep 23, 2025 •

edited by github-actions bot

Loading

Uh oh!

github-actions bot commented Sep 23, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 23, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 23, 2025

Uh oh!

greptile-apps bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

test: add test cases for visualization using "Select *" queries #8589

test: add test cases for visualization using "Select *" queries #8589

Uh oh!

Conversation

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

Uh oh!

User description

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

github-actions bot commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit eef523e)

Uh oh!

github-actions bot commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

github-actions bot commented Sep 23, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ktx-akshay commented Sep 23, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Sep 23, 2025 •

edited

Loading

(Review updated until commit `eef523e`)

github-actions bot commented Sep 23, 2025 •

edited

Loading