test: add parallel invocation of querystring benchmark#16052
test: add parallel invocation of querystring benchmark#16052jeskew wants to merge 2 commits intonodejs:masterfrom jeskew:test/querystring-benchmark
Conversation
|
|
||
| const runBenchmark = require('../common/benchmark'); | ||
|
|
||
| runBenchmark('querystring', ['n=1', 'input=', 'type=']); |
There was a problem hiding this comment.
I believe n=1 is probably enough. Setting type= would leave some codepaths uncovered.
There was a problem hiding this comment.
I've added a type and input from the regular benchmark run. This is my first time working with the node.js benchmarks; thanks for reviewing and your help.
|
|
||
| const runBenchmark = require('../common/benchmark'); | ||
|
|
||
| runBenchmark('querystring', ['n=1', 'input=', 'type=']); |
There was a problem hiding this comment.
This would result in multiple undefined values so we should at least use a existing value.
There was a problem hiding this comment.
Thanks for the context; I was trying to replicate the pattern used in other benchmark invocations, where an empty string is used. I've pulled in what looked like the appropriate "no-op" values from the existing benchmark.
|
Thanks for the contribution! |
|
Landed in ee587f3, thanks for your contribution and welcome to Node.js! |
PR-URL: #16052 Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs/node#16052 Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #16052 Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test