vitest icon indicating copy to clipboard operation
vitest copied to clipboard

fix: rewrite benchmark reporter without `log-update`

Open AriPerkkio opened this issue 1 year ago • 7 comments

Description

  • Fixes https://github.com/vitest-dev/vitest/issues/5895
  • Patches tinyexec for https://github.com/tinylibs/tinyexec/issues/40. Alternatively I could remove the patch and use node:child_process in test cases.
  • Removes log-update and cli-truncate -> -92 kB from bundle
  • Uses summary-reporter so users can now scroll up and see previous benchmark results while tests are running.

Test preview release with:

  • [x] https://github.com/radashi-org/radashi/tree/main/benchmarks
    • They have 98 benchmark files. In vitest@v2 they could not see previous results during the run as log-update limits the rows to the terminal height. Now you can actually scroll up during the run and see previous results. No need to wait for that ~8min run to finish.
    • Having the summary reporter on bottom of terminal helps here a lot as you can actually see how many benchmark files are left.
  • [x] https://github.com/toss/es-toolkit/tree/main/benchmarks/performance
    • Noticed tests being reported as 'pass' multiple times, fixed.
  • [x] https://github.com/vuejs/core/blob/main/packages/reactivity/benchmarks/ref.bench.ts

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • [x] It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
  • [x] Ideally, include a test that fails without this PR but passes with it.
  • [x] Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

  • [x] Run the tests with pnpm test:ci.

Documentation

  • [ ] If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

  • [x] Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

AriPerkkio avatar Dec 04 '24 10:12 AriPerkkio

Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
Latest commit 9c9e69b1f6b089555e5d7ffb570e5efd9f688c9b
Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/676c3dc88a0d4f0008fb9abc
Deploy Preview https://deploy-preview-7019--vitest-dev.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Dec 04 '24 10:12 netlify[bot]

@vitest/browser

npm i https://pkg.pr.new/@vitest/browser@7019
@vitest/coverage-istanbul

npm i https://pkg.pr.new/@vitest/coverage-istanbul@7019
@vitest/expect

npm i https://pkg.pr.new/@vitest/expect@7019
@vitest/coverage-v8

npm i https://pkg.pr.new/@vitest/coverage-v8@7019
@vitest/mocker

npm i https://pkg.pr.new/@vitest/mocker@7019
@vitest/pretty-format

npm i https://pkg.pr.new/@vitest/pretty-format@7019
@vitest/runner

npm i https://pkg.pr.new/@vitest/runner@7019
@vitest/snapshot

npm i https://pkg.pr.new/@vitest/snapshot@7019
@vitest/spy

npm i https://pkg.pr.new/@vitest/spy@7019
@vitest/ui

npm i https://pkg.pr.new/@vitest/ui@7019
@vitest/utils

npm i https://pkg.pr.new/@vitest/utils@7019
vite-node

npm i https://pkg.pr.new/vite-node@7019
@vitest/web-worker

npm i https://pkg.pr.new/@vitest/web-worker@7019
vitest

npm i https://pkg.pr.new/vitest@7019
@vitest/ws-client

npm i https://pkg.pr.new/@vitest/ws-client@7019

commit: 5bb20e1

pkg-pr-new[bot] avatar Dec 04 '24 10:12 pkg-pr-new[bot]

Looks like benchmark runner is reporting tests of nested suites twice. Need to resolve this, otherwise summary is showing too high test count.

AriPerkkio avatar Dec 11 '24 17:12 AriPerkkio

Does this also add a project name to the output? I just noticed we don't print |name|

sheremet-va avatar Dec 11 '24 17:12 sheremet-va

Project name is visible now:

AriPerkkio avatar Dec 11 '24 17:12 AriPerkkio

It's not in table output though, should it be here too?

AriPerkkio avatar Dec 13 '24 08:12 AriPerkkio

It's not in table output though, should it be here too?

Yeah

sheremet-va avatar Dec 13 '24 08:12 sheremet-va

About tinyexec patch, it's not a blocker, but the situation is not ideal since patch is only applied for our local dev, but tinyexec is also used for external dependencies in a few places.

I agree. I'll replace tinyexec usage+patching with node:child_process or maybe even startVitest when using preserveAnsi option from https://github.com/vitest-dev/vitest/pull/7090/files#diff-eee947831abd0685dc52e26f42f2c7c3adfb802e478ca8e252ba649f1a1ab2f6R5-R13.

AriPerkkio avatar Dec 23 '24 08:12 AriPerkkio