From 77fead8f4ce05e5e7e4843cb5f4f377039ee859e Mon Sep 17 00:00:00 2001 From: Joe Li Date: Thu, 14 May 2026 10:28:09 -0700 Subject: [PATCH] test(playwright): standardize delete-modal wait and clarify count-vs-visibility comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use waitForVisible() on the dashboard single-delete site to match the other four delete flows; fillConfirmationInput already auto-waits for the input, so the lone waitForReady() added no extra signal. Rewrite "(deleted rows leave the DOM)" — "leave" is ambiguous in English (depart vs remain) — to spell out that the row is removed and that's why these assertions use toHaveCount(0) instead of not.toBeVisible(). Co-Authored-By: Claude Sonnet 4.6 --- superset-frontend/playwright/tests/chart/chart-list.spec.ts | 4 ++-- .../playwright/tests/dashboard/dashboard-list.spec.ts | 6 +++--- .../playwright/tests/dataset/dataset-list.spec.ts | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/superset-frontend/playwright/tests/chart/chart-list.spec.ts b/superset-frontend/playwright/tests/chart/chart-list.spec.ts index 669b041bd1c..cade2e43117 100644 --- a/superset-frontend/playwright/tests/chart/chart-list.spec.ts +++ b/superset-frontend/playwright/tests/chart/chart-list.spec.ts @@ -89,7 +89,7 @@ test('should delete a chart with confirmation', async ({ const toast = new Toast(page); await expect(toast.getSuccess()).toBeVisible(); - // Verify chart is removed from list (deleted rows leave the DOM) + // Verify chart is removed from list (deleted rows are removed from the DOM, so assert count rather than visibility) await expect(chartListPage.getChartRow(chartName)).toHaveCount(0); // Backend verification: API returns 404 @@ -248,7 +248,7 @@ test('should bulk delete multiple charts', async ({ const toast = new Toast(page); await expect(toast.getSuccess()).toBeVisible(); - // Verify both charts are removed from list (deleted rows leave the DOM) + // Verify both charts are removed from list (deleted rows are removed from the DOM, so assert count rather than visibility) await expect(chartListPage.getChartRow(chart1.name)).toHaveCount(0); await expect(chartListPage.getChartRow(chart2.name)).toHaveCount(0); diff --git a/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts b/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts index d1ed3561277..522bb6fa75f 100644 --- a/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts +++ b/superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts @@ -79,7 +79,7 @@ test('should delete a dashboard with confirmation', async ({ // Delete confirmation modal should appear const deleteModal = new DeleteConfirmationModal(page); - await deleteModal.waitForReady(); + await deleteModal.waitForVisible(); // Type "DELETE" to confirm await deleteModal.fillConfirmationInput('DELETE'); @@ -197,7 +197,7 @@ test('should bulk delete multiple dashboards', async ({ const toast = new Toast(page); await expect(toast.getSuccess()).toBeVisible(); - // Verify both dashboards are removed from list (deleted rows leave the DOM) + // Verify both dashboards are removed from list (deleted rows are removed from the DOM, so assert count rather than visibility) await expect(dashboardListPage.getDashboardRow(dashboard1.name)).toHaveCount( 0, ); @@ -308,7 +308,7 @@ test.describe('import dashboard', () => { label: `Dashboard ${dashboardId}`, }); - // Refresh to confirm dashboard is no longer in the list (deleted rows leave the DOM) + // Refresh to confirm dashboard is no longer in the list (deleted rows are removed from the DOM, so assert count rather than visibility) await dashboardListPage.goto(); await dashboardListPage.waitForTableLoad(); await expect(dashboardListPage.getDashboardRow(dashboardName)).toHaveCount( diff --git a/superset-frontend/playwright/tests/dataset/dataset-list.spec.ts b/superset-frontend/playwright/tests/dataset/dataset-list.spec.ts index 818d83e2f0d..962149db60b 100644 --- a/superset-frontend/playwright/tests/dataset/dataset-list.spec.ts +++ b/superset-frontend/playwright/tests/dataset/dataset-list.spec.ts @@ -134,7 +134,7 @@ test('should delete a dataset with confirmation', async ({ await expect(toast.getSuccess()).toBeVisible(); await expect(toast.getMessage()).toContainText('Deleted'); - // Verify dataset is removed from list (deleted rows leave the DOM) + // Verify dataset is removed from list (deleted rows are removed from the DOM, so assert count rather than visibility) await expect(datasetListPage.getDatasetRow(datasetName)).toHaveCount(0); // Verify via API that dataset no longer exists (404) @@ -440,7 +440,7 @@ test('should bulk delete multiple datasets', async ({ const toast = new Toast(page); await expect(toast.getSuccess()).toBeVisible(); - // Verify both datasets are removed from list (deleted rows leave the DOM) + // Verify both datasets are removed from list (deleted rows are removed from the DOM, so assert count rather than visibility) await expect(datasetListPage.getDatasetRow(dataset1.name)).toHaveCount(0); await expect(datasetListPage.getDatasetRow(dataset2.name)).toHaveCount(0); @@ -487,7 +487,7 @@ test.describe('import dataset', () => { label: `Dataset ${datasetId}`, }); - // Refresh to confirm dataset is no longer in the list (deleted rows leave the DOM) + // Refresh to confirm dataset is no longer in the list (deleted rows are removed from the DOM, so assert count rather than visibility) await datasetListPage.goto(); await datasetListPage.waitForTableLoad(); await expect(datasetListPage.getDatasetRow(datasetName)).toHaveCount(0);