-
-
Notifications
You must be signed in to change notification settings - Fork 0
release: bring audit stack (19 PRs) into develop #285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3412e4e
03da9cb
07a76d6
e27f1ba
c519fc3
5b1cc55
0eb49a5
402e280
dd4823d
5607100
c51c4d3
d5f33ef
5ba96d4
2bd1396
fd9b809
a3d7c1b
0ec48b3
cca7e04
1a0df95
9034c71
390503c
b668828
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # E2E tests (Playwright + Electron) | ||
|
|
||
| End-to-end tests for the desktop app, driven through Playwright's `_electron` API. Tests launch the **built** Electron bundle in `out/`, so you must run `pnpm build` (or `pnpm dev` for headed iteration) before they pass. | ||
|
|
||
| ## Running locally | ||
|
|
||
| ```bash | ||
| # From repo root | ||
| pnpm --filter @readied/desktop build # produces out/main/index.js | ||
| pnpm --filter @readied/desktop e2e # headless | ||
| pnpm --filter @readied/desktop e2e:headed # opens the window | ||
| ``` | ||
|
|
||
| First run also downloads Playwright's browser binaries: | ||
|
|
||
| ```bash | ||
| npx playwright install --with-deps | ||
| ``` | ||
|
|
||
| (`--with-deps` only matters on Linux, where it installs system libs.) | ||
|
|
||
| ## Isolation | ||
|
|
||
| `launchApp()` in `fixtures.ts` creates a fresh temp `userData` dir per test, so: | ||
|
|
||
| - The SQLite DB starts empty every time. | ||
| - Settings, license cache, AI keys, etc. don't leak between tests. | ||
| - The host's real Readied data is never touched. | ||
|
|
||
| Set `READIED_E2E_KEEP_USERDATA=1` to keep the temp dir on failure for post-mortem inspection. | ||
|
|
||
| ## What we test | ||
|
|
||
| | Spec | What it covers | | ||
| | --------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `smoke.spec.ts` | App launches, main window renders, IPC bridge present, no uncaught console errors during initial mount. This is the regression catch for #266 (editor mount crashes producing blank windows). | | ||
| | `notes.spec.ts` | Notes IPC contract — create / list / get roundtrip, FTS5 search returns freshly-created notes. We deliberately drive the **preload bridge** (`window.readied.notes.*`) rather than the editor UI; selectors churn but the contract is stable. | | ||
|
|
||
| ## What we deliberately don't test (yet) | ||
|
|
||
| - **Editor UI interactions** (typing, formatting, hotkeys). The CodeMirror surface is too prone to flake without per-spec selectors. Worth doing once the editor is split (see PR-G in the audit). | ||
| - **AI panel streaming.** Needs a mock provider and is more useful as a vitest test against `@readied/ai-core`. | ||
| - **Sync flows.** Need a fake server. | ||
|
|
||
| These will be follow-ups once the basics are stable in CI. | ||
|
|
||
| ## CI | ||
|
|
||
| The `e2e` job in `.github/workflows/ci.yml` runs on Linux + xvfb. It starts as `continue-on-error: true` — the goal of this PR is to land the infrastructure, not to gate every PR on E2E green. Once the suite is verified end-to-end on a real CI run, flip the flag off in a follow-up. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| /** | ||
| * Shared E2E fixtures for Electron app tests. | ||
| * | ||
| * `launchApp()` launches a fresh Electron instance with an isolated | ||
| * userData directory so tests don't interfere with each other or with | ||
| * a developer's local Readied install. Each test should call this in | ||
| * its own `beforeEach`. | ||
| */ | ||
|
|
||
| import { mkdtemp, rm } from 'fs/promises'; | ||
| import { tmpdir } from 'os'; | ||
| import { join } from 'path'; | ||
| import { _electron as electron, type ElectronApplication, type Page } from '@playwright/test'; | ||
|
|
||
| interface LaunchedApp { | ||
| app: ElectronApplication; | ||
| window: Page; | ||
| userDataDir: string; | ||
| /** Call in afterEach. */ | ||
| cleanup: () => Promise<void>; | ||
| } | ||
|
|
||
| /** | ||
| * Launches the desktop app and waits for the first window to be ready. | ||
| * | ||
| * Uses a fresh temp `userData` so the test gets an empty database every | ||
| * time. Set READIED_E2E_KEEP_USERDATA=1 to keep the dir on failure for | ||
| * post-mortem. | ||
| */ | ||
| export async function launchApp(): Promise<LaunchedApp> { | ||
| const userDataDir = await mkdtemp(join(tmpdir(), 'readied-e2e-')); | ||
|
|
||
| const app = await electron.launch({ | ||
| args: [ | ||
| '.', | ||
| `--user-data-dir=${userDataDir}`, | ||
| // Disable updates / external network checks during tests. | ||
| '--disable-features=AutoUpdate', | ||
| ], | ||
| env: { | ||
| ...process.env, | ||
| NODE_ENV: 'test', | ||
| READIED_E2E: '1', | ||
| // Pin the data root explicitly so the app uses our temp dir for | ||
| // its SQLite database too, not just for Electron's userData. | ||
| READIED_DATA_DIR: userDataDir, | ||
| }, | ||
| }); | ||
|
|
||
| const window = await app.firstWindow(); | ||
| // Wait for the renderer to finish initial paint. | ||
| await window.waitForLoadState('domcontentloaded'); | ||
|
|
||
| return { | ||
| app, | ||
| window, | ||
| userDataDir, | ||
| cleanup: async () => { | ||
| await app.close().catch(() => {}); | ||
| if (process.env.READIED_E2E_KEEP_USERDATA !== '1') { | ||
| await rm(userDataDir, { recursive: true, force: true }).catch(() => {}); | ||
| } | ||
| }, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| import { test, expect } from '@playwright/test'; | ||
| import { launchApp } from './fixtures.js'; | ||
|
|
||
| /** | ||
| * Notes CRUD end-to-end. | ||
| * | ||
| * We exercise the IPC contract directly through the preload bridge | ||
| * (`window.readied.notes`) rather than driving the editor UI. This is | ||
| * intentional: | ||
| * - The UI elements (selectors, labels, hotkeys) churn often. Asserting | ||
| * against the IPC surface gives us regression coverage on the | ||
| * *contract* that survives renderer refactors. | ||
| * - Anything that breaks here also breaks the desktop's renderer code, | ||
| * because the renderer uses the same bridge. | ||
| */ | ||
| test.describe('notes IPC contract', () => { | ||
| test('create → list → read roundtrip', async () => { | ||
| const { window, cleanup } = await launchApp(); | ||
| try { | ||
| const noteId = `e2e-${Date.now()}-${Math.floor(Math.random() * 1e6)}`; | ||
| const content = '# E2E note\n\nbody from playwright'; | ||
|
|
||
| const createResult = await window.evaluate( | ||
| async ([id, body]) => { | ||
| const api = ( | ||
| window as unknown as { | ||
| readied: { | ||
| notes: { | ||
| create: (input: { | ||
| id?: string; | ||
| content: string; | ||
| notebookId?: string; | ||
| }) => Promise<unknown>; | ||
| list: ( | ||
| opts?: Record<string, unknown> | ||
| ) => Promise<Array<{ id: string; title: string; content: string }>>; | ||
| get: (id: string) => Promise<unknown>; | ||
| }; | ||
| }; | ||
| } | ||
| ).readied; | ||
| const created = await api.notes.create({ id, content: body }); | ||
| return { created }; | ||
| }, | ||
| [noteId, content] as const | ||
| ); | ||
|
|
||
| expect(createResult.created).toBeTruthy(); | ||
|
|
||
| const list = await window.evaluate( | ||
| async () => | ||
| ( | ||
| window as unknown as { | ||
| readied: { | ||
| notes: { | ||
| list: () => Promise<Array<{ id: string; title: string; content: string }>>; | ||
| }; | ||
| }; | ||
| } | ||
| ).readied.notes.list(), | ||
| undefined | ||
| ); | ||
|
|
||
| const ourNote = list.find(n => n.id === noteId); | ||
| expect(ourNote, `note ${noteId} missing from list`).toBeDefined(); | ||
| expect(ourNote!.content).toContain('body from playwright'); | ||
| } finally { | ||
| await cleanup(); | ||
| } | ||
| }); | ||
|
|
||
| test('search returns the freshly-created note via FTS5', async () => { | ||
| const { window, cleanup } = await launchApp(); | ||
| try { | ||
| const marker = `marker_${Date.now()}_unique`; | ||
| await window.evaluate( | ||
| async ([body]) => { | ||
| const api = ( | ||
| window as unknown as { | ||
| readied: { | ||
| notes: { create: (input: { content: string }) => Promise<unknown> }; | ||
| }; | ||
| } | ||
| ).readied; | ||
| await api.notes.create({ content: `# Searchable\n\n${body}` }); | ||
| }, | ||
| [marker] as const | ||
| ); | ||
|
|
||
| const results = await window.evaluate( | ||
| async ([q]) => | ||
| ( | ||
| window as unknown as { | ||
| readied: { | ||
| notes: { | ||
| search: ( | ||
| query: string, | ||
| limit?: number | ||
| ) => Promise<Array<{ id: string; content: string }>>; | ||
| }; | ||
| }; | ||
| } | ||
| ).readied.notes.search(q, 10), | ||
| [marker] as const | ||
| ); | ||
|
|
||
| expect(results.length).toBeGreaterThan(0); | ||
| expect(results.some(r => r.content.includes(marker))).toBe(true); | ||
| } finally { | ||
| await cleanup(); | ||
| } | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import { test, expect } from '@playwright/test'; | ||
| import { launchApp } from './fixtures.js'; | ||
|
|
||
| test.describe('app launch (smoke)', () => { | ||
| test('launches and shows the main window', async () => { | ||
| const { app, window, cleanup } = await launchApp(); | ||
| try { | ||
| // Title is "Readied" in production. Allow any non-empty title in case | ||
| // dev/test envs use a different one. | ||
| const title = await window.title(); | ||
| expect(title.length).toBeGreaterThan(0); | ||
|
|
||
| // First window must render *something* — a <body> element with non-zero | ||
| // size is a low bar that catches the regression class from PR #266 | ||
| // (editor mount crashes that produced a blank window). | ||
| const bodyBox = await window.locator('body').boundingBox(); | ||
| expect(bodyBox).not.toBeNull(); | ||
| expect(bodyBox!.width).toBeGreaterThan(0); | ||
| expect(bodyBox!.height).toBeGreaterThan(0); | ||
|
|
||
| // Sanity: the app exposed its IPC bridge. | ||
| const hasBridge = await window.evaluate( | ||
| () => typeof (window as unknown as { readied?: unknown }).readied !== 'undefined' | ||
| ); | ||
| expect(hasBridge).toBe(true); | ||
|
|
||
| expect(app.windows().length).toBeGreaterThanOrEqual(1); | ||
| } finally { | ||
| await cleanup(); | ||
| } | ||
| }); | ||
|
|
||
| test('console does not log uncaught errors during initial render', async () => { | ||
| const { window, cleanup } = await launchApp(); | ||
| const consoleErrors: string[] = []; | ||
| window.on('console', msg => { | ||
| if (msg.type() === 'error') consoleErrors.push(msg.text()); | ||
| }); | ||
| window.on('pageerror', err => consoleErrors.push(`pageerror: ${err.message}`)); | ||
|
|
||
| try { | ||
| // Give the renderer 3s to throw any early errors during mount. | ||
| await window.waitForTimeout(3000); | ||
|
|
||
| // Known non-fatal noise that the app emits in test/dev environments. | ||
| // Strip these out before asserting "no errors". | ||
| const ignored = [ | ||
| /\[Sentry\]/, // "No DSN configured" — expected without VITE_SENTRY_DSN | ||
| /Failed to load resource: net::ERR_/, // network during dev sometimes | ||
| ]; | ||
| const real = consoleErrors.filter(line => !ignored.some(re => re.test(line))); | ||
|
Comment on lines
+47
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error filter patterns are too broad and may hide real failures. The regex patterns 🔍 Proposed fix with more specific patterns const ignored = [
- /\[Sentry\]/, // "No DSN configured" — expected without VITE_SENTRY_DSN
- /Failed to load resource: net::ERR_/, // network during dev sometimes
+ /\[Sentry\].*No DSN configured/, // Expected without VITE_SENTRY_DSN
+ /Failed to load resource:.*net::ERR_FILE_NOT_FOUND/, // Dev HMR artifacts
];🤖 Prompt for AI Agents |
||
|
|
||
| expect(real, real.join('\n')).toEqual([]); | ||
| } finally { | ||
| await cleanup(); | ||
| } | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "extends": "../../../tsconfig.base.json", | ||
| "compilerOptions": { | ||
| "module": "ESNext", | ||
| "moduleResolution": "bundler", | ||
| "rootDir": ".", | ||
| "noEmit": true | ||
| }, | ||
| "include": ["**/*.ts"] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource leak if launch fails after temp directory creation.
If
electron.launch()orapp.firstWindow()throws, the temp directory created on line 31 is never cleaned up, since thecleanupfunction isn't returned. While CI runners clean up automatically, local development can accumulate orphaned temp directories if launch failures persist.🛡️ Proposed fix to ensure cleanup on launch failure
export async function launchApp(): Promise<LaunchedApp> { const userDataDir = await mkdtemp(join(tmpdir(), 'readied-e2e-')); + + try { + const app = await electron.launch({ + args: [ + '.', + `--user-data-dir=${userDataDir}`, + '--disable-features=AutoUpdate', + ], + env: { + ...process.env, + NODE_ENV: 'test', + READIED_E2E: '1', + READIED_DATA_DIR: userDataDir, + }, + }); - const app = await electron.launch({ - args: [ - '.', - `--user-data-dir=${userDataDir}`, - // Disable updates / external network checks during tests. - '--disable-features=AutoUpdate', - ], - env: { - ...process.env, - NODE_ENV: 'test', - READIED_E2E: '1', - // Pin the data root explicitly so the app uses our temp dir for - // its SQLite database too, not just for Electron's userData. - READIED_DATA_DIR: userDataDir, - }, - }); - - const window = await app.firstWindow(); - // Wait for the renderer to finish initial paint. - await window.waitForLoadState('domcontentloaded'); - - return { - app, - window, - userDataDir, - cleanup: async () => { - await app.close().catch(() => {}); - if (process.env.READIED_E2E_KEEP_USERDATA !== '1') { - await rm(userDataDir, { recursive: true, force: true }).catch(() => {}); - } - }, - }; + const window = await app.firstWindow(); + await window.waitForLoadState('domcontentloaded'); + + return { + app, + window, + userDataDir, + cleanup: async () => { + await app.close().catch(() => {}); + if (process.env.READIED_E2E_KEEP_USERDATA !== '1') { + await rm(userDataDir, { recursive: true, force: true }).catch(() => {}); + } + }, + }; + } catch (err) { + await rm(userDataDir, { recursive: true, force: true }).catch(() => {}); + throw err; + } }📝 Committable suggestion
🤖 Prompt for AI Agents