WebUI: rewrite and simplify E2E Playwright tests #10112
Open
+1,158
−978
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
github-actions
bot
added
area/testing
nopcoder
added
the
exclude-changelog
- Add custom Playwright fixtures to eliminate POM construction boilerplate - Add lakeFSApi helper wrapping Playwright APIRequestContext for test setup - Split RepositoryPage god object into focused sub-POMs (branches, objects, changes, commits, revert) with a thin facade - Add named timeout constants replacing magic numbers across all specs - Separate setup infrastructure (auth state saving) from setup validation tests so validation failures don't break all downstream projects - Add test.step() for logical grouping in long test scenarios - Pin @duckdb/duckdb-wasm to 1.28.0 (1.32.0 has a regression where COPY TO lakefs:// silently fails) - Fix objectViewerPage.clickExecuteButton to wait for query completion, eliminating a false positive in DuckDB result verification - Fix pullsPage.getPullsListCount to handle empty pull request lists - Fix repositoriesPage.createRepository to fill Storage Namespace when the server has no default_namespace_prefix configured - Consolidate duplicate credential types in credentialsFile.ts - Remove test-upload.txt side-effect file; use Buffer.from() instead
- Use TypeScript parameter properties across all POMs (eliminates manual field + constructor assignment boilerplate) - Extract shared createLakeFSApi factory in fixtures.ts (dedup lakeFSApi and lakeFSApiWorker fixture code) - Combine chained locator selectors into single CSS selectors (e.g. table.table tbody tr instead of 3 chained .locator calls) - Collapse single-line fixture definitions to reduce visual noise - Extract SETUP_SPECS constant in playwright config to DRY testIgnore - Simplify createRepository: single heading assertion with ternary instead of duplicated if/return/else branches - Add test/README.md with instructions for running tests, headed mode, debugging (inspector, traces, UI mode, VS Code), env vars, and project structure
nopcoder
force-pushed
the
webui/e2e-test-rewrite
branch
2 times, most recently
from
3ddc346 to
091518c
Compare
nopcoder
force-pushed
the
webui/e2e-test-rewrite
branch
from
05f998f to
669e641
Compare
Reduce documentation surface area and prevent stale structure docs as tests evolve.
Avoid redirects to /setup in environments where only the comm prefs step is pending, improving e2e test reliability when SKIP_SETUP is set.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the WebUI E2E Playwright suite to reduce boilerplate and flakiness by introducing fixtures, decomposing the repository POM into focused “operations” sub-POMs, and standardizing timeouts and setup via a small API client.
Changes:
- Split the former
RepositoryPage“god object” into a facade that composesBranchOperations,ObjectOperations,ChangesOperations,RevertOperations, andCommitsOperations. - Added a shared fixture layer (
fixtures.ts) and an API helper (LakeFSApi) to simplify test setup and reduce per-test POM construction. - Introduced named timeout constants and updated tests/specs accordingly; added a new E2E README and updated Playwright project configuration.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| webui/test/e2e/timeouts.ts | Adds named timeout constants for E2E waits. |
| webui/test/e2e/poms/setupPage.ts | Minor POM cleanup via constructor parameter property. |
| webui/test/e2e/poms/revertOperations.ts | New sub-POM for revert flows. |
| webui/test/e2e/poms/repositoryPage.ts | Converts RepositoryPage into a thin facade composing sub-POMs. |
| webui/test/e2e/poms/repositoriesPage.ts | Uses named timeouts; adds conditional storage-namespace handling. |
| webui/test/e2e/poms/pullsPage.ts | Updates pull-list selectors and tab navigation helper. |
| webui/test/e2e/poms/objectViewerPage.ts | Simplifies selectors and adds “wait for execute” logic. |
| webui/test/e2e/poms/objectOperations.ts | New sub-POM for uploads/deletes in object views. |
| webui/test/e2e/poms/loginPage.ts | Minor POM cleanup via constructor parameter property. |
| webui/test/e2e/poms/commitsOperations.ts | New sub-POM for commit-list queries. |
| webui/test/e2e/poms/changesOperations.ts | New sub-POM for uncommitted changes, commits, merges. |
| webui/test/e2e/poms/branchOperations.ts | New sub-POM for branch creation/switching/compare controls. |
| webui/test/e2e/lakeFSApi.ts | New API wrapper for setup actions using Basic auth. |
| webui/test/e2e/fixtures.ts | Adds Playwright fixtures for POMs + worker-scoped setup/api helpers. |
| webui/test/e2e/credentialsFile.ts | Consolidates credential/storage-state paths and read/write helpers. |
| webui/test/e2e/consts.ts | Removes now-redundant constants file. |
| webui/test/e2e/common/viewParquetObject.spec.ts | Migrates spec to fixtures + new POM structure. |
| webui/test/e2e/common/uploadFile.spec.ts | Migrates spec to fixtures; uses in-memory upload payloads. |
| webui/test/e2e/common/test-upload.txt | Removes checked-in upload artifact. |
| webui/test/e2e/common/syncCommitMerge.spec.ts | Migrates spec to fixtures, steps, named timeouts, and sub-POM ops. |
| webui/test/e2e/common/setupInfra.spec.ts | New setup project to save auth state + credentials. |
| webui/test/e2e/common/setup.spec.ts | Narrows to setup validation tests (infra moved elsewhere). |
| webui/test/e2e/common/revertCommit.spec.ts | Migrates spec to fixtures + new revert/commit ops. |
| webui/test/e2e/common/repositoriesPage.spec.ts | Migrates spec to fixtures. |
| webui/test/e2e/common/readOnlyRepository.spec.ts | Uses lakeFSApi for setup and fixtures for POM access. |
| webui/test/e2e/common/quickstart.spec.ts | Migrates spec to fixtures, steps, and sub-POM operations. |
| webui/test/README.md | Adds documentation for running/debugging/extending E2E tests and conventions. |
| webui/playwright.config.ts | Updates project graph for setup vs SKIP_SETUP runs; switches to new storage-state path. |
| webui/.gitignore | Ignores generated Playwright credential/state JSON files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix resource leak, wrong POM references, missing directory creation, magic timeouts, and flaky visibility checks identified in code review.
Contributor
Author
|
Addressed all 6 review comments in 2380499:
|
nopcoder
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nopcoder
added
minor-change
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
Rewrites and simplifies the E2E Playwright test infrastructure. Net result is -92 lines (844 added, 936 removed) with better architecture.
What changed
Decomposed God Object:
repositoryPage.tswas a 246-line class handling branches, uploads, commits, merges, reverts, and navigation. It's now a 52-line facade that composes 5 focused sub-POMs (branchOperations,objectOperations,changesOperations,commitsOperations,revertOperations), accessed viarepositoryPage.branches.createBranch(...),repositoryPage.changes.commitChanges(...), etc.Playwright fixtures: Tests no longer manually construct POMs. A custom fixture system in
fixtures.tsprovides POMs via destructuring (async ({ repositoriesPage, repositoryPage }) => { ... }), eliminating 2-4 lines of boilerplate per test.Named timeouts: Replaced magic numbers (
5000,10000,120000) scattered through tests with self-documenting constants:TIMEOUT_LONG_OPERATION(120s),TIMEOUT_ELEMENT_VISIBLE(10s),TIMEOUT_NAVIGATION(5s).API client: New
lakeFSApi.tswraps Playwright'sAPIRequestContextwith Basic auth for test setup via the lakeFS REST API.Auth simplification: Flattened directory structure, removed
consts.ts, consolidated credential paths intocredentialsFile.ts. Added.gitignoreentry for generated credential files.Bug fix:
getUncommittedCount()used.isVisible()(a boolean check that doesn't wait) instead ofexpect().toBeVisible()(an assertion that auto-retries). This could return 0 on slow page loads.Test README: Added
webui/test/README.mddocumenting how to run, debug, and extend the tests, plus architecture and design principles.Test plan
SKIP_SETUP=true npx playwright test— passing tests still pass (4/4 unrelated to sample data creation)