History

agents, notes

Rhys Sullivan committed 6 months ago

Commit bfb87a01 parent 63cdca32 files · +200 −0Browse files

Showing the changes to notes (1 of 2 files). Show all

Unified · Split

notes/livestore-effect-testing-porting.md+196−0
@@ -0,0 +1,196 @@
1+# LiveStore Effect Testing Patterns Worth Porting
2+
3+LiveStore is useful as a reference repo for Effect-heavy test ergonomics, not
4+as a wholesale tooling model. Executor already has the stricter Effect lint
5+posture and a simpler Bun/Turbo toolchain. The part worth copying is the small
6+test harness shape that makes scoped Effect tests easier to write and easier to
7+debug when they hang.
8+
9+Reference checkout:
10+
11+```txt
12+.reference/livestore
13+```
14+
15+## What to Port
16+
17+### Effect test context helper
18+
19+LiveStore's `packages/@livestore/utils-dev/src/node-vitest/Vitest.ts` wraps
20+`@effect/vitest` with:
21+
22+- `withTestCtx(test)` for per-test Effect setup.
23+- `makeWithTestCtx(...)` for reusable layer/timeout config.
24+- A default timeout, longer in CI.
25+- `Effect.logWarnIfTakesLongerThan` before the timeout trips.
26+- `Effect.timeout(...)`.
27+- `Effect.provide(...)` for per-test layers.
28+- `Effect.scoped` so finalizers and spans close predictably.
29+- Optional tracing/log layer wiring.
30+
31+Executor should port the shape, not the exact implementation. LiveStore is on
32+Effect 3, while Executor is on Effect 4 beta. The local helper should be tiny
33+and typed against the current `@effect/vitest` and `effect` APIs.
34+
35+Likely home:
36+
37+```txt
38+packages/core/sdk/src/testing.ts
39+```
40+
41+If the helper starts pulling in cloud-specific, Node-server, or plugin-specific
42+dependencies, stop and keep those helpers package-local instead.
43+
44+### Timeout diagnostics for slow Effect tests
45+
46+Several Executor tests already have manual sleeps, `Promise.race`, test-level
47+timeouts, or long-running fixtures. A `withTestCtx`-style helper would make
48+failures more readable by logging before timeout rather than only surfacing a
49+late Vitest timeout.
50+
51+Good initial targets:
52+
53+- `apps/cloud/src/mcp-miniflare.e2e.node.test.ts`
54+- `apps/cloud/src/mcp-session.e2e.node.test.ts`
55+- `packages/plugins/openapi/src/sdk/oauth-refresh.test.ts`
56+- `packages/plugins/mcp/src/sdk/connection-pool.test.ts`
57+- `packages/core/execution/src/tool-invoker.test.ts`
58+
59+The first slice should convert only one painful test cluster. If it makes the
60+test body clearer and failure output better, then expand.
61+
62+### Scoped fixture/runtime pattern
63+
64+LiveStore's `tests/sync-provider/src/sync-provider.test.ts` builds a shared
65+`ManagedRuntime` in `beforeAll`, disposes it in `afterAll`, then creates
66+per-test providers with isolated ids. That pattern is a good fit for expensive
67+Executor integration fixtures:
68+
69+- local HTTP protocol servers,
70+- MCP servers,
71+- OAuth mock servers,
72+- Miniflare environments,
73+- database-backed cloud services,
74+- plugin SDK tests that need realistic server behavior.
75+
76+Executor already uses `layer(TestLayer)(...)` in some OpenAPI and cloud tests.
77+Keep that where it works. Use the LiveStore runtime pattern only where the
78+fixture is expensive enough that rebuilding it per test is wasteful or flaky.
79+
80+### Property-test wrapper later
81+
82+LiveStore's `asProp` wrapper normalizes FastCheck options and makes shrinking
83+progress clearer. Do not port it preemptively.
84+
85+It becomes useful if Executor adds property tests for:
86+
87+- OpenAPI parameter encoding,
88+- form/multipart request bodies,
89+- schema round-trips,
90+- scope ordering and shadowing,
91+- tool/source dedupe,
92+- storage adapter conformance.
93+
94+Until then, direct `@effect/vitest` property tests are enough.
95+
96+### Possibly scoped React perf lint
97+
98+LiveStore enables `react-perf` oxlint rules for JSX props:
99+
100+- no new functions as props,
101+- no new objects as props,
102+- no JSX as props,
103+- no new arrays as props.
104+
105+This might be useful in `packages/react`, but it should start as a scoped
106+experiment. Do not enable it repo-wide. The risk is turning useful UI work into
107+memoization churn.
108+
109+## What Not to Port
110+
111+### Biome
112+
113+LiveStore uses Biome for formatting/import organization plus oxlint for linting.
114+Executor already uses `oxfmt` and oxlint:
115+
116+```txt
117+bun run format
118+bun run format:check
119+bun run lint
120+```
121+
122+Adding Biome would split formatter ownership without solving an Executor
123+problem.
124+
125+### devenv / genie / effect-utils repo generation
126+
127+LiveStore's config generation and `devenv` task setup are substantial
128+infrastructure. Executor's Bun/Turbo setup is smaller and easier to reason
129+about. Do not port that unless there is a concrete repo-management problem that
130+cannot be solved locally.
131+
132+### LiveStore's lint rules wholesale
133+
134+Executor's lint posture is already stronger and more domain-specific. Executor
135+currently bans or guides:
136+
137+- raw Vitest imports,
138+- conditional tests,
139+- double casts,
140+- cross-package relative imports,
141+- missing effect-atom reactivity keys,
142+- Effect escape hatches,
143+- unsupported Effect APIs,
144+- `new Error`,
145+- `try`/`catch` and raw `throw`,
146+- `Promise.catch` / `Promise.reject`,
147+- raw fetch outside approved boundaries,
148+- manual tagged-error checks,
149+- duplicated schema/value-derived types.
150+
151+LiveStore disables several things Executor intentionally cares about, including
152+some broad TypeScript strictness. Their rule set should stay reference-only.
153+
154+### An Effect barrel module
155+
156+LiveStore centralizes Effect exports through `@livestore/utils/effect`.
157+Executor mostly imports from `effect` directly. Do not introduce a broad
158+`@executor-js/.../effect` barrel just for symmetry. It would add another import
159+surface without a concrete need.
160+
161+## First Implementation Slice
162+
163+Add a tiny test helper, then use it in one test cluster.
164+
165+Possible local API:
166+
167+```ts
168+export const withTestCtx =
169+ (test: TestContext, options?: TestContextOptions) =>
170+ <A, E, R>(effect: Effect.Effect<A, E, R>) =>
171+ effect.pipe(
172+ Effect.logWarnIfTakesLongerThan({
173+ duration: "...",
174+ label: "...",
175+ }),
176+ Effect.timeout("..."),
177+ Effect.provide(options?.layer ?? Layer.empty),
178+ Effect.scoped,
179+ );
180+```
181+
182+Keep the helper small:
183+
184+- no tracing dependency in the first pass,
185+- no generic framework package,
186+- no large fixture registry,
187+- no migration of every test.
188+
189+Validation should be narrow:
190+
191+```txt
192+vitest run <converted test file> --testNamePattern "<converted test>"
193+```
194+
195+If the first conversion produces clearer test bodies and better failure output,
196+then expand it to the other long-running Effect integration tests.