diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7b0ff6a42..b13a71812 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,6 +18,9 @@ permissions: # - Tests: the *-coverage jobs run the suites and upload lcov; s3-contract # runs the MinIO contract suite. Coverage jobs run tests only — with the # gates cleared in preflight, they can only fail because tests failed. +# client-e2e drives the real app in Chromium and WebKit, and client-bundle +# builds the client and checks each map engine keeps a chunk of its own. +# Neither feeds the scan. # - Scan `needs:` exactly the four coverage jobs. A coverage job failing # must stop the scan — a missing lcov file would make every line of that # package report as uncovered, and the dashboard would show a collapse @@ -261,6 +264,93 @@ jobs: - name: Typecheck run: npm run typecheck + client-bundle: + name: Client Bundle + runs-on: ubuntu-latest + timeout-minutes: 45 + needs: [plugin-facts, i18n-parity, server-quality, client-quality, plugin-sdk-quality] + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-node@v6 + with: + node-version: 24 + cache: npm + cache-dependency-path: package-lock.json + + - name: Install dependencies + run: npm ci --workspace shared && npm ci --workspace client + + - name: Build shared + run: npm run build --workspace=shared + + - name: Build client + run: cd client && npm run build + + - name: Each map engine in a chunk of its own + # One static import of mapbox-gl or maplibre-gl in the map tree folds + # both engines into one chunk again, and nothing else notices: the build + # stays green and the app still works, only every map user downloads + # both. scripts/check-gl-split.mjs explains the markers it looks for. + run: cd client && npm run check:gl-split + + client-e2e: + name: Client End-to-End + runs-on: ubuntu-latest + timeout-minutes: 45 + needs: [plugin-facts, i18n-parity, server-quality, client-quality, plugin-sdk-quality] + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-node@v6 + with: + node-version: 24 + cache: npm + cache-dependency-path: package-lock.json + + # The run boots the real server (e2e/server-launch.mjs builds it and + # starts it against a throwaway SQLite file) and the Vite dev server in + # front of it, so it needs every workspace. + - name: Install dependencies + run: npm ci + + - name: Build shared + run: npm run build --workspace=shared + + # Keyed on the lockfile, which pins @playwright/test and with it the + # browser builds. The system libraries the browsers need are not cached + # and are installed on every run. + - name: Cache browsers + id: browsers + uses: actions/cache@v6 + with: + path: ~/.cache/ms-playwright + key: playwright-${{ runner.os }}-${{ hashFiles('package-lock.json') }} + + # Chromium for the desktop and phone flows, WebKit for the iPad and + # iPhone ones (every browser on iOS and iPadOS is WebKit underneath). + - name: Install browsers + if: steps.browsers.outputs.cache-hit != 'true' + run: cd client && npx playwright install --with-deps chromium webkit + + - name: Install browser dependencies + if: steps.browsers.outputs.cache-hit == 'true' + run: cd client && npx playwright install-deps chromium webkit + + # The public and app projects are the regression flows. The screenshot + # and help-media projects make pictures for the wiki and the help center + # and assert nothing, so they stay out of CI. + - name: Run the end-to-end flows + run: cd client && npx playwright test --project=public --project=app + + - name: Upload traces + if: failure() + uses: actions/upload-artifact@v6 + with: + name: e2e-results + path: client/test-results/ + retention-days: 7 + s3-contract: name: S3 Driver Contract (MinIO) runs-on: ubuntu-latest diff --git a/CLAUDE.md b/CLAUDE.md index 4beea8616..dd6e7f15b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,7 +32,7 @@ npm run test --workspace=server # vitest run (server); also t npm run test:e2e # server e2e (boots Nest against a temp SQLite) npm run test:cov # coverage (lcov) for all four packages incl. plugin-sdk cd client && npm run test # client vitest run -cd client && npm run e2e # Playwright (local only) +cd client && npm run e2e # Playwright (CI runs the public and app projects; the screenshot and help-media ones are local) cd client && npm run lint:pages # enforce the Page pattern (CI gate) cd client && npm run lint:rtl # physical left/right sides may only shrink per file (CI gate) cd client && npm run theme:lint # flag styling that bypasses appearance tokens (local only) @@ -108,7 +108,7 @@ These principles come out of a full-repo audit and shape all new code: ## Quality gates in CI -`.github/workflows/test.yml` runs, per package: typecheck (server also `typecheck:tests`), `lint:check`, `lint:pages`, `lint:rtl`, `check:plugin-facts`, `i18n:parity:strict`, the S3 contract suite, coverage for all four packages, then a SonarCloud scan. `lint-prettier.yml` additionally runs `lint` + `format:check` on `shared/`. `plugin-sdk` has no lint script and is outside the eslint gates. **Not in CI**: `theme:lint`, Playwright, `check:gl-split` — run them locally. There are no git hooks; nothing runs before a commit except you. Other workflows build Docker images, scan them, and publish `trek-plugin-sdk` on `plugin-sdk-v*` tags. +`.github/workflows/test.yml` runs, per package: typecheck (server also `typecheck:tests`), `lint:check`, `lint:pages`, `lint:rtl`, `check:plugin-facts`, `i18n:parity:strict`, the S3 contract suite, the browser end-to-end flows (Chromium and WebKit), a client build with `check:gl-split`, coverage for all four packages, then a SonarCloud scan. `lint-prettier.yml` additionally runs `lint` + `format:check` on `shared/`. `plugin-sdk` has no lint script and is outside the eslint gates. **Not in CI**: `theme:lint` and the screenshot and help-media Playwright projects; run them locally. There are no git hooks; nothing runs before a commit except you. Other workflows build Docker images, scan them, and publish `trek-plugin-sdk` on `plugin-sdk-v*` tags. **SonarCloud** (project `liketrek_TREK`, the built-in Sonar way gate; the run takes 20–25 min, so get it right before pushing) measures **new code only** on a PR; any failing condition blocks it: diff --git a/client/CLAUDE.md b/client/CLAUDE.md index e2d41f49c..772a7da7e 100644 --- a/client/CLAUDE.md +++ b/client/CLAUDE.md @@ -12,10 +12,10 @@ npm run lint # eslint . (lint:check in CI) npm run lint:pages # enforce the Page pattern (CI gate) npm run lint:rtl # physical left/right sides may only shrink per file (CI gate; --list shows them, --update lowers the baseline) npm run test # vitest run (tests/** + co-located src/**/*.test.{ts,tsx}); also test:unit / test:integration / test:coverage -npm run e2e # Playwright (local only; e2e:report opens the last report) +npm run e2e # Playwright (CI runs --project=public --project=app; e2e:report opens the last report) npm run shots # Playwright screenshot project (shots:promote to accept) npm run theme:lint # theme conformance audit (theme:lint:strict exits 1; local only) -npm run check:gl-split # after build: fails if one chunk bundles both mapbox-gl and maplibre-gl (local only) +npm run check:gl-split # after build: fails if one chunk bundles both mapbox-gl and maplibre-gl (CI gate) ``` Single test: `npx vitest run src/store/slices/budgetSlice.test.ts`, or `npx vitest run -t "optimistically adds the place"`. diff --git a/client/e2e/helpers.ts b/client/e2e/helpers.ts index fa6d43d3d..966b82eed 100644 --- a/client/e2e/helpers.ts +++ b/client/e2e/helpers.ts @@ -10,11 +10,13 @@ import type { Page } from '@playwright/test' * scoped INSIDE the dialog — an unscoped /next/i can match dashboard buttons * (carousel arrows) and satisfy the wait before the modal even mounts. * - * A notice closes one of two ways depending on its shape: + * A notice closes one of three ways depending on its shape: * - CTA-bearing notices (e.g. the thank-you/support modal) only offer the * X button (`aria-label="Dismiss"`), shown on the last page. * - CTA-less notices show an "OK" button that pages forward and dismisses on * the last page. + * - The release notes (ReleaseNoticeModal), shown again after every update, + * close through their X (`aria-label="Close"`). * Multi-page notices are paged through via the pager's Next button first. * Dismissal is persisted server-side per user, so clearing once keeps it * cleared for every later spec in the run (shared test DB). @@ -32,8 +34,10 @@ export async function dismissSystemNotices(page: Page, appearTimeoutMs = 3_000): } const dismiss = dialog.getByRole('button', { name: 'Dismiss', exact: true }) const ok = dialog.getByRole('button', { name: 'OK', exact: true }) + const close = dialog.getByRole('button', { name: 'Close', exact: true }) if (await dismiss.isVisible().catch(() => false)) await dismiss.click() else if (await ok.isVisible().catch(() => false)) await ok.click() + else if (await close.isVisible().catch(() => false)) await close.click() else break // Exit animation + the next queued notice mounting. await page.waitForTimeout(400) diff --git a/client/e2e/ipad-scroll-1432.spec.ts b/client/e2e/ipad-scroll-1432.spec.ts index c7cc854a9..71f3a5f05 100644 --- a/client/e2e/ipad-scroll-1432.spec.ts +++ b/client/e2e/ipad-scroll-1432.spec.ts @@ -5,16 +5,19 @@ import { dismissSystemNotices } from './helpers' // // A tablet is a coarse-pointer device at a *desktop* viewport width, so the width-based // "is this mobile" check that 3.2.1 shipped left `draggable` armed on iPad: the swipe -// became an HTML5 drag and raised the drop-to-import overlay instead of scrolling. Drag -// is now gated on `(pointer: coarse)` (useIsTouch), and only a real device context proves -// it — a jsdom unit test cannot express "coarse pointer at 834px". +// became an HTML5 drag and raised the drop-to-import overlay instead of scrolling. What +// turned the swipe into a drag was the drag-drop-touch polyfill, confined to hybrid +// laptops since. #1616 then gave tablets their drag back on purpose, through the +// long-press bridge ([data-touch-drag]): a row may be draggable, but only a press that +// is held becomes a drag, so a swipe still scrolls. Only a real device context proves +// that; a jsdom unit test cannot express "coarse pointer at 834px". // // Needs WebKit (`npx playwright install webkit`, plus libmanette-0.2-0 and libwoff1 on // Debian/Ubuntu). WebKit is the right engine here, not a nicety: every browser on iPadOS // is WebKit underneath, which is why the reporter saw this in all three they tried. test.use({ ...devices['iPad Pro 11'] }) -test('#1432 iPad: places list is scrollable, not draggable', async ({ page }) => { +test('#1432 iPad: the places list scrolls under a swipe', async ({ page }) => { await page.goto('/dashboard') await dismissSystemNotices(page) @@ -58,12 +61,17 @@ test('#1432 iPad: places list is scrollable, not draggable', async ({ page }) => expect(env.coarse, 'iPad reports a coarse primary pointer').toBe(true) expect(env.width, 'iPad sits above the 768px "mobile" breakpoint').toBeGreaterThanOrEqual(768) - // 1. Rows must not be draggable — a draggable row is what swallowed the scroll gesture. + // 1. A swipe must not become a drag: a row is either not draggable, or draggable + // only behind the long-press bridge (#1616). const row = page.locator('div[draggable]').filter({ hasText: 'Place 1' }).first() - await expect(row).toHaveAttribute('draggable', 'false') + await expect(row).toBeVisible() + const swipeSafe = await row.evaluate(el => el.getAttribute('draggable') !== 'true' || !!el.closest('[data-touch-drag]')) + expect(swipeSafe, 'a draggable row sits behind the long-press bridge').toBe(true) // 2. The list must scroll, and no drop-to-import overlay may appear. - const scroller = page.locator('div[draggable]').first().locator('xpath=ancestor::div[@class="trek-stagger"]') + // The list's scroll container carries trek-stagger among its other classes. Taken + // from the row above: the first draggable on the page can be another panel's. + const scroller = row.locator('xpath=ancestor::div[contains(concat(" ", normalize-space(@class), " "), " trek-stagger ")]') const before = await scroller.evaluate(el => el.scrollTop) const box = (await scroller.boundingBox())! await page.touchscreen.tap(box.x + box.width / 2, box.y + 40) @@ -72,17 +80,11 @@ test('#1432 iPad: places list is scrollable, not draggable', async ({ page }) => expect(after, 'places list scrolled').toBeGreaterThan(before) await expect(page.getByText('Drop to import')).toHaveCount(0) - // 3. Drag being off means the arrow buttons are the only reorder affordance left — - // they must be visible (they were opacity:0 above 767px). - const arrowOpacity = await page.evaluate(() => { - const el = document.querySelector('.reorder-buttons') - return el ? getComputedStyle(el).opacity : 'absent' - }) - expect(['1', 'absent']).toContain(arrowOpacity) - - // 4. The iPad must still get the desktop shell — isMobile stayed width-based. One + // 3. The iPad must still get the desktop shell — isMobile stayed width-based. One // panel at a time (#2247), so opening Places closed the day plan and left its // tab as the way back. await expect(page.locator('.leaflet-container')).toBeVisible() - await expect(page.locator('button[aria-label="Plan"]')).toBeVisible() + // The top tab bar has a button named Plan as well; the way back is the panel's + // own tab, which is accented while its panel is closed. + await expect(page.locator('button[aria-label="Plan"].bg-accent')).toBeVisible() }) diff --git a/client/e2e/urlbar-scroll-1809.spec.ts b/client/e2e/urlbar-scroll-1809.spec.ts index 917748f76..586725d3b 100644 --- a/client/e2e/urlbar-scroll-1809.spec.ts +++ b/client/e2e/urlbar-scroll-1809.spec.ts @@ -71,18 +71,29 @@ test('#1809 iPhone: flow screens scroll the document, full-screen ones do not', await expect.poll(() => page.evaluate(() => document.scrollingElement!.scrollTop)).toBe(0) // 4. The shared body scroll lock: with the document as the scroller, an open - // sheet must freeze the page behind it and give the position back. + // sheet must freeze the page behind it and give the position back. The + // lock is body overflow: hidden, which reaches the viewport only while + // html stays overflow: visible (index.css), and there it stops the swipe. + // It does not stop a script: window.scrollBy moves an overflow: hidden + // viewport in every engine, so the lock is read where it takes effect. await page.evaluate(() => window.scrollBy(0, 300)) const locked = await page.evaluate(readScroll) expect(locked.top).toBeGreaterThan(0) await page.getByRole('button', { name: 'New Trip' }).click() await expect(page.getByRole('dialog')).toBeVisible() - await page.evaluate(() => window.scrollBy(0, 300)) - expect(await page.evaluate(() => document.scrollingElement!.scrollTop)).toBe(locked.top) + const lock = await page.evaluate(() => ({ + body: getComputedStyle(document.body).overflowY, + html: getComputedStyle(document.documentElement).overflowY, + top: document.scrollingElement!.scrollTop, + })) + expect(lock.body, 'the open sheet locks body').toBe('hidden') + expect(lock.html, 'html stays visible, so the lock reaches the viewport').toBe('visible') + expect(lock.top, 'opening the sheet does not move the page').toBe(locked.top) await page.keyboard.press('Escape') await expect(page.getByRole('dialog')).toHaveCount(0) + expect(await page.evaluate(() => getComputedStyle(document.body).overflowY), 'the lock is released').not.toBe('hidden') expect(await page.evaluate(() => document.scrollingElement!.scrollTop)).toBe(locked.top) await page.evaluate(() => window.scrollBy(0, 200)) expect(await page.evaluate(() => document.scrollingElement!.scrollTop)).toBeGreaterThan(locked.top)