From 027fe2cbc34194939115b222b987e43bcfc4e572 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 21 Jul 2026 01:01:34 -0500 Subject: [PATCH] docs(website): add Discord bot test plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document the test plan for the bot/ workspace — the last of the three website npm workspaces without a suite (server + client landed in website PR #86). Records the shared node --test conventions (no jest/vitest/jsdom; DB pool at a dead port; Discord objects hand-faked) and maps the meaningful bot behavior to lock: the normalize/duration/spam pure logic, the invite/site-api/internal-key single-collaborator units, the messageFilter pipeline (bypass-first decision order + fixed-duration mute + best-effort recording), and the models with real shaping logic. Includes the CI + SonarQube coverage wiring to mirror PR #86 and a suggested phasing. Co-Authored-By: Claude --- website/test-plan.md | 154 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 website/test-plan.md diff --git a/website/test-plan.md b/website/test-plan.md new file mode 100644 index 0000000..2a6d285 --- /dev/null +++ b/website/test-plan.md @@ -0,0 +1,154 @@ +# Runic Gateway Website — Test Plan (Discord bot) + +> Companion to [website-README.md](website-README.md) (overview) and +> [BACKEND_DESIGN.md](BACKEND_DESIGN.md) (server API/schema/security contract). +> Establishes the test plan for the **`bot/` workspace** (the Discord bot), the +> last of the three `website` npm workspaces without a suite. The server and +> client suites already exist (website PR #86); this doc closes the bot gap and +> records the shared conventions so all three stay consistent. + +## 1. Goal & philosophy + +Cover **meaningful bot behavior** — the moderation/filter decisions a future +change could silently break — not a coverage number. The bot's value is in *what +it decides to delete, warn, mute, or let through*; a good test reads "given this +message + config, what action does the bot take?", never "did this function call +that function?". + +The whole `website` repo tests on **Node's built-in runner (`node --test`)** with +**zero external test dependencies** — no jest, no vitest, no jsdom. Unit tests +run without a database or a live Discord gateway: the DB pool is pointed at a dead +port and every collaborator (`db.query`, a model, a Discord `message`/`client`) is +replaced with an in-memory fake. This mirrors the server suite exactly (the bot is +CommonJS, like the server — `require`/`module.exports` — so the same patterns +apply verbatim). + +## 2. Current state + +| Workspace | Runner | Status | +|---|---|---| +| `server/` | `node --test` | **Exists** — models, controllers, auth/session, shard ingest. | +| `client/` | `node --test` (pure-logic ESM) | **Exists** (PR #86) — `lib/`, `api/`, `data/`. | +| `bot/` | — | **This plan** — no runner or tests yet. | + +The 46% SonarQube aggregate is dragged down by the bot (and the client's +un-unit-testable React components) reading as 0% covered. Standing up the bot +suite lifts the real number and, more importantly, locks the filter/moderation +rules. + +## 3. Harness + +Add a test script to `bot/package.json` (mirrors server/client): + +```json +"scripts": { "test": "node --test" } +``` + +Conventions, identical to `server/test/`: + +- **No DB.** Set `process.env.DB_HOST = '127.0.0.1'` / `DB_PORT = '59999'` at the + top of any test whose module transitively `require`s `../db`, so the pool is + built against a dead port and a stray query fails fast instead of hanging. + Models are exercised by monkeypatching `db.query` (or the model method the unit + under test calls) with an in-memory fake; restore it in `afterEach`. +- **No Discord.** The `discord.js` `Client`, `Message`, `GuildMember`, and + `Interaction` objects are hand-rolled fakes carrying only the fields the unit + reads (e.g. `message.mentions.users.size`, `message.member.roles.cache`, + `message.client.fetchInvite`). Never construct a real client or open a gateway + connection. +- **Tests live in `bot/test/*.test.js`.** One file per module under test. + +## 4. Units to cover + +Ordered high-value first. Each row names the module, the behavior worth locking, +and the seam a test drives it through. + +### 4.1 Pure logic (no mocks beyond inputs) + +| Module | Behaviors to lock | Notes | +|---|---|---| +| `filter/normalize.js` | leetspeak folding (`b4d`→`bad`, `@ss`→`ass`), 3+-repeat collapse (`sooooo`→`so`), case-fold; `matches()` is word-boundary anchored (so `bad` does not fire inside `badminton`); `findMatch()` returns the first `{word, severity}` row or `null`; the *documented gaps* stay gaps (spaced-out `b a d` is **not** caught). | The core obfuscation-resistance contract. Pin the gaps too, so a future tightening is a deliberate, test-visible change. | +| `utils/duration.js` | `"30s"/"10m"/"2h"/"1d"` → correct ms; whitespace tolerated; garbage/empty/unknown-unit → `null`; `MAX_TIMEOUT_MS` is 28 days (the Discord cap callers clamp to). | Feeds `/mute` and temp-role expiry — a wrong parse mutes for the wrong span. | +| `filter/spamFilter.js` | `isRateLimited` trips on the 6th message inside the 5 s window and is per-`guild:user`; it has a **side effect** (records the timestamp) so it must be called once; `isMassMention` counts users **+** roles against the threshold; `isMassEmoji` counts custom `<:name:id>` and unicode pictographs. | Drive time with a stubbed `Date.now` (or accept the real clock and use tight windows). The module-load `setInterval(...).unref()` sweep must not keep the runner alive — `.unref()` already handles this; assert no leak by letting the suite exit. | + +### 4.2 Logic with a single mocked collaborator + +| Module | Behaviors to lock | Seam / mock | +|---|---|---| +| `filter/inviteFilter.js` | returns the **code** (not a bool) of the first *foreign* invite; an invite resolving to the current guild is allowed; an invite that **fails to resolve** (expired/invalid) is treated as foreign (fail-closed); no invite in the message → `null`. | Fake `message.client.fetchInvite(code)` — resolve with `{ guild: { id } }` for local/foreign, or throw for unresolvable. Set `message.guildId` + `message.content`. | +| `site/siteApiClient.js` | never throws on a network/timeout/HTTP failure — always returns the `{ ok, ... }` shape so a command never breaks when the site is down: `{ ok: true, data }` on success, `{ ok: false, error }` on failure, and the distinct `{ ok: false, maintenance: true, message }` for the site's 503 maintenance response. (Public read client — **no** shared secret; the keyed channel is `botInternalClient.js`.) | Stub `global.fetch` (resolve various statuses/bodies, reject/timeout via `AbortController`). Same non-throwing contract the server's `uoLinkClient` follows. | +| `internal/requireInternalKey.js` | `401` on a missing/wrong key; `next()` on the configured key; **fail-closed** when `BOT_INTERNAL_KEY` is empty (never matches); timing-safe compare (equal-length + `crypto.timingSafeEqual`, no early-exit length leak). | Call the middleware with a fake `req` (`req.get('X-Internal-Key')`) + `res`/`next` spies; mirrors `server/test/requireInternalKey.test.js`. | + +### 4.3 The filter pipeline (orchestration) + +`discord/messageFilter.js` is the highest-value integration seam — it composes +all of §4.1–4.2. Lock the **decision order and the actions**, with every +collaborator faked: + +- **Bypass wins first:** an allow-listed channel, or a member holding an + allow-listed role, short-circuits the whole pipeline (`isBypassed`) — no + delete, no hit recorded. +- **Order:** invite link → banned word → spam (rate-limit → mass-mention → + mass-emoji). `detectSpam` must evaluate `isRateLimited` **first and exactly + once** (it has the timestamp side effect). +- **Actions:** invite/spam always delete + warn (no severity tiers); a + word-filter hit applies the action for its severity; filter-triggered mutes use + the fixed `FILTER_MUTE_SECONDS` (600 s). +- **Best-effort recording never breaks moderation:** `recordFilterHit` / + `recordSpamHit` throwing must be swallowed — the delete/warn still happens. + +Mocks: `filterCache` (in-memory `allowChannels`/`allowRoles` Sets + words), a fake +`message` (content, author, member roles, `delete()`, `guildId`), and spies on +`warnings`/`filterHits`/`spamHits`/`modLog` to assert what was called. + +### 4.4 Models (`bot/model/*.js`) + +Thin `db.query` wrappers (e.g. `warnings.js`, `filterWords.js`, `filterHits.js`, +`spamHits.js`, `memberEvents.js`, `guildConfig.js`, `scheduledMessages.js`, +`tempRoles.js`). Cover only those with **shaping/branching logic** (a query that +maps rows, applies an "active/not-expired" predicate, or upserts) by asserting the +**SQL params** passed to a fake `db.query` and the shape returned. Skip pure +one-line inserts with no logic — testing those only proves the mock. + +## 5. Explicitly out of scope + +- **`discord.js` internals / the live gateway** — not ours to test; never open a + real connection. +- **Thin command wrappers** (`discord/commands/*.command.js`) that only validate + args and call a Discord API + a model — cover their *logic* (arg parsing, + duration clamping) if any, but not the Discord round-trip. +- **`node-cron` scheduling itself** — test the job's callback logic, not that cron + fires. + +## 6. CI & coverage wiring + +Mirror what PR #86 did for the client: + +- **`.gitea/workflows/pr-checks.yml`** — add a `bot-tests` job (or a + `Run bot tests` step) that runs `npm test --prefix bot`, gating PRs into `main`. +- **`.gitea/workflows/sonarqube.yml`** — generate a bot LCOV from the repo root so + `SF:` paths resolve to `bot/src/...`: + + ``` + node --test --experimental-test-coverage \ + --test-reporter=lcov --test-reporter-destination=bot/coverage/lcov.info \ + bot/test/*.test.js + ``` + +- **`sonar-project.properties`** — add `bot/test` to `sonar.tests`, the glob to + `sonar.test.inclusions`, `bot/coverage/lcov.info` to + `sonar.javascript.lcov.reportPaths`, and `bot/coverage/**` to `sonar.exclusions`. + (`bot/src` is already in `sonar.sources`.) + +Bot units need no `npm ci` to run when they import only relative files + Node +built-ins; a test that stubs `global.fetch` or fakes `db` needs no `discord.js` +either. Keep the pool pointed at a dead port so the run is hermetic. + +## 7. Suggested phasing + +1. **Harness + pure logic** — `test` script, then `normalize`, `duration`, + `spamFilter`. Fast, zero mocks, immediate value. +2. **Single-collaborator units** — `inviteFilter`, `siteApiClient`, + `requireInternalKey`. +3. **Pipeline** — `messageFilter` decision order + actions (the payoff test). +4. **Models with logic**, then the **CI/Sonar wiring** so it all counts.