Repository navigation
feat: Support Vitest 5 - #113
Conversation
Vitest 5 changed the Assertion type parameters to <R, T>, so the vitest-evals matcher augmentations failed to merge. Match the new signature and move the workspace to Vitest 5. vitest-evals now needs vitest >=5 <6: a Vitest 4 Assertion<T> cannot merge with the new declaration. The GitHub reporter does not augment Vitest types, so it accepts both majors. Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
i assume its too copmlicated to support both versions? |
|
For the runtime, no. The reporter and matchers run the same on Vitest 4 and 5. For the types, mostly yes. Vitest 5 changed
So one I tested this branch's build in a Vitest 4.1.12 project:
Real dual support would mean shipping a second augmentation for v4, for example a So we're keeping this PR v5-only and shipping it as 0.18.0. |
The Eval workflow on `main` reports green, but no evals have run. `eval:ci` fails at startup: ``` Failed to load custom Reporter from vitest-evals/reporter cause: Package subpath './reporters' is not defined by "exports" (ERR_PACKAGE_PATH_NOT_EXPORTED) ``` The cause is that vitest-evals 0.4.0 imports `vitest/reporters`, which vitest 5 removed. `continue-on-error` on "Run evals" hides the failure, so no `eval-results.json` gets written and the status check step throws. Example run: https://github.com/getsentry/toolkit/actions/runs/41951775819 This PR moves the eval package to vitest-evals 0.18.0, which supports vitest 5 (getsentry/vitest-evals#113, peer range `vitest >=5 <6`). The whole repo stays on vitest 5; there is no vitest 4 pin. ## Changes - **Bump `vitest-evals` to `^0.18.0`.** I added `vitest-evals@0.18.0` and its `@vitest-evals/core@0.18.0` and `@vitest-evals/report-ui@0.18.0` dependencies to `minimumReleaseAgeExclude`, because they were published today. Drop those entries once they are past the 24h window. - **Import from `vitest-evals/legacy`.** 0.18 keeps the scorer-first API (`describeEval` with `data`/`task`/`scorers`, `ToolCallScorer`, `Score`, `ScoreFn`, `BaseScorerOptions`) under `vitest-evals/legacy`. This is an import-path change only, in the 33 eval imports and the example in `docs/testing/overview.md`. The eval code is unchanged. - **Remove the unused root `vitest-evals` dev dependency.** Nothing at the root imports it. - **Set `SENTRY_CLI_NO_TELEMETRY=1` for the scorer's `sentry mcp` process.** - `ToolPredictionScorer` starts `pnpm --dir packages/cli run cli mcp` from source. On `main`, that process exits immediately with ``Package subpath './client' is not defined by "exports"``, and every scorer then fails with `MCPClientError: Connection closed`. - The exit comes from `lazyTelemetry()` in `src/lib/db/index.ts`. It loads `../telemetry.js` with `createRequire`, so its imports resolve `@sentry/api/client` under the `require` condition, but `@sentry/api` exports `./client` only for `import`. - Evals shouldn't send CLI telemetry anyway. The CLI bug itself is not fixed here: `pnpm --filter sentry run cli help` also fails from source. It needs its own CLI fix. `continue-on-error` and the eval status check are unchanged. ## Verification - **Local, no LLM key:** with no key in the sandbox, `pnpm eval:ci` on vitest 5.0.5 loads the reporter, collects and runs all 27 files and 121 evals, and writes `eval-results.json`. On `main` it stops at startup. Every eval failed, and the only cause was the missing provider key. No `Connection closed` errors. - **Checks:** `pnpm run tsc`, `pnpm run lint`, and `pnpm install --frozen-lockfile` pass. The full `pnpm run test` didn't finish inside my sandbox's time limit, so I'm relying on CI for it. ## Related - #1132 (draft, "Migrate to vitest-evals harness") moves these evals to the harness API. It touches the same eval files, so expect import conflicts there. This PR keeps the current scorer-first suite running until then. - #1972 (Roach recordings for evals) depends on this, because Roach records nothing while the suite doesn't run. Both PRs change `pnpm-lock.yaml` and `pnpm-workspace.yaml`, so #1972 needs a rebase after this merges. <!-- junior-request-attribution:start --> via @dcramer. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3A1de0f02c391e347294cc7ce28353bcfe --> -- [View Session](https://junior-prod.sentry.dev/conversations/agent%3A1de0f02c391e347294cc7ce28353bcfe) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3A1de0f02c391e347294cc7ce28353bcfe/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: David Cramer <23610+dcramer@users.noreply.github.com>
Makes
vitest-evalswork on Vitest 5. getsentry/toolkit uses Vitest 5, and its eval suite can't load the reporter from thevitest-evals0.4.0 release it depends on, because that release importsvitest/reporters, which Vitest 5 removed.The reporter on
mainalready imports fromvitest/node, and Vitest 5 still exportsDefaultReporterandVerboseReporterfrom there, so it runs. What broke were the types and the test fixtures.Warning
Breaking change for Vitest 4 consumers. The
vitest-evalspeer dependency changes fromvitest >=4 <5tovitest >=5 <6. Onedeclare module "vitest"block can't merge with both the Vitest 4Assertion<T>and the Vitest 5Assertion<R, T>, so the typed matchers no longer support Vitest 4. Ship this as a minor bump (0.18.0), not a patch.Changes
Assertion<R extends void | Promise<void> = void, T = unknown>. The augmentations inindex.ts(toSatisfyJudge) andlegacy.ts(toEval) usedAssertion<T = any>, sotscfailed with TS2428. Both now use the Vitest 5 signature.vitest-evalsrequiresvitest >=5 <6, as the warning above describes.@vitest-evals/github-reporterdoesn't augment Vitest types, so it accepts>=4 <6.vitestand@vitest/coverage-v8move to^5.1.0, and the lockfile is regenerated with pnpm 10.33.0.statefrom the context and callsbenchmarks()on each test case, so the fake context andTestCasenow provide both. The verbose fallback test now expectsfile:12, because Vitest 5 prints the line number without the column.Verification
pnpm typecheck,pnpm lint,pnpm build, andpnpm release:checkpass.pnpm test:ci: 408 tests pass and 1 fails. The failure is inreport-ui, "returns 500 when asset reads fail". It fails the same way onmainwith Vitest 4 in my sandbox, where the file is still readable afterchmod 000, so this change didn't cause it.main, which runs Vitest 5.0.5. With its imports switched fromvitest-evalstovitest-evals/legacy,vitest run --reporter=vitest-evals/reporterloaded the reporter. All 27 files and 121 evals ran and wroteeval-results.json, andtscpassed for the eval package. The evals themselves failed only because the sandbox has no provider API keys.Follow-up
After 0.18.0 is released, toolkit can move from
vitest-evals0.4.0 to it and keep Vitest 5. That change will replace getsentry/toolkit#1973's current approach, which pins Vitest 4 for the eval package.via @dcramer.
--
View Session [Sentry]