(superset) 02/02: test(sqllab): align Playwright tests with sc-102088 story requirements

[jl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jli pushed a commit to branch feat-sql-lab-to-playwright
in repository https://gitbox.apache.org/repos/asf/superset.git

commit b0a82b948810002c071c6db015a14ed593cba703
Author: Joe Li <jo...@preset.io>
AuthorDate: Thu Apr 2 09:40:41 2026 -0700

    test(sqllab): align Playwright tests with sc-102088 story requirements
    
    Add 5 new tests and reorganize files to match the story spec:
    - query-execution.spec.ts: error handling + re-run query tests
    - tabs.spec.ts: create/close/preserve-state tests
    - save-and-share.spec.ts: moved save query + new dataset creation test
    - SqlLabPage POM: error alert, save dataset button/modal methods
    
    Co-Authored-By: Claude Opus 4.6 (1M context) <no...@anthropic.com>
---
 superset-frontend/playwright/pages/SqlLabPage.ts   |  20 ++++
 .../tests/sqllab/query-execution.spec.ts           |  81 ++++++++---------
 ...ry-execution.spec.ts => save-and-share.spec.ts} | 101 ++++++++++++++++-----
 .../playwright/tests/sqllab/tabs.spec.ts           |  72 ++++++++++-----
 4 files changed, 184 insertions(+), 90 deletions(-)

diff --git a/superset-frontend/playwright/pages/SqlLabPage.ts b/superset-frontend/playwright/pages/SqlLabPage.ts
index bfe60f9e1ba..1b6fea415f8 100644
--- a/superset-frontend/playwright/pages/SqlLabPage.ts
+++ b/superset-frontend/playwright/pages/SqlLabPage.ts
@@ -50,6 +50,7 @@ export class SqlLabPage {
     DATABASE_SELECTOR: '[data-test="DatabaseSelector"]',
     LIMIT_DROPDOWN: '.limitDropdown',
     TAB_REMOVE: '[aria-label="remove"]',
+    SAVE_DATASET_BUTTON: 'button[aria-label="Save dataset"]',
   } as const;
 
   constructor(page: Page) {
@@ -237,6 +238,10 @@ export class SqlLabPage {
     return this.page.locator(SqlLabPage.SELECTORS.SOUTH_PANE);
   }
 
+  getErrorAlert(): Locator {
+    return this.getResultsPane().locator('.ant-alert-error');
+  }
+
   // ── Row Limit ──
 
   async getRowLimit(): Promise<string> {
@@ -261,6 +266,21 @@ export class SqlLabPage {
     return new Modal(this.page, SqlLabPage.SELECTORS.SAVE_QUERY_MODAL);
   }
 
+  // ── Save Dataset ──
+
+  async clickSaveDatasetButton(): Promise<void> {
+    await this.activePanel
+      .locator(SqlLabPage.SELECTORS.SAVE_DATASET_BUTTON)
+      .click();
+  }
+
+  getSaveDatasetModal(): Modal {
+    return new Modal(
+      this.page,
+      '[data-test="Save or Overwrite Dataset-modal"] .ant-modal',
+    );
+  }
+
   // ── Create Chart ──
 
   getCreateChartButton(): Locator {
diff --git a/superset-frontend/playwright/tests/sqllab/query-execution.spec.ts b/superset-frontend/playwright/tests/sqllab/query-execution.spec.ts
index 2ccfa0ed392..0cfdef1254a 100644
--- a/superset-frontend/playwright/tests/sqllab/query-execution.spec.ts
+++ b/superset-frontend/playwright/tests/sqllab/query-execution.spec.ts
@@ -17,10 +17,9 @@
  * under the License.
  */
 
-import { test, expect } from '../../helpers/fixtures/testAssets';
+import { test, expect } from '@playwright/test';
 import { SqlLabPage } from '../../pages/SqlLabPage';
 import { waitForPost } from '../../helpers/api/intercepts';
-import { apiGetSavedQuery } from '../../helpers/api/savedQuery';
 import { TIMEOUT } from '../../utils/constants';
 
 let sqlLabPage: SqlLabPage;
@@ -32,7 +31,9 @@ test.beforeEach(async ({ page }) => {
   await sqlLabPage.ensureEditorReady();
 });
 
-test('should execute a query and display results', async ({ page }) => {
+test('executes a simple SELECT query and displays results', async ({
+  page,
+}) => {
   // Verify the left sidebar database selector is visible and interactive (#38833)
   await expect(sqlLabPage.getDatabaseSelectorText()).toBeVisible();
 
@@ -54,57 +55,51 @@ test('should execute a query and display results', async ({ page }) => {
   expect(headers.some(h => h.includes('test_col'))).toBe(true);
 });
 
-test('should save and reload a query', async ({ page, testAssets }) => {
-  const queryText = 'SELECT 1 AS saved_test_col';
-  const savedQueryTitle = `pw_test_saved_query_${Date.now()}`;
-
-  // Verify left sidebar is interactive
-  await expect(sqlLabPage.getDatabaseSelectorText()).toBeVisible();
-
-  // Set and run query
-  await sqlLabPage.setQuery(queryText);
+test('shows error message for invalid SQL', async ({ page }) => {
+  await sqlLabPage.setQuery(
+    'SELECT * FROM a_table_that_does_not_exist_xyz_pw',
+  );
 
   const executePromise = waitForPost(page, 'api/v1/sqllab/execute/', {
     timeout: TIMEOUT.QUERY_EXECUTION,
   });
   await sqlLabPage.runQuery();
   await executePromise;
-  await sqlLabPage.waitForQueryResults();
 
-  // Open the save query modal
-  await sqlLabPage.clickSaveButton();
-  const saveModal = sqlLabPage.getSaveQueryModal();
-  await saveModal.waitForReady();
+  // Wait for error alert to render in south pane
+  const errorAlert = sqlLabPage.getErrorAlert();
+  await expect(errorAlert).toBeVisible({ timeout: TIMEOUT.QUERY_EXECUTION });
 
-  // Fill in the query name
-  await saveModal.body.locator('input[type="text"]').first().clear();
-  await saveModal.body
-    .locator('input[type="text"]')
-    .first()
-    .fill(savedQueryTitle);
+  // Verify the south pane contains an error indicator (engine-agnostic)
+  const southPane = sqlLabPage.getResultsPane();
+  await expect(southPane).toContainText(/error/i);
+});
 
-  // Save and intercept the API response
-  const savePromise = waitForPost(page, 'api/v1/saved_query/', {
-    timeout: TIMEOUT.API_RESPONSE,
+test('re-runs a query and refreshes results', async ({ page }) => {
+  // First query
+  await sqlLabPage.setQuery('SELECT 1 AS first_col');
+  const firstExecute = waitForPost(page, 'api/v1/sqllab/execute/', {
+    timeout: TIMEOUT.QUERY_EXECUTION,
   });
-  await saveModal.footer
-    .getByRole('button', { name: 'Save', exact: true })
-    .click();
-  const saveResponse = await savePromise;
-  expect(saveResponse.status()).toBe(201);
+  await sqlLabPage.runQuery();
+  const firstResponse = await firstExecute;
+  expect(firstResponse.status()).toBe(200);
+  await sqlLabPage.waitForQueryResults();
 
-  // Extract saved query ID for cleanup
-  const saveBody = await saveResponse.json();
-  const savedQueryId: number = saveBody.id ?? saveBody.result?.id;
-  expect(savedQueryId).toBeTruthy();
-  testAssets.trackSavedQuery(savedQueryId);
+  const firstHeaders = await sqlLabPage.getResultsGrid().getHeaderTexts();
+  expect(firstHeaders.some(h => h.includes('first_col'))).toBe(true);
 
-  // Verify the modal closed
-  await saveModal.waitForHidden();
+  // Second query (re-run with different SQL)
+  await sqlLabPage.setQuery('SELECT 2 AS second_col');
+  const secondExecute = waitForPost(page, 'api/v1/sqllab/execute/', {
+    timeout: TIMEOUT.QUERY_EXECUTION,
+  });
+  await sqlLabPage.runQuery();
+  const secondResponse = await secondExecute;
+  expect(secondResponse.status()).toBe(200);
+  await sqlLabPage.waitForQueryResults();
 
-  // Verify the saved query via API (round-trip persistence check)
-  const getResponse = await apiGetSavedQuery(page, savedQueryId);
-  const savedQuery = (await getResponse.json()).result;
-  expect(savedQuery.sql).toContain('saved_test_col');
-  expect(savedQuery.label).toBe(savedQueryTitle);
+  const secondHeaders = await sqlLabPage.getResultsGrid().getHeaderTexts();
+  expect(secondHeaders.some(h => h.includes('second_col'))).toBe(true);
+  expect(secondHeaders.some(h => h.includes('first_col'))).toBe(false);
 });
diff --git a/superset-frontend/playwright/tests/sqllab/query-execution.spec.ts b/superset-frontend/playwright/tests/sqllab/save-and-share.spec.ts
similarity index 57%
copy from superset-frontend/playwright/tests/sqllab/query-execution.spec.ts
copy to superset-frontend/playwright/tests/sqllab/save-and-share.spec.ts
index 2ccfa0ed392..2f5c2b3bc37 100644
--- a/superset-frontend/playwright/tests/sqllab/query-execution.spec.ts
+++ b/superset-frontend/playwright/tests/sqllab/save-and-share.spec.ts
@@ -17,8 +17,10 @@
  * under the License.
  */
 
-import { test, expect } from '../../helpers/fixtures/testAssets';
+import { expect } from '@playwright/test';
+import { test } from '../../helpers/fixtures/testAssets';
 import { SqlLabPage } from '../../pages/SqlLabPage';
+import { ExplorePage } from '../../pages/ExplorePage';
 import { waitForPost } from '../../helpers/api/intercepts';
 import { apiGetSavedQuery } from '../../helpers/api/savedQuery';
 import { TIMEOUT } from '../../utils/constants';
@@ -32,29 +34,10 @@ test.beforeEach(async ({ page }) => {
   await sqlLabPage.ensureEditorReady();
 });
 
-test('should execute a query and display results', async ({ page }) => {
-  // Verify the left sidebar database selector is visible and interactive (#38833)
-  await expect(sqlLabPage.getDatabaseSelectorText()).toBeVisible();
-
-  // Set the query
-  await sqlLabPage.setQuery('SELECT 1 AS test_col');
-
-  // Run query and wait for API response
-  const executePromise = waitForPost(page, 'api/v1/sqllab/execute/', {
-    timeout: TIMEOUT.QUERY_EXECUTION,
-  });
-  await sqlLabPage.runQuery();
-  const response = await executePromise;
-  expect(response.status()).toBe(200);
-
-  // Verify results appear in the AG Grid
-  await sqlLabPage.waitForQueryResults();
-  const grid = sqlLabPage.getResultsGrid();
-  const headers = await grid.getHeaderTexts();
-  expect(headers.some(h => h.includes('test_col'))).toBe(true);
-});
-
-test('should save and reload a query', async ({ page, testAssets }) => {
+test('saves a query and loads it from saved queries', async ({
+  page,
+  testAssets,
+}) => {
   const queryText = 'SELECT 1 AS saved_test_col';
   const savedQueryTitle = `pw_test_saved_query_${Date.now()}`;
 
@@ -108,3 +91,73 @@ test('should save and reload a query', async ({ page, testAssets }) => {
   expect(savedQuery.sql).toContain('saved_test_col');
   expect(savedQuery.label).toBe(savedQueryTitle);
 });
+
+test('creates a dataset from query results', async ({ page, testAssets }) => {
+  // Select database to enable the "Save dataset" toolbar button
+  await sqlLabPage.selectDatabase('examples');
+
+  const queryText = 'SELECT 1 AS ds_test_col';
+  await sqlLabPage.setQuery(queryText);
+
+  const executePromise = waitForPost(page, 'api/v1/sqllab/execute/', {
+    timeout: TIMEOUT.QUERY_EXECUTION,
+  });
+  await sqlLabPage.runQuery();
+  const executeResponse = await executePromise;
+
+  if (executeResponse.status() !== 200) {
+    test.skip(true, 'Query execution failed — database may not be configured');
+  }
+
+  await sqlLabPage.waitForQueryResults();
+
+  // Click "Save dataset" button in the toolbar
+  await sqlLabPage.clickSaveDatasetButton();
+
+  // Wait for the Save Dataset modal
+  const saveDatasetModal = sqlLabPage.getSaveDatasetModal();
+  await saveDatasetModal.waitForReady();
+
+  // Fill in a unique dataset name
+  const datasetName = `pw_test_dataset_${Date.now()}`;
+  const nameInput = saveDatasetModal.body.locator(
+    'input[placeholder="Dataset name"]',
+  );
+  await nameInput.clear();
+  await nameInput.fill(datasetName);
+
+  // Set up intercepts before clicking save:
+  // 1. Dataset creation API
+  // 2. New browser tab (SaveDatasetModal opens Explore via window.open)
+  const datasetCreatePromise = waitForPost(page, 'api/v1/dataset/', {
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  const newPagePromise = page.context().waitForEvent('page', {
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+
+  // Click "Save & Explore"
+  await saveDatasetModal.footer
+    .getByRole('button', { name: /Save & Explore/i })
+    .click();
+
+  // Capture dataset ID for cleanup
+  const createResponse = await datasetCreatePromise;
+  const createBody = await createResponse.json();
+  const datasetId: number = createBody.result?.id ?? createBody.id;
+  expect(datasetId).toBeTruthy();
+  testAssets.trackDataset(datasetId);
+
+  // Wait for the new tab with Explore page
+  const newPage = await newPagePromise;
+  await newPage.waitForLoadState();
+
+  const explorePage = new ExplorePage(newPage);
+  await explorePage.waitForPageLoad({ timeout: TIMEOUT.PAGE_LOAD });
+
+  // Verify the dataset name appears in the Explore datasource control
+  const loadedDatasetName = await explorePage.getDatasetName();
+  expect(loadedDatasetName).toContain(datasetName);
+
+  await newPage.close();
+});
diff --git a/superset-frontend/playwright/tests/sqllab/tabs.spec.ts b/superset-frontend/playwright/tests/sqllab/tabs.spec.ts
index a687815ebb1..1efeb941274 100644
--- a/superset-frontend/playwright/tests/sqllab/tabs.spec.ts
+++ b/superset-frontend/playwright/tests/sqllab/tabs.spec.ts
@@ -30,44 +30,70 @@ test.beforeEach(async ({ page }) => {
   await sqlLabPage.ensureEditorReady();
 });
 
-test('should create tabs that persist across page reload', async ({
-  page,
-}) => {
+test('creates a new tab via button', async () => {
   const initialTabCount = await sqlLabPage.getTabCount();
 
-  // Create a new tab and verify count incremented
   await sqlLabPage.addTab();
   await sqlLabPage.getEditor().waitForReady();
-  expect(await sqlLabPage.getTabCount()).toBe(initialTabCount + 1);
 
-  // Set up tab state intercept before making changes (avoid race with debounced save)
-  const tabStatePromise = waitForPost(page, 'tabstateview');
+  expect(await sqlLabPage.getTabCount()).toBe(initialTabCount + 1);
 
-  // Type a distinctive query so we can verify persistence
-  const testQuery = `SELECT 'persistence_test_${Date.now()}'`;
-  await sqlLabPage.setQuery(testQuery);
+  // Verify new tab has default SQL content
+  const defaultContent = await sqlLabPage.getQuery();
+  expect(defaultContent).toContain('SELECT');
 
-  // Trigger a save by blurring the editor (tab state saves on editor changes)
-  await page.locator('body').click();
-  await tabStatePromise;
+  // Clean up
+  await sqlLabPage.closeLastTab();
+  expect(await sqlLabPage.getTabCount()).toBe(initialTabCount);
+});
 
-  // Reload the page
-  await page.reload();
-  await sqlLabPage.waitForPageLoad();
-  await sqlLabPage.ensureEditorReady();
+test('closes a tab via close button', async () => {
+  const initialTabCount = await sqlLabPage.getTabCount();
 
-  // Verify the tab survived the reload
+  // Create a tab so we have something to close
+  await sqlLabPage.addTab();
+  await sqlLabPage.getEditor().waitForReady();
   expect(await sqlLabPage.getTabCount()).toBe(initialTabCount + 1);
 
-  // Verify the editor content survived the reload
-  const restoredQuery = await sqlLabPage.getQuery();
-  expect(restoredQuery).toContain('persistence_test_');
-
-  // Clean up: close the tab we created
+  // Close the tab via the × button
   await sqlLabPage.closeLastTab();
   expect(await sqlLabPage.getTabCount()).toBe(initialTabCount);
 });
 
+test('preserves query state when switching tabs', async () => {
+  const tabOneSql = `SELECT 'tab_one_${Date.now()}'`;
+  const tabTwoSql = `SELECT 'tab_two_${Date.now()}'`;
+
+  // Get the first tab's name for switching back later
+  const tabNames = await sqlLabPage.getTabNames();
+  const firstTabName = tabNames[tabNames.length - 1];
+
+  // Set query in the first (current) tab
+  await sqlLabPage.setQuery(tabOneSql);
+
+  // Create second tab and set a different query
+  await sqlLabPage.addTab();
+  await sqlLabPage.getEditor().waitForReady();
+  await sqlLabPage.setQuery(tabTwoSql);
+
+  // Switch back to first tab and verify its content is preserved
+  await sqlLabPage.getTab(firstTabName).click();
+  await sqlLabPage.getEditor().waitForReady();
+  const firstContent = await sqlLabPage.getQuery();
+  expect(firstContent).toContain('tab_one_');
+
+  // Switch to second tab and verify its content is preserved
+  const updatedNames = await sqlLabPage.getTabNames();
+  const secondTabName = updatedNames[updatedNames.length - 1];
+  await sqlLabPage.getTab(secondTabName).click();
+  await sqlLabPage.getEditor().waitForReady();
+  const secondContent = await sqlLabPage.getQuery();
+  expect(secondContent).toContain('tab_two_');
+
+  // Clean up
+  await sqlLabPage.closeLastTab();
+});
+
 test('should open new tab by keyboard shortcut with correct defaults', async ({
   page,
 }) => {