Review

Review: 0002 - Domain API POC Tests

Implementation Summary

This spec implemented unit and integration tests for the Domain API POC, covering:

1. Mock server unit tests (Task 8.5)
2. Cross-API integration tests (Task 12.7)
3. Error handling unit tests (Task 13.2)

Files Changed

New Files:

• repos/domain-apis/tests/helpers/mock-server-manager.ts - Helper for spawning and managing Prism mock servers
• repos/domain-apis/tests/unit/mock-server.test.ts - Unit tests for mock server functionality
• repos/domain-apis/tests/integration/cross-api-traversal.test.ts - Integration tests for cross-API link traversal
• repos/domain-apis/tests/unit/error-handling.test.ts - Error handling tests for 404 and 400 responses

Lessons Learned

1. Prism Dynamic Mode vs Example Mode

Issue: Initially used Prism’s -d (dynamic) flag which generates random data instead of using OpenAPI spec examples.

Solution: Removed the -d flag so Prism returns deterministic data from the spec’s examples sections. This is critical for link traversal tests that depend on consistent IDs.

Recommendation: Always use static example mode for tests requiring predictable data. Dynamic mode is useful for stress testing/fuzzing but not for deterministic tests.

2. Jest Process Cleanup with Child Processes

Issue: Tests spawning Prism servers caused Jest to hang after test completion with “Jest did not exit one second after the test run has completed.”

Solution: Implemented more aggressive cleanup in stopMockServer() with SIGTERM followed by SIGKILL fallback. Tests should be run with --runInBand for tests involving process spawning.

Recommendation: When spawning external processes in tests, always:

• Use detached: false to keep processes in the same process group
• Implement cleanup in afterAll hooks
• Consider using --forceExit flag if cleanup issues persist

3. TypeScript Type Safety with response.json()

Issue: TypeScript’s fetch returns unknown type for response.json(), causing type errors when accessing properties.

Solution: Used explicit type casts: const data: any = await response.json() or await response.json() as any.

Recommendation: For test files, pragmatic type casts are acceptable. For production code, define proper interfaces for expected response shapes.

4. Spec vs Implementation Test Framework Discrepancy

Issue: The spec mentioned “Vitest” but the actual codebase uses Jest with ts-jest.

Solution: Followed the actual codebase patterns (Jest) rather than the spec text.

Recommendation: Always verify actual tooling in the codebase before starting implementation. Specs may have aspirational or outdated information.

5. Multi-Repo Workspace Complexity

Issue: The codev workspace contains multiple repos (including domain-apis). Worktrees for the codev workspace don’t include the referenced repos.

Solution: Made changes directly in the domain-apis repo with a new branch, rather than trying to work within the codev worktree.

Recommendation: For specs that modify files in referenced repos, clarify the commit/branch strategy upfront. Consider whether changes should be in the main workspace worktree or the target repo.

Test Coverage Analysis

Test Category	Tests Added	Coverage
Mock server startup	6	3 APIs x 2 tests each
Schema conformance	6	3 APIs x 2 tests each
Link format validation	9	3 APIs x 3 link types each
Helper functions	7	ID validation + URL validation
Cross-API traversal	8	All relationship directions
Error handling (404)	6	Non-existent resources
Error handling (400)	10	Invalid input scenarios

Deferred Work

The following items were identified but deferred:

• Negative integration tests: Testing link traversal to non-existent resources
• Timeout value specifications: Explicit timeout expectations in tests
• Coverage thresholds: No coverage requirements were set

Reviewer Notes

External review via Claude returned verdict: COMMENT with confidence: HIGH

Key issues raised (addressed or acknowledged):

1. Jest vs Vitest inconsistency - documentation issue, implementation uses Jest
2. Missing edge cases - deferred as nice-to-haves
3. Prism response determinism - addressed by removing -d flag

Conclusion

The implementation satisfies all spec requirements:

• Mock server tests verify Prism startup, schema conformance, and link format
• Integration tests verify cross-API link traversal for all relationship types
• Error handling tests verify 404 and 400 responses

The existing Lambda tests already cover 502 gateway error scenarios, so no additional tests were needed for that requirement.