FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: fix test runner arg regression by stefanmb · Pull Request #5446 · nodejs/node · GitHub

/ node Public

test: fix test runner arg regression - #5446

Closed
stefanmb wants to merge 1 commit into
nodejs:masterfrom
stefanmb:fix-test-runner-arg-regression
Closed

test: fix test runner arg regression#5446
stefanmb wants to merge 1 commit into
nodejs:masterfrom
stefanmb:fix-test-runner-arg-regression

Conversation

Copy link
Copy Markdown
Contributor

Affected core subsystem(s)

test

Description of change

In #5376 I introduced the "--node-args" facility to test.py by overwriting the test configuration additional argument list. Instead, I should have simply appended to the list to preserve the existing arguments. This PR fixes the regression introduced and reported in #5442.

After this PR the test runner will append --node-args to the existing argument list (if it is present), instead of simply overwriting it.

Append --node-args to existing list, don't overwrite arg list.

Copy link
Copy Markdown
Contributor Author

@mhdawson Please review this PR, it resolves the issue you reported in #5442.

mscdex added the test Issues and PRs related to the tests. label Feb 26, 2016

Copy link
Copy Markdown
Member

Fishrock123 added the build Issues and PRs related to build files or the CI. label Feb 26, 2016

Copy link
Copy Markdown
Member

LGTM.

CI run above is green so will land

mhdawson self-assigned this Feb 26, 2016
mhdawson pushed a commit that referenced this pull request Feb 26, 2016
Append --node-args to existing list, don't overwrite arg list.

Fixes: #5442
PR-URL: #5446
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-by: Michael Dawson <michael_dawson@ca.ibm.com>

Copy link
Copy Markdown
Member

Landed as c98d159

mhdawson closed this Feb 26, 2016
rvagg pushed a commit that referenced this pull request Feb 27, 2016
Append --node-args to existing list, don't overwrite arg list.

Fixes: #5442
PR-URL: #5446
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-by: Michael Dawson <michael_dawson@ca.ibm.com>
rvagg pushed a commit that referenced this pull request Feb 27, 2016
Append --node-args to existing list, don't overwrite arg list.

Fixes: #5442
PR-URL: #5446
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-by: Michael Dawson <michael_dawson@ca.ibm.com>
Fishrock123 mentioned this pull request Mar 1, 2016
5 tasks

jasnell commented Mar 2, 2016

Copy link
Copy Markdown
Member

marking this and it's dependent PRs as don't land on v4.x... we can revisit that later if necessary.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL