mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
ci: run the browser flows and the map bundle check on every change
Both only ran on a developer's machine, and both had stopped telling the truth. The end-to-end flows now run in CI in Chromium and in WebKit (the iPad and iPhone cases need the engine every iOS browser uses), with the browsers cached on the lockfile and the traces uploaded when a flow fails. A second job builds the client and fails when one chunk carries both map engines. Making them pass meant fixing what had rotted. The setup closed every first-run notice but the release notes, whose X is called Close, so the notes covered the dashboard and every flow after login timed out on a click. The iPad case still expected undraggable place rows; tablets drag on purpose since #1616, behind the long-press bridge, so it now checks what #1432 is about, a swipe that scrolls and raises no drop overlay, with locators for the scroller and the Plan tab that match today's markup. The iPhone case read the body scroll lock through window.scrollBy, which moves an overflow: hidden viewport in every engine; it now reads the lock where it takes effect and checks the page keeps its place.
This commit is contained in:
Maurice
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
|
||||
+2
-2
@@ -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"`.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user