Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add median to --output-json #5745

Merged
merged 1 commit into from
May 27, 2024
Merged

Conversation

Joristdh
Copy link
Contributor

@Joristdh Joristdh commented May 17, 2024

Description

In #5398 the JSON reporter got reworked for the benchmark results. In this rework, the samples array was dropped to prevent huge output files. However, without the samples it's not possible anymore to manually calculate some extra metrics after the bench run. The median is simple enough to calculate when having acces to the samples and can provide some interesting insights (e.g. significant difference between mean and median could indicate outliers).

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

  • 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.
  • Ideally, include a test that fails without this PR but passes with it.
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

  • 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

  • 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:.

@Joristdh Joristdh changed the title feat: Calculate median for benchmark results feat: add median to --output-json May 17, 2024
@Joristdh Joristdh changed the title feat: add median to --output-json feat: add median to --output-json May 17, 2024
@Joristdh
Copy link
Contributor Author

If the Vitest framework isn't the right place for new metrics, the change could also be migrated to the underlying Tinybench results.

@sheremet-va
Copy link
Member

If the Vitest framework isn't the right place for new metrics, the change could also be migrated to the underlying Tinybench results.

I guess it makes sense to have it in Vitest because you can extrapolate it in tinybench from samples, although it might also be nice for tinybench users to have a quick access to median on the object, so I am fine either way

@Joristdh
Copy link
Contributor Author

I think indeed that calculating the median is trivial when having access to all the samples, so this mainly adds value for Vitest users.

Maybe it'd be even better to just add a flag to preserve the original samples. Then Vitest users can deal with/use the results however they see fit.

In any case, that would be something for another PR. Is there something I just do to get this merged after the approval @sheremet-va?

@sheremet-va sheremet-va merged commit 0766b7f into vitest-dev:main May 27, 2024
14 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants