Repository navigation
Create a test for each benchmark file #187
Description
Some benchmarks simply fail, and we are not tracking that. Example: nodejs/node#59173.
Some benchmarks simply fail, and we are not tracking that. Example: nodejs/node#59173.
Activity
RafaelGSS commentedon Aug 15, 2025
For context:
We already wrote tests for each benchmark. See:
test/parallel/test-benchmark-*, but it runs just for one group of config, and it hides potential bugs.My suggestion is to run all configs, but set
n=1(for the ones applicable)RafaelGSS commentedon Aug 15, 2025
Patch suggestion: https://gist.github.com/RafaelGSS/653f031e800eda40cdb272252da56e25
Important: We must not increase the execution time of
test-benchmark-*files as it will affect the whole test suite.brunocroh commentedon Aug 19, 2025
So, I started working on that one, but after running the tests in
test/common/benchmark, I checked the benchmark output to see if more than one config was executed. Can I remove this validation?https://github.com/nodejs/node/blob/bdcab711b8048af2c4dcd4a2c33d506ce1621c6a/test/common/benchmark.js#L31-L50
And for tests that use duration, would it be better to set
duration=1(like was done withn), or should we keep the default duration?Questions about PR's: do I need to open one PR for each test I change, or can I open a single PR with a batch of changes?
cheers
brunocroh commentedon Aug 19, 2025
I think it’s inevitable that the test duration will increase if we run all options, even with
n=1, right?So ideally, we should run all configs but use
n=1so they run as fast as possible, right?In this example, I set
duration=1andn=1, and even with the flags to reduce the scope, it still took 8 times longer than the original test.Is this acceptable, or is there something else I could do to improve it?
RafaelGSS commentedon Aug 21, 2025
Can you measure which tests are taking more time?
brunocroh commentedon Aug 21, 2025
Hmm, in the actual state? Or after update to run configs with n=1?
Have some cli/tool that can I use to generate something like #186 csv output?
RafaelGSS commentedon Aug 21, 2025
You can use nodejs/node#59174
--trackand use n=1brunocroh commentedon Aug 21, 2025
Brillant, got it. Cheers
brunocroh commentedon Aug 27, 2025
So I created this spreadsheet for comparision
Results
Tests with
testargvTests witouth
test, and using n=1I had issues running the
napi,http, andhttp2suites, so these are missing from the results.Please let me know how I should proceed. Thank you!
RafaelGSS commentedon Aug 27, 2025
For the ones that got more time, it's because they don't use
nas an iteration parameter. For instance, the websocket usesroundtrips,so you would need to sendroundtrips=1.So, I believe we should go slightly differently. Instead of adjusting the test/parallel to include all configs. Let's create a different workflow that runs once per week and runs without test and using
n=1 roundtrips=1 .... We will need to normalize some benchmarks that aren't usingnto usen- let's also update the writing benchmarks document.This can be done in baby steps; the first step is to normalize benchmarks to use
nwhenever possible (except for cases wherenon-nmakes more sense, for instanceroundtrips).thisalihassan commentedon Mar 3, 2026
Hi @brunocroh @RafaelGSS can you please check this PR nodejs/node#62084 it fixes a bug introduced by nodejs/node#59872
benchmark: fix destructuring in dgram/single-buffer
benchmark: fix destructuring in dgram/single-buffer
benchmark: fix destructuring in dgram/single-buffer
benchmark: fix destructuring in dgram/single-buffer
benchmark: fix destructuring in dgram/single-buffer
benchmark: fix destructuring in dgram/single-buffer
benchmark: fix destructuring in dgram/single-buffer
benchmark: fix destructuring in dgram/single-buffer
benchmark: fix destructuring in dgram/single-buffer
benchmark: fix destructuring in dgram/single-buffer