diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97b751fb9..5a0e33014 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,9 +4,19 @@ on: push: branches: [ main, develop ] pull_request: - branches: [ main, develop ] + # Deliberately NOT filtered by target branch. The retired pr-tests.yml ran on + # PRs into *any* branch; this workflow used to be limited to main/develop, so + # narrowing it here would silently leave PRs targeting release/feature + # branches with no test run at all. types: [ opened, synchronize, reopened ] +# Inherited from the retired pr-tests.yml: without this, every push to a PR +# branch starts a full run and the superseded ones keep burning runners to +# completion. +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: test: name: Test Suite @@ -135,6 +145,35 @@ jobs: echo "Checking server build..." ls -la dist/server/ + docs-build: + name: Documentation Build + runs-on: ubuntu-latest + + # Absorbed from the retired pr-tests.yml, where it was the ONE job with + # coverage this workflow did not already have. It is not redundant with + # deploy-docs.yml: that only runs on push to main, i.e. after merge — so + # without this, a VitePress build break is caught by the deploy, not the PR. + # + # Run unconditionally rather than behind a dorny/paths-filter `docs` gate. + # The old gate keyed on `docs/**` and `**.md`, which missed the VitePress + # config and the docs' own dependencies, and the build is ~1 min. Dropping + # the gate is what let the `changed-files` job retire with pr-tests.yml. + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + + - name: Setup Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + with: + node-version: '24.x' + cache: 'npm' + + - name: Install dependencies + run: npm install --prefer-offline --no-audit --legacy-peer-deps + + - name: Build documentation + run: npm run docs:build + docker: name: Docker Build runs-on: ubuntu-latest diff --git a/.github/workflows/dependabot-auto-merge.yml b/.github/workflows/dependabot-auto-merge.yml index 2ba1e7909..afe9c6ef1 100644 --- a/.github/workflows/dependabot-auto-merge.yml +++ b/.github/workflows/dependabot-auto-merge.yml @@ -25,7 +25,9 @@ jobs: uses: lewagon/wait-on-check-action@2271c86c146b96545b4e871b855e10ffa6f50773 # v1.9.0 with: ref: ${{ github.event.pull_request.head.sha }} - check-regexp: '(Build Check|Test Suite|Security Scan|Docker Build|Quick Tests).*' + # "Quick Tests" retired with pr-tests.yml (its coverage was a strict + # subset of "Test Suite"); "Documentation Build" moved into ci.yml. + check-regexp: '(Build Check|Test Suite|Security Scan|Docker Build|Documentation Build).*' repo-token: ${{ secrets.GITHUB_TOKEN }} wait-interval: 30 allowed-conclusions: success,skipped,neutral diff --git a/.github/workflows/pr-tests.yml b/.github/workflows/pr-tests.yml deleted file mode 100644 index 4271c87af..000000000 --- a/.github/workflows/pr-tests.yml +++ /dev/null @@ -1,225 +0,0 @@ -name: PR Tests - -on: - pull_request: - types: [opened, synchronize, reopened] - paths: - - '**.ts' - - '**.tsx' - - '**.js' - - '**.jsx' - - '**.json' - - '**.md' - - 'docs/**' - - '.github/workflows/pr-tests.yml' - -# Cancel in-progress runs for the same PR -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -jobs: - quick-test: - name: Quick Tests - runs-on: ubuntu-latest - # The full unit suite (npm run test:run) keeps growing: ~10 min when the - # cap was first raised (#3385), ~14 min by 2026-07 — so the 15-min cap - # started timing the job out at the boundary again (GitHub reports the - # timeout as "cancelled"). Keep real headroom above the observed runtime. - timeout-minutes: 25 - - services: - postgres: - image: postgres:16 - env: - POSTGRES_USER: test - POSTGRES_PASSWORD: test - POSTGRES_DB: meshmonitor_test - ports: - - 5433:5432 - options: >- - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - mysql: - image: mysql:8 - env: - MYSQL_ROOT_PASSWORD: root - MYSQL_USER: test - MYSQL_PASSWORD: test - MYSQL_DATABASE: meshmonitor_test - ports: - - 3307:3306 - options: >- - --health-cmd "mysqladmin ping -h localhost" - --health-interval 10s - --health-timeout 5s - --health-retries 5 - - steps: - - name: Checkout code - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - fetch-depth: 0 # Fetch all history for better diffing - submodules: recursive # Initialize protobufs submodule for tests - - - name: Setup Node.js - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 - with: - node-version: '24.x' - cache: 'npm' - - - name: Install dependencies - run: | - npm install --prefer-offline --no-audit --legacy-peer-deps - - - name: Check formatting - run: | - echo "Checking code formatting..." - # Add prettier check when configured - # npm run format:check - - - name: Run lint ratchet - run: | - echo "Running ESLint ratchet..." - # BLOCKING: fails only on new violations above the checked-in baseline. - # Existing debt is frozen in eslint-baseline.json and burns down over time. - # Regenerate baseline: npm run lint:baseline. Tracking: #3962 Task 1.4. - npm run lint:ci - - - name: Type check - run: | - echo "Running TypeScript compiler..." - npm run typecheck - - - name: Type-check tests (non-blocking) - run: npm run typecheck:tests - # NON-BLOCKING: ~283 pre-existing test-type errors. Flip to blocking (drop continue-on-error) - # when `npm run typecheck:tests` reports 0. Tracking: #3962 Task 1.2. - continue-on-error: true - - - name: Run unit tests - run: | - echo "Running test suite..." - npm run test:run - - - name: Generate test summary - if: always() - run: | - echo "## Test Results Summary" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - if [ $? -eq 0 ]; then - echo "✅ All tests passed!" >> $GITHUB_STEP_SUMMARY - else - echo "❌ Some tests failed. Please check the logs." >> $GITHUB_STEP_SUMMARY - fi - - changed-files: - name: Detect Changed Files - runs-on: ubuntu-latest - outputs: - backend: ${{ steps.changes.outputs.backend }} - frontend: ${{ steps.changes.outputs.frontend }} - docker: ${{ steps.changes.outputs.docker }} - docs: ${{ steps.changes.outputs.docs }} - - steps: - - name: Checkout code - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - - - name: Check for changes - uses: dorny/paths-filter@7b450fff21473bca461d4b92ce414b9d0420d706 # v4 - id: changes - with: - filters: | - backend: - - 'src/server/**' - - 'src/services/**' - - 'package.json' - - 'tsconfig.server.json' - frontend: - - 'src/**' - - '!src/server/**' - - '!src/services/**' - - 'index.html' - - 'vite.config.ts' - docker: - - 'Dockerfile' - - '.dockerignore' - docs: - - 'docs/**' - - '**.md' - - focused-tests: - name: Focused Tests - runs-on: ubuntu-latest - needs: changed-files - if: needs.changed-files.outputs.backend == 'true' || needs.changed-files.outputs.frontend == 'true' - - steps: - - name: Checkout code - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - submodules: recursive # Initialize protobufs submodule for tests - - - name: Setup Node.js - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 - with: - node-version: '24.x' - cache: 'npm' - - - name: Install dependencies - run: npm install --prefer-offline --no-audit --legacy-peer-deps - - - name: Test backend changes - if: needs.changed-files.outputs.backend == 'true' - run: | - echo "Testing backend changes..." - npm run test:run -- src/services/database.test.ts src/server/server.simple.test.ts - - - name: Test frontend changes - if: needs.changed-files.outputs.frontend == 'true' - run: | - echo "Testing frontend changes..." - npm run test:run -- src/components/ - - - name: Build check - run: | - if [[ "${{ needs.changed-files.outputs.backend }}" == "true" ]]; then - echo "Building server..." - npm run build:server - fi - if [[ "${{ needs.changed-files.outputs.frontend }}" == "true" ]]; then - echo "Building frontend..." - npm run build - fi - - docs-build: - name: Documentation Build - runs-on: ubuntu-latest - needs: changed-files - if: needs.changed-files.outputs.docs == 'true' - - steps: - - name: Checkout code - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - - - name: Setup Node.js - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 - with: - node-version: '24.x' - cache: 'npm' - - - name: Install dependencies - run: npm install --prefer-offline --no-audit --legacy-peer-deps - - - name: Build documentation - run: | - echo "Building VitePress documentation..." - npm run docs:build - - # The PR Status Check job has been removed as GitHub Actions - # automatically provides its own status checks for each job. - # Attempting to create additional commit statuses requires - # special permissions that are not available by default. \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md index f68da2463..59182c2a8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -154,7 +154,7 @@ Three lint commands: ```bash npm run lint:ci 2>&1 | grep '^FAIL' | grep -v '.claude/worktrees' ``` -Empty output = the CI gate passes. The same applies to Vitest, which scans those worktrees and inflates the suite count. +Empty output = the CI gate passes. Vitest **no longer** has this problem — `vitest.config.ts` excludes `**/.claude/worktrees/**`. (It used to: two leftover worktrees tripled the local suite, adding ~11 min and ~22k phantom tests, and reported failures from the worktrees' own stale dependencies.) ESLint still walks them, so the `grep -v` above is still required for `lint:ci`. **`npx eslint ` exiting 0 does not mean the ratchet passes.** The ratchet compares *per-file, per-rule counts* against the baseline, so adding one `react-hooks/exhaustive-deps` violation to an already-baselined file fails CI while plain ESLint reports nothing new. Always confirm with `lint:ci` before pushing. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ac824a928..86b4d05ec 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -127,18 +127,19 @@ Our CI/CD pipeline runs automatically on all PRs: ### GitHub Actions Workflows -1. **PR Tests** (`pr-tests.yml`) - - Runs on every PR - - Quick validation of changes - - Type checking and unit tests - -2. **Full CI** (`ci.yml`) - - Comprehensive testing - - Multiple Node.js versions - - Docker build validation +1. **CI** (`ci.yml`) + - Runs on every PR, whatever branch it targets + - Lint ratchet, type checking, and the full unit suite on Node 22 / 24 / 25 + (against real PostgreSQL and MySQL service containers) + - Frontend, server, documentation, and Docker build validation - Security scanning -3. **Release Pipeline** (`release.yml`) + A separate `pr-tests.yml` used to run alongside this one, but every job in it + was a strict subset of a job here — it re-ran the same suite a fourth time on + Node 24 and then re-ran parts of it again. Its one unique job, the VitePress + documentation build, now lives in `ci.yml`. + +2. **Release Pipeline** (`release.yml`) - Runs on version tags - Full test suite - Multi-platform Docker builds diff --git a/README.md b/README.md index 2a277e48d..b145381a3 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,6 @@ # MeshMonitor [![CI](https://github.com/Yeraze/meshmonitor/actions/workflows/ci.yml/badge.svg)](https://github.com/Yeraze/meshmonitor/actions/workflows/ci.yml) -[![PR Tests](https://github.com/Yeraze/meshmonitor/actions/workflows/pr-tests.yml/badge.svg)](https://github.com/Yeraze/meshmonitor/actions/workflows/pr-tests.yml) [![Docker Image](https://ghcr-badge.egpl.dev/yeraze/meshmonitor/latest_tag?color=%235b4566&ignore=latest,main,dev&label=version&trim=)](https://github.com/Yeraze/meshmonitor/pkgs/container/meshmonitor) [![Docker Pulls](https://ghcr-badge.egpl.dev/yeraze/meshmonitor/size?color=%235b4566&tag=latest&label=image%20size&trim=)](https://github.com/Yeraze/meshmonitor/pkgs/container/meshmonitor) [![License](https://img.shields.io/github/license/Yeraze/meshmonitor)](https://github.com/Yeraze/meshmonitor/blob/main/LICENSE) diff --git a/src/server/test-helpers/userTestHelper.ts b/src/server/test-helpers/userTestHelper.ts index fd7062894..25543a17f 100644 --- a/src/server/test-helpers/userTestHelper.ts +++ b/src/server/test-helpers/userTestHelper.ts @@ -14,7 +14,25 @@ import bcrypt from 'bcrypt'; import { AuthRepository } from '../../db/repositories/auth.js'; import { User, CreateUserInput, UpdateUserInput } from '../../types/auth.js'; -const SALT_ROUNDS = 12; +/** + * bcrypt cost factor for test fixtures ONLY. + * + * This is deliberately the bcrypt minimum (4), not production's 10 + * (`localAuth.ts`). The cost factor is a brute-force hardening knob — nothing + * in the suite asserts on it, and every call here hashes a throwaway literal + * like 'password123' into an in-memory DB that dies with the process. + * + * It is worth this comment because the number is load-bearing for CI time: + * these helpers run inside `beforeEach`, so the cost is paid per test, not per + * file. At 12 a single hash costs ~209ms; at 4 it costs ~2ms. That one constant + * accounted for ~185s of the ~352s total suite execution time across the five + * files that use this helper (meshcoreRoutes, userRoutes, authRoutes, + * auditRoutes, packetRoutes). + * + * Do NOT raise this to "match production" — production hashing is covered by + * `localAuth.ts`/`mfa.ts`, which set their own rounds. + */ +const SALT_ROUNDS = 4; /** * Map a DbUser (authMethod field) to the legacy User shape (authProvider field). diff --git a/vitest.config.ts b/vitest.config.ts index b95b74f4e..00720de85 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -2,32 +2,110 @@ import { defineConfig } from 'vitest/config'; import react from '@vitejs/plugin-react'; import path from 'path'; +/** + * Test files that talk to the SHARED PostgreSQL / MySQL test databases + * (`meshmonitor_test` on :5433 / :3307, via `src/db/repositories/test-utils.ts`). + * + * These cannot run concurrently with each other. Each one issues + * `DROP TABLE IF EXISTS CASCADE` + `CREATE TABLE ` against a single shared + * database at import time, so two of them in flight at once will drop a table out + * from under the other's queries. The overlaps are real, not theoretical: + * `users` — auth, channelDatabase, notifications + * `nodes` — nodes, keyRepair.multidb, notifications + * + * They therefore run in their own serial project (see `projects` below). Locally + * these skip entirely unless the containers are up (`npm run test:db:up`), so the + * serial cost is a CI-only ~1 min; in CI both containers are service containers. + * + * Add a file here whenever it starts using `createPostgresBackend` / + * `createMysqlBackend`. If it only ever uses `createSqliteBackend`, leave it out — + * SQLite backends are per-process `:memory:` and are safe to parallelize. + */ +const SHARED_DB_TESTS = [ + 'src/db/repositories/atakContacts.test.ts', + 'src/db/repositories/auth.test.ts', + 'src/db/repositories/autoTraceroute.test.ts', + 'src/db/repositories/backupHistory.test.ts', + 'src/db/repositories/channelDatabase.test.ts', + 'src/db/repositories/channels.test.ts', + 'src/db/repositories/ignoredNodes.test.ts', + 'src/db/repositories/keyRepair.multidb.test.ts', + 'src/db/repositories/neighbors.test.ts', + 'src/db/repositories/newsCache.test.ts', + 'src/db/repositories/nodes.test.ts', + 'src/db/repositories/notifications.test.ts', + 'src/db/repositories/settings.test.ts', + 'src/db/repositories/solarEstimates.test.ts', + 'src/db/repositories/traceroutes.test.ts', +]; + +const COMMON_EXCLUDE = [ + '**/node_modules/**', + '**/dist/**', + '**/.paperclip/**', + 'takpacket-sdk/**', // git submodule — has its own vitest suite (#4317) + // Agent worktrees are git-excluded, so CI never sees them, but locally they are + // real files on disk that Vitest happily collects — one leftover worktree + // re-runs the ENTIRE suite an extra time (measured: +11 min, +22k phantom + // tests, and phantom failures from the worktree's own stale deps). Same class + // of gotcha as the lint ratchet's `.claude/worktrees` caveat in CLAUDE.md. + '**/.claude/worktrees/**', +]; + +/** Settings shared by both projects. */ +const COMMON_TEST = { + globals: true, + environment: 'node' as const, // tests override per-file with @vitest-environment + setupFiles: './src/test/setup.ts', + isolate: true, + testTimeout: 10000, + hookTimeout: 10000, + pool: 'forks' as const, + env: { + DATABASE_PATH: ':memory:', + }, +}; + export default defineConfig({ plugins: [react()], test: { - globals: true, - environment: 'node', // Default to node, tests can override with @vitest-environment - setupFiles: './src/test/setup.ts', - isolate: true, - testTimeout: 10000, - hookTimeout: 10000, - // Run tests serially to avoid OOM issues - fileParallelism: false, - pool: 'forks', - poolOptions: { - forks: { - singleFork: true, + // Test files run in parallel across forks — Vitest's default. + // + // History (read before re-serializing): `fileParallelism: false` plus + // `poolOptions.forks.singleFork` were added in #295 (2025-10-24) as a drive-by + // in an unrelated feature PR, when this suite was 47 files, to dodge an OOM. + // The suite is now ~790 files, and that one line was the single largest cost + // in CI: the per-file import + transform cost (~1s each) was paid one file at + // a time, so ~20 min of the run was startup, not testing. `poolOptions` had + // also become dead weight — Vitest 4 removed it, so `singleFork` silently + // stopped applying and only `fileParallelism: false` was still serializing. + // + // Memory is now bounded by `maxWorkers`, not by running one file at a time. + // If OOM ever returns, lower `maxWorkers` — do not re-serialize the suite. + maxWorkers: process.env.CI ? 4 : '75%', + exclude: COMMON_EXCLUDE, + projects: [ + { + // `extends: true` inherits plugins + resolve.alias from this file. + extends: true, + test: { + ...COMMON_TEST, + name: 'unit', + exclude: [...COMMON_EXCLUDE, ...SHARED_DB_TESTS], + }, + }, + { + extends: true, + test: { + ...COMMON_TEST, + name: 'shared-db', + include: SHARED_DB_TESTS, + exclude: COMMON_EXCLUDE, + // Serial: these share one PostgreSQL/MySQL database. See SHARED_DB_TESTS. + fileParallelism: false, + }, }, - }, - exclude: [ - '**/node_modules/**', - '**/dist/**', - '**/.paperclip/**', - 'takpacket-sdk/**', // git submodule — has its own vitest suite (#4317) ], - env: { - DATABASE_PATH: ':memory:', - }, coverage: { provider: 'v8', reporter: ['text', 'json', 'html'], @@ -65,4 +143,4 @@ export default defineConfig({ ), } } -}); \ No newline at end of file +});