You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/05/08 22:56:36 UTC

(superset) branch fix-flakey-playwright-tests created (now 369c60811b8)

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

sadpandajoe pushed a change to branch fix-flakey-playwright-tests
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 369c60811b8 fix(playwright): extend de-flake hardening across list specs

This branch includes the following new commits:

     new 4c4f56cd8a2 fix(playwright): de-flake dashboard-list delete and bulk-export tests
     new 369c60811b8 fix(playwright): extend de-flake hardening across list specs

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(superset) 02/02: fix(playwright): extend de-flake hardening across list specs

Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

sadpandajoe pushed a commit to branch fix-flakey-playwright-tests
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 369c60811b8511fa8fce07f26231ea17c0b5f733
Author: Joe Li <jo...@preset.io>
AuthorDate: Thu May 7 23:52:23 2026 -0700

    fix(playwright): extend de-flake hardening across list specs
    
    Apply the same flake-pattern fixes from 4c4f56cd8a to the rest of the
    list-spec test surface so the hardening is consistent end-to-end.
    
    Spec-level (dashboard/chart/dataset):
    - Row-visibility waits on freshly-created rows now use TIMEOUT.API_RESPONSE
      (15s) so the asynchronous list query has time on slow CI.
    - Toast assertions switched from `expect(toast).toBeVisible()` to
      `toast.waitFor({ state: 'visible' })` so a fast auto-dismiss is detected.
    - Post-delete row assertions switched from `not.toBeVisible()` to
      `toHaveCount(0)` since deleted rows are removed from the DOM.
    - Bulk export specs (chart, dataset) now set `test.setTimeout(SLOW_TEST)`
      and `waitForGet({ timeout: SLOW_TEST })` to match the bulk-delete budget;
      the prior implicit budget was capped by Playwright's 30s test timeout.
    
    Page object:
    - BulkSelect.deselectRow mirrors selectRow's visibility wait + state
      assertion so any lingering selection surfaces at the call site.
---
 .../playwright/components/ListView/BulkSelect.ts   |  10 +-
 .../playwright/tests/chart/chart-list.spec.ts      | 101 +++++++++++++------
 .../tests/dashboard/dashboard-list.spec.ts         |  74 ++++++++------
 .../playwright/tests/dataset/dataset-list.spec.ts  | 108 ++++++++++++++-------
 4 files changed, 195 insertions(+), 98 deletions(-)

diff --git a/superset-frontend/playwright/components/ListView/BulkSelect.ts b/superset-frontend/playwright/components/ListView/BulkSelect.ts
index 3286a67e1b5..794803ec718 100644
--- a/superset-frontend/playwright/components/ListView/BulkSelect.ts
+++ b/superset-frontend/playwright/components/ListView/BulkSelect.ts
@@ -93,11 +93,17 @@ export class BulkSelect {
   }
 
   /**
-   * Deselects a row's checkbox in bulk select mode
+   * Deselects a row's checkbox in bulk select mode.
+   * Mirrors selectRow: waits for visibility and asserts the unchecked state
+   * so any lingering selection surfaces here rather than as a stale bulk-action
+   * count later.
    * @param rowName - The name/text identifying the row to deselect
    */
   async deselectRow(rowName: string): Promise<void> {
-    await this.getRowCheckbox(rowName).uncheck();
+    const checkbox = this.getRowCheckbox(rowName);
+    await checkbox.element.waitFor({ state: 'visible' });
+    await checkbox.uncheck();
+    await expect(checkbox.element).not.toBeChecked();
   }
 
   /**
diff --git a/superset-frontend/playwright/tests/chart/chart-list.spec.ts b/superset-frontend/playwright/tests/chart/chart-list.spec.ts
index b7aad6ffda8..8e78361fbac 100644
--- a/superset-frontend/playwright/tests/chart/chart-list.spec.ts
+++ b/superset-frontend/playwright/tests/chart/chart-list.spec.ts
@@ -32,6 +32,7 @@ import {
   expectStatusOneOf,
   expectValidExportZip,
 } from '../../helpers/api/assertions';
+import { TIMEOUT } from '../../utils/constants';
 
 /**
  * Extend testWithAssets with chartListPage navigation (beforeEach equivalent).
@@ -62,8 +63,11 @@ test('should delete a chart with confirmation', async ({
   await chartListPage.goto();
   await chartListPage.waitForTableLoad();
 
-  // Verify chart is visible in list
-  await expect(chartListPage.getChartRow(chartName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created chart appears.
+  await expect(chartListPage.getChartRow(chartName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Click delete action button
   await chartListPage.clickDeleteAction(chartName);
@@ -81,12 +85,13 @@ test('should delete a chart with confirmation', async ({
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears. Use waitFor instead of toBeVisible so we
+  // detect the toast even if it auto-dismisses on a fast machine.
   const toast = new Toast(page);
-  await expect(toast.getSuccess()).toBeVisible();
+  await toast.getSuccess().waitFor({ state: 'visible' });
 
-  // Verify chart is removed from list
-  await expect(chartListPage.getChartRow(chartName)).not.toBeVisible();
+  // Verify chart is removed from list (deleted rows leave the DOM)
+  await expect(chartListPage.getChartRow(chartName)).toHaveCount(0);
 
   // Backend verification: API returns 404
   await expectDeleted(page, ENDPOINTS.CHART, chartId, {
@@ -111,8 +116,11 @@ test('should edit chart name via properties modal', async ({
   await chartListPage.goto();
   await chartListPage.waitForTableLoad();
 
-  // Verify chart is visible in list
-  await expect(chartListPage.getChartRow(chartName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created chart appears.
+  await expect(chartListPage.getChartRow(chartName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Click edit action to open properties modal
   await chartListPage.clickEditAction(chartName);
@@ -137,9 +145,10 @@ test('should edit chart name via properties modal', async ({
   // Modal should close
   await propertiesModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears. Use waitFor instead of toBeVisible so we
+  // detect the toast even if it auto-dismisses on a fast machine.
   const toast = new Toast(page);
-  await expect(toast.getSuccess()).toBeVisible();
+  await toast.getSuccess().waitFor({ state: 'visible' });
 
   // Backend verification: API returns updated name
   const response = await apiGetChart(page, chartId);
@@ -164,8 +173,11 @@ test('should export a chart as a zip file', async ({
   await chartListPage.goto();
   await chartListPage.waitForTableLoad();
 
-  // Verify chart is visible in list
-  await expect(chartListPage.getChartRow(chartName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created chart appears.
+  await expect(chartListPage.getChartRow(chartName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Set up API response intercept for export endpoint
   const exportResponsePromise = waitForGet(page, ENDPOINTS.CHART_EXPORT);
@@ -202,9 +214,14 @@ test('should bulk delete multiple charts', async ({
   await chartListPage.goto();
   await chartListPage.waitForTableLoad();
 
-  // Verify both charts are visible in list
-  await expect(chartListPage.getChartRow(chart1.name)).toBeVisible();
-  await expect(chartListPage.getChartRow(chart2.name)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created charts appear.
+  await expect(chartListPage.getChartRow(chart1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(chartListPage.getChartRow(chart2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Enable bulk select mode
   await chartListPage.clickBulkSelectButton();
@@ -229,13 +246,14 @@ test('should bulk delete multiple charts', async ({
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears. Use waitFor instead of toBeVisible so we
+  // detect the toast even if it auto-dismisses on a fast machine.
   const toast = new Toast(page);
-  await expect(toast.getSuccess()).toBeVisible();
+  await toast.getSuccess().waitFor({ state: 'visible' });
 
-  // Verify both charts are removed from list
-  await expect(chartListPage.getChartRow(chart1.name)).not.toBeVisible();
-  await expect(chartListPage.getChartRow(chart2.name)).not.toBeVisible();
+  // Verify both charts are removed from list (deleted rows leave the DOM)
+  await expect(chartListPage.getChartRow(chart1.name)).toHaveCount(0);
+  await expect(chartListPage.getChartRow(chart2.name)).toHaveCount(0);
 
   // Backend verification: Both return 404
   for (const chart of [chart1, chart2]) {
@@ -259,8 +277,11 @@ test('should edit chart name from card view', async ({ page, testAssets }) => {
   await cardListPage.gotoCardView();
   await cardListPage.waitForCardLoad();
 
-  // Verify chart card is visible
-  await expect(cardListPage.getChartCard(chartName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created chart card appears.
+  await expect(cardListPage.getChartCard(chartName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Open card dropdown and click edit
   await cardListPage.clickCardEditAction(chartName);
@@ -285,13 +306,17 @@ test('should edit chart name from card view', async ({ page, testAssets }) => {
   // Modal should close
   await propertiesModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears. Use waitFor instead of toBeVisible so we
+  // detect the toast even if it auto-dismisses on a fast machine.
   const toast = new Toast(page);
-  await expect(toast.getSuccess()).toBeVisible();
+  await toast.getSuccess().waitFor({ state: 'visible' });
 
   // Verify the renamed card appears in card view and old name is gone
-  await expect(cardListPage.getChartCard(newName)).toBeVisible();
-  await expect(cardListPage.getChartCard(chartName)).not.toBeVisible();
+  // (the old card name is removed from the DOM after the rename re-render).
+  await expect(cardListPage.getChartCard(newName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(cardListPage.getChartCard(chartName)).toHaveCount(0);
 
   // Backend verification: API returns updated name
   const response = await apiGetChart(page, chartId);
@@ -304,6 +329,11 @@ test('should bulk export multiple charts', async ({
   chartListPage,
   testAssets,
 }) => {
+  // Chains create×2 → refresh → bulk select → export. Matches the
+  // sibling bulk-delete test's budget so the export response wait below
+  // can exceed the 30s default without hitting the test timeout.
+  test.setTimeout(TIMEOUT.SLOW_TEST);
+
   // Create 2 throwaway charts for bulk export
   const [chart1, chart2] = await Promise.all([
     createTestChart(page, testAssets, test.info(), {
@@ -318,9 +348,14 @@ test('should bulk export multiple charts', async ({
   await chartListPage.goto();
   await chartListPage.waitForTableLoad();
 
-  // Verify both charts are visible in list
-  await expect(chartListPage.getChartRow(chart1.name)).toBeVisible();
-  await expect(chartListPage.getChartRow(chart2.name)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created charts appear.
+  await expect(chartListPage.getChartRow(chart1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(chartListPage.getChartRow(chart2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Enable bulk select mode
   await chartListPage.clickBulkSelectButton();
@@ -329,8 +364,12 @@ test('should bulk export multiple charts', async ({
   await chartListPage.selectChartCheckbox(chart1.name);
   await chartListPage.selectChartCheckbox(chart2.name);
 
-  // Set up API response intercept for export endpoint
-  const exportResponsePromise = waitForGet(page, ENDPOINTS.CHART_EXPORT);
+  // Set up API response intercept BEFORE the click that triggers it.
+  // Exports of multiple charts can take longer than 30s under load,
+  // so use SLOW_TEST instead of the default test-timeout-bound budget.
+  const exportResponsePromise = waitForGet(page, ENDPOINTS.CHART_EXPORT, {
+    timeout: TIMEOUT.SLOW_TEST,
+  });
 
   // Click bulk export action
   await chartListPage.clickBulkAction('Export');
diff --git a/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts b/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts
index 6e6073c3f11..1d14bda5d73 100644
--- a/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts
+++ b/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts
@@ -121,8 +121,11 @@ test('should export a dashboard as a zip file', async ({
   await dashboardListPage.goto();
   await dashboardListPage.waitForTableLoad();
 
-  // Verify dashboard is visible in list
-  await expect(dashboardListPage.getDashboardRow(dashboardName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dashboard appears.
+  await expect(dashboardListPage.getDashboardRow(dashboardName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Set up API response intercept for export endpoint
   const exportResponsePromise = waitForGet(page, ENDPOINTS.DASHBOARD_EXPORT);
@@ -159,13 +162,14 @@ test('should bulk delete multiple dashboards', async ({
   await dashboardListPage.goto();
   await dashboardListPage.waitForTableLoad();
 
-  // Verify both dashboards are visible in list
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard1.name),
-  ).toBeVisible();
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard2.name),
-  ).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dashboards appear.
+  await expect(dashboardListPage.getDashboardRow(dashboard1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(dashboardListPage.getDashboardRow(dashboard2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Enable bulk select mode
   await dashboardListPage.clickBulkSelectButton();
@@ -190,17 +194,18 @@ test('should bulk delete multiple dashboards', async ({
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears. Use waitFor instead of toBeVisible so we
+  // detect the toast even if it auto-dismisses on a fast machine.
   const toast = new Toast(page);
-  await expect(toast.getSuccess()).toBeVisible();
+  await toast.getSuccess().waitFor({ state: 'visible' });
 
-  // Verify both dashboards are removed from list
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard1.name),
-  ).not.toBeVisible();
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard2.name),
-  ).not.toBeVisible();
+  // Verify both dashboards are removed from list (deleted rows leave the DOM)
+  await expect(dashboardListPage.getDashboardRow(dashboard1.name)).toHaveCount(
+    0,
+  );
+  await expect(dashboardListPage.getDashboardRow(dashboard2.name)).toHaveCount(
+    0,
+  );
 
   // Backend verification: Both return 404
   for (const dashboard of [dashboard1, dashboard2]) {
@@ -215,6 +220,11 @@ test('should bulk export multiple dashboards', async ({
   dashboardListPage,
   testAssets,
 }) => {
+  // Chains create×2 → refresh → bulk select → export. Matches the
+  // sibling bulk-delete test's budget so the export response wait below
+  // can exceed the 30s default without hitting the test timeout.
+  test.setTimeout(TIMEOUT.SLOW_TEST);
+
   // Create 2 throwaway dashboards for bulk export
   const [dashboard1, dashboard2] = await Promise.all([
     createTestDashboard(page, testAssets, test.info(), {
@@ -246,10 +256,10 @@ test('should bulk export multiple dashboards', async ({
   await dashboardListPage.selectDashboardCheckbox(dashboard2.name);
 
   // Set up API response intercept BEFORE the click that triggers it.
-  // Use the configured API_RESPONSE timeout — exports of multiple dashboards
-  // can run longer than Playwright's default 30s timeout under load.
+  // Exports of multiple dashboards can take longer than 30s under load,
+  // so use SLOW_TEST instead of the default test-timeout-bound budget.
   const exportResponsePromise = waitForGet(page, ENDPOINTS.DASHBOARD_EXPORT, {
-    timeout: TIMEOUT.API_RESPONSE,
+    timeout: TIMEOUT.SLOW_TEST,
   });
 
   // Click bulk export action (waits for the action button to render)
@@ -300,12 +310,12 @@ test.describe('import dashboard', () => {
       label: `Dashboard ${dashboardId}`,
     });
 
-    // Refresh to confirm dashboard is no longer in the list
+    // Refresh to confirm dashboard is no longer in the list (deleted rows leave the DOM)
     await dashboardListPage.goto();
     await dashboardListPage.waitForTableLoad();
-    await expect(
-      dashboardListPage.getDashboardRow(dashboardName),
-    ).not.toBeVisible();
+    await expect(dashboardListPage.getDashboardRow(dashboardName)).toHaveCount(
+      0,
+    );
 
     // Click the import button
     await dashboardListPage.clickImportButton();
@@ -357,18 +367,20 @@ test.describe('import dashboard', () => {
     // Modal should close on success
     await importModal.waitForHidden({ timeout: TIMEOUT.FILE_IMPORT });
 
-    // Verify success toast appears
+    // Verify success toast appears. Use waitFor instead of toBeVisible so we
+    // detect the toast even if it auto-dismisses on a fast machine.
     const toast = new Toast(page);
-    await expect(toast.getSuccess()).toBeVisible({ timeout: 10000 });
+    await toast.getSuccess().waitFor({ state: 'visible', timeout: 10000 });
 
     // Refresh to see the imported dashboard
     await dashboardListPage.goto();
     await dashboardListPage.waitForTableLoad();
 
-    // Verify dashboard appears in list
-    await expect(
-      dashboardListPage.getDashboardRow(dashboardName),
-    ).toBeVisible();
+    // The list query is asynchronous; allow extra time on slow CI before the
+    // freshly-imported dashboard appears.
+    await expect(dashboardListPage.getDashboardRow(dashboardName)).toBeVisible({
+      timeout: TIMEOUT.API_RESPONSE,
+    });
 
     // Track for cleanup: look up the reimported dashboard by title
     const reimported = await getDashboardByName(page, dashboardName);
diff --git a/superset-frontend/playwright/tests/dataset/dataset-list.spec.ts b/superset-frontend/playwright/tests/dataset/dataset-list.spec.ts
index e0b281f6232..26c9c739887 100644
--- a/superset-frontend/playwright/tests/dataset/dataset-list.spec.ts
+++ b/superset-frontend/playwright/tests/dataset/dataset-list.spec.ts
@@ -107,8 +107,11 @@ test('should delete a dataset with confirmation', async ({
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
 
-  // Verify dataset is visible in list
-  await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dataset appears.
+  await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Click delete action button
   await datasetListPage.clickDeleteAction(datasetName);
@@ -126,14 +129,15 @@ test('should delete a dataset with confirmation', async ({
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears with correct message
+  // Verify success toast appears with correct message. Use waitFor instead of
+  // toBeVisible so we detect the toast even if it auto-dismisses fast.
   const toast = new Toast(page);
   const successToast = toast.getSuccess();
-  await expect(successToast).toBeVisible();
+  await successToast.waitFor({ state: 'visible' });
   await expect(toast.getMessage()).toContainText('Deleted');
 
-  // Verify dataset is removed from list
-  await expect(datasetListPage.getDatasetRow(datasetName)).not.toBeVisible();
+  // Verify dataset is removed from list (deleted rows leave the DOM)
+  await expect(datasetListPage.getDatasetRow(datasetName)).toHaveCount(0);
 
   // Verify via API that dataset no longer exists (404)
   await expectDeleted(page, ENDPOINTS.DATASET, datasetId, {
@@ -155,10 +159,13 @@ test('should duplicate a dataset with new name', async ({
   );
   const duplicateName = `duplicate_${Date.now()}_${test.info().parallelIndex}`;
 
-  // Navigate to list and verify original dataset is visible
+  // Navigate to list and verify original dataset is visible.
+  // The list query is asynchronous; allow extra time on slow CI.
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
-  await expect(datasetListPage.getDatasetRow(originalName)).toBeVisible();
+  await expect(datasetListPage.getDatasetRow(originalName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Set up response intercept to capture duplicate dataset ID
   const duplicateResponsePromise = waitForPost(
@@ -201,9 +208,14 @@ test('should duplicate a dataset with new name', async ({
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
 
-  // Verify both datasets exist in list
-  await expect(datasetListPage.getDatasetRow(originalName)).toBeVisible();
-  await expect(datasetListPage.getDatasetRow(duplicateName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // duplicate appears alongside the original.
+  await expect(datasetListPage.getDatasetRow(originalName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(datasetListPage.getDatasetRow(duplicateName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // API Verification: Fetch both datasets via detail API for consistent comparison
   // (list API may return undefined for fields that detail API returns as null)
@@ -256,6 +268,11 @@ test('should export multiple datasets via bulk select action', async ({
   datasetListPage,
   testAssets,
 }) => {
+  // Chains create×2 → refresh → bulk select → export. Matches the
+  // sibling bulk-delete test's budget so the export response wait below
+  // can exceed the 30s default without hitting the test timeout.
+  test.setTimeout(TIMEOUT.SLOW_TEST);
+
   // Create 2 throwaway datasets for bulk export
   const [dataset1, dataset2] = await Promise.all([
     createTestDataset(page, testAssets, test.info(), {
@@ -270,9 +287,14 @@ test('should export multiple datasets via bulk select action', async ({
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
 
-  // Verify both datasets are visible in list
-  await expect(datasetListPage.getDatasetRow(dataset1.name)).toBeVisible();
-  await expect(datasetListPage.getDatasetRow(dataset2.name)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created datasets appear.
+  await expect(datasetListPage.getDatasetRow(dataset1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(datasetListPage.getDatasetRow(dataset2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Enable bulk select mode
   await datasetListPage.clickBulkSelectButton();
@@ -281,8 +303,12 @@ test('should export multiple datasets via bulk select action', async ({
   await datasetListPage.selectDatasetCheckbox(dataset1.name);
   await datasetListPage.selectDatasetCheckbox(dataset2.name);
 
-  // Set up API response intercept for export endpoint
-  const exportResponsePromise = waitForGet(page, ENDPOINTS.DATASET_EXPORT);
+  // Set up API response intercept BEFORE the click that triggers it.
+  // Exports of multiple datasets can take longer than 30s under load,
+  // so use SLOW_TEST instead of the default test-timeout-bound budget.
+  const exportResponsePromise = waitForGet(page, ENDPOINTS.DATASET_EXPORT, {
+    timeout: TIMEOUT.SLOW_TEST,
+  });
 
   // Click bulk export action
   await datasetListPage.clickBulkAction('Export');
@@ -312,8 +338,11 @@ test('should edit dataset name via modal', async ({
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
 
-  // Verify dataset is visible in list
-  await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dataset appears.
+  await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Click edit action to open modal
   await datasetListPage.clickEditAction(datasetName);
@@ -348,9 +377,10 @@ test('should edit dataset name via modal', async ({
   // Modal should close
   await editModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears. Use waitFor instead of toBeVisible so we
+  // detect the toast even if it auto-dismisses on a fast machine.
   const toast = new Toast(page);
-  await expect(toast.getSuccess()).toBeVisible({ timeout: 10000 });
+  await toast.getSuccess().waitFor({ state: 'visible', timeout: 10000 });
 
   // Verify via API that name was saved
   const updatedDatasetRes = await apiGetDataset(page, datasetId);
@@ -377,9 +407,14 @@ test('should bulk delete multiple datasets', async ({
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
 
-  // Verify both datasets are visible in list
-  await expect(datasetListPage.getDatasetRow(dataset1.name)).toBeVisible();
-  await expect(datasetListPage.getDatasetRow(dataset2.name)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created datasets appear.
+  await expect(datasetListPage.getDatasetRow(dataset1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(datasetListPage.getDatasetRow(dataset2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Enable bulk select mode
   await datasetListPage.clickBulkSelectButton();
@@ -404,13 +439,14 @@ test('should bulk delete multiple datasets', async ({
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears. Use waitFor instead of toBeVisible so we
+  // detect the toast even if it auto-dismisses on a fast machine.
   const toast = new Toast(page);
-  await expect(toast.getSuccess()).toBeVisible();
+  await toast.getSuccess().waitFor({ state: 'visible' });
 
-  // Verify both datasets are removed from list
-  await expect(datasetListPage.getDatasetRow(dataset1.name)).not.toBeVisible();
-  await expect(datasetListPage.getDatasetRow(dataset2.name)).not.toBeVisible();
+  // Verify both datasets are removed from list (deleted rows leave the DOM)
+  await expect(datasetListPage.getDatasetRow(dataset1.name)).toHaveCount(0);
+  await expect(datasetListPage.getDatasetRow(dataset2.name)).toHaveCount(0);
 
   // Verify via API that datasets no longer exist (404)
   await expectDeleted(page, ENDPOINTS.DATASET, dataset1.id, {
@@ -455,10 +491,10 @@ test.describe('import dataset', () => {
       label: `Dataset ${datasetId}`,
     });
 
-    // Refresh to confirm dataset is no longer in the list
+    // Refresh to confirm dataset is no longer in the list (deleted rows leave the DOM)
     await datasetListPage.goto();
     await datasetListPage.waitForTableLoad();
-    await expect(datasetListPage.getDatasetRow(datasetName)).not.toBeVisible();
+    await expect(datasetListPage.getDatasetRow(datasetName)).toHaveCount(0);
 
     // Click the import button
     await datasetListPage.clickImportButton();
@@ -507,16 +543,20 @@ test.describe('import dataset', () => {
     // Modal should close on success
     await importModal.waitForHidden({ timeout: TIMEOUT.FILE_IMPORT });
 
-    // Verify success toast appears
+    // Verify success toast appears. Use waitFor instead of toBeVisible so we
+    // detect the toast even if it auto-dismisses on a fast machine.
     const toast = new Toast(page);
-    await expect(toast.getSuccess()).toBeVisible({ timeout: 10000 });
+    await toast.getSuccess().waitFor({ state: 'visible', timeout: 10000 });
 
     // Refresh to see the imported dataset
     await datasetListPage.goto();
     await datasetListPage.waitForTableLoad();
 
-    // Verify dataset appears in list
-    await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible();
+    // The list query is asynchronous; allow extra time on slow CI before the
+    // freshly-imported dataset appears.
+    await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible({
+      timeout: TIMEOUT.API_RESPONSE,
+    });
 
     // Track for cleanup: the dataset import API returns {"message": "OK"}
     // with no ID, so look up the reimported dataset by name.


(superset) 01/02: fix(playwright): de-flake dashboard-list delete and bulk-export tests

Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

sadpandajoe pushed a commit to branch fix-flakey-playwright-tests
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 4c4f56cd8a28879002512ebfcc49fe127344f755
Author: Joe Li <jo...@preset.io>
AuthorDate: Wed May 6 22:00:13 2026 -0700

    fix(playwright): de-flake dashboard-list delete and bulk-export tests
    
    Harden the delete-confirmation modal and bulk-select page objects against
    known Playwright race patterns: explicit data-test selectors, an
    enabled-state wait before clicking the disabled-by-default Delete button,
    a header-toggle wait after enabling bulk select, and visibility gates on
    checkboxes and action buttons that mount only after a row is selected.
    Also extend list-row visibility timeouts for slow CI and switch the
    post-delete assertion to toHaveCount(0).
    
    Co-Authored-By: Claude Opus 4.7 (1M context) <no...@anthropic.com>
---
 .../playwright/components/ListView/BulkSelect.ts   | 30 ++++++++++---
 .../components/modals/DeleteConfirmationModal.ts   | 28 +++++++++++--
 .../tests/dashboard/dashboard-list.spec.ts         | 49 ++++++++++++----------
 3 files changed, 76 insertions(+), 31 deletions(-)

diff --git a/superset-frontend/playwright/components/ListView/BulkSelect.ts b/superset-frontend/playwright/components/ListView/BulkSelect.ts
index 3e4d2dbf87b..3286a67e1b5 100644
--- a/superset-frontend/playwright/components/ListView/BulkSelect.ts
+++ b/superset-frontend/playwright/components/ListView/BulkSelect.ts
@@ -17,12 +17,13 @@
  * under the License.
  */
 
-import { Locator, Page } from '@playwright/test';
+import { Locator, Page, expect } from '@playwright/test';
 import { Button, Checkbox, Table } from '../core';
 
 const BULK_SELECT_SELECTORS = {
   CONTROLS: '[data-test="bulk-select-controls"]',
   ACTION: '[data-test="bulk-select-action"]',
+  HEADER_TOGGLE: '[data-test="header-toggle-all"]',
 } as const;
 
 /**
@@ -56,10 +57,17 @@ export class BulkSelect {
   }
 
   /**
-   * Enables bulk selection mode by clicking the toggle button
+   * Enables bulk selection mode by clicking the toggle button.
+   *
+   * Waits for the bulk-select column header checkbox to render so the next
+   * row interaction does not race the table re-render that adds the
+   * checkbox column.
    */
   async enable(): Promise<void> {
     await this.getToggleButton().click();
+    await this.page
+      .locator(BULK_SELECT_SELECTORS.HEADER_TOGGLE)
+      .waitFor({ state: 'visible' });
   }
 
   /**
@@ -72,11 +80,16 @@ export class BulkSelect {
   }
 
   /**
-   * Selects a row's checkbox in bulk select mode
+   * Selects a row's checkbox in bulk select mode.
+   * Asserts the checkbox is checked afterwards so any state-update race
+   * surfaces here rather than as a missing bulk-action button later.
    * @param rowName - The name/text identifying the row to select
    */
   async selectRow(rowName: string): Promise<void> {
-    await this.getRowCheckbox(rowName).check();
+    const checkbox = this.getRowCheckbox(rowName);
+    await checkbox.element.waitFor({ state: 'visible' });
+    await checkbox.check();
+    await expect(checkbox.element).toBeChecked();
   }
 
   /**
@@ -107,10 +120,15 @@ export class BulkSelect {
   }
 
   /**
-   * Clicks a bulk action button by name (e.g., "Export", "Delete")
+   * Clicks a bulk action button by name (e.g., "Export", "Delete").
+   *
+   * The action buttons only render once a row is selected; waiting for
+   * visibility makes that timing contract explicit before the click.
    * @param actionName - The name of the bulk action to click
    */
   async clickAction(actionName: string): Promise<void> {
-    await this.getActionButton(actionName).click();
+    const button = this.getActionButton(actionName);
+    await button.element.waitFor({ state: 'visible' });
+    await button.click();
   }
 }
diff --git a/superset-frontend/playwright/components/modals/DeleteConfirmationModal.ts b/superset-frontend/playwright/components/modals/DeleteConfirmationModal.ts
index 44dca9e6b26..776b44838da 100644
--- a/superset-frontend/playwright/components/modals/DeleteConfirmationModal.ts
+++ b/superset-frontend/playwright/components/modals/DeleteConfirmationModal.ts
@@ -17,6 +17,7 @@
  * under the License.
  */
 
+import { expect } from '@playwright/test';
 import { Modal, Input } from '../core';
 
 /**
@@ -27,7 +28,8 @@ import { Modal, Input } from '../core';
  */
 export class DeleteConfirmationModal extends Modal {
   private static readonly SELECTORS = {
-    CONFIRMATION_INPUT: 'input[type="text"]',
+    CONFIRMATION_INPUT: '[data-test="delete-modal-input"]',
+    CONFIRM_BUTTON: '[data-test="modal-confirm-button"]',
   };
 
   /**
@@ -36,12 +38,16 @@ export class DeleteConfirmationModal extends Modal {
   private get confirmationInput(): Input {
     return new Input(
       this.page,
-      this.body.locator(DeleteConfirmationModal.SELECTORS.CONFIRMATION_INPUT),
+      this.element.locator(
+        DeleteConfirmationModal.SELECTORS.CONFIRMATION_INPUT,
+      ),
     );
   }
 
   /**
    * Fills the confirmation input with the specified text.
+   * Waits for the input to be visible before filling so callers don't race
+   * with the modal's open animation / focus effect.
    *
    * @param confirmationText - The text to type
    * @param options - Optional fill options (timeout, force)
@@ -57,11 +63,21 @@ export class DeleteConfirmationModal extends Modal {
     confirmationText: string,
     options?: { timeout?: number; force?: boolean },
   ): Promise<void> {
+    await this.confirmationInput.element.waitFor({
+      state: 'visible',
+      timeout: options?.timeout,
+    });
     await this.confirmationInput.fill(confirmationText, options);
   }
 
   /**
-   * Clicks the Delete button in the footer
+   * Clicks the Delete button in the footer.
+   *
+   * Waits for the confirm button to become enabled before clicking. The button
+   * is disabled until the confirmation text matches "DELETE", and React's state
+   * update from fillConfirmationInput is asynchronous; the explicit
+   * toBeEnabled() check makes the timing contract explicit and avoids racing
+   * the disabled→enabled transition.
    *
    * @param options - Optional click options (timeout, force, delay)
    */
@@ -70,6 +86,10 @@ export class DeleteConfirmationModal extends Modal {
     force?: boolean;
     delay?: number;
   }): Promise<void> {
-    await this.clickFooterButton('Delete', options);
+    const confirmButton = this.element.locator(
+      DeleteConfirmationModal.SELECTORS.CONFIRM_BUTTON,
+    );
+    await expect(confirmButton).toBeEnabled({ timeout: options?.timeout });
+    await confirmButton.click(options);
   }
 }
diff --git a/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts b/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts
index 1def93acecb..6e6073c3f11 100644
--- a/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts
+++ b/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts
@@ -68,33 +68,35 @@ test('should delete a dashboard with confirmation', async ({
   await dashboardListPage.goto();
   await dashboardListPage.waitForTableLoad();
 
-  // Verify dashboard is visible in list
-  await expect(dashboardListPage.getDashboardRow(dashboardName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dashboard appears.
+  await expect(dashboardListPage.getDashboardRow(dashboardName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Click delete action button
   await dashboardListPage.clickDeleteAction(dashboardName);
 
   // Delete confirmation modal should appear
   const deleteModal = new DeleteConfirmationModal(page);
-  await deleteModal.waitForVisible();
+  await deleteModal.waitForReady();
 
   // Type "DELETE" to confirm
   await deleteModal.fillConfirmationInput('DELETE');
 
-  // Click the Delete button
+  // Click the Delete button (waits for it to become enabled)
   await deleteModal.clickDelete();
 
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears. Use waitFor instead of toBeVisible so we
+  // detect the toast even if it auto-dismisses on a fast machine.
   const toast = new Toast(page);
-  await expect(toast.getSuccess()).toBeVisible();
+  await toast.getSuccess().waitFor({ state: 'visible' });
 
   // Verify dashboard is removed from list
-  await expect(
-    dashboardListPage.getDashboardRow(dashboardName),
-  ).not.toBeVisible();
+  await expect(dashboardListPage.getDashboardRow(dashboardName)).toHaveCount(0);
 
   // Backend verification: API returns 404
   await expectDeleted(page, ENDPOINTS.DASHBOARD, dashboardId, {
@@ -227,25 +229,30 @@ test('should bulk export multiple dashboards', async ({
   await dashboardListPage.goto();
   await dashboardListPage.waitForTableLoad();
 
-  // Verify both dashboards are visible in list
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard1.name),
-  ).toBeVisible();
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard2.name),
-  ).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dashboards appear.
+  await expect(dashboardListPage.getDashboardRow(dashboard1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(dashboardListPage.getDashboardRow(dashboard2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
-  // Enable bulk select mode
+  // Enable bulk select mode (waits for the checkbox column to render)
   await dashboardListPage.clickBulkSelectButton();
 
-  // Select both dashboards
+  // Select both dashboards (each call asserts the checkbox is checked)
   await dashboardListPage.selectDashboardCheckbox(dashboard1.name);
   await dashboardListPage.selectDashboardCheckbox(dashboard2.name);
 
-  // Set up API response intercept for export endpoint
-  const exportResponsePromise = waitForGet(page, ENDPOINTS.DASHBOARD_EXPORT);
+  // Set up API response intercept BEFORE the click that triggers it.
+  // Use the configured API_RESPONSE timeout — exports of multiple dashboards
+  // can run longer than Playwright's default 30s timeout under load.
+  const exportResponsePromise = waitForGet(page, ENDPOINTS.DASHBOARD_EXPORT, {
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
-  // Click bulk export action
+  // Click bulk export action (waits for the action button to render)
   await dashboardListPage.clickBulkAction('Export');
 
   // Wait for export API response and validate zip contains both dashboards