wtclaude
027fe2cbc3
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 <noreply@anthropic.com>
155 lines
9.3 KiB
Markdown
155 lines
9.3 KiB
Markdown
# 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.
|