test(playwright): convert and create new dataset list playwright tests #36196
+1,796
−95
Conversation
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
…test Implements Phase 0 and Phase 1 of Playwright E2E test migration: Infrastructure: - Global authentication setup with storageState for test performance - Base Modal component with inheritance pattern for specific modals - Table component for ListView interactions - Timeout constants for maintainability - Enhanced AuthPage with resilient login validation Components: - DeleteConfirmationModal and DuplicateDatasetModal - DatasetListPage and ExplorePage page objects First E2E test validates dataset → Explore navigation flow using existing birth_names fixture dataset. All code follows YAGNI/DRY principles - minimal implementation for Test 1, with room to grow as additional tests are added. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1. Auth State Isolation:
- Move test.use({ storageState: undefined }) to file scope
- Previously scoped inside test.describe, which ran after worker
loaded global auth state, causing login redirects
- File-scope override creates fresh context for login tests only
2. Directory Creation:
- Add mkdir for playwright/.auth before saving storageState
- Prevents failure on first run when directory doesn't exist
3. Code Quality:
- Remove test.describe nesting (Superset testing guidelines)
- Create authPage per-test instead of file-scoped variable
- Ensures tests remain parallel-safe if mode changes later
- Use TIMEOUT.PAGE_LOAD constant instead of magic 10000
- Use URL.WELCOME constant instead of hardcoded string
- Rely on Playwright's inferred fixture types
Fixes broken auth/login tests while keeping fast global auth for
all other tests.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Root Cause:
- playwright.config.ts line 67 hardcodes storageState for ALL tests
- File-scope test.use({ storageState: undefined }) runs AFTER worker
initialization with cached auth state
- Login tests immediately redirect (already authenticated) → timeout
Solution:
- Create 'chromium-unauth' Playwright project for auth tests
- testMatch routes tests/auth/**/*.spec.ts to unauth project
- Project-level storageState: undefined overrides global setting
- Workers initialize with correct auth state from the start
Changes:
1. playwright.config.ts: Add chromium-unauth project definition
2. login.spec.ts: Remove file-scope test.use() workaround
Why This Works:
- Project-level config overrides global config at worker init time
- testMatch provides declarative routing (no runtime overrides needed)
- Follows Playwright best practices for multiple auth states
- Future-proof: easy to add signup/reset password unauthenticated tests
Fixes failing CI tests from commit 5c9ae89.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
…pers Two critical fixes for Playwright API helper utilities: 1. Fix Import Path (datasets.ts:22): - Changed: '../utils/constants' - To: '../../utils/constants' - Issue: Wrong path resolved to playwright/helpers/utils/constants.ts (doesn't exist) - Fix: Correct path resolves to playwright/utils/constants.ts - Impact: TypeScript compilation would fail for any test using datasets helper 2. Fix Hardcoded localhost (client.ts:58): - Changed: return 'http://localhost:8088' - To: return process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:8088' - Issue: CI uses http://superset:8088, API calls before first navigation fail - Fix: Respect PLAYWRIGHT_BASE_URL environment variable - Impact: API helpers work correctly in CI environment Fallback Chain for baseURL: 1. Extract from page.url() if page has navigated 2. Use process.env.PLAYWRIGHT_BASE_URL (CI: http://superset:8088) 3. Fallback to localhost (local dev: http://localhost:8088) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Switch auth tests to per-test login via beforeEach instead of fighting with storageState configuration. This follows the Cypress pattern and provides clear separation: - Auth tests: Per-test login (simple, isolated) - E2E tests: Global auth (fast, login once) Changes: 1. login.spec.ts: Add beforeEach hook for navigation 2. playwright.config.ts: Document hybrid approach 3. global-setup.ts: Clarify it's for non-auth tests only 4. client.ts: Prettier formatting (auto-applied) This eliminates the storageState complexity that was causing auth test failures while keeping E2E tests fast. Pattern: - Auth tests run in chromium-unauth project with beforeEach navigation - E2E tests run in chromium project with global-setup.ts auth - Each project has clear purpose documented in config Benefits: - Simple, maintainable auth tests (no storageState debugging) - Fast E2E tests (login once, reuse state) - Follows proven Cypress approach - Clear separation of concerns 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Root cause: Auth tests inherited global storageState, causing immediate redirect to /welcome and login form never rendering (100% failure rate). Problem: - Auth tests ran in BOTH chromium and chromium-unauth projects (4 runs instead of 2) - Global storageState at config root inherited by all projects - chromium-unauth override to undefined didn't prevent cookie inheritance - Worker loads config-level storageState before project overrides apply - Tests navigate to /login but redirect to /welcome (already authenticated) - waitForLoginForm() times out after 5000ms (form never renders) Solution (file-based routing): - Add testIgnore to chromium project (skip tests/auth/**) - Move storageState from global use to chromium project only - chromium-unauth now starts with clean browser (no cached cookies) - Directory structure enforces auth context (no manual tagging needed) Changes: - Remove global storageState from config root (line 67) - Add testIgnore to chromium project (line 80) - Move storageState to chromium project only (line 85) - Remove storageState: undefined override from chromium-unauth (no longer needed) Expected result: - Before: Running 4 tests → ××××FF××××FF (4 failures) - After: Running 2 tests → ✓✓ (2 passes in chromium-unauth only) Architecture pattern established: - tests/auth/ → chromium-unauth project (clean browser) - tests/dataset/ etc → chromium project (global auth, fast E2E) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…ucture Add comprehensive API infrastructure and delete dataset E2E test: **API Infrastructure:** - Generic request helpers with CSRF token handling (requests.ts) - DRY header building for all mutation requests - Database and dataset CRUD operations - Google Sheets database factory for test data isolation - Response validation with clear error messages **Toast Component:** - Reusable toast verification component - Type-specific getters (success, danger, warning, info) - SELECTORS constants (no magic strings) **Delete Dataset Test:** - Creates test dataset via API (Google Sheets) - Tests full delete flow: click → modal → confirm → verify removal - Validates success toast appears with correct message - Clean afterEach cleanup pattern (no race conditions) **Code Quality:** - No private Playwright APIs (uses env variables) - Path prefix support for prefixed deployments - All selectors in constants - Type-safe throughout 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Convert absolute API paths (/api/v1/...) to relative paths (api/v1/...) in Playwright API helpers to properly handle baseURL with path prefix. Problem: - CI test "should delete a dataset" was failing in app_prefix configuration - Absolute paths starting with / bypass Playwright's baseURL - With baseURL=http://localhost:8081/app/prefix/, requests went to http://localhost:8081/api/v1/... (missing /app/prefix/) Solution: - Remove leading / from all API endpoint constants - Playwright now correctly prepends baseURL to relative paths - Works in both standard (no prefix) and app_prefix configurations Files changed: - playwright/helpers/api/database.ts - DATABASE endpoint - playwright/helpers/api/dataset.ts - DATASET endpoint - playwright/helpers/api/requests.ts - CSRF token endpoint + comment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
After rebasing onto master, reorganize Playwright tests to follow the experimental pattern that was adopted in master: - Move dataset E2E tests to tests/experimental/ directory - Update import paths to reflect new directory structure (../../ → ../../../) - Update chromium project testIgnore to respect experimental pattern - Add comprehensive README explaining experimental test workflow - Auth tests remain stable (always run by default) - Dataset tests are now opt-in (require INCLUDE_EXPERIMENTAL=true) Test organization: - tests/auth/ - Stable auth tests (2 tests) - tests/experimental/dataset/ - Experimental dataset tests (2 tests) All supporting infrastructure remains in stable locations: - playwright/components/ - Modal, Table, Toast, etc. - playwright/pages/ - AuthPage, DatasetListPage, ExplorePage - playwright/helpers/api/ - Full CRUD + factories Verification: - Without INCLUDE_EXPERIMENTAL: 2 tests (auth only) - With INCLUDE_EXPERIMENTAL=true: 4 tests (auth + dataset)
- Remove hardcoded birth_names dependency (create test data via API) - Fix JSON template injection in createGsheetsDatabase (use JSON.stringify) - Add TypeScript interfaces for API payloads (DatabaseCreatePayload, DatasetCreatePayload) Tests remain in experimental/ directory until proven stable. Keeps test.describe structure per Playwright best practices. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Wrap dataset creation in try/catch to clean up orphaned database if apiPostDataset fails. Without this, failed test runs accumulate GSheets database connections that never get deleted by afterEach cleanup. The issue: createGsheetsDatabase succeeds and creates dbId, but if apiPostDataset throws before returning, the test's testResources never receives dbId, so afterEach can't clean it up. The fix: catch errors during dataset creation, delete the database with failOnStatusCode: false, then rethrow the original error for visibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Creates test dataset via API (hermetic) - Clicks duplicate action button - Fills DuplicateDatasetModal with new name - Verifies success toast appears - Verifies both original and duplicate exist in list - Cleans up duplicate via UI delete - Tests real backend dataset creation (not mocked) Closes gap: Component tests mock API, E2E verifies actual creation
…on and API verification
**Problem 1: Delete dependency**
- Previous version deleted duplicate via UI
- If delete failed, couldn't tell if duplicate or delete was broken
- Violated "one test, one concern" principle
**Problem 2: Missing usability verification**
- Only verified duplicate appeared in list
- Didn't verify SQL/schema/database were actually copied
- No proof duplicate was usable, just that it existed
**Solutions:**
1. Response interception (like Cypress intercept):
- Use page.waitForResponse() to capture duplicate API response
- Extract duplicate dataset ID directly from backend
- Track in testResources for cleanup
2. Multi-dataset cleanup pattern:
- Changed testResources from { datasetId } to { datasetIds: [] }
- Updated cleanup to loop through all dataset IDs
- All tests now push to array instead of replacing
3. API verification of duplicate:
- Added apiGetDataset() helper to fetch dataset details
- Compare original vs duplicate: sql, database.id, schema
- Verify table_name changed to new duplicate name
- Proves duplicate is a true copy, not just a shell
4. Removed UI delete from test:
- No dependency on delete test
- Relies on afterEach cleanup via API
- Clean separation of concerns
**Benefits:**
- ✅ No test interdependencies
- ✅ Verifies duplicate is actually usable
- ✅ Cleaner, more focused test
- ✅ Reusable multi-dataset cleanup pattern
Before (3 lines):
const { datasetId, dbId } = await createTestDataset(page, name);
testResources.datasetIds.push(datasetId);
testResources.dbId = dbId;
After (2 lines):
const result = await createTestDataset(page, name);
testResources = { datasetIds: [result.datasetId], dbId: result.dbId };
Benefits:
- Single assignment instead of mutation
- Fewer lines (2 vs 3)
- More explicit about array creation
- Easier to read
**Problem**: Test created physical dataset, but duplicate button only shows
for virtual datasets (those with SQL queries).
**Solution**: Use existing 'members_channels_2' virtual example dataset
**Changes**:
1. Added getDatasetByName() helper (dataset.ts):
- Searches datasets by table_name using filter API
- Returns { id, data } - both ID and full payload in one call
- No hardcoded IDs, works in any environment
- Reusable for future tests
2. Updated duplicate test to use example dataset:
- Gets members_channels_2 by name (ID varies by environment)
- Uses returned data for verification (no extra API call)
- Expects dataset to exist (fails gracefully if example data not loaded)
- Only cleans up duplicate (original is example data)
- Simpler: no dataset/database creation needed
**Benefits**:
- ✅ Works with virtual datasets (duplicate button appears)
- ✅ No hardcoded dataset IDs
- ✅ Optimized: One less API call (uses data from getDatasetByName)
- ✅ Fewer API calls overall (no dataset/database creation)
- ✅ More realistic (uses actual Superset data)
- ✅ Still hermetic (only duplicate gets cleaned up)
**Testing**: CI will validate with loaded example data
**Problem**: Playwright duplicate test fails in CI because `members_channels_2` dataset is not loaded with `--load-test-data` flag. The `--load-test-data` flag only loads files matching `*.test.*` pattern (e.g., `unicode_test.test.yaml`). Regular example datasets like `members_channels_2.yaml` are skipped, causing the duplicate dataset test to fail with null. **Root Cause**: - CI uses `superset load_examples --load-test-data` (line 115 in bashlib.sh) - This loads ONLY `*.test.*` files (1 dataset: unicode_test) - Playwright test expects `members_channels_2` virtual dataset - Ephemeral environments use `superset load_examples` (all datasets) **Solution**: Create separate data loading function for Playwright 1. Added `playwright_testdata()` function in bashlib.sh: - Loads ALL examples via `superset load_examples` (no flag) - Includes all 17 datasets (16 virtual + 1 test) - Matches ephemeral environment behavior 2. Updated superset-e2e.yml: - Playwright job now uses `playwright_testdata` instead of `testdata` - Cypress tests still use `testdata` (unchanged) **Impact**: - ✅ Playwright tests can use realistic example datasets - ✅ Matches ephemeral environment data availability - ✅ No impact on Cypress tests (still use --load-test-data) - ⏱️ CI data loading ~30-60s slower (acceptable tradeoff) **Testing**: Will be validated by CI run after push 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
**Problem**: Experimental Playwright tests still failing after commit 8eb510f. **Root Cause**: There are TWO Playwright workflow files: - `.github/workflows/superset-e2e.yml` - Updated in commit 8eb510f ✅ - `.github/workflows/superset-playwright.yml` - NOT updated ❌ The experimental tests (`playwright-tests-experimental` job) run from the `superset-playwright.yml` file which was still using `testdata` instead of `playwright_testdata`, causing `members_channels_2` dataset to not be loaded. **Solution**: Update `superset-playwright.yml` to use `playwright_testdata` Changed line 100 from: run: testdata To: run: playwright_testdata This matches the fix in commit 8eb510f and ensures experimental tests load all 17 example datasets including virtual datasets. **Testing**: Will be validated by CI run after push 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…bstring collisions
**Problem**: Duplicate dataset test failing with strict mode violation:
```
Error: locator().filter({ hasText: 'members_channels_2' }) resolved to 2 elements:
1) duplicate_members_channels_2_1763101756082
2) members_channels_2
```
**Root Cause**: The `filter({ hasText: rowText })` uses substring matching,
so searching for 'members_channels_2' matches both the original dataset
AND any duplicates that contain that substring in their name.
**Solution**: Use exact cell match instead of substring text match
Changed `Table.getRow()` from:
```typescript
.filter({ hasText: rowText })
```
To:
```typescript
.filter({ has: this.page.getByRole('cell', { name: rowText, exact: true }) })
```
**Why this works**:
- Matches the cell with EXACT text (not substring)
- Won't match 'duplicate_members_channels_2_XXX' when looking for 'members_channels_2'
- More precise and resilient to leftover test data
- Uses semantic selector (role='cell') + exact match
**Impact**: All tests using `getDatasetRow()` will now use exact matching:
- Navigate test (finds specific dataset in list)
- Delete test (finds correct dataset to delete)
- Duplicate test (distinguishes original from duplicate)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
github-actions
bot
added
the
github_actions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION