chore(deps): upgrade tinybench to v6#7100
chore(deps): upgrade tinybench to v6#7100jerome-benoit wants to merge 20 commits intovitest-dev:mainfrom
Conversation
Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
|
@jerome-benoit any progress with this?... Can you migrate this to v4? 🙏🏻 |
|
It's still on my TODO list, but I had no time to finish the migration. |
|
@jerome-benoit thank you for the quick response... I hope you find some time to finish 👍🏻 |
…rade # Conflicts: # packages/vitest/src/runtime/runners/benchmark.ts # test/benchmark/test/reporter.test.ts
…stream merge - Use latency.mean instead of deprecated mean in benchmark ranking sort - Guard filter with benchmark?.latency to handle in-progress benchmarks - Simplify error handler with null-safe access (e.task?.result?.error) - Remove dead test:benchmark script referencing deleted workspace
jerome-benoit
changed the title
There was a problem hiding this comment.
Pull request overview
Upgrades Vitest’s benchmarking integration to work with tinybench@6, adapting internal result types and benchmark reporting/runner logic to match tinybench’s new discriminated-union results and latency/throughput statistics model.
Changes:
- Bump
tinybenchdependency to^6.0.0(lockfile + package dependency). - Refactor runtime benchmark runner/types to use tinybench v6 task/results and map
includeSamplestoretainSamples. - Update benchmark reporters/tests to read from
latency.*/throughput.*instead of deprecated top-level stats.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/cli/test/benchmarking.test.ts | Updates include-samples assertions to match v6 latency.samples behavior. |
| pnpm-lock.yaml | Locks tinybench@6.0.0 and updates related dependency metadata. |
| packages/vitest/src/runtime/types/benchmark.ts | Redefines benchmark result/statistics types for v6 (latency/throughput, samples handling). |
| packages/vitest/src/runtime/runners/benchmark.ts | Refactors benchmark execution to Bench.run() and v6 events/results. |
| packages/vitest/src/runtime/benchmark.ts | Tightens benchmark options storage/types and adds formatted name into options. |
| packages/vitest/src/public/index.ts | Updates public type re-exports to new tinybench v6 aliases. |
| packages/vitest/src/node/reporters/benchmark/tableRender.ts | Updates table rendering to use latency.* and compare via throughput.mean. |
| packages/vitest/src/node/reporters/benchmark/reporter.ts | Updates sorting/ranking logic to use latency.mean. |
| packages/vitest/src/node/reporters/benchmark/json-formatter.ts | Stops forcibly overriding samples in JSON output. |
| packages/vitest/src/node/reporters/base.ts | Updates benchmark summary ratio calculation to use latency.mean. |
| packages/vitest/package.json | Bumps tinybench dependency to ^6.0.0. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
You can also share your feedback on Copilot code review. Take the survey.
…y, fix comparison display
Guard complete handler against errored/aborted tasks to prevent TypeError crash. Track error state to correctly mark failed benchmarks. Replace Object.assign with selective field extraction to prevent tinybench runtime metadata from leaking into BenchmarkResult and JSON output. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Move retainSamples (includeSamples → retainSamples) mapping from the runner call site into getBenchOptions, which now takes the runner config and returns ready-to-use tinybench BenchOptions. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Rename numberOfSamples to samplesCount to match tinybench Statistics.samplesCount. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
|
Hey @sheremet-va @hi-ogawa @AriPerkkio 👋 This PR is finally ready for review! It's been sitting around since December 2024, but it's now fully up to date with main and covers the complete tinybench v2.9.0 → v6.0.0 upgrade. Beyond the API migration, this includes a few correctness fixes found during an audit of the benchmarking code path (error handling in the CI is green (the browser test flake on the previous run was from the upstream merge and has since been fixed). Happy to address any feedback. Thanks! |
|
Hey! Thank you for the PR and sorry for the lack of communication, but we currently don't want to accept PRs targeting benchmarking features because we have a big rewrite on our road map that changes the whole architecture to not treat By accepting this PR we might introduce bugs that need to be fixed in an already kind of softly deprecated feature, so I would rather avoid any turbulence at this time. |
Fair enough. But given the actual minimal code churn and the obviousness of the trivial fixes (data structure fields rename, proper condition and code flow, grouped steps, ... and all of it relying on a library version that does not has single bug report since), I do not see how it be worse with it. The current implementation is buggy. Making it just works better will not introduce any maintenance overhead, at all, unless the revamp has already started and that PR will introduce conflicts in another PR. |
It could be worse because it can break existing integrations, like codespeed, or it might not! There are almost no tests for benchmarking, so it is hard to catch. The changes in interfaces is already breaking enough, for example. Basically, I don't want to break the ecosystem multiple times. |
It can't be worse, the code is already telling it.
It can be reverted to keep the exact same API if it's an issue for a minor revision beta cycle.
Understood but here it's basically trivial bug fixes that can be made totally transparent if needed.: vitest already abstract out tinybench internals: API, tunables, in and out data structure, ... I can add simplified tests ported from tinybench to vitest API in another PR to ensure that PR will pass a more comprehensive of real world usage tests suite for benchmark. |
|
All of these just increases maintenance, someone needs to write write this, then review this, triage the issue to see if it needs to be reverted. Benchmarking has a very low priority at the moment, it won't be merged anyway. And when priority increases, the work will be focused on the new API. |
Discouraging external contributions made by people having a clue on what they are doing is a very bad move for a project maintenance point of view. Especially on a low priority and under maintained component orthogonal to the project main use case. And is even worse if the proposal is offering a baseline unit tests integration that will establish a solid TDD for any future enhancements. |
Anything added to benchmarking will be removed and rewritten within half a year, the feature in its current state is deprecated. So yes, we discourage any contribution to it as I said in my initial comment. Please, do not take this personally. As a person who has a clue on what you are doing and whose opinion on benchmarking I respect, you should leave feedback on the next iteration of benchmarking here: #7850 |
Description
Upgrade tinybench from v2.9.0 to v6.0.0.
Breaking changes adapted:
TaskResultis now a discriminated union —BenchmarkResultredefined as standalone vitest-owned interface withBenchmarkStatisticstype andcreateEmptyStatistics()helper for type-safe defaultshz,samples,mean, etc.) — usinglatency.*,throughput.*insteadretainSamplesoption (defaults tofalse) replaces manualsamples.length = 0clearing — mapped to vitest'sincludeSamplesvia centralizedgetBenchOptionsBenchEventis now a typed class — event handlers use typed event APIBenchmarkResult.sampleCountrenamed tosamplesCountto align withStatistics.samplesCountBug fixes included:
completeevent handler crash when task errors — tinybench v6 firescompleteaftererror, but errored tasks have nolatencyproperty. Added task state guard and error trackingpass— now correctly marked asfailObject.assign(result, task.result)with selective field extraction — prevents tinybench runtime metadata (runtime,runtimeVersion,timestampProviderName) from pollutingBenchmarkResultand JSON outputdiffFixedcheck ('1.0.0'→'1.00') with properelse ifchainUser-facing type change:
BenchmarkResult.sampleCount→BenchmarkResult.samplesCount.BenchTaskResultre-export is now the tinybench v6TaskResultdiscriminated union.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.Changesets