docs: clarified test isolation requirements and mock restoration patterns

- Clarified test isolation requirements to prohibit `mock.module()` due to global registry leakage across test files.
- Added guidance on using `spyOn` with proper restoration via `vi.restoreAllMocks()` for reliable per-test isolation.
- Documented workaround for pass and package dependencies using namespace imports and spy restoration in `afterEach`.
This commit is contained in:
can1357
2026-04-02 02:00:20 +02:00
parent 84e71f2c66
commit db763e327d
+2 -1
View File
@@ -454,7 +454,8 @@ When adding or changing tests, test the contract the system exposes — not the
- Do not add placeholder tests, tautologies, or assertions that only prove the code executed (`expect(true).toBe(true)`, `not.toThrow()`, non-empty string checks, array length growth checks, or "prompt exists" checks without a stronger semantic assertion).
- Prefer contract-level tests over implementation-detail tests. Avoid asserting internal helper wiring, field assignment, singleton identity, incidental ordering, prompt boilerplate, or passthrough option forwarding unless another component depends on that exact detail as a documented contract.
- Do not duplicate coverage across abstraction levels. If an integration or public-surface test already proves the behavior, delete or avoid the narrower unit test that only restates it through mocks or internal plumbing.
- Tests MUST be full-suite safe, not just file-local safe. Do not use top-level `mock.module()` for shared workspace packages (`@oh-my-pi/pi-utils`, `@oh-my-pi/pi-ai`, `@oh-my-pi/pi-natives`) or long-lived file-wide mutations of globals like `Bun.*`, `process.platform`, `process.env`, or `Bun.env` when a narrower seam exists. Prefer per-test `vi.spyOn(...)`, local fakes, and immediate restoration. A test that passes in isolation but poisons later files is broken.
- Tests MUST be full-suite safe, not just file-local safe. Do not use long-lived file-wide mutations of globals like `Bun.*`, `process.platform`, `process.env`, or `Bun.env` when a narrower seam exists. Prefer per-test `vi.spyOn(...)`, local fakes, and immediate restoration via `vi.restoreAllMocks()`. A test that passes in isolation but poisons later files is broken.
- Never use `mock.module()`. Bun's `mock.module()` mutates the global module registry and leaks across test files ([oven-sh/bun#12823](https://github.com/oven-sh/bun/issues/12823)). There is no reliable per-file isolation. Use `spyOn` on the imported module object instead, and restore in `afterEach`. For pass dependencies, import the pass object and spy on its `run` method. For package dependencies, use a namespace import and spy on the exported function.
- For lifecycle or stateful code, prefer one test per invariant or transition over several tiny tests that each assert one field from the same transition.
- For error handling, prefer tests that trigger the real failure path and assert the surfaced error contract over tests that directly instantiate error classes or inspect purely internal metadata.
- Smoke tests are only acceptable when they detect a failure mode narrower tests would miss. A test that only proves a package boots or a command starts is not enough.