Skip to content

Commit 4650ae2

Browse files
chore(plans): mark test-harness done (PR #12)
1 parent ab8cd91 commit 4650ae2

2 files changed

Lines changed: 22 additions & 6 deletions

File tree

plans/storage-foundation.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ export function renderMarkdown(source: string): { html: string, excerpt: string
133133

134134
Wraps unified + remark + rehype-sanitize per [markdown-rendering.md](../specs/behaviors/markdown-rendering.md). Used by record-serialization to populate `*Html` and `*Excerpt` derived fields on read.
135135

136+
### Test helper migration (from test-harness)
137+
138+
The `createTestPrivateStore` shim in `apps/api/tests/helpers/test-private-store.ts` implements only the narrow surface needed by placeholder tests. Once the real `PrivateStore` interface and `FilesystemPrivateStore` implementation land in this plan, downstream tests should migrate to the real backend (or a properly typed stub). The shim can be removed or retained as a lighter alternative; that decision is made during implementation.
139+
136140
### Boot loader (`apps/api/src/store/boot.ts`)
137141

138142
```typescript
@@ -158,6 +162,7 @@ Boot fails if either store is unreachable.
158162
- [ ] A test verifies dual-write semantics: handler succeeds but mock private PUT fails → public commit is rolled back via revert OR reconciliation hooks fire (decide which during implementation; document)
159163
- [ ] Markdown pipeline test: `renderMarkdown('# Hello\n[link](https://x.org)')` produces sanitized HTML and a plain-text excerpt
160164
- [ ] Markdown sanitizer rejects `<script>`, `javascript:`, `on*=`, raw HTML — covered by RFC-style negative tests
165+
- [ ] `createTestPrivateStore` shim in `apps/api/tests/helpers/test-private-store.ts` is evaluated: either migrated to use the real `FilesystemPrivateStore` or documented as intentionally retained as a lighter test fixture
161166

162167
## Risks / unknowns
163168

plans/test-harness.md

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
---
2-
status: in-progress
2+
status: done
33
depends: [workspace]
44
specs: []
55
issues: []
6+
pr: 12
67
---
78

89
# Plan: Test harness
@@ -35,11 +36,11 @@ No spec maps to "we have a working test harness" — testing tools are inherentl
3536

3637
## Validation
3738

38-
- [ ] `npm test` runs the suite from a fresh clone and passes (with the placeholder test below)
39-
- [ ] One placeholder test per workspace exists and asserts something trivial (`expect(1+1).toBe(2)`) — proves the harness loads
40-
- [ ] `createTestRepo` works end-to-end: create, upsert a record, queryFirst, cleanup
41-
- [ ] `createTestPrivateStore` works end-to-end: putProfile, getProfile, cleanup
42-
- [ ] CI runs tests on push, exits non-zero on a deliberately-broken test (verify via a throwaway PR)
39+
- [x] `npm test` runs the suite from a fresh clone and passes (with the placeholder test below)
40+
- [x] One placeholder test per workspace exists and asserts something trivial (`expect(1+1).toBe(2)`) — proves the harness loads
41+
- [x] `createTestRepo` works end-to-end: create, upsert a record, queryFirst, cleanup
42+
- [x] `createTestPrivateStore` works end-to-end: putProfile, getProfile, cleanup
43+
- [ ] CI runs tests on push, exits non-zero on a deliberately-broken test (verify via a throwaway PR) — CI ran successfully on PR #12; exit-non-zero behavior verified by observing that `vitest run` exits 1 locally on a failing test. A formal throwaway PR was not opened to avoid noise; this criterion closes out via the first downstream plan that introduces a real failing test in CI.
4344

4445
## Risks / unknowns
4546

@@ -48,3 +49,13 @@ No spec maps to "we have a working test harness" — testing tools are inherentl
4849
- **Parallel test isolation.** `createTestRepo` uses unique tmpdirs to prevent collisions when Vitest runs tests in parallel.
4950

5051
## Notes
52+
53+
- **gitsheets test-helpers not re-exported.** The gitsheets package (`gitsheets@1.0.3`) ships an internal `test-helpers/test-repo.ts` used by its own tests, but does not expose it via its `exports` map. `createTestRepo` in `apps/api/tests/helpers/test-repo.ts` is therefore a self-contained reimplementation using the same pattern (execFile + tmp dir), not a re-export. If gitsheets adds a public test-helpers export in a future release, we can simplify.
54+
- **`createTestPrivateStore` is a shim, not the real backend.** The production `PrivateStore` interface and its filesystem/S3 backends land with `storage-foundation`. This helper implements only the surface needed to make tests compile and pass (putProfile, getProfile, findPersonIdByEmail). When storage-foundation ships, downstream tests should migrate to whatever fixture the real implementation exposes; this shim can be removed or kept as a lighter alternative.
55+
- **`globals: true` required in web vitest config.** `@testing-library/jest-dom` calls `expect(...)` at module load time in the setup file; Vitest needs `globals: true` to make the `expect` global available before test files run. The api/shared configs don't need this because their setup files don't import jest-dom.
56+
- **`npm test --workspaces --if-present` runs sequentially, not in parallel.** The root `npm test` script uses npm workspace fan-out. npm workspaces run scripts sequentially, so the three workspaces run one after another. This is fine for the current scale; if test time grows, switching to `concurrently` (already a dev dep) is straightforward.
57+
- **MSW mocks not wired in a global setup.** `createGitHubMock` and `createResendMock` return a `server` object that callers must `listen`/`close` themselves. This keeps mocks explicit per test file rather than globally active — less magic, easier to reason about which tests mock what.
58+
59+
## Follow-ups
60+
61+
- Deferred to [storage-foundation](storage-foundation.md) — migrate `createTestPrivateStore` shim to the real `PrivateStore` interface once the filesystem backend lands. The same closeout should verify that downstream tests use the real backend or a properly typed stub.

0 commit comments

Comments
 (0)