290 lines
12 KiB
Markdown
290 lines
12 KiB
Markdown
# Test design: add a rate-limit
|
|
|
|
> Deliverable for the `p-test` step of `c-design-build` (subject: "add a rate-limit").
|
|
> Matches the existing `test/` conventions: `node --test` + `node:assert/strict`,
|
|
> small independent tests, no LLM, no real clock by default.
|
|
|
|
## 1. Existing conventions
|
|
|
|
| Aspect | Convention in `test/` | Implication for rate-limit tests |
|
|
|---|---|---|
|
|
| Framework | `node:test` + `node:assert/strict` | Use the same; no jest/vitest. |
|
|
| File layout | `test/<feature>.test.ts` next to `support/` | New file: `test/rate-limit.test.ts`. |
|
|
| Imports | `import { describe, it, before } from "node:test";` | Identical. |
|
|
| Assertions | `assert.equal`, `assert.deepEqual`, `assert.match` | Use strict asserts; avoid `assert.ok(x === y)`. |
|
|
| Module type | `"type": "module"` in `package.json` | Use ESM, `.ts` extension in import specifiers. |
|
|
| Test discovery | `node --test --experimental-strip-types <file>` | Same one-liner; no jest config. |
|
|
| Time / randomness | Not yet used; tests are static and synchronous | Inject a `now()` function so we can advance the clock deterministically. |
|
|
| Fixtures | `test/fixtures/valid-prompt.md` is a one-off example | Reuse the same pattern only if a non-trivial fixture is needed; prefer inline data. |
|
|
| Support helpers | `test/support/frontmatter.ts` exposes `parseFrontmatter`, `parseChain` | Add `test/support/rate-limit.ts` for the SUT + clock injection; keep helpers pure. |
|
|
|
|
## 2. Test cases to add
|
|
|
|
Grouped by behavior. Each test is one `it(...)` and asserts one thing.
|
|
|
|
### Happy path
|
|
- `allowRequest` returns `true` for the first request from a fresh key.
|
|
- A burst up to the configured `limit` inside the window all return `true`.
|
|
- A single request after the window has elapsed returns `true` again (full reset on window expiry).
|
|
- Per-key isolation: a request from key `A` does not consume a slot for key `B`.
|
|
- The limiter is independent across two distinct limiter instances (no shared global state).
|
|
- `getStats(key)` reports `used`, `limit`, `remaining`, and `resetAt` matching the last operation.
|
|
|
|
### Edge cases
|
|
- `limit = 0`: every request is rejected (degenerate but defined).
|
|
- `limit = 1`: second request in the same window is rejected, the first is allowed.
|
|
- Window boundary: request at exactly `windowMs` after the first is treated as a fresh window (off-by-one). Use a `now` function that returns the boundary value explicitly.
|
|
- Two consecutive requests straddling the window: first allowed, second allowed, counts reset.
|
|
- Large `limit` (e.g. `1_000_000`): behaves like an effectively unbounded limiter; one test with a high value is enough.
|
|
- Empty / whitespace key string: rejected up front with a typed error, never reaches the bucket logic.
|
|
- `now` going backwards (clock skew): does not grant extra capacity; refuses to decrement below zero.
|
|
- `now` going forwards by exactly `windowMs + 1`: bucket fully reset; one extra request is allowed.
|
|
|
|
### Error cases
|
|
- Missing required options (`limit` or `windowMs`): throws `TypeError` with a message that includes the missing key.
|
|
- `limit` is negative or non-integer: throws `RangeError` (or `TypeError` — pick one and assert it consistently).
|
|
- `windowMs` is zero or negative: throws `RangeError`.
|
|
- Unknown option key (e.g. `limt` typo) is rejected to catch silent config bugs; throws with the unknown key in the message.
|
|
- A custom `now` that throws propagates the error (no swallowed exceptions).
|
|
- Concurrency: 100 synchronous calls to `allowRequest` for the same key with `limit = 50` result in exactly 50 `true` and 50 `false`, no off-by-one, no double-allow.
|
|
|
|
## 3. Test code
|
|
|
|
Target file: `test/rate-limit.test.ts`. The SUT is not yet in this repo; the
|
|
tests assume it is implemented at `src/rate-limit.ts` with this minimal
|
|
contract (declared here so the tests are runnable as soon as the SUT lands):
|
|
|
|
```ts
|
|
// test/rate-limit.test.ts
|
|
|
|
import { describe, it } from "node:test";
|
|
import assert from "node:assert/strict";
|
|
|
|
import {
|
|
createRateLimiter,
|
|
type RateLimiter,
|
|
type RateLimiterOptions,
|
|
} from "../src/rate-limit.ts";
|
|
|
|
// --- helpers -------------------------------------------------------------
|
|
|
|
function makeLimiter(
|
|
overrides: Partial<RateLimiterOptions> = {},
|
|
now: () => number = () => 0,
|
|
): RateLimiter {
|
|
return createRateLimiter(
|
|
{
|
|
limit: 3,
|
|
windowMs: 1_000,
|
|
...overrides,
|
|
},
|
|
now,
|
|
);
|
|
}
|
|
|
|
// --- happy path ----------------------------------------------------------
|
|
|
|
describe("rate-limit: happy path", () => {
|
|
it("allows the first request from a fresh key", () => {
|
|
const rl = makeLimiter();
|
|
assert.equal(rl.allowRequest("user:1"), true);
|
|
});
|
|
|
|
it("allows a burst up to the configured limit inside the window", () => {
|
|
const rl = makeLimiter({ limit: 3 });
|
|
assert.equal(rl.allowRequest("user:1"), true);
|
|
assert.equal(rl.allowRequest("user:1"), true);
|
|
assert.equal(rl.allowRequest("user:1"), true);
|
|
});
|
|
|
|
it("rejects the (limit + 1)th request in the same window", () => {
|
|
const rl = makeLimiter({ limit: 3 });
|
|
for (let i = 0; i < 3; i++) rl.allowRequest("user:1");
|
|
assert.equal(rl.allowRequest("user:1"), false);
|
|
});
|
|
|
|
it("allows again after the window has fully elapsed", () => {
|
|
let t = 0;
|
|
const clock = () => t;
|
|
const rl = makeLimiter({ limit: 1, windowMs: 1_000 }, clock);
|
|
assert.equal(rl.allowRequest("user:1"), true);
|
|
assert.equal(rl.allowRequest("user:1"), false);
|
|
t = 1_001; // strictly past the window
|
|
assert.equal(rl.allowRequest("user:1"), true);
|
|
});
|
|
|
|
it("isolates buckets per key", () => {
|
|
const rl = makeLimiter({ limit: 1 });
|
|
assert.equal(rl.allowRequest("a"), true);
|
|
assert.equal(rl.allowRequest("a"), false);
|
|
assert.equal(rl.allowRequest("b"), true);
|
|
assert.equal(rl.allowRequest("b"), false);
|
|
});
|
|
|
|
it("keeps two limiter instances independent", () => {
|
|
const a = makeLimiter({ limit: 1 });
|
|
const b = makeLimiter({ limit: 1 });
|
|
assert.equal(a.allowRequest("k"), true);
|
|
assert.equal(b.allowRequest("k"), true);
|
|
});
|
|
|
|
it("getStats reports remaining and resetAt matching the bucket state", () => {
|
|
let t = 0;
|
|
const rl = makeLimiter({ limit: 2, windowMs: 1_000 }, () => t);
|
|
rl.allowRequest("user:1");
|
|
const stats = rl.getStats("user:1");
|
|
assert.equal(stats.used, 1);
|
|
assert.equal(stats.remaining, 1);
|
|
assert.equal(stats.resetAt, 1_000);
|
|
});
|
|
});
|
|
|
|
// --- edge cases ----------------------------------------------------------
|
|
|
|
describe("rate-limit: edge cases", () => {
|
|
it("limit = 0 rejects every request", () => {
|
|
const rl = makeLimiter({ limit: 0 });
|
|
assert.equal(rl.allowRequest("k"), false);
|
|
});
|
|
|
|
it("limit = 1 allows exactly one request then rejects", () => {
|
|
const rl = makeLimiter({ limit: 1 });
|
|
assert.equal(rl.allowRequest("k"), true);
|
|
assert.equal(rl.allowRequest("k"), false);
|
|
});
|
|
|
|
it("treats a request at exactly windowMs as a fresh window (off-by-one)", () => {
|
|
let t = 0;
|
|
const rl = makeLimiter({ limit: 1, windowMs: 1_000 }, () => t);
|
|
assert.equal(rl.allowRequest("k"), true);
|
|
t = 1_000; // exactly at the boundary
|
|
assert.equal(rl.allowRequest("k"), true);
|
|
});
|
|
|
|
it("does not grant extra capacity when the clock goes backwards", () => {
|
|
let t = 1_000;
|
|
const rl = makeLimiter({ limit: 1, windowMs: 1_000 }, () => t);
|
|
assert.equal(rl.allowRequest("k"), true);
|
|
t = 500; // clock skew backwards
|
|
assert.equal(rl.allowRequest("k"), false);
|
|
assert.equal(rl.getStats("k").used, 1);
|
|
});
|
|
|
|
it("rejects an empty or whitespace key", () => {
|
|
const rl = makeLimiter();
|
|
assert.throws(() => rl.allowRequest(""), /key/);
|
|
assert.throws(() => rl.allowRequest(" "), /key/);
|
|
});
|
|
|
|
it("handles a very large limit without overflowing", () => {
|
|
const rl = makeLimiter({ limit: 1_000_000 });
|
|
for (let i = 0; i < 10; i++) {
|
|
assert.equal(rl.allowRequest("k"), true);
|
|
}
|
|
});
|
|
});
|
|
|
|
// --- error cases ---------------------------------------------------------
|
|
|
|
describe("rate-limit: error cases", () => {
|
|
it("throws when limit is missing", () => {
|
|
// @ts-expect-error: limit intentionally omitted
|
|
assert.throws(() => createRateLimiter({ windowMs: 1_000 }), /limit/);
|
|
});
|
|
|
|
it("throws when windowMs is missing", () => {
|
|
// @ts-expect-error: windowMs intentionally omitted
|
|
assert.throws(() => createRateLimiter({ limit: 1 }), /windowMs/);
|
|
});
|
|
|
|
it("throws on a negative limit", () => {
|
|
assert.throws(() => makeLimiter({ limit: -1 }), /limit/);
|
|
});
|
|
|
|
it("throws on a non-integer limit", () => {
|
|
assert.throws(() => makeLimiter({ limit: 1.5 }), /limit/);
|
|
});
|
|
|
|
it("throws on a zero or negative windowMs", () => {
|
|
assert.throws(() => makeLimiter({ windowMs: 0 }), /windowMs/);
|
|
assert.throws(() => makeLimiter({ windowMs: -1 }), /windowMs/);
|
|
});
|
|
|
|
it("rejects an unknown option key", () => {
|
|
assert.throws(
|
|
() => makeLimiter({ limt: 3 } as unknown as Partial<RateLimiterOptions>),
|
|
/limt/,
|
|
);
|
|
});
|
|
|
|
it("propagates errors from a custom now()", () => {
|
|
const rl = createRateLimiter(
|
|
{ limit: 1, windowMs: 1_000 },
|
|
() => { throw new Error("clock broken"); },
|
|
);
|
|
assert.throws(() => rl.allowRequest("k"), /clock broken/);
|
|
});
|
|
|
|
it("is exact under synchronous burst: exactly limit trues, then falses", () => {
|
|
const rl = makeLimiter({ limit: 50 });
|
|
let allowed = 0;
|
|
let rejected = 0;
|
|
for (let i = 0; i < 100; i++) {
|
|
if (rl.allowRequest("k")) allowed++;
|
|
else rejected++;
|
|
}
|
|
assert.equal(allowed, 50);
|
|
assert.equal(rejected, 50);
|
|
});
|
|
});
|
|
```
|
|
|
|
## 4. How to run
|
|
|
|
From the repository root:
|
|
|
|
```bash
|
|
# all tests in this directory
|
|
cd test && npm test
|
|
|
|
# only the new rate-limit tests
|
|
cd test && node --test --experimental-strip-types rate-limit.test.ts
|
|
|
|
# a single test by name pattern
|
|
cd test && node --test --experimental-strip-types \
|
|
--test-name-pattern="rejects the \\(limit \\+ 1\\)th" rate-limit.test.ts
|
|
```
|
|
|
|
Notes:
|
|
- `--experimental-strip-types` is required because the existing `test/package.json`
|
|
already targets Node's built-in TS stripping; no separate tsconfig needed.
|
|
- These tests are pure and synchronous, so the whole file should finish in
|
|
well under 100 ms once the SUT lands.
|
|
|
|
## 5. Coverage gaps (flagged, not refactored in this pass)
|
|
|
|
1. **SUT does not exist yet.** `src/rate-limit.ts` is assumed. Until it
|
|
lands with the contract declared at the top of section 3, the test file
|
|
will fail to import. This is the single largest blocker.
|
|
2. **Async / distributed cases are not covered.** The contract above is
|
|
synchronous. If the production limiter is async (e.g. backed by Redis
|
|
with `INCR` + `EXPIRE`), the burst test will need to be reworked; a
|
|
`before`/`after` test that injects an in-memory store would be the
|
|
smallest refactor.
|
|
3. **Concurrency under a real event loop is not exercised.** A single
|
|
synchronous `for` loop does not stress the microtask queue. If the SUT
|
|
becomes async, add a `Promise.all` of N concurrent calls and assert
|
|
that the count of `true` results equals `limit`.
|
|
4. **No metrics / observability surface is assumed.** If the limiter is
|
|
later expected to emit counters or events, that needs an interface
|
|
change first; tests would follow in a follow-up.
|
|
5. **No per-route or weighted limits.** A real production limiter often
|
|
supports per-route buckets or weighted costs per request. Out of scope
|
|
here; would require a contract change.
|
|
6. **No clock injection in the SUT contract yet.** The design assumes
|
|
`createRateLimiter(opts, now?)`. If the chosen implementation uses a
|
|
module-level `Date.now()`, the clock-dependent tests in this file
|
|
cannot be written deterministically; the SUT should be refactored to
|
|
accept a `now` parameter (smallest viable change) before these tests
|
|
are added.
|