Conversation
There was a problem hiding this comment.
Pull request overview
This pull request migrates the Spectral testing framework from Jest to Vitest and updates fast-check to version 4.x with the Vitest connector integration. The migration maintains backward compatibility by keeping jest-mock as a dependency since it's part of the public API.
Changes:
- Replaced Jest with Vitest as the test runner
- Updated fast-check from 2.16.0 to 4.5.3 and adopted the @fast-check/vitest connector
- Refactored property-based tests to use it.prop syntax
- Updated test assertions for better type safety (toStrictEqual → toBe where appropriate)
- Removed TypeScript workaround that was needed for Jest
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removed Jest dependencies, added Vitest and related packages |
| packages/spectral/vitest.config.ts | New Vitest configuration file |
| packages/spectral/tsconfig.test.json | Removed jest types, kept node types |
| packages/spectral/package.json | Updated test scripts and dev dependencies |
| packages/spectral/jest.config.js | Removed obsolete Jest configuration |
| packages/spectral/src/util.test.ts | Migrated to Vitest with it.prop pattern |
| packages/spectral/src/conditionalLogic/index.test.ts | Migrated to Vitest syntax |
| packages/spectral/src/serverTypes/*.test.ts | Updated mock usage (jest.fn → vi.fn) |
| packages/spectral/src/serverTypes/context.ts | Removed TypeScript error suppression workaround |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ? memoryUsageInMB() | ||
| : // @ts-expect-error: memoryUsage.rss() is documented but not typed | ||
| (memoryUsage.rss() as number) / MEMORY_USAGE_CONVERSION; | ||
| const usage = showDetail ? memoryUsageInMB() : memoryUsage.rss() / MEMORY_USAGE_CONVERSION; |
There was a problem hiding this comment.
nitpick: Definitely not a blocker. In a ternary with complex expressions, I like to use parentheses to ensure clarity:
const usage = showDetail ? memoryUsageInMB() : (memoryUsage.rss() / MEMORY_USAGE_CONVERSION);Otherwise, even if we know how the interpreter will interpret it, a human reading it could interpret it as equivalent to:
const usage = (showDetail ? memoryUsageInMB() : memoryUsage.rss()) / MEMORY_USAGE_CONVERSION;There was a problem hiding this comment.
This is actually something I have no control over :D prettier (or biome in our case) takes this away
Migrate spectral to
vitestand updatefast-check(property based testing library) to latest.This leaves
jest-mockin place since we expose a util that uses it as a part of our public API.I've adopted the vitest connector for fast-check which appears to be the preferred way to use the library. That necessitates a larger refactor of our usage but ultimately seems to land in a better spot.
Main:

This Branch:
