chore(tests): promote Playwright experimental tests to stable#38924
Merged
sadpandajoe merged 14 commits intomasterfrom Apr 2, 2026
Merged
chore(tests): promote Playwright experimental tests to stable#38924sadpandajoe merged 14 commits intomasterfrom
sadpandajoe merged 14 commits intomasterfrom
Conversation
…an up superseded Cypress tests Move chart, dashboard, and dataset Playwright E2E tests from experimental/ to stable test directories so they run as required CI blockers. Delete the Cypress chart_list and dataset_list tests that are now fully replaced, and remove orphaned helpers from Cypress utils. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Promotes previously “experimental” Playwright E2E coverage for chart, dashboard, and dataset flows into the stable Playwright test suite so these scenarios run as required CI blockers, and removes now-superseded Cypress coverage/helpers.
Changes:
- Moved Playwright chart/dashboard/dataset tests out of
tests/experimental/**and updated their relative imports accordingly. - Deleted superseded Cypress E2E specs for chart list and dataset list.
- Removed unused Cypress helpers/constants that no longer have consumers.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| superset-frontend/playwright/tests/dataset/dataset-test-helpers.ts | Updates helper imports to match new (non-experimental) test location. |
| superset-frontend/playwright/tests/dataset/dataset-list.spec.ts | Updates imports/fixture path after promotion to stable Playwright tests. |
| superset-frontend/playwright/tests/dataset/create-dataset.spec.ts | Updates imports after promotion to stable Playwright tests. |
| superset-frontend/playwright/tests/dashboard/theme.spec.ts | Updates imports after promotion to stable Playwright tests. |
| superset-frontend/playwright/tests/dashboard/export.spec.ts | Updates imports after promotion to stable Playwright tests. |
| superset-frontend/playwright/tests/dashboard/dashboard-test-helpers.ts | Updates helper imports after promotion to stable Playwright tests. |
| superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts | Updates imports after promotion to stable Playwright tests. |
| superset-frontend/playwright/tests/chart/chart-test-helpers.ts | Updates helper imports after promotion to stable Playwright tests. |
| superset-frontend/playwright/tests/chart/chart-list.spec.ts | Updates imports after promotion to stable Playwright tests. |
| superset-frontend/cypress-base/cypress/utils/urls.ts | Removes unused Cypress dataset list URL constant. |
| superset-frontend/cypress-base/cypress/utils/index.ts | Removes unused Cypress list/grid + bulk-select helpers. |
| superset-frontend/cypress-base/cypress/e2e/explore/utils.ts | Removes unused chart-list-specific intercept/filter helpers. |
| superset-frontend/cypress-base/cypress/e2e/dataset/dataset_list.test.ts | Deletes superseded Cypress dataset list spec. |
| superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts | Deletes superseded Cypress chart list spec. |
The gsheets driver (shillelagh[gsheetsapi]) is a base dependency in pyproject.toml, so it is always available after `pip install -e .`. Remove test.skip() logic that silently swallowed engine-not-found errors, replacing it with hard failures so these promoted tests are real CI blockers. Keep the timeout skip for transient Google Sheets API issues. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
superset-frontend/playwright/tests/dataset/dataset-list.spec.ts:523
- The surrounding comments are now inconsistent: this test no longer skips when the GSheets connector/engine is unavailable (it fails hard below), but there’s still an earlier note saying it will skip. Please update/remove that note so the test’s documented behavior matches the implementation.
superset-frontend/playwright/tests/dataset/dataset-list.spec.ts:526 - When the import fails, the thrown error omits the HTTP status and drops all details if the response isn’t valid JSON (it becomes
{}). For CI-debuggability, includeimportResponse.status()(and ideally fall back toawait importResponse.text()whenjson()fails) in the error message/attachments.
superset-frontend/playwright/tests/dataset/create-dataset.spec.ts:88 - In the failure path,
await createDbRes.json()can itself throw if the backend returns a non-JSON error (proxy/500 HTML/etc), which can obscure the real failure cause. Consider includingcreateDbRes.status()and falling back toawait createDbRes.text()when JSON parsing fails so CI logs stay actionable.
…verage P1: create-dataset.spec.ts depends on an external Google Sheet that can be rate-limited or go offline, making it unsuitable as a required CI blocker. Move it back to experimental/ where failures don't block merges. P2: The deleted Cypress chart_list spec was the only test exercising card view. Add card-view methods to ChartListPage (gotoCardView, getChartCard, clickCardDeleteAction, clickCardEditAction) and a smoke test that deletes a chart from card view, preserving coverage for the card UI path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace blanket TimeoutError skip in create-dataset with API-level
verification: intercept GET /api/v1/database/{id}/tables and only
skip when the response is empty (Google Sheets transient) or never
arrives. Non-OK responses and non-empty results missing the expected
sheet now fail hard, catching Superset regressions instead of hiding
them behind a skip.
- Replace broken card-view delete test with a card-view edit test that
exercises the card dropdown → properties modal flow, including UI
assertions that the renamed card appears and the old name disappears.
- Remove card-view delete test (ConfirmStatusChange inside Ant Design
Dropdown loses state on close — known framework issue, not fixable
in the test layer).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The dataset ID from create responses must exist — if it doesn't, the test should fail rather than silently leak test data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… imports Remove dead Cypress helpers (interceptExploreJson, interceptFormDataKey, clearAllInputs, getChartAliasBySpec, getChartAliasesBySpec, toSlicelike, ALERT_LIST, REPORT_LIST, LOGIN, REGISTER) and unexport internal-only functions (interceptExploreGet, getChartGridComponent). Flatten Playwright test imports to use barrel index files for components/modals, components/core, and helpers/fixtures. Remove unused clickCardDeleteAction from ChartListPage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Re-export getChartAliasBySpec, getChartAliasesBySpec, vizPlugins, and parsePostForm from cypress/utils — these are still imported by the kept _skip.* test files and their removal broke Cypress type checking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sadpandajoe
requested review from
betodealmeida,
eschutho,
geido,
mistercrunch and
rusackas
as code owners
Restore original create-dataset.spec.ts without the over-engineered gsheets API interception logic. The test is promoted out of experimental and is allowed to fail if Google Sheets is unavailable. Remove dual test/testWithAssets export from fixtures/index.ts — use the same import pattern as all other test files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
superset-frontend/playwright/tests/dataset/dataset-list.spec.ts:462
- The comment above
importedDatasetNamesays the test will skip if the gsheets connector is unavailable, but the updated logic below now always throws on failed imports. Please update the comment to match the current behavior (or reintroduce the skip if that was still intended).
superset-frontend/playwright/tests/dataset/dataset-list.spec.ts:525 - On import failure you only attempt
response.json()and fall back to{}, which can produce an unhelpful error (e.g.Import failed: {}). Consider including the HTTP status and falling back toresponse.text()when JSON parsing fails so CI failures are actionable.
The previous commit removed the `test` named export from
fixtures/index.ts but didn't update three spec files that still
imported `{ test as testWithAssets }`. Changed to `{ testWithAssets }`.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
superset-frontend/playwright/tests/dataset/dataset-list.spec.ts:462
- The inline comment above this test says the import will be skipped if the gsheets connector is unavailable, but the updated logic now fails hard on any non-OK response (no skip path). Please update/remove this comment so it matches the new behavior (and CI expectations).
superset-frontend/playwright/tests/dataset/dataset-list.spec.ts:526 - On import failure, the thrown error only includes a best-effort JSON body (or
{}if parsing fails). This can hide useful debugging info (HTTP status, status text, or non-JSON response bodies like HTML 500 pages). Consider includingimportResponse.status()and falling back toawait importResponse.text()when JSON parsing fails.
Contributor
Code Review Agent Run #0e4e31Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
The comment said the test would skip if the gsheets connector was unavailable, but the implementation fails hard on import failure since shillelagh[gsheetsapi] is a base dependency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Code Review Agent Run #7fb7c0Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #38924 +/- ##
=======================================
Coverage 64.41% 64.41%
=======================================
Files 2536 2536
Lines 131155 131155
Branches 30450 30450
=======================================
Hits 84485 84485
Misses 45207 45207
Partials 1463 1463
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
Promote all 9 chart, dashboard, and dataset Playwright E2E tests from `experimental/` to stable test directories so they run as required CI blockers. Clean up the Cypress tests they supersede and remove orphaned Cypress helpers.
Moved to stable:
Cypress cleanup:
No CI workflow changes needed — `playwright.config.ts` `testIgnore: '/experimental/'` automatically picks up files moved out of `experimental/`.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — no UI changes
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION