benchmark: remove deprecated argument #27091
Conversation
|
The benchmark files are not tested with the CI (@nodejs/build could we add such a job? That way we could just start that in case benchmark files are changed), so I marked this as |
It's mildly roundabout, but they are run in node-daily-master. So to run them in a PR, you could:
I've done all that. https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/5971/ |
Interestingly, it looks like some other recently-landed commit broke the benchmark tests. |
The benchmarks for dns.lookup() include calling it with an empty hostname which results in a deprecation warning. This benchmark seems to be subject to some odd side effects (see Ref below) and we probably generally don't want to benchmark deprecated things by default anyway. Remove the deprecated value from the default list. Bonus is that this will speed up the benchmark. Refs: nodejs#27081 (comment)
|
Landed in 77dee25 |
The benchmarks for dns.lookup() include calling it with an empty hostname which results in a deprecation warning. This benchmark seems to be subject to some odd side effects (see Ref below) and we probably generally don't want to benchmark deprecated things by default anyway. Remove the deprecated value from the default list. Bonus is that this will speed up the benchmark. Refs: nodejs#27081 (comment) PR-URL: nodejs#27091 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The benchmarks for dns.lookup() include calling it with an empty
hostname which results in a deprecation warning. This benchmark seems to
be subject to some odd side effects (see Ref below) and we probably
generally don't want to benchmark deprecated things by default anyway.
Remove the deprecated value from the default list. Bonus is that this
will speed up the benchmark.
Refs: #27081 (comment)
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes