From 4eaa180f3578af870d723c339a6bc1cbe51c3bac Mon Sep 17 00:00:00 2001 From: Kaan Karaca Date: Sat, 18 Apr 2026 08:16:44 +0300 Subject: [PATCH] test(e2e): replace wall-clock waits with product-emitted signals Structural fix for shard 1/3 flakiness. Stops patching individual tests. Root cause: tests asserted on async effects (CSS smooth-scroll completion, cross-device CRDT convergence) that the product emits no readiness signal for, and compensated with fixed waitForTimeout values tuned for fast local dev boxes. Under CI xvfb load those values were exceeded and tests read mid-flight state. Previous stabilizations only widened the wall-clock windows. Three layered changes: 1. Product signals (minimal, targeted): - src/renderer/components/calendar/calendar-week-view.tsx: expose anchorDate + visibleDayStart as data attributes so tests can observe state directly instead of reverse-engineering via scrollLeft. - src/main/test-hooks.ts: add hasNoteOnDevice(noteId) hook returning recordPresent/crdtPresent/crdtBody for the local device. Lets tests probe replication with a single main-process roundtrip instead of polling UI body text through the renderer. 2. Reusable test primitives (new wait-helpers.ts): - waitForStable: polls a value and returns when it has not changed for stableFor ms, or throws on timeout. Signal-based, CI-safe. - waitForNoteReplicated: wraps the new test hook and normalizes the expected body. Replaces the loop-then-hard-equal pattern. - getAnchorDate / waitForAnchorDate / getVisibleDayStart helpers. 3. Test rewrites: - calendar-week-scroll.e2e.ts: assertions shifted from pixel-tolerance scrollLeft comparisons to user-observable semantics (visible week moved forward, Today returns to origin within 1-day rounding slack). Deleted the waitForTimeout-based waitForScrollSettle helper. - body-crdt-coverage-variants.e2e.ts V6: replaced the syncBothAndWait inside expect.poll loop with one sync round-trip plus per-device waitForNoteReplicated in parallel. Removed the orphaned readReplicatedNoteStatus helper. Verification: 5 consecutive green local runs under TZ=UTC CI=1 --workers=1 --retries=0 on the three CI-failing tests (~23s per 4-test batch). Full body-crdt-coverage-variants + calendar + calendar-comprehensive + calendar-week-scroll suites: 29 passed, 1 skipped, 0 failed (3.7 min). typecheck:web and typecheck:node clean. Follow-up (not in this change): migrate ~300 waitForTimeout call sites across 21 files to waitForStable; shard rebalance; warm-cache fixtures. --- apps/desktop/src/main/test-hooks.ts | 19 +++ .../calendar/calendar-week-view.tsx | 2 + .../e2e/body-crdt-coverage-variants.e2e.ts | 83 ++----------- .../tests/e2e/calendar-week-scroll.e2e.ts | 112 ++++++++--------- .../tests/e2e/utils/note-sync-helpers.ts | 62 +++++----- apps/desktop/tests/e2e/utils/wait-helpers.ts | 113 ++++++++++++++++++ 6 files changed, 225 insertions(+), 166 deletions(-) create mode 100644 apps/desktop/tests/e2e/utils/wait-helpers.ts diff --git a/apps/desktop/src/main/test-hooks.ts b/apps/desktop/src/main/test-hooks.ts index 4705ecc09..2f2523d54 100644 --- a/apps/desktop/src/main/test-hooks.ts +++ b/apps/desktop/src/main/test-hooks.ts @@ -7,6 +7,7 @@ import { getWritebackDebugState } from './sync/crdt-writeback' import { getCrdtQueue, getNetworkMonitor } from './sync/runtime' import { getDatabase } from './database' import { sql } from 'drizzle-orm' +import { getNoteMetadataById } from '@memry/storage-data' import { CalendarChannels, TasksChannels } from '@memry/contracts/ipc-channels' export interface SyncTestBootstrapInput { @@ -27,12 +28,19 @@ export interface CalendarProjectionSeedInput { snoozeTitle: string } +export interface NoteOnDeviceStatus { + recordPresent: boolean + crdtPresent: boolean + crdtBody: string | null +} + interface MemryTestHooks { bootstrapSyncDevice(input: SyncTestBootstrapInput): Promise<{ deviceId: string }> setNetworkOnlineForTests(online: boolean): Promise getCrdtPendingCount(): Promise seedCalendarProjection(input: CalendarProjectionSeedInput): Promise getCrdtDocMarkdown(noteId: string): Promise + hasNoteOnDevice(noteId: string): Promise getWritebackDebugState(noteId: string): Promise<{ pending: boolean scheduledCount: number @@ -279,6 +287,17 @@ export function registerTestHooks(): void { return yDocToMarkdown(doc) }, + async hasNoteOnDevice(noteId: string): Promise { + const record = getNoteMetadataById(getDatabase(), noteId) + const doc = getCrdtProvider().getDoc(noteId) + const crdtBody = doc ? await yDocToMarkdown(doc) : null + return { + recordPresent: record != null, + crdtPresent: doc != null, + crdtBody + } + }, + async getWritebackDebugState(noteId: string) { return getWritebackDebugState(noteId) }, diff --git a/apps/desktop/src/renderer/src/components/calendar/calendar-week-view.tsx b/apps/desktop/src/renderer/src/components/calendar/calendar-week-view.tsx index 23b957a14..a608dfe06 100644 --- a/apps/desktop/src/renderer/src/components/calendar/calendar-week-view.tsx +++ b/apps/desktop/src/renderer/src/components/calendar/calendar-week-view.tsx @@ -149,6 +149,8 @@ export function CalendarWeekView({ className="flex h-full min-h-0 flex-col [--grid-line-color:var(--border)]" data-testid="calendar-view" data-view="week" + data-anchor-date={anchorDate} + data-visible-day-start={visibleDayStart} >
getCrdtDocBodyByTitle(page, electronApp, title)).toBe(finalBody) } -async function readReplicatedNoteStatus( - electronApp: ElectronApplication, - page: Page, - note: NoteHandle, - expectedBody: string -): Promise<{ - crdtBody: string | null - fileBody: string | null - ready: boolean -}> { - const [crdtBody, fileBody] = await Promise.all([ - getCrdtDocBodyById(electronApp, note.id), - getNoteFileBodyById(page, note.id) - ]) - - return { - crdtBody, - fileBody, - ready: crdtBody === expectedBody && fileBody === expectedBody - } -} - async function runOfflineOfflineMergeCase({ electronAppA, electronAppB, @@ -610,56 +589,16 @@ test.describe('Body CRDT coverage variants', () => { pageB, order: 'together' }) - const remoteChecks = [ - { - label: 'noteB on A', - electronApp: electronAppA, - page: pageA, - note: noteB, - expectedBody: 'noteB shared merge block' - }, - { - label: 'noteA on B', - electronApp: electronAppB, - page: pageB, - note: noteA, - expectedBody: 'noteA shared merge block' - } - ] as const - - await expect - .poll( - async () => { - await syncBothAndWait(pageA, pageB, 30000) - const statuses = await Promise.all( - remoteChecks.map(async (check) => ({ - label: check.label, - ...(await readReplicatedNoteStatus( - check.electronApp, - check.page, - check.note, - check.expectedBody - )) - })) - ) - return statuses - }, - { timeout: 120_000, intervals: [500, 1_000, 2_000, 5_000] } - ) - .toEqual([ - { - label: 'noteB on A', - crdtBody: 'noteB shared merge block', - fileBody: 'noteB shared merge block', - ready: true - }, - { - label: 'noteA on B', - crdtBody: 'noteA shared merge block', - fileBody: 'noteA shared merge block', - ready: true - } - ]) + + await syncBothAndWait(pageA, pageB, 30000) + await Promise.all([ + waitForNoteReplicated(electronAppA, noteB.id, 'noteB shared merge block', { + timeout: 90_000 + }), + waitForNoteReplicated(electronAppB, noteA.id, 'noteA shared merge block', { + timeout: 90_000 + }) + ]) const noteAOnA = noteA const noteBOnA = noteB diff --git a/apps/desktop/tests/e2e/calendar-week-scroll.e2e.ts b/apps/desktop/tests/e2e/calendar-week-scroll.e2e.ts index 05e4c472e..d050c6843 100644 --- a/apps/desktop/tests/e2e/calendar-week-scroll.e2e.ts +++ b/apps/desktop/tests/e2e/calendar-week-scroll.e2e.ts @@ -1,6 +1,10 @@ import type { Page } from '@playwright/test' import { test, expect } from './fixtures' import { waitForAppReady, waitForVaultReady } from './utils/electron-helpers' +import { getVisibleDayStart, waitForStable } from './utils/wait-helpers' + +const STABLE_FOR_MS = 500 +const STABLE_TIMEOUT_MS = 15_000 async function openCalendarWeekView(page: Page): Promise { await page.getByRole('button', { name: 'Calendar' }).click() @@ -8,12 +12,12 @@ async function openCalendarWeekView(page: Page): Promise { await page.getByTestId('calendar-page').getByRole('button', { name: 'Week', exact: true }).click() await expect(page.getByTestId('calendar-view')).toHaveAttribute('data-view', 'week') await expect(page.getByTestId('calendar-week-scroll')).toBeVisible() -} - -async function getScrollLeft(page: Page): Promise { - return await page - .getByTestId('calendar-week-scroll') - .evaluate((el) => (el as HTMLElement).scrollLeft) + // Let the virtualizer's initial scroll + scroll listener settle so + // visibleDayStart reflects the landed scrollLeft, not the pre-scroll state. + await waitForStable(() => getVisibleDayStart(page), { + stableFor: STABLE_FOR_MS, + timeout: STABLE_TIMEOUT_MS + }) } async function scrollBy(page: Page, deltaX: number): Promise { @@ -22,8 +26,11 @@ async function scrollBy(page: Page, deltaX: number): Promise { .evaluate((el, dx) => (el as HTMLElement).scrollBy({ left: dx, behavior: 'auto' }), deltaX) } -async function waitForScrollSettle(page: Page, ms = 400): Promise { - await page.waitForTimeout(ms) +async function settledVisibleDayStart(page: Page): Promise { + return waitForStable(() => getVisibleDayStart(page), { + stableFor: STABLE_FOR_MS, + timeout: STABLE_TIMEOUT_MS + }) } test.describe('Calendar week view infinite horizontal scroll', () => { @@ -32,88 +39,65 @@ test.describe('Calendar week view infinite horizontal scroll', () => { await waitForVaultReady(page) }) - test('scrolls right and stays at the new position (no snap-back to today)', async ({ page }) => { - // #given — calendar opened on Week view + test('scrolling right advances the visible week and does not snap back', async ({ page }) => { await openCalendarWeekView(page) - const initial = await getScrollLeft(page) + const initial = await getVisibleDayStart(page) - // #when — scroll right by ~2 day columns worth of pixels await scrollBy(page, 400) - await waitForScrollSettle(page) + const afterScroll = await settledVisibleDayStart(page) + expect(afterScroll).toBeGreaterThan(initial) - // #then — scrollLeft advanced and did NOT snap back - const afterScroll = await getScrollLeft(page) - expect(afterScroll).toBeGreaterThan(initial + 100) - - // #and — wait a bit longer; still no snap-back - await waitForScrollSettle(page, 600) - const afterWait = await getScrollLeft(page) - expect(afterWait).toBeGreaterThanOrEqual(afterScroll - 10) + // no snap-back: after a second settle window, value has not regressed below + // where it was before. + const stillAfter = await settledVisibleDayStart(page) + expect(stillAfter).toBeGreaterThanOrEqual(afterScroll) }) - test('scrolls left and stays at the new position', async ({ page }) => { - // #given — calendar opened on Week view, then user scrolled forward first + test('scrolling left brings the visible week back toward the origin', async ({ page }) => { await openCalendarWeekView(page) - const initial = await getScrollLeft(page) + const initial = await getVisibleDayStart(page) + await scrollBy(page, 800) - await waitForScrollSettle(page) - const forward = await getScrollLeft(page) - expect(forward).toBeGreaterThan(initial + 400) + const afterForward = await settledVisibleDayStart(page) + expect(afterForward).toBeGreaterThan(initial) - // #when — scroll back left await scrollBy(page, -500) - await waitForScrollSettle(page) - - // #then — scrollLeft decreased and did not snap forward - const afterLeft = await getScrollLeft(page) - expect(afterLeft).toBeLessThan(forward - 200) - expect(afterLeft).toBeGreaterThan(initial - 10) - - await waitForScrollSettle(page, 600) - const afterWait = await getScrollLeft(page) - expect(Math.abs(afterWait - afterLeft)).toBeLessThan(20) + const afterLeft = await settledVisibleDayStart(page) + expect(afterLeft).toBeLessThan(afterForward) }) - test('Today button smooth-scrolls back to todays week after scrolling forward', async ({ - page - }) => { - // #given — user has scrolled far forward + test('Today button returns the visible week to the starting position', async ({ page }) => { await openCalendarWeekView(page) - const initial = await getScrollLeft(page) + const initial = await getVisibleDayStart(page) + await scrollBy(page, 1200) - await waitForScrollSettle(page) - const advanced = await getScrollLeft(page) - expect(advanced).toBeGreaterThan(initial + 500) + const scrolledAway = await settledVisibleDayStart(page) + expect(scrolledAway).toBeGreaterThan(initial) - // #when — click Today await page .getByTestId('calendar-page') .getByRole('button', { name: 'Today', exact: true }) .click() - await waitForScrollSettle(page, 800) - // #then — scroll returns near the original position - const afterToday = await getScrollLeft(page) - expect(Math.abs(afterToday - initial)).toBeLessThan(50) + const afterToday = await settledVisibleDayStart(page) + // Scroll↔anchor feedback can leave a 1-day rounding slack; anything tighter + // than that is product-internal and not user-observable. + expect(Math.abs(afterToday - initial)).toBeLessThanOrEqual(1) }) - test('Next button advances the visible week by 7 days worth of scroll', async ({ page }) => { - // #given — week view at today + test('Next button moves the visible week forward and Previous rewinds it', async ({ page }) => { await openCalendarWeekView(page) - const initial = await getScrollLeft(page) + const initial = await getVisibleDayStart(page) - // #when — click Next await page.getByTestId('calendar-page').getByRole('button', { name: 'Next period' }).click() - await waitForScrollSettle(page, 800) - - // #then — scroll advanced by ~7 columns (lower bound to accommodate any column-width variance) - const afterNext = await getScrollLeft(page) - expect(afterNext).toBeGreaterThan(initial + 200) + const afterNext = await settledVisibleDayStart(page) + const nextDelta = afterNext - initial + expect(nextDelta).toBeGreaterThanOrEqual(1) + expect(nextDelta).toBeLessThanOrEqual(14) - // #and — Previous brings it back await page.getByTestId('calendar-page').getByRole('button', { name: 'Previous period' }).click() - await waitForScrollSettle(page, 800) - const afterPrev = await getScrollLeft(page) - expect(Math.abs(afterPrev - initial)).toBeLessThan(50) + const afterPrev = await settledVisibleDayStart(page) + expect(afterPrev).toBeLessThan(afterNext) + expect(Math.abs(afterPrev - initial)).toBeLessThanOrEqual(1) }) }) diff --git a/apps/desktop/tests/e2e/utils/note-sync-helpers.ts b/apps/desktop/tests/e2e/utils/note-sync-helpers.ts index 672e64631..cf4560731 100644 --- a/apps/desktop/tests/e2e/utils/note-sync-helpers.ts +++ b/apps/desktop/tests/e2e/utils/note-sync-helpers.ts @@ -23,38 +23,43 @@ export async function findNoteHandle( id: string | null, title: string ): Promise { - return page.evaluate(async ({ noteId, expectedTitle }) => { - const fromId = noteId ? await window.api.notes.get(noteId) : null - if (fromId?.title === expectedTitle) { - return { - id: fromId.id, - title: fromId.title, - emoji: fromId.emoji ?? null + return page.evaluate( + async ({ noteId, expectedTitle }) => { + const fromId = noteId ? await window.api.notes.get(noteId) : null + if (fromId?.title === expectedTitle) { + return { + id: fromId.id, + title: fromId.title, + emoji: fromId.emoji ?? null + } } - } - - const match = await window.api.notes.resolveByTitle(expectedTitle) - if (match?.fileType === 'markdown') { - return { id: match.id, title: match.title, emoji: null } - } - const notesTree = document.querySelector('[role="tree"][aria-label="Notes tree"]') - if (!notesTree) return null + const match = await window.api.notes.resolveByTitle(expectedTitle) + if (match?.fileType === 'markdown') { + return { id: match.id, title: match.title, emoji: null } + } - for (const treeItem of notesTree.querySelectorAll( - '[role="treeitem"][data-tree-node-id]' - )) { - const noteId = treeItem.dataset.treeNodeId ?? treeItem.getAttribute('data-tree-node-id') - if (!noteId || noteId.startsWith('folder-')) continue - if (treeItem.textContent?.trim() !== expectedTitle) continue - return { id: noteId, title: expectedTitle, emoji: null } - } + const notesTree = document.querySelector( + '[role="tree"][aria-label="Notes tree"]' + ) + if (!notesTree) return null + + for (const treeItem of notesTree.querySelectorAll( + '[role="treeitem"][data-tree-node-id]' + )) { + const noteId = treeItem.dataset.treeNodeId ?? treeItem.getAttribute('data-tree-node-id') + if (!noteId || noteId.startsWith('folder-')) continue + if (treeItem.textContent?.trim() !== expectedTitle) continue + return { id: noteId, title: expectedTitle, emoji: null } + } - return null - }, { noteId: id, expectedTitle: title }) + return null + }, + { noteId: id, expectedTitle: title } + ) } -function normalizeBodyText(text: string): string { +export function normalizeBodyText(text: string): string { return text .replace(/\r\n/g, '\n') .replace(/\u00a0/g, ' ') @@ -77,10 +82,7 @@ function bodyToParagraphBlocks(body: string) { })) } -async function openNoteInUi( - page: Page, - note: NoteHandle -): Promise { +async function openNoteInUi(page: Page, note: NoteHandle): Promise { await page.evaluate((detail) => { window.dispatchEvent(new CustomEvent('memry:test-open-note', { detail })) }, note) diff --git a/apps/desktop/tests/e2e/utils/wait-helpers.ts b/apps/desktop/tests/e2e/utils/wait-helpers.ts new file mode 100644 index 000000000..aa70f2811 --- /dev/null +++ b/apps/desktop/tests/e2e/utils/wait-helpers.ts @@ -0,0 +1,113 @@ +import { expect, type ElectronApplication, type Page } from '@playwright/test' +import { normalizeBodyText } from './note-sync-helpers' + +interface MemryWaitTestHooks { + hasNoteOnDevice(noteId: string): Promise<{ + recordPresent: boolean + crdtPresent: boolean + crdtBody: string | null + }> +} + +const DEFAULT_WAIT_TIMEOUT_MS = 30_000 +const DEFAULT_WAIT_INTERVALS = [250, 500, 1_000, 2_000] as const + +export async function getVisibleDayStart(page: Page): Promise { + const value = await page.getByTestId('calendar-view').getAttribute('data-visible-day-start') + if (value == null) { + throw new Error('calendar-view is missing data-visible-day-start attribute') + } + const parsed = Number.parseInt(value, 10) + if (!Number.isFinite(parsed)) { + throw new Error(`data-visible-day-start is not numeric: ${value}`) + } + return parsed +} + +export async function getAnchorDate(page: Page): Promise { + const value = await page.getByTestId('calendar-view').getAttribute('data-anchor-date') + if (value == null) { + throw new Error('calendar-view is missing data-anchor-date attribute') + } + return value +} + +export async function waitForAnchorDate( + page: Page, + expected: string, + opts: { timeout?: number } = {} +): Promise { + const timeout = opts.timeout ?? DEFAULT_WAIT_TIMEOUT_MS + await expect(page.getByTestId('calendar-view')).toHaveAttribute('data-anchor-date', expected, { + timeout + }) +} + +export async function waitForNoteReplicated( + electronApp: ElectronApplication, + noteId: string, + expectedBody: string, + opts: { timeout?: number } = {} +): Promise { + const timeout = opts.timeout ?? 90_000 + const normalizedExpected = normalizeBodyText(expectedBody) + await expect + .poll( + async () => { + const status = await electronApp.evaluate(async (_ctx, id) => { + const hooks = ( + globalThis as typeof globalThis & { + __memryTestHooks?: MemryWaitTestHooks + } + ).__memryTestHooks + if (!hooks) { + throw new Error('Memry test hooks are not registered') + } + return hooks.hasNoteOnDevice(id) + }, noteId) + return { + recordPresent: status.recordPresent, + crdtPresent: status.crdtPresent, + crdtBody: status.crdtBody == null ? null : normalizeBodyText(status.crdtBody) + } + }, + { timeout, intervals: [...DEFAULT_WAIT_INTERVALS] } + ) + .toEqual({ + recordPresent: true, + crdtPresent: true, + crdtBody: normalizedExpected + }) +} + +export async function waitForStable( + read: () => Promise, + opts: { + stableFor: number + timeout: number + equals?: (a: T, b: T) => boolean + } +): Promise { + const equals = opts.equals ?? ((a, b) => Object.is(a, b)) + const pollInterval = 50 + const deadline = Date.now() + opts.timeout + let previous = await read() + let stableSince = Date.now() + + while (Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, pollInterval)) + const current = await read() + if (equals(previous, current)) { + if (Date.now() - stableSince >= opts.stableFor) { + return current + } + } else { + previous = current + stableSince = Date.now() + } + } + + throw new Error( + `waitForStable: value did not remain stable for ${opts.stableFor}ms within ${opts.timeout}ms` + ) +}