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

benchmark: fix benchmark/run.js handling of --set #41334

Merged
merged 1 commit into from
Dec 29, 2021

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 27, 2021

run.js does not work with --set as it tries to include it as
options to fork() rather than as part of argv for fork(). This
doesn't throw an error because of a quirk in fork() that silently
accepts arrays for options objects. This will be changing in Node.js
18.x.

run.js does not work with --set as it tries to include it as
options to `fork()` rather than as part of argv for `fork()`. This
doesn't throw an error because of a quirk in `fork()` that silently
accepts arrays for options objects. This will be changing in Node.js
18.x.
@nodejs-github-bot nodejs-github-bot added the benchmark Issues and PRs related to the benchmark subsystem. label Dec 27, 2021
@Trott
Copy link
Member Author

Trott commented Dec 27, 2021

This is the part of #41305 that is not semver-major. There is no test because the current issue would cause test failures when that PR lands.

@Trott Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 29, 2021
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 29, 2021
@nodejs-github-bot nodejs-github-bot merged commit db02f6f into nodejs:master Dec 29, 2021
@nodejs-github-bot
Copy link
Collaborator

Landed in db02f6f

@Trott Trott deleted the fix-benchmark-run branch December 30, 2021 00:41
targos pushed a commit that referenced this pull request Jan 14, 2022
run.js does not work with --set as it tries to include it as
options to `fork()` rather than as part of argv for `fork()`. This
doesn't throw an error because of a quirk in `fork()` that silently
accepts arrays for options objects. This will be changing in Node.js
18.x.

PR-URL: #41334
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 31, 2022
run.js does not work with --set as it tries to include it as
options to `fork()` rather than as part of argv for `fork()`. This
doesn't throw an error because of a quirk in `fork()` that silently
accepts arrays for options objects. This will be changing in Node.js
18.x.

PR-URL: #41334
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
run.js does not work with --set as it tries to include it as
options to `fork()` rather than as part of argv for `fork()`. This
doesn't throw an error because of a quirk in `fork()` that silently
accepts arrays for options objects. This will be changing in Node.js
18.x.

PR-URL: nodejs#41334
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
run.js does not work with --set as it tries to include it as
options to `fork()` rather than as part of argv for `fork()`. This
doesn't throw an error because of a quirk in `fork()` that silently
accepts arrays for options objects. This will be changing in Node.js
18.x.

PR-URL: #41334
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants