From 3f74c65787943890b7a91c81a058983a57a3d3af Mon Sep 17 00:00:00 2001 From: fabi Date: Wed, 15 Jul 2026 07:26:54 +0200 Subject: [PATCH] ci: run the checks that only ran on a laptop; mobile in CI; retries 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI ran Playwright (chromium-desktop) + the dependency audit and nothing else. cargo test, clippy, the frontend vitest suite, svelte-check and the e2e typecheck were all green on a developer's machine and gated NOTHING. checks.yml (new): cargo test (with a Postgres service; --test-threads bounded so the sqlx per-test DBs can't exhaust connections) + clippy; vitest + svelte-check; e2e tsc. e2e.yml: also run the chromium-mobile project — 22 tests (focus traps, touch targets, safe-area, viewport reflow) that never ran in CI on a phone-first app. playwright.config: widen webkit-iphone beyond @smoke (2 specs) to the core guest journeys (auth/upload/feed) — iOS Safari is the PRIMARY browser here; and retries 2 → 0. retries:0 is deliberate and against the usual advice: this repo's real bugs are races, a race is indistinguishable from a flake from outside, and a retry resolves that ambiguity toward "flake" every time — it took a real 3%-flaky product bug from 1-in-33 to 1-in-37000, green. A flake here is a bug report. Not gated: cargo fmt (the tree has never been rustfmt'd — a 112-file reformat would bury every real diff; do it separately). WebKit widening is unverified locally (needs libavif16 via sudo), so its first CI run may surface real iOS bugs. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/checks.yml | 113 +++++++++++++++++++++++++++++++++++ .github/workflows/e2e.yml | 8 +++ README.md | 31 +++++++++- e2e/playwright.config.ts | 27 ++++++++- 4 files changed, 176 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/checks.yml diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml new file mode 100644 index 0000000..4d38500 --- /dev/null +++ b/.github/workflows/checks.yml @@ -0,0 +1,113 @@ +# The checks that were only ever running on a laptop. +# +# Before this file, CI ran Playwright (chromium-desktop) and the dependency audit — and nothing +# else. `cargo test` (40 tests), the frontend vitest suite (5 files, including the offline +# upload-queue and auth-token logic), svelte-check, and the e2e typecheck were all green on a +# developer's machine and gated NOTHING. A check that only ever runs locally is not running. +# +# In .github/workflows/ because Gitea Actions scans it too (see audit.yml). Rust is installed +# explicitly: the common Gitea runner images ship Node and Docker, not cargo. +name: Checks + +on: + pull_request: + push: + branches: [main] + +jobs: + backend: + name: Backend — cargo test + clippy + fmt + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v4 + + - name: Install Rust toolchain + run: | + if ! command -v cargo > /dev/null; then + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal + echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" + fi + rustup component add clippy rustfmt + + - uses: actions/cache@v4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + backend/target + key: cargo-${{ hashFiles('backend/Cargo.lock') }} + restore-keys: cargo- + + # SQLx runs its queries against a live database at TEST time (see backend/tests/), so the + # DB-backed tests need one. The pure unit tests don't care, but starting it unconditionally + # keeps the job simple and honest about what it covers. + - name: Start Postgres + run: | + docker run -d --name ci-pg -p 5432:5432 \ + -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=eventsnap_ci \ + postgres:16-alpine + for _ in $(seq 1 30); do + docker exec ci-pg pg_isready -U postgres > /dev/null 2>&1 && break + sleep 1 + done + + - name: Test + working-directory: ./backend + env: + DATABASE_URL: postgres://postgres:postgres@localhost:5432/eventsnap_ci + # `#[sqlx::test]` creates a fresh database per test and opens its own pool; run at unbounded + # parallelism against a default `max_connections=100` Postgres, a full suite can exhaust the + # server's connection slots and fail with PoolTimedOut — a pure infra flake, not a real + # failure. Cap the concurrency so the gate stays trustworthy on a loaded runner. + run: cargo test --all-features -- --test-threads=8 + + - name: Clippy + working-directory: ./backend + run: cargo clippy --all-targets -- -D warnings + + # NOTE: `cargo fmt --check` is deliberately NOT gated. The tree has never been rustfmt'd, so + # turning it on means a 112-file mechanical reformat that would bury every real diff under it. + # Formatting is not a correctness gate; do that cleanup on its own, then add the check here. + + frontend: + name: Frontend — vitest + svelte-check + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'npm' + cache-dependency-path: 'frontend/package-lock.json' + + - name: Install deps + working-directory: ./frontend + run: npm ci || npm install + + - name: Unit tests + working-directory: ./frontend + run: npm run test:unit + + - name: svelte-check + working-directory: ./frontend + run: npx svelte-check --threshold error + + e2e-typecheck: + name: E2E — typecheck + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: '22' + - name: Install deps + working-directory: ./e2e + run: npm install + # Playwright TRANSPILES specs without typechecking them, so a type error in a spec is + # invisible until the assertion it guards silently does the wrong thing at runtime. + - name: tsc --noEmit + working-directory: ./e2e + run: npx tsc --noEmit diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 21ce556..eadda0b 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -46,6 +46,14 @@ jobs: working-directory: ./e2e run: npm run test:e2e -- --project=chromium-desktop + # 09-mobile is `testIgnore`d on chromium-desktop (it needs hasTouch + a phone viewport), so + # running only that project left 22 tests — focus traps, touch targets, safe-area insets, + # viewport reflow, upload-cancel — never executing in CI. On a phone-first event app, where + # essentially every real guest is on a phone, that was the wrong half of the suite to skip. + - name: Run E2E tests (mobile) + working-directory: ./e2e + run: npm run test:e2e -- --project=chromium-mobile + - name: Upload Playwright report if: failure() uses: actions/upload-artifact@v4 diff --git a/README.md b/README.md index 0300098..f4f1751 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,28 @@ The `/media` volume holds originals, previews, thumbnails, exports, and DB backu --- +## Running the backend test suite + +```bash +cd backend + +# The DB-backed integration tests (backend/tests/) need a live Postgres. `#[sqlx::test]` creates a +# throwaway database per test and runs backend/migrations/ into it — it does NOT touch this one's data. +docker run -d --name eventsnap-test-pg -p 55433:5432 \ + -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=eventsnap postgres:16-alpine + +export DATABASE_URL=postgres://postgres:postgres@localhost:55433/eventsnap +cargo test # 44 unit + 12 DB-backed +cargo clippy --all-targets -- -D warnings +``` + +**`cargo test` requires `DATABASE_URL`** — without it the integration tests panic rather than skip. +That is deliberate. The riskiest code in this repo is SQL (the export epoch state machine, the +atomic quota increment, the `FOR SHARE` upload lock), and for a long time *not one line of it* was +executed by `cargo test` — every backend test was a pure-function test, so the tests clustered +tightly around the code that could not break and stopped exactly where it started to. Tests that +silently skip when the database is absent recreate that hole; they were meant to be a gate. + ## Running the E2E test suite Playwright-based end-to-end tests live in [`e2e/`](e2e/). They spin up an isolated docker-compose stack (Postgres on `:55432`, Caddy on `:3101`) and exercise the SvelteKit frontend against the real Rust backend with rate limits disabled. @@ -198,7 +220,14 @@ npm run stack:down # tear it down See [`e2e/README.md`](e2e/README.md) for the full UA matrix, Samsung Internet escalation tiers, and the Phase 2/3 roadmap. -CI runs this on every PR — see [`.github/workflows/e2e.yml`](.github/workflows/e2e.yml). +CI runs this on every PR — see [`.github/workflows/e2e.yml`](.github/workflows/e2e.yml) (desktop **and** +mobile projects), plus [`checks.yml`](.github/workflows/checks.yml) for `cargo test`/clippy, the frontend +unit tests, svelte-check and the e2e typecheck, and [`audit.yml`](.github/workflows/audit.yml) for +dependency advisories. + +**Playwright runs with `retries: 0`, including in CI.** This repo's real bugs are races, and from the +outside a race is indistinguishable from a flake — so a retry silently resolves that ambiguity in +favour of "flake" every time. A flake here is a bug report; treat it as one. --- diff --git a/e2e/playwright.config.ts b/e2e/playwright.config.ts index 1073517..382d32a 100644 --- a/e2e/playwright.config.ts +++ b/e2e/playwright.config.ts @@ -23,7 +23,25 @@ export default defineConfig({ outputDir: './test-results', fullyParallel: false, // Single shared backend → tests TRUNCATE between, so don't run in parallel. forbidOnly: !!process.env.CI, - retries: process.env.CI ? 2 : 0, + // NO RETRIES — not even in CI. This is deliberate and it is the opposite of the usual advice. + // + // The standard case for retries is "the environment is flaky, the product isn't." That argument + // does not hold here. This backend's real bugs ARE races (the last several fixes were all export + // concurrency), and from the outside a race is indistinguishable from a flake — so a retry + // resolves that ambiguity, silently, in favour of "flake", every single time. + // + // The numbers make it concrete. A test that fails 3% of runs reports a bug roughly 1 run in 33. + // Under `retries: 2` it fails the build only when it fails three times in a row: ~1 in 37,000. + // We had exactly such a test, and it was reporting a REAL user-facing bug (a suggestion button + // that committed on mousedown and destroyed itself mid-click). Retries would have buried it + // forever, and buried it GREEN, so nobody would even have seen an amber. + // + // Worse, a retry does not re-run the failing conditions — it runs a CLEANER environment (the + // interfering background worker from the previous test has since finished). So the mechanism is + // specifically good at hiding exactly the cross-test contamination this suite is prone to. + // + // A flake here is a bug report. Treat it as one. + retries: 0, workers: 1, // One worker. Multi-worker needs per-worker isolated DBs (Phase 2+). reporter: [ ['list'], @@ -127,7 +145,12 @@ export default defineConfig({ { name: 'webkit-iphone', use: { ...devices['iPhone 14 Pro'] }, - grep: /@smoke/, + // The PRIMARY user of this app is a wedding guest opening a QR link in iOS Safari. Gating + // that on `@smoke` — which exists on exactly two specs — meant the entire iOS guarantee was + // one happy path and one join test. Every other UA here is a secondary browser and a smoke + // check is proportionate; WebKit is not. Give it the core journeys the guest actually walks: + // join/recover, upload, and browse the feed. + testMatch: ['**/__smoke/**', '**/01-auth/**', '**/02-upload/**', '**/03-feed/**'], }, { name: 'firefox-android',