Introduce adapter injection and migrate test suite
Replace direct adapter/persistence imports with context-based injection (AdapterContext + useAdapters) so tests use in-memory implementations instead of vi.mock. Migrate component tests from context mocking to AllProviders with real hooks. Extract export/import logic from ActionBar into useEncounterExportImport hook. Add bestiary-cache and bestiary-index-adapter test suites. Raise adapter coverage thresholds (68→80 lines, 56→62 branches). 77 test files, 891 tests, all passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
51
CLAUDE.md
51
CLAUDE.md
@@ -70,11 +70,60 @@ docs/agents/ RPI skill artifacts (research reports, plans)
|
||||
- **TypeScript strict mode** with `verbatimModuleSyntax`. Use `.js` extensions in relative imports.
|
||||
- **Branded types** for identity values (e.g., `CombatantId`). Prefer immutability/`readonly` where practical.
|
||||
- **Domain events** are plain data objects with a `type` discriminant — no classes.
|
||||
- **Tests** live in `packages/*/src/__tests__/*.test.ts`. Test pure functions directly; map acceptance scenarios from specs to individual `it()` blocks.
|
||||
- **Tests** live in `__tests__/` directories adjacent to source. See the Testing section below for conventions on mocking, assertions, and per-layer approach.
|
||||
- **Quality gates** are enforced at pre-commit via Lefthook (parallel jobs). No gate may exist only as a CI step or manual process.
|
||||
|
||||
For component prop rules, export format compatibility, and ADRs, see [`docs/conventions.md`](docs/conventions.md).
|
||||
|
||||
## Testing
|
||||
|
||||
### Philosophy
|
||||
|
||||
Test **user-visible behavior**, not implementation details. A good test answers "does this feature work?" not "does this internal function get called?"
|
||||
|
||||
### Adapter Injection
|
||||
|
||||
Adapters (storage, cache, browser APIs) are provided via `AdapterContext`. Production wires real implementations; tests wire in-memory implementations. This means:
|
||||
- No `vi.mock()` for adapter or persistence modules
|
||||
- Tests control adapter behavior by configuring the in-memory implementation
|
||||
- Type changes in adapter interfaces are caught at compile time
|
||||
|
||||
### Per-Layer Approach
|
||||
|
||||
| Layer | How to test |
|
||||
|---|---|
|
||||
| Domain (`packages/domain`) | Pure unit tests, no mocks, test invariants and acceptance scenarios |
|
||||
| Application (`packages/application`) | Mock port interfaces only, use real domain logic |
|
||||
| Hooks (context-wrapped) | Test via `renderHook` with `AllProviders` wrapping in-memory adapters |
|
||||
| Hooks (component-specific) | Test through the component that uses them |
|
||||
| Components | Render with `AllProviders`, use in-memory adapters, use `userEvent` for interactions |
|
||||
|
||||
### Test Data
|
||||
|
||||
Use factory functions from `apps/web/src/__tests__/factories/` to construct domain objects. Each factory provides sensible defaults overridden via `Partial<T>`:
|
||||
|
||||
```typescript
|
||||
import { buildEncounter } from "../../__tests__/factories/build-encounter.js";
|
||||
import { buildCombatant } from "../../__tests__/factories/build-combatant.js";
|
||||
|
||||
const encounter = buildEncounter({
|
||||
combatants: [buildCombatant({ name: "Goblin" })],
|
||||
activeIndex: 0,
|
||||
roundNumber: 1,
|
||||
});
|
||||
```
|
||||
|
||||
Add new factory files as needed (one per domain type). Don't inline test data construction — use factories so type changes are caught at compile time.
|
||||
|
||||
### Anti-Patterns
|
||||
|
||||
- **`vi.mock()` for adapters**: Use in-memory adapter implementations via `AdapterContext` instead
|
||||
- **Mocking contexts**: Use `AllProviders` and drive state through real hooks instead of `vi.mock("../../contexts/...")`. Exception: context mocks are acceptable when the component under test requires specific state machine states that cannot be reached through adapter configuration alone — document the reason in a comment at the top of the test file.
|
||||
- **Stubbing child components**: Render real children; stub only if the child has heavy I/O that can't be mocked at the adapter level
|
||||
- **Asserting mock call counts**: Prefer asserting what the user sees (`screen.getByText(...)`) over `expect(mockFn).toHaveBeenCalledWith(...)`
|
||||
- **Testing internal state**: Don't assert `result.current.suggestionIndex === 0`; assert the first suggestion is highlighted
|
||||
- **Assertion-free tests**: Every `it()` block must contain at least one `expect()`. Tests that render without asserting inflate coverage without catching bugs.
|
||||
|
||||
## Self-Review Checklist
|
||||
|
||||
Before finishing a change, consider:
|
||||
|
||||
Reference in New Issue
Block a user