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
Fix broken bench scripts #1291
Fix broken bench scripts #1291
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1291 +/- ##
==========================================
+ Coverage 94.14% 94.18% +0.03%
==========================================
Files 44 44
Lines 4098 4126 +28
==========================================
+ Hits 3858 3886 +28
Misses 240 240
Continue to review full report at Codecov.
|
@ShogunPanda @RafaelGSS could you take a look? |
@@ -129,6 +129,8 @@ function printResults (results) { | |||
] | |||
}) | |||
|
|||
console.log(results) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console.log(results) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed for debugging this and showing that it's using one sample anyway
@@ -10,7 +10,7 @@ const { WritableStream } = require('stream/web') | |||
|
|||
const { Pool, Client, fetch, Agent, setGlobalDispatcher } = require('..') | |||
|
|||
const iterations = (parseInt(process.env.SAMPLES, 10) || 100) + 1 | |||
const iterations = (parseInt(process.env.SAMPLES, 10) || 10) + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does it need to change? I have run it locally and it's working fine. When 10
it uses only 1 sample.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing, I committed my current branch.
Co-authored-by: Rafael Silva <rafael.nunu@hotmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
* partial fix for bench * fix lint * Update benchmarks/benchmark.js Co-authored-by: Rafael Silva <rafael.nunu@hotmail.com> Co-authored-by: Rafael Silva <rafael.nunu@hotmail.com>
* partial fix for bench * fix lint * Update benchmarks/benchmark.js Co-authored-by: Rafael Silva <rafael.nunu@hotmail.com> Co-authored-by: Rafael Silva <rafael.nunu@hotmail.com>
* partial fix for bench * fix lint * Update benchmarks/benchmark.js Co-authored-by: Rafael Silva <rafael.nunu@hotmail.com> Co-authored-by: Rafael Silva <rafael.nunu@hotmail.com>
Our bench CI job is not working anymore. This PR partially fixes it but the number of iterations do not work anymore.