refactor(coding-agent): migrated test mocks to vitest spyOn with Symbol.dispose cleanup

- Migrated test mocking from bun:test mock API to vitest spyOn pattern across 9 test files.
- Extracted mock setup logic into reusable helper functions with Symbol.dispose cleanup pattern.
- Replaced manual beforeEach/afterEach and try-finally blocks with TypeScript 5.2 using declarations.
- Removed 178 lines of boilerplate mock initialization and restoration code from test suite.
This commit is contained in:
can1357
2026-03-26 16:10:12 +01:00
parent 4ff0f309a1
commit df369dd7fd
10 changed files with 233 additions and 403 deletions
+1
View File
@@ -454,6 +454,7 @@ 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.
- 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.