test: move frontend-only e2e specs to the vitest-browser tier#563
Open
mariusvniekerk wants to merge 15 commits into
Open
test: move frontend-only e2e specs to the vitest-browser tier#563mariusvniekerk wants to merge 15 commits into
mariusvniekerk wants to merge 15 commits into
Conversation
The provider-capabilities e2e-full spec exercised two pieces of pure UI rendering -- the locked-PR chip and the GitLab read-only issue timeline -- through a seeded Go server and two Playwright browser projects. That is slow and flaky for behavior that only needs a real DOM, so this reimplements both cases on the vitest-browser tier: the real App is mounted in Chromium with the capability data mocked at the fetch boundary, so the rendered chip and the presence/absence of the timeline edit control reflect exactly what the capability fields produced. The spec's raw /api/v1 capabilities JSON sub-assertion is intentionally not carried over. GitLab's real capability default is CommentMutation=true (internal/platform/gitlab/client.go); the e2e-server fixture constructs the read-only scenario, so that assertion only pinned the test server's own fixture wiring. The provider capability contract is owned by the Go platform tests (e.g. internal/platform/gitlab/mutation_test.go). First unit of reimplementing PR #499's Playwright->Vitest demotion on the current browser tier, which postdates and supersedes #499's jsdom approach. Validation: full browser project green (15 files, 50 tests). Generated with Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The force-mobile-routes e2e-full spec checked a pure presentation contract: the __MIDDLEMAN_FORCE_MOBILE_ROUTES__ global makes /issues render the focus layout (no mobile shell, no app header) even on a desktop-width viewport. That needs only a real DOM, so it moves to the vitest-browser tier, where the real App is mounted in Chromium with the flag set before render. A contrast case without the flag pins that the same route falls back to the standard desktop shell, proving the presentation is flag-driven rather than incidental. App.svelte reads the flag at render time (shouldForceMobileRoutes), so setting window.__MIDDLEMAN_FORCE_MOBILE_ROUTES__ before mountBrowserApp is sufficient and the value is reset between cases. Validation: browser project file green (2 tests). Generated with Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The stack-status e2e-full spec covered three things through a seeded Go server: the stack panel rendering (summary, per-member conflict labels, member rows, passive base row), focus-route member navigation, and the activity-drawer member navigation. The first two are pure stack-data rendering plus client-side routing, so they move to the vitest-browser tier: the real App is mounted in Chromium with the acme/tools auth-stack detail mocked at the fetch boundary, and the rendered panel and the post-navigation URL are the genuine outputs of that data. The member data mirrors internal/testutil/fixtures.go. The raw DB marks only #10 dirty, but the stack API propagates downstack conflicts upward (the spec asserts the conflict label three times), so the mocked stack response carries mergeable_state dirty on every member -- the response shape the panel renders. The activity-drawer case stays in Playwright: opening the drawer from a selected= URL param and rewriting the route query on member navigation is cross-layer drawer/routing glue better exercised against the live backend. Validation: browser project file green (2 tests). Generated with Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The link-navigation-repo-sync spec is a regression guard: deep-linking to a PR/issue used to leave the repo dropdown and sidebar list pinned to a previously chosen repo while the detail jumped elsewhere. The PR-side cases (initial cross-repo deep-link, back-to-back PR switches via popstate, all-repos selection, and preserving the chosen repo on a bare /pulls) are client-side routing + dropdown state, so they move to the vitest-browser tier: the real App is mounted in Chromium with the list/detail mocked at the fetch boundary, and the pull list honors the repo= query param the store sends so the sidebar group reflects the same filter the live backend applies. Reproducing the sync requires both repos configured (the normalization resolves only configured repos) and driving the global-repo store directly, since its state initializes from localStorage once at module load. The issue deep-link variant stays in Playwright: the issues-route sync interacts with the issue detail and settings load in a way exercised most faithfully end-to-end. Validation: browser project file green (4 tests). Generated with Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The collapsible-repos spec exercised repo/status group collapse, the retained header count, localStorage persistence across reload, and the independence of the pulls vs issues collapse sets. These are real DOM + localStorage behaviors with no backend dependency, so they move to the vitest-browser tier: the app is mounted in Chromium with the pull/issue lists mocked at the fetch boundary, and a remount stands in for the reload to assert the persisted state. The keyboard-activation case (Enter/Space on the header button) stays in Playwright: native button activation is a real key event that the browser tier's synthetic KeyboardEvent dispatch cannot trigger. Validation: browser project file green (7 tests). Generated with Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The activity-row-link spec guards that flat and threaded activity rows keep a link button that opens the provider URL via window.open without opening the in-app drawer, that threaded rows stay clickable after the split detail pane opens, and that the compact threaded row keeps the expand chevron separate from the type chip. These are real rendering, real window.open, and real geometry, so the whole spec moves to the vitest-browser tier: the app is mounted in Chromium with the activity feed mocked at the fetch boundary, window.open is spied with vi.spyOn, and the chevron gap is measured with getBoundingClientRect. Validation: browser project file green (4 tests). Generated with Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tier The navigation spec's app-shell flows that need only the activity feed and the PR/issue lists move to the vitest-browser tier: selecting an activity item and returning to it after visiting PRs or the settings gear preserves the selection in the URL and reopens the split view; the legacy /mail route falls through to Activity rather than Messages; and clicking a list row opens the detail pane. The app is mounted in Chromium with those feeds mocked at the fetch boundary. The cross-mode tab tour, the Kata-shell repo-selector/shortcut specifics, the direct Docs/Messages loads, and the settings/diff route toggle stay in Playwright: they depend on the external Kata/Docs/Messages mode-shell backends and diff rendering, which are full-stack concerns. Validation: browser project file green (5 tests). Generated with Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both cross-repository timeline-reference cases (tracked -> internal navigation, untracked -> provider window.open) are real fetch-boundary behavior the browser tier reproduces without a live Go backend, so they move to App.item-reference-routing.browser.svelte.ts and the e2e-full Playwright spec is removed. The detail title (.detail-title) only renders when the repo capabilities include state_mutation; the mocked repoRef now sets it so the title binds, matching the seeded server's GitHub capability set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The browser-tier force-mobile test only checked that .focus-layout .focus-list existed in the DOM, so a regression that left the focus layout mounted but hidden by CSS would have passed. The deleted Playwright spec asserted visibility; restore that guarantee with a visibility assertion plus a non-empty bounding rect (roborev #2385). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The timeline rendering and filter behavior computed entirely in the Svelte EventTimeline component moves to App.pr-timeline-filters.browser.svelte.ts: seeded commit/system event rendering, merge/close lifecycle collapse into one purple row, hiding and restoring the View-menu event buckets, and persisting the bucket filter across a reload. The event fixture mirrors the Go seed. The Playwright spec keeps only the cases that genuinely need the live backend: the force-push commit-generation ordering and fresh-import ordering assert the order the Go sync computes via commit_order_key metadata (the frontend falls back to event ID order without it, so a hand fixture cannot exercise the algorithm), the compact review-thread reply layout depends on backend-transformed review-thread event cards plus reply round-trips, and the regroup case drives a live __e2e server hook. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The grouping toggle is one store shared across the PR list, issue list, and threaded activity view plus a hide-org-name preference, all of which render deterministically without real geometry. App.grouping-toggle.browser.svelte.ts covers them end to end: PR-list grouped headers vs per-item repo chips, persistence across a reload, sync into the issue list and threaded activity, cross-repo thread separation, the threaded-only grouping control, hide-org-name relabeling in flat and threaded activity, and the threaded empty state. The Playwright spec keeps only j/k navigation, which asserts selection follows the flat visual order and needs native key handling plus scroll-into-view. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…state The grouping persistence test remounts the app (the browser harness cannot do a true page reload without losing the API mocks), which would also pass if a regression kept the grouping choice only in module-level state. Assert the persisted middleman:groupingMode value at toggle time so persistence is proven to route through localStorage, the same value a real reload rereads (roborev #2561). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The repo-tree selector component logic (multi-select checkboxes, owner cascade, stale provider-qualified value normalization, provider-collision qualification, single-repo-owner flatten labels) is already covered by the jsdom component tests RepoTypeahead/RepoTreeNode/repoTree.test.ts, and the backend repo filter by internal/server TestAPIRepoFilterAcceptsMultipleRepos. Those four e2e-full cases were redundant with that faster coverage. The spec keeps only the cases that genuinely need the live backend: the provider-collision persistence drives a server seeded with two hosts that collide by path, and the native-click focus retention proves a real checkbox click does not blur the filter input (a native interaction jsdom cannot reproduce). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The app-startup readiness-gating and settings-timeout cases cannot move to the vitest-browser tier: the browser mock hard-codes /healthz and /livez to 200 so ordinary browser tests do not spin on startup, which makes the 503 readiness gating impossible to reproduce there, and the timeout case fast-forwards a fake clock past the 8s settings-startup timeout, which the browser harness has no equivalent for. Record that decision in a header so the migration's coverage map stays legible; the spec behavior is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The navigation browser spec's settings-gear case routes to /settings, which renders FleetSettings; that component dereferences fleet.sessions at init, so the hand-built settings override crashed with an unhandled 'reading enabled of undefined' during the full Vitest run (the assertions still passed, but the unhandled error risks false positives). Carry the same fleet block the default mock provides. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
roborev: Review Unavailable (
|
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
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.
Reimplements PR #499's Playwright→Vitest e2e demotions on the current vitest-browser test tier. Test-only; no production code changes.
e2e-fullPlaywright specs to browser-tierApp.*.browser.svelte.tsspecs that mount the real app against mocked fetch: provider capabilities, force-mobile routes, stack-panel rendering, repo-dropdown route sync, repo-group collapse, view navigation, activity row links, item-reference routing, PR timeline filters, and the repo grouping toggle.commit_order_key), review-thread reply layout, the live__e2eregroup hook, provider-collision persistence, and native checkbox-focus behavior.repo-filter-multiselectto its live-stack cases only; its component logic is already covered by existing jsdomRepoTypeahead/RepoTreeNode/repoTreetests and Go's repo-filter test.app-startupfull-stack (the browser mock can't gate/healthzor fast-forward the settings timeout) with a header documenting why.🤖 Generated with Claude Code
generated by a clanker