test(sqllab): migrate Cypress E2E tests to Playwright#39071
Merged
sadpandajoe merged 5 commits intomasterfrom Apr 14, 2026
Merged
test(sqllab): migrate Cypress E2E tests to Playwright#39071sadpandajoe merged 5 commits intomasterfrom
sadpandajoe merged 5 commits intomasterfrom
Conversation
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (99.81%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #39071 +/- ##
=======================================
Coverage 64.44% 64.44%
=======================================
Files 2555 2555
Lines 132688 132688
Branches 30791 30791
=======================================
Hits 85511 85511
Misses 45691 45691
Partials 1486 1486
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:
|
msyavuz
reviewed
Apr 3, 2026
Member
msyavuz
left a comment
msyavuz
left a comment
There was a problem hiding this comment.
Thank you for this! It looks much bettern than the cypress tests already. Do you need anything to get this out of draft?
sadpandajoe
force-pushed
the
feat-sql-lab-to-playwright
branch
3 times, most recently
from
1dfb09c to
92a3cd7
Compare
Member
Author
No still doing some internal testing. Before last commit it actually was saying it was passing but some jobs were failing so fixing that up now. I'll let you know once its out of draft. |
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates the SQL Lab E2E test suite from Cypress to Playwright by introducing a SQL Lab Page Object Model and a new set of smoke tests that cover core SQL Lab workflows.
Changes:
- Added
SqlLabPagePOM plus supporting core components (AgGrid,EditableTabs) and API helpers for Saved Queries. - Introduced 4 Playwright spec files covering query execution, tab behaviors, save/share, dataset creation, and Explore navigation.
- Removed the legacy Cypress SQL Lab E2E tests/helpers that are now superseded by Playwright coverage.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| superset-frontend/playwright/utils/constants.ts | Adds a dedicated timeout for SQL query execution in Playwright tests. |
| superset-frontend/playwright/tests/experimental/sqllab/tabs.spec.ts | New Playwright tests for SQL Lab tab creation, closing, persistence, and shortcut behavior. |
| superset-frontend/playwright/tests/experimental/sqllab/save-and-share.spec.ts | New Playwright tests for saving queries and creating datasets → Explore verification. |
| superset-frontend/playwright/tests/experimental/sqllab/query-execution.spec.ts | New Playwright tests for successful execution, error handling, and re-run behavior. |
| superset-frontend/playwright/tests/experimental/sqllab/create-chart-from-query.spec.ts | New Playwright test for SQL Lab → Create chart → Explore navigation. |
| superset-frontend/playwright/pages/SqlLabPage.ts | Introduces a SQL Lab Page Object Model used by the new Playwright suite. |
| superset-frontend/playwright/pages/AuthPage.ts | Adjusts login navigation to use domcontentloaded for HMR/dev-server stability. |
| superset-frontend/playwright/helpers/fixtures/testAssets.ts | Extends test asset cleanup to track and delete saved queries. |
| superset-frontend/playwright/helpers/api/savedQuery.ts | Adds API helpers to GET/DELETE saved queries for assertions and cleanup. |
| superset-frontend/playwright/components/modals/EditDatasetModal.ts | Updates modal tab scoping to align with the updated Tabs component behavior. |
| superset-frontend/playwright/components/core/Tabs.ts | Changes Tabs scoping logic to avoid matching nested AntD tab bars. |
| superset-frontend/playwright/components/core/index.ts | Exposes newly added core components via the barrel export. |
| superset-frontend/playwright/components/core/EditableTabs.ts | Adds an editable-card tabs helper (add/remove tab actions). |
| superset-frontend/playwright/components/core/AgGrid.ts | Adds an AG Grid wrapper to simplify result grid assertions in tests. |
| superset-frontend/cypress-base/cypress/e2e/sqllab/tabs.test.ts | Removes Cypress SQL Lab tab E2E coverage (replaced by Playwright). |
| superset-frontend/cypress-base/cypress/e2e/sqllab/sqllab.helper.js | Removes Cypress SQL Lab helper utilities no longer needed. |
| superset-frontend/cypress-base/cypress/e2e/sqllab/query.test.ts | Removes Cypress SQL Lab query E2E coverage (replaced by Playwright). |
| superset-frontend/cypress-base/cypress/e2e/sqllab/_skip.sourcePanel.index.test.js | Removes a previously skipped/flaky Cypress test that is now out of scope. |
sadpandajoe
force-pushed
the
feat-sql-lab-to-playwright
branch
2 times, most recently
from
86e6461 to
ad19e8c
Compare
SqlLabPage page object with AceEditor, AgGrid, DatabaseSelector, and EditableTabs component wrappers. Shared test helpers for API assertions (expectStatus, extractIdFromResponse), saved-query API, and request intercepts (waitForPost/waitForGet). SaveQueryModal and SaveDatasetModal page objects. Timeout constants and URL registry.
sadpandajoe
force-pushed
the
feat-sql-lab-to-playwright
branch
from
aab42e6 to
0bc388b
Compare
Replace 3 Cypress spec files with a single Playwright spec covering 10 tests: query execution (3), tab management (4), save/share (2), and chart creation (1). Add chromium-sqllab project in playwright config with fullyParallel: false for server-side tab state isolation. Deleted Cypress files: - cypress/e2e/sqllab/query.test.ts - cypress/e2e/sqllab/tabs.test.ts - cypress/e2e/sqllab/sqllab.helper.js - cypress/e2e/sqllab/_skip.sourcePanel.index.test.js
sadpandajoe
force-pushed
the
feat-sql-lab-to-playwright
branch
from
0bc388b to
1d54bc2
Compare
sadpandajoe
requested review from
betodealmeida,
eschutho,
geido,
mistercrunch and
rusackas
as code owners
Contributor
Code Review Agent Run #b0da64Actionable Suggestions - 0Additional Suggestions - 1
Review 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 |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| name: 'chromium-sqllab', | ||
| testMatch: '**/tests/sqllab/**/*.spec.ts', | ||
| fullyParallel: false, | ||
| use: { |
Contributor
There was a problem hiding this comment.
Suggestion: fullyParallel: false only disables in-file test parallelism; it does not prevent different SQL Lab spec files from running on multiple workers. Because SQL Lab tab state is shared per authenticated user, this still allows cross-worker interference and nondeterministic failures. Pin this project to a single worker. [race condition]
Severity Level: Major ⚠️
- ⚠️ SQL Lab Playwright suite may be flaky in CI.
- ⚠️ CI runs for SQL Lab may intermittently fail.
- ⚠️ Debugging intermittent tabstate issues becomes more difficult.
Suggested change
| use: { | |
| workers: 1, |
Steps of Reproduction ✅
1. In CI, run Playwright from `superset-frontend/` with `CI=1` so global workers is `2`
per `superset-frontend/playwright.config.ts:43-46` (`workers: process.env.CI ? 2 : 1`).
2. Execute SQL Lab tests only, e.g. `PLAYWRIGHT_BASE_URL=http://localhost:8088 CI=1 npx
playwright test tests/sqllab/ --project=chromium-sqllab`, which uses project
`chromium-sqllab` defined at `superset-frontend/playwright.config.ts:107-122`.
3. Observe that `chromium-sqllab` has `fullyParallel: false` at `line 116`, but no
project-specific `workers` override, so Playwright still spawns two workers (from the
global config) and distributes different `**/tests/sqllab/**/*.spec.ts` files across them
per `testMatch` at `line 115`.
4. Because both workers reuse the same authenticated user via `storageState:
'playwright/.auth/user.json'` (`lines 117-121`) and SQL Lab tab state is stored
server-side at `/tabstateview/*` (`superset-frontend/src/SqlLab/actions/sqlLab.ts:13-32`
and comment at `playwright.config.ts:108-113`), concurrent spec files can create/delete
tabs for the same user in parallel, leading to nondeterministic tab counts and
intermittent assertion failures when tests read tab state.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/playwright.config.ts
**Line:** 117:117
**Comment:**
*Race Condition: `fullyParallel: false` only disables in-file test parallelism; it does not prevent different SQL Lab spec files from running on multiple workers. Because SQL Lab tab state is shared per authenticated user, this still allows cross-worker interference and nondeterministic failures. Pin this project to a single worker.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
| // The form submission is async (SupersetClient.postForm uses ensureAuth) | ||
| // so listen for the page reload before triggering the login | ||
| await Promise.all([ | ||
| page.waitForEvent('load', { timeout: TIMEOUT.PAGE_LOAD }), |
Contributor
There was a problem hiding this comment.
Suggestion: Waiting for the 'load' event after submitting invalid credentials is brittle here and can timeout, because this test suite already documents that full page load is not reliable in this app flow. Use navigation waiting with domcontentloaded instead so the test waits for the server-rendered login response without depending on a potentially missing load event. [possible bug]
Severity Level: Major ⚠️
- ❌ Auth negative-login spec can timeout despite successful server response.
- ⚠️ Flaky chromium-unauth project increases CI runtime and retries.
Suggested change
| page.waitForEvent('load', { timeout: TIMEOUT.PAGE_LOAD }), | |
| page.waitForNavigation({ | |
| waitUntil: 'domcontentloaded', | |
| timeout: TIMEOUT.PAGE_LOAD, | |
| }), |
Steps of Reproduction ✅
1. From `superset-frontend/`, run the unauth project tests, e.g. `npx playwright test
--project=chromium-unauth playwright/tests/auth/login.spec.ts` (project `chromium-unauth`
is defined in `superset-frontend/playwright.config.ts:124-133` to run
`tests/auth/**/*.spec.ts` without stored auth).
2. The `beforeEach` in `playwright/tests/auth/login.spec.ts:37-42` constructs `AuthPage`
and calls `authPage.goto()`, which in `playwright/pages/AuthPage.ts:52-56` uses
`page.goto(URL.LOGIN, { waitUntil: 'domcontentloaded' })` with a comment stating the
default `'load'` event may never fire with the HMR WebSocket.
3. The test `"should redirect to login with incorrect username and password"` at
`login.spec.ts:44-63` executes `await Promise.all([ page.waitForEvent('load', { timeout:
TIMEOUT.PAGE_LOAD }), authPage.loginWithCredentials('wronguser', 'wrongpassword') ])`
(lines 49-52), while `loginWithCredentials` in `AuthPage.ts:70-87` fills the login form
and clicks the submit button, triggering a POST to `/login/` and a reload of the login
page with an error.
4. In environments where the login page is server-rendered but the page `'load'` event is
unreliable or never fires due to the HMR/WebSocket behavior documented in
`AuthPage.ts:52-55`, the POST `/login/` completes and the error state renders, but
`page.waitForEvent('load', { timeout: TIMEOUT.PAGE_LOAD })` (using the 10s `PAGE_LOAD`
timeout from `playwright/utils/constants.ts:31-40`) can time out, causing this test to
fail even though the DOM is ready; waiting instead for `page.waitForNavigation({
waitUntil: 'domcontentloaded' })` would track the actual navigation lifecycle and avoid
depending on the brittle `'load'` event.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/playwright/tests/auth/login.spec.ts
**Line:** 50:50
**Comment:**
*Possible Bug: Waiting for the `'load'` event after submitting invalid credentials is brittle here and can timeout, because this test suite already documents that full page load is not reliable in this app flow. Use navigation waiting with `domcontentloaded` instead so the test waits for the server-rendered login response without depending on a potentially missing `load` event.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
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
Replace the SQL Lab Cypress E2E test suite with Playwright tests. Creates a
SqlLabPagePage Object Model and 10 smoke tests across 4 spec files covering query execution, tab management, save/share workflows, and chart creation from query results.All 4 Cypress SQL Lab test files are deleted — the 3 skipped tests are redesigned as reliable Playwright tests, and the source panel test is dropped (covered by existing
SqlEditorLeftBarRTL unit tests).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — test infrastructure change, no UI changes.
TESTING INSTRUCTIONS
Expected: 10 tests pass across 4 spec files (~2.5 min):
query-execution.spec.ts(3): execute query, error handling, re-run with refreshtabs.spec.ts(4): create via button, close via button, preserve state across tabs, keyboard shortcutsave-and-share.spec.ts(2): save query + API verification, create dataset → Explorecreate-chart-from-query.spec.ts(1): SQL Lab → Create Chart → Explore navigationADDITIONAL INFORMATION