Skip to content
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

test: add test-benchmark-buffer #15175

Closed
wants to merge 2 commits into from
Closed

test: add test-benchmark-buffer #15175

wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Sep 4, 2017

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.
@Trott 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 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
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jasnell
Copy link
Member

jasnell commented Sep 7, 2017

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

jasnell pushed a commit that referenced this pull request Sep 7, 2017
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 <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com
jasnell pushed a commit that referenced this pull request Sep 7, 2017
Add minimal test forbuffer benchmarks.

PR-URL: #15175
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com
@jasnell
Copy link
Member

jasnell commented Sep 7, 2017

Landed in b62343d and 19294c2

@jasnell jasnell closed this Sep 7, 2017
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
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 <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
Add minimal test forbuffer benchmarks.

PR-URL: #15175
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com
@MylesBorins MylesBorins mentioned this pull request Sep 10, 2017
MylesBorins pushed a commit that referenced this pull request Sep 11, 2017
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 <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com
MylesBorins pushed a commit that referenced this pull request Sep 11, 2017
Add minimal test forbuffer benchmarks.

PR-URL: #15175
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
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 <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
Add minimal test forbuffer benchmarks.

PR-URL: #15175
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com
addaleax pushed a commit to addaleax/node that referenced this pull request Sep 13, 2017
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 <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com
addaleax pushed a commit to addaleax/node that referenced this pull request Sep 13, 2017
Add minimal test forbuffer benchmarks.

PR-URL: nodejs#15175
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com
@MylesBorins
Copy link
Contributor

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 Trott deleted the vacay-10 branch January 13, 2022 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants