test: add test-benchmark-buffer #15175

Trott · 2017-09-04T05:31:09Z

First commit:

benchmark: add default configs to buffer benchmark

Add default values to use for `type` and `method` in `buffer` benchmarks
when the provided configuration value is an empty string. This is
primarily useful for testing, so the test can request a single iteration
without having to worry about providing different valid values for the
different benchmarks.

While making this change, some `var` instances in immediately
surrounding code were changed to `const`. In some cases, `var` had been
preserved so that the benchmarks would continue to run in versions of
Node.js prior to 4.0.0. However, now that `const` has been introduced
into the benchmark `common` module, the benchmarks will no longer run
with those versions of Node.js anyway.

Second commit:

test: add test-benchmark-buffer

Add minimal test forbuffer benchmarks.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

test benchmark buffer

Add default values to use for `type` and `method` in `buffer` benchmarks when the provided configuration value is an empty string. This is primarily useful for testing, so the test can request a single iteration without having to worry about providing different valid values for the different benchmarks. While making this change, some `var` instances in immediately surrounding code were changed to `const`. In some cases, `var` had been preserved so that the benchmarks would continue to run in versions of Node.js prior to 4.0.0. However, now that `const` has been introduced into the benchmark `common` module, the benchmarks will no longer run with those versions of Node.js anyway.

Add minimal test forbuffer benchmarks.

mhdawson

LGTM

jasnell · 2017-09-07T05:37:38Z

CI: https://ci.nodejs.org/job/node-test-pull-request/9973/

BridgeAR · 2017-09-07T22:47:21Z

benchmark/buffers/buffer-creation.js

@@ -19,6 +19,7 @@ function main(conf) {
  const len = +conf.len;
  const n = +conf.n;
  switch (conf.type) {
+    case '':


It would be good to add a short description why this is necessary.
E.g. // Used for testing

Oy... I just landed this then saw this comment, my apologies for missing it

Add default values to use for `type` and `method` in `buffer` benchmarks when the provided configuration value is an empty string. This is primarily useful for testing, so the test can request a single iteration without having to worry about providing different valid values for the different benchmarks. While making this change, some `var` instances in immediately surrounding code were changed to `const`. In some cases, `var` had been preserved so that the benchmarks would continue to run in versions of Node.js prior to 4.0.0. However, now that `const` has been introduced into the benchmark `common` module, the benchmarks will no longer run with those versions of Node.js anyway. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

Add minimal test forbuffer benchmarks. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

jasnell · 2017-09-07T22:52:12Z

Landed in b62343d and 19294c2

Add default values to use for `type` and `method` in `buffer` benchmarks when the provided configuration value is an empty string. This is primarily useful for testing, so the test can request a single iteration without having to worry about providing different valid values for the different benchmarks. While making this change, some `var` instances in immediately surrounding code were changed to `const`. In some cases, `var` had been preserved so that the benchmarks would continue to run in versions of Node.js prior to 4.0.0. However, now that `const` has been introduced into the benchmark `common` module, the benchmarks will no longer run with those versions of Node.js anyway. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

Add minimal test forbuffer benchmarks. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

Add default values to use for `type` and `method` in `buffer` benchmarks when the provided configuration value is an empty string. This is primarily useful for testing, so the test can request a single iteration without having to worry about providing different valid values for the different benchmarks. While making this change, some `var` instances in immediately surrounding code were changed to `const`. In some cases, `var` had been preserved so that the benchmarks would continue to run in versions of Node.js prior to 4.0.0. However, now that `const` has been introduced into the benchmark `common` module, the benchmarks will no longer run with those versions of Node.js anyway. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

Add minimal test forbuffer benchmarks. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

Add default values to use for `type` and `method` in `buffer` benchmarks when the provided configuration value is an empty string. This is primarily useful for testing, so the test can request a single iteration without having to worry about providing different valid values for the different benchmarks. While making this change, some `var` instances in immediately surrounding code were changed to `const`. In some cases, `var` had been preserved so that the benchmarks would continue to run in versions of Node.js prior to 4.0.0. However, now that `const` has been introduced into the benchmark `common` module, the benchmarks will no longer run with those versions of Node.js anyway. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

Add minimal test forbuffer benchmarks. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

Add default values to use for `type` and `method` in `buffer` benchmarks when the provided configuration value is an empty string. This is primarily useful for testing, so the test can request a single iteration without having to worry about providing different valid values for the different benchmarks. While making this change, some `var` instances in immediately surrounding code were changed to `const`. In some cases, `var` had been preserved so that the benchmarks would continue to run in versions of Node.js prior to 4.0.0. However, now that `const` has been introduced into the benchmark `common` module, the benchmarks will no longer run with those versions of Node.js anyway. PR-URL: nodejs#15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

Add minimal test forbuffer benchmarks. PR-URL: nodejs#15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

MylesBorins · 2017-09-20T20:07:57Z

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

Trott added 2 commits September 3, 2017 22:29

test: add test-benchmark-buffer

ff792be

Add minimal test forbuffer benchmarks.

Trott added benchmark Issues and PRs related to the benchmark subsystem. buffer Issues and PRs related to the buffer subsystem. test Issues and PRs related to the tests. labels Sep 4, 2017

nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. buffer Issues and PRs related to the buffer subsystem. test Issues and PRs related to the tests. labels Sep 4, 2017

jasnell approved these changes Sep 5, 2017

View reviewed changes

mhdawson approved these changes Sep 6, 2017

View reviewed changes

BridgeAR reviewed Sep 7, 2017

View reviewed changes

jasnell pushed a commit that referenced this pull request Sep 7, 2017

test: add test-benchmark-buffer

19294c2

Add minimal test forbuffer benchmarks. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

jasnell closed this Sep 7, 2017

MylesBorins pushed a commit that referenced this pull request Sep 10, 2017

test: add test-benchmark-buffer

704faa1

Add minimal test forbuffer benchmarks. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

MylesBorins mentioned this pull request Sep 10, 2017

v8.5.0 proposal #15308

Merged

MylesBorins pushed a commit that referenced this pull request Sep 11, 2017

test: add test-benchmark-buffer

7078c2f

Add minimal test forbuffer benchmarks. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

MylesBorins pushed a commit that referenced this pull request Sep 12, 2017

test: add test-benchmark-buffer

7106734

Add minimal test forbuffer benchmarks. PR-URL: #15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

addaleax pushed a commit to addaleax/node that referenced this pull request Sep 13, 2017

test: add test-benchmark-buffer

9cfa57d

Add minimal test forbuffer benchmarks. PR-URL: nodejs#15175 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]

MylesBorins added the backport-requested-v6.x label Sep 20, 2017

Trott added dont-land-on-v4.x and removed backport-requested-v6.x labels Sep 25, 2017

Trott deleted the vacay-10 branch January 13, 2022 22:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add test-benchmark-buffer #15175

test: add test-benchmark-buffer #15175

Trott commented Sep 4, 2017

mhdawson left a comment

jasnell commented Sep 7, 2017

BridgeAR Sep 7, 2017

jasnell Sep 7, 2017

jasnell commented Sep 7, 2017

MylesBorins commented Sep 20, 2017

test: add test-benchmark-buffer #15175

test: add test-benchmark-buffer #15175

Conversation

Trott commented Sep 4, 2017

Checklist

Affected core subsystem(s)

mhdawson left a comment

Choose a reason for hiding this comment

jasnell commented Sep 7, 2017

BridgeAR Sep 7, 2017

Choose a reason for hiding this comment

jasnell Sep 7, 2017

Choose a reason for hiding this comment

jasnell commented Sep 7, 2017

MylesBorins commented Sep 20, 2017