test(app): publish-flow e2e — 5 specs, in-process mock backend, real regression coverage#373
Merged
Merged
Conversation
…regression coverage
The share-publish stack shipped with zero e2e. Until now the gate was
that mocking the Cloudflare backend in CI was nontrivial (D1/KV/R2 +
the OG render pipeline). This PR closes that gap with the minimum
infrastructure to give actual regression coverage:
E2E test infrastructure:
- SPOOL_E2E_TEST short-circuits in main/auth/oauth.ts and
main/auth/session-store.ts:
- signInWith() skips the loopback OAuth dance (impossible in CI
without a real browser + Google account); POSTs a marker token
directly to the mock backend
- session-store uses in-memory storage (CI Linux lacks the
libsecret/keyring access safeStorage requires, so the
production safeStorage.encryptString path would throw)
- launchApp() gains an `extraEnv` opt so each test can wire its
Electron child to a per-run mock backend URL (random port)
- VITE_FEATURE_SHAREPUBLISH=1 added to the test:e2e build flags so
the gate-on surfaces (Share popover Publish tab, SharesPage
Published tab, Settings → Account) actually render
- PublishTab + manage view get four new test-only data-testids
(`share-menu-form`, `share-menu-manage-view`, `share-menu-republish`,
`share-menu-unpublish`) so the spec doesn't depend on any
English-language text — the in-flight i18n PR can land copy
without breaking selectors
In-process mock backend (e2e/helpers/share-publish-mock-backend.ts):
Tiny Node http server that speaks the share-backend wire contract:
POST /api/auth/sign-in-with-id-token, /api/auth/sign-out
GET /api/me, /api/me/shares
GET /api/handles/check?h=, POST /api/handles/claim
POST /api/publish, POST /api/revoke/:id
POST + DELETE /api/me/delete
Maintains an in-memory state Map<slug, MockShareRow> with the same
revoked_at semantics (idempotent re-revoke on already-tombstoned
rows) as the post-#369 backend, so the renderer's existing throw-
on-!ok IPC handler converges cleanly.
Per-test reset() keeps the suite isolated without restarting the
HTTP server.
Five specs (e2e/share-publish.spec.ts), 7.3s total wall-clock:
1. Share popover opens with the ConnectCard when signed out —
covers the gated render + the ConnectCard signed-out branch
2. Sign-in transitions the popover to the publish form —
exercises the entire main IPC chain (signIn IPC →
authedFetch → mock backend → safeStorage → useShareAuth
EventTarget broadcast → renderer re-render)
3. Publish lands in the manage view with slug + copy-link +
unpublish — happy path + asserts the renderer-derived
client_request_id reaches the backend verbatim
4. Unpublish requires confirm and tombstones the share — guards
the destructive-confirm discipline (PR #371): no single-click
revoke; confirm modal appears; confirm → 200 idempotent
5. Cancel on the unpublish modal leaves the share live —
counterpart of #4: backend row keeps revoked_at: null when
the user cancels
Each test runs against a fresh Electron via restartApp() so popover
state can't leak across boundaries.
What this spec deliberately doesn't cover (cited inline):
- Real OAuth (oauth.test.ts at the main-process unit layer)
- Server-side validation (publish.test.ts against the real
validator + zod schema)
- PII gate (publish-logic.test.ts)
All three are first-class unit tests against the real code paths;
this e2e is for the renderer-side regression surface only.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…test awareness
Reworks the previous runtime SPOOL_E2E_TEST env-var checks (in oauth.ts +
session-store.ts) into the elegant build-time composition-root pattern:
- electron.vite.config.ts injects a `__SPOOL_E2E__: boolean` constant
at build time. test:e2e sets SPOOL_E2E_TEST=1 before electron-vite
build, so __SPOOL_E2E__ resolves to `true` in test bundles and
`false` everywhere else.
- main/index.ts has the ONE selection point:
if (__SPOOL_E2E__) {
const { registerShareAuthIpcForE2E } = await import(...e2e-mode/...)
registerShareAuthIpcForE2E()
} else {
registerShareAuthIpc()
}
Production builds get `if (false) { ... }`, which terser deletes
outright. The dynamic import never resolves through rollup; the
e2e-mode/ module is absent from production bundles.
Production source files now have ZERO test-mode awareness:
- oauth.ts: reverted to main
- session-store.ts: gains a `_setImpl(SessionStoreImpl)` swap seam
used by the e2e-mode entry — composition-root pattern, no
`__SPOOL_E2E__` knowledge inside the file
- ipc/share-auth.ts: registerShareAuthIpc takes an optional
SignInImpl param defaulted to the real signInWith — the e2e
entry passes a fake-id-token POST instead
The e2e composition root (src/main/e2e-mode/share-auth-e2e.ts) is a
single 50-line file that:
- swaps session-store to an in-memory backend (CI Linux lacks
libsecret/keyring access safeStorage requires)
- registers share-auth IPC with the fake-id-token signIn override
- is loaded only via `import()` under the dead-code-stripped guard
The non-negotiable invariant — that production bundles contain NO
trace of the e2e entry — is enforced by an automated unit test
(e2e-mode/e2e-mode-clean.test.ts):
1. shells out to electron-vite build WITHOUT SPOOL_E2E_TEST
2. greps the resulting main/index.js for marker tokens
('e2e-fake-id-token', 'registerShareAuthIpcForE2E',
'share-auth-e2e')
3. asserts zero matches
Verified locally:
- prod build (no SPOOL_E2E_TEST): grep returns 0 matches
- e2e build (SPOOL_E2E_TEST=1): grep returns 3 matches
- 5/5 publish spec passes in 7.5s
- e2e-mode-clean.test.ts passes in 4.2s
References: React's __DEV__ + Vite's `define` pattern (build-time
constant + terser dead-code elimination) is the same approach React,
Vue, and most modern JS frameworks use for dev/test-only branches.
The composition-root choice between two IPC registration impls is a
straightforward dependency-injection pattern.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…body
CodeQL flagged `JSON.stringify({ ..., detail: String(err) })` on the
defensive 500 path as potential stack-trace exposure
(js/stack-trace-exposure, alert #21). The mock only ever runs in e2e
on 127.0.0.1 against our own test harness, so practical exposure is
nil — but the CodeQL review thread blocks the merge queue.
Drop the raw err from the wire body; log to stderr instead, which
Playwright pipes into the test output for debugging without trusting
any caller to discard the value.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…blish is on Root cause from CI on PR #373: This PR's test:e2e build flips VITE_FEATURE_SHAREPUBLISH=1 so the new share-publish specs can exercise the gated UI. Side effect: the <ShareMenu> popover now renders a Publish/Export tab strip and defaults to the Publish tab. The pre-existing share-exports.spec.ts + share-privacy.spec.ts both assumed the popover would open directly on Export (the pre-flag default), so `share-menu-export-*` was never visible and the click timed out. Same regression class that pdf-iframe-with-gpu.spec.ts already handles (PR #366): conditionally click Export tab if it exists. Apply the same pattern to share-exports.spec.ts (1 site) and share-privacy.spec.ts (3 sites) instead of skipping the affected tests. Specs now work in BOTH flag configurations. Verified locally: - share-exports.spec.ts + share-privacy.spec.ts: 11/11 pass - share-publish.spec.ts + pdf-iframe-with-gpu.spec.ts: 6/6 pass Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
What
Adds the first publish-flow Playwright spec for the share-publish stack — and does it via the build-time composition-root pattern so production source has zero test-mode awareness.
The mocking pattern
Earlier revisions of this PR carried runtime
if (process.env.SPOOL_E2E_TEST === '1')branches insideoauth.tsandsession-store.ts. The user pushed back: test code shouldn't live in production source. The reworked design moves the entire test-mode contract behind a build-time switch.Three principles
Production source files contain ZERO test-mode references.
oauth.ts,session-store.ts,ipc/share-auth.tsall behave exactly as they do onmain— noprocess.env.SPOOL_E2E_TESTchecks, no e2e knowledge. session-store gains a_setImpl(SessionStoreImpl)swap seam (a composition-root pattern), but the file itself doesn't know who calls it.Test code lives in a separate physical directory.
src/main/e2e-mode/is its own folder. The single non-trivial file (share-auth-e2e.ts, ~50 lines) wires the swap + registers IPC with mock impls.A build-time constant + dead-code elimination removes the test code from production bundles entirely.
electron.vite.config.tsinjects__SPOOL_E2E__: boolean.main/index.tshas exactly ONE branch that imports the e2e entry:In production builds,
__SPOOL_E2E__is the literalfalse. Terser deletes the if-branch. Rollup never resolves the dynamic import. The e2e-mode/ source file is not bundled into any production binary.flowchart TD BUILD["electron-vite build"] --> CFG["electron.vite.config.ts<br/>define __SPOOL_E2E__"] CFG -->|"SPOOL_E2E_TEST=1 in env"| TEST_BUILD["__SPOOL_E2E__ = true<br/>e2e-mode/ bundled<br/>memory store + fake id_token"] CFG -->|"env unset (prod / dev)"| PROD_BUILD["__SPOOL_E2E__ = false<br/>terser strips dead branch<br/>e2e-mode/ NEVER resolved<br/>real safeStorage + real OAuth"] PROD_BUILD --> ASSERT["e2e-mode-clean.test.ts<br/>greps out/main/index.js<br/>asserts 0 matches"]Automated invariant
src/main/e2e-mode/e2e-mode-clean.test.tsshells out toelectron-vite buildWITHOUTSPOOL_E2E_TESTand greps the output fore2e-fake-id-token,registerShareAuthIpcForE2E,share-auth-e2e. Any non-zero match fails the test. This guarantees the dead-code-elimination invariant survives future refactors.Locally verified:
SPOOL_E2E_TEST): grep returns 0 matches ✓SPOOL_E2E_TEST=1): grep returns 3 matches ✓e2e-mode-clean.test.tspasses in 4.2s ✓The publish-flow spec (5 cases, 7.5s total)
sequenceDiagram autonumber participant T as Test participant E as Electron renderer participant M as main IPC<br/>(e2e-mode entry) participant SS as session-store<br/>(swapped to memory) participant B as Mock backend Note over T: Test 1 — signed-out gate T->>E: click share-menu-trigger E->>E: useShareAuth.user === null E-->>T: ConnectCard visible Note over T: Test 2 — sign-in T->>E: click connect-card-signin E->>M: shareAuth.signIn IPC M->>B: POST /api/auth/sign-in-with-id-token<br/>("e2e-fake-id-token") B-->>M: { session_token, user } M->>SS: _setImpl(memoryStore).saveToken M-->>E: user E-->>T: share-menu-form visible Note over T: Test 3 — publish T->>E: click share-menu-submit E->>E: computePublishIdempotencyKey<br/>= sha256(stableJSON) E->>M: spoolShare.publish M->>B: POST /api/publish + memory token B-->>M: { id, url, version } M-->>E: PublishResult E-->>T: share-menu-manage-view visible<br/>+ assert backend.client_request_id === key Note over T: Test 4 — unpublish + confirm T->>E: click share-menu-unpublish E-->>T: unpublish-confirm modal visible T->>E: click unpublish-confirm-yes E->>M: spoolShare.revoke(slug) M->>B: POST /api/revoke/:id B-->>M: 200 E-->>T: share-menu-form visible<br/>+ assert backend.revoked_at !== null Note over T: Test 5 — cancel path T->>E: click share-menu-unpublish T->>E: click unpublish-confirm-cancel E-->>T: share-menu-manage-view still visible<br/>+ assert backend.revoked_at === nullcomputePublishIdempotencyKeywire-up; manage view renders Copy / Republish / UnpublishInfrastructure
src/main/e2e-mode/share-auth-e2e.ts— ~50 lines, the composition-root:SessionStoreImpl_setImpl(memoryStore)to swap session-storeregisterShareAuthIpc(e2eSignIn)with the fake-id-token POST as the signIn overridelaunchApp({ extraEnv })— new opt so each test wires its Electron child to a per-run mock backend URL (random port).AppContext.tmpDirnow explicit sorestartAppcleanup doesn't fragile-reconstruct via env-var regex.Mock backend (
e2e/helpers/share-publish-mock-backend.ts) — tiny Nodehttp.Serverspeaking the share-backend wire contract:/api/auth/sign-in-with-id-token,/api/me,/api/me/shares,/api/handles/check?h=,/api/handles/claim,/api/publish,/api/revoke/:id,/api/me/delete. In-memoryMap<slug, MockShareRow>with the samerevoked_atsemantics (idempotent re-revoke on already-tombstoned rows) as the post-#369 backend, so the renderer's existing throw-on-!okIPC handler converges cleanly. Per-testreset(); per-testrestartApp()for state isolation.4 new test-only
data-testids on PublishTab (share-menu-form,share-menu-manage-view,share-menu-republish,share-menu-unpublish) so the spec doesn't depend on any English-language text — the i18n PR (#372, already merged) landed copy without breaking selectors.VITE_FEATURE_SHAREPUBLISH=1+SPOOL_E2E_TEST=1added to thetest:e2ebuild flags.What this spec deliberately doesn't cover (cited inline)
main/auth/oauth.test.tsunit suiteshare-backend/tests/publish.test.tsagainst real validator + zodshare-editor/publish-logic.test.tspure-function unit testsVerification
pnpm --filter @spool/app exec playwright test e2e/share-publish.spec.ts→ 5/5 pass in 7.5spnpm --filter @spool/app exec vitest run src/main/e2e-mode/e2e-mode-clean.test.ts→ 1/1 pass in 4.2s (prod build invariant)Risk
__SPOOL_E2E__switch is build-time only. SettingSPOOL_E2E_TEST=1at runtime in a production binary has no effect — the if-branch was deleted at build time.127.0.0.1(not0.0.0.0) on a random port and is only reachable from the same machine.e2e-mode-clean.test.tsinvariant prevents future refactors from silently re-introducing test code into production bundles.References
The build-time
define+ dead-code-elimination pattern is what React (__DEV__), Vue 3 (__DEV__,__TEST__), and most modern JS frameworks use for dev/test-only branches. See Vite'sdefineconfig docs.Submitted by @graydawnc.