CenterForDigitalHumanities / CenterForDigitalHumanities/TinyPen

Test coverage is ~40% of TinyNode's — port route-level behavioral tests

Open
#48 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
0
Forks
0
PR merge metrics
No merged PRs in 30d

Description

## Summary

TinyPen's test suite is roughly **40% the size and depth** of its sister project [TinyNode](https://github.com/CenterForDigitalHumanities/TinyNode/tree/main/test). The Jest → `node:test` migration in #47 is complete and clean, but it did not bring over the route-level behavioral tests that TinyNode has. The two repos are meant to be kin; the testing harness should be kin too.

## Coverage gap

| Subject | TinyNode | TinyPen | Status |
|---|---:|---:|---|
| `create.test.js` | 200 lines | — | Missing |
| `delete.test.js` | 171 lines | — | Missing |
| `overwrite.test.js` | 247 lines | — | Missing |
| `query.test.js` | 178 lines | — | Missing |
| `update.test.js` | 182 lines | — | Missing |
| `rest.test.js` (`verifyJsonContentType`) | 31 lines | 0 lines | Missing |
| `mount.test.js` (HTTP-driven via supertest) | 73 lines | 97 lines, but stack-inspection only | Weak |
| `tokens.test.js` | 158 | 172 ✓ | At parity |
| `rerum.test.js` | 89 | 88 ✓ | At parity |
| `openapi_sync_artifacts.test.js` | 44 | 77 ✓ | At parity (or stronger) |
| **Total** | **~1616** | **~654** | **~40%** |

## Concrete blind spots (mutation-verified)

These code mutations in `routes/` pass the current TinyPen suite. They would fail TinyNode's:

- `routes/create.js:50` changing `res.status(201)` to `res.status(200)` — the contract declares `201` but nothing verifies it on the wire.
- `routes/delete.js:38` changing `res.status(204)` to `res.status(200)` — same.
- `routes/create.js:13` removing the `_id = req.body.id.split('/').pop()` extraction — nothing verifies the upstream body shape.
- `routes/overwrite.js:52-58` short-circuiting the `409` branch so conflicts are mapped to `502` — the conflict-passthrough contract is undocumented in tests.
- `app.js:99` swapping `app.use('/create', createRouter)` to `app.use('/create', deleteRouter)` — `mount.test.js`'s stack-inspection accepts it (path matcher still matches); only an HTTP-driven request would notice.
- `rest.js:17-31` (`hasMultipleContentTypes`) — this is the MIME-smuggling detector. Its own docstring enumerates 8 attack patterns. Zero of them are tested. Mutating the function to `return false` passes the suite.

## Root cause

The Jest → `node:test` migration in #47 also removed `supertest` as a dev dependency (commit `c38ad90`). TinyNode's route tests are all `supertest`-driven against a minimal Express tester app. Without `supertest`, the same patterns can't be ported one-for-one.

## Proposed work

1. **Re-add `supertest` to `devDependencies`.** It's a peer-of-`express` testing utility with no runtime impact and TinyNode is already using it on the same Express 5 stack.
2. **Port the five route test files from TinyNode** (`create`, `delete`, `overwrite`, `query`, `update`). They are already in `node:test + assert` form. The only TinyPen-specific adaptations are:
- `/create` `_id` extraction from `req.body.id`
- `/overwrite` `If-Overwritten-Version` header passthrough and `409` conflict-body JSON response
3. **Add a `rest_content_type.test.js`** (or expand `rest.test.js`) covering all 8 MIME-smuggling cases documented in `rest.js:1-13`.
4. **Replace `mount.test.js` with an HTTP-driven version** following TinyNode's pattern (or delete it once the route-level tests cover registration implicitly).
5. **Walk through `__tests__/*.testcases.md`** and convert documented behaviors into real `it(...)` blocks. Delete or significantly trim the Markdown punch lists once their cases have a real test home.

## Acceptance criteria

- `__tests__/` total line count is roughly at parity with TinyNode (~1500+ lines).
- Each of the six route-level mutations listed above causes at least one test to fail.
- `npm test` continues to be `c8 --reporter=… node --test` — no Jest, no global mocks.
- All sync workflows continue to pass.

## Context

Surfaced during the #47 static review. Out-of-scope for that PR by agreement; tracked here.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.