test: reduce run time for misc benchmark tests #16120
Conversation
Run only one benchmark for each benchmark file tested by test-benchmark-misc.js.
Trott
added
benchmark
nodejs-github-bot
added
benchmark
| @@ -93,6 +93,8 @@ function runUsingArgumentsAndApply(n, concat) { | |||
| function main(conf) { | |||
| const n = +conf.n; | |||
| switch (conf.method) { | |||
| // '' is a default case for tests | |||
There was a problem hiding this comment.
Why are we always running the first case? Are the default cases missing a break? I'd rather have explicit calls then the first one as default. (Maybe I shouldn't comment if I'm missing context)
There was a problem hiding this comment.
@fhinkel We are limiting to the first case in tests only. conf.method is assigned a default value earlier in the code (line 16). Tests override it with a blank string by using --set method= as a command line option.
The reason is so that we don't have to change parameter names in all the benchmarks.
Let's say test-benchmark-misc runs a benchmark called foo.js. It takes an configuration option called method. The method option may be readFile or readFileSync. If it's not one of those, the benchmark throws.
But test-benchmark-misc also runs a benchmark called bar.js. It also takes a method option but throws if they are anything other than chdir or chmod.
It is now impossible to specify a configuration option (so that each benchmark file only runs a single benchmark) for method without causing one or the other to throw.
Rather than try to figure out unique names for each benchmark file (which seems like an anti-pattern--I don't want to have to know that foo uses method but bar uses function or something like that), we assign a default for the case where the configuration option is explicitly set to an empty value.
There was a problem hiding this comment.
That makes a lot of sense Thanks for the explanation!
Run only one benchmark for each benchmark file tested by test-benchmark-misc.js. PR-URL: nodejs#16120 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
|
Landed in ce848a4 |
Run only one benchmark for each benchmark file tested by test-benchmark-misc.js. PR-URL: nodejs/node#16120 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Run only one benchmark for each benchmark file tested by test-benchmark-misc.js. PR-URL: #16120 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Run only one benchmark for each benchmark file tested by test-benchmark-misc.js. PR-URL: #16120 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Run only one benchmark for each benchmark file tested by test-benchmark-misc.js. PR-URL: #16120 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Run only one benchmark for each benchmark file tested by test-benchmark-misc.js. PR-URL: #16120 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Run only one benchmark for each benchmark file tested by
test-benchmark-misc.js.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test benchmark