From f9111c06a7d5f78e50b11dd1e15033d5a30f2ba3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 23:47:18 -0400 Subject: [PATCH] test(extension): unit suite for lib/ + version-consistency specs extension/ had no test harness at all -- web-ext lint was the only signal, so the URL-normalization fix in 8214afe shipped with nothing exercising it. Adds vitest (mirroring frontend/vitest.config.js) and three specs: - url.spec.js normalizeApiUrl / webRootFromApiUrl, including the #2393 regression: instance-root input must reach /api/credentials, idempotence, trailing-slash and whitespace handling, and that empty input never yields a bare "/api" (which isConfigured() would read as configured). - platforms.spec.js getPlatformFromUrl / isArtistPage, pinning the #1485 regression -- all three Patreon creator URL shapes (bare, /c/, /cw/) plus inner pages, with nav pages excluded -- and table-integrity checks. - version.spec.js manifest.json and package.json versions in lockstep, AMO-safe version format, and url.js ordered before api.js in background.scripts (classic scripts share one scope, so a reorder is a runtime ReferenceError with no build signal). Specs load lib/*.js by evaluating the real file as a classic script (test/helpers/loadLib.js) instead of adding module.exports shims to production code that would never run in the browser. The suite therefore exercises exactly the bytes packaged into the XPI. Two packaging consequences, both handled: - web-ext would otherwise bundle test/ and vitest.config.js INTO the XPI; both are now in --ignore-files across all four web-ext scripts. - ci.yml's extension-version guard must ignore the same paths, or editing a spec would demand a pointless version bump. The dangerous drift direction is the opposite one -- a guard exclusion for a file that DOES ship would let a real change pass unnoticed -- so version.spec.js asserts every :(exclude) in ci.yml appears in --ignore-files. extension.yml also triggers on ci.yml now, since version.spec.js reads it. Refs #2393, #2397 Co-Authored-By: Claude Opus 5 (1M context) --- .forgejo/workflows/ci.yml | 11 ++- .forgejo/workflows/extension.yml | 17 +++- ci-requirements.md | 18 +++++ extension/package.json | 10 ++- extension/test/helpers/loadLib.js | 29 +++++++ extension/test/platforms.spec.js | 127 ++++++++++++++++++++++++++++++ extension/test/url.spec.js | 93 ++++++++++++++++++++++ extension/test/version.spec.js | 71 +++++++++++++++++ extension/vitest.config.js | 13 +++ 9 files changed, 379 insertions(+), 10 deletions(-) create mode 100644 extension/test/helpers/loadLib.js create mode 100644 extension/test/platforms.spec.js create mode 100644 extension/test/url.spec.js create mode 100644 extension/test/version.spec.js create mode 100644 extension/vitest.config.js diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 184e9b3..5f17846 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -123,13 +123,18 @@ jobs: fi # Exclusions mirror --ignore-files in extension/package.json's web-ext # scripts: these files are not packaged into the XPI, so touching them - # (e.g. Renovate bumping the web-ext devDep) changes nothing shipped - # and must not demand a version bump. + # (e.g. Renovate bumping the web-ext devDep, or editing a spec) + # changes nothing shipped and must not demand a version bump. + # KEEP IN SYNC with --ignore-files — a file packaged into the XPI but + # excluded here is exactly the silent-stale-ship this job exists to + # prevent. test/version.spec.js pins the two lists' shared intent. CHANGED=$(git diff --name-only "$BASE" HEAD -- extension/ \ ':(exclude)extension/package.json' \ ':(exclude)extension/package-lock.json' \ ':(exclude)extension/README.md' \ - ':(exclude)extension/.gitignore') + ':(exclude)extension/.gitignore' \ + ':(exclude)extension/vitest.config.js' \ + ':(exclude)extension/test/**') if [ -z "$CHANGED" ]; then echo "No packaged extension files changed since $BASE — nothing to guard." echo "OK: extension version $PKG" diff --git a/.forgejo/workflows/extension.yml b/.forgejo/workflows/extension.yml index 3a50314..a67c578 100644 --- a/.forgejo/workflows/extension.yml +++ b/.forgejo/workflows/extension.yml @@ -1,5 +1,5 @@ name: extension -# Lint-only workflow. The sign-and-publish dance moved into build.yml's +# Lint + unit tests. The sign-and-publish dance moved into build.yml's # `sign-extension` job (2026-05-25) — `:latest` now always bundles the XPI # because sign-extension runs as a build-web dependency in the SAME workflow, # eliminating the prior race between build.yml and a separate extension.yml. @@ -10,10 +10,15 @@ on: paths: - 'extension/**' - '.forgejo/workflows/extension.yml' + # test/version.spec.js asserts ci.yml's extension-version guard never + # ignores a file web-ext actually packages, so a ci.yml-only edit can + # break this suite and must trigger it. + - '.forgejo/workflows/ci.yml' pull_request: branches: [main] paths: - 'extension/**' + - '.forgejo/workflows/ci.yml' workflow_dispatch: jobs: @@ -23,7 +28,13 @@ jobs: image: node:24-bookworm-slim steps: - uses: actions/checkout@v4 - - name: Install web-ext - run: cd extension && npm install --no-save --no-audit --no-fund + # Not --no-save: vitest and web-ext are both real devDependencies now, + # and the suite needs vitest resolvable from node_modules. + - name: Install dev dependencies + run: cd extension && npm install --no-audit --no-fund - name: Lint run: cd extension && npm run lint + # Pure-logic specs over lib/url.js and lib/platforms.js plus manifest / + # package version-consistency checks. No browser, no network. + - name: Unit tests + run: cd extension && npm run test:unit diff --git a/ci-requirements.md b/ci-requirements.md index c5cebd3..c7a5438 100644 --- a/ci-requirements.md +++ b/ci-requirements.md @@ -13,10 +13,20 @@ git.fabledsword.com/bvandeusen/ci-python:3.14 - node (frontend job: `npm install` + vitest + vite build) - docker CLI + buildx (`.forgejo/workflows/build.yml`: build-web, build-ml — Fabled-Git registry push) +## Secondary runtime image + +node:24-bookworm-slim — `.forgejo/workflows/extension.yml` only. + +The extension lane is the one job that does NOT run on `ci-python:3.14`: it +needs a current Node for `web-ext` and vitest and nothing Python at all. Kept +on the upstream slim image rather than adding a Node toolchain to `ci-python`, +per `docs/process.md`'s "add deps to the image when used by >1 project". + ## Per-job tool installs - `pip install -r requirements.txt pytest pytest-asyncio` — in `backend-lint-and-test` and `integration` jobs - `npm install --no-audit --no-fund` — in `frontend-build` job +- `npm install --no-audit --no-fund` — in `extension.yml`'s `lint` job (web-ext + vitest) ## Notes @@ -35,3 +45,11 @@ git.fabledsword.com/bvandeusen/ci-python:3.14 memory bans `npm install` locally). Using `npm install` rather than `npm ci` until a lockfile lands. - No `imagemagick` / `pandoc` per-job installs needed. +- `extension/`'s vitest specs load `lib/*.js` by evaluating the real file as a + classic script (`test/helpers/loadLib.js`) rather than adding `module.exports` + shims to production code — the libs ship as `background.scripts`, not ES + modules, so the specs exercise exactly the bytes packaged into the XPI. +- Extension test files are excluded from the XPI via `--ignore-files` in + `extension/package.json`, and the same paths are excluded from `ci.yml`'s + `extension-version` guard. Those two lists must agree — `test/version.spec.js` + asserts the guard never ignores a file web-ext actually packages. diff --git a/extension/package.json b/extension/package.json index 8230440..c441e82 100644 --- a/extension/package.json +++ b/extension/package.json @@ -4,12 +4,14 @@ "private": true, "description": "Firefox extension for FabledCurator", "scripts": { - "lint": "web-ext lint --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore", - "start": "web-ext run --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore --firefox=firefox", - "build": "web-ext build --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore --overwrite-dest", - "sign": "web-ext sign --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore --channel=unlisted --api-key=$WEB_EXT_API_KEY --api-secret=$WEB_EXT_API_SECRET" + "lint": "web-ext lint --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\"", + "start": "web-ext run --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\" --firefox=firefox", + "build": "web-ext build --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\" --overwrite-dest", + "sign": "web-ext sign --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\" --channel=unlisted --api-key=$WEB_EXT_API_KEY --api-secret=$WEB_EXT_API_SECRET", + "test:unit": "vitest run" }, "devDependencies": { + "vitest": "^4.0.0", "web-ext": "^10.0.0" } } diff --git a/extension/test/helpers/loadLib.js b/extension/test/helpers/loadLib.js new file mode 100644 index 0000000..e5ded02 --- /dev/null +++ b/extension/test/helpers/loadLib.js @@ -0,0 +1,29 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import path from 'node:path' + +const LIB_DIR = path.join(path.dirname(fileURLToPath(import.meta.url)), '..', '..', 'lib') + +/** + * Load an extension lib and hand back the globals it declares. + * + * The files under lib/ are CLASSIC scripts, not ES modules: manifest.json + * lists them in `background.scripts` and options.html pulls them in with a + * plain