[v10.x] test: mark test-repl-envvars as flaky on AIX #25219
Closed
Conversation
This file contains 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
also... return TextDecoder directly from factories PR-URL: nodejs#23625 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
The fixed-size buffer on the stack is unnecessary and way too large for most applications. This change removes it and allocates the required memory directly instead of copying into heap later. PR-URL: nodejs#23427 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Refael Ackermann <refack@gmail.com>
PR-URL: nodejs#23734 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Introduced use of smart pointers instead of MallocedBuffer to manage memory allocated in the cares library. PR-URL: nodejs#23628 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
Picking a path according to a boolean is essentially free, compared to the cost of a function call. Also, remove an unnecessary `TryCatch`. PR-URL: nodejs#23782 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matheus Marchini <mat@mmarchini.me> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
The convertProtocols() function now throws a range error when the byte length of a protocol is too long to fit in a Buffer. Also added a test case in test/parallel/test-tls-basic-validations.js to cover this. PR-URL: nodejs#23606 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Using `process.nextTick()` or `Promise`, it is possible to escape the `timeout` set when running code with `vm.runInContext()`, `vm.runInThisContext()`, and `vm.runInNewContext()`. This documents the issue and adds two known_issues tests. Refs: nodejs#3020 PR-URL: nodejs#23743 Refs: nodejs#3020 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
These are known issues that can be flaky on certain platforms because they rely entirely on timing differences. PR-URL: nodejs#23743 Refs: nodejs#3020 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Those categories are necessary to build addons that depends on libcurl and libssh, the following were the missing symbols: libcurl: OCSP_cert_status_str OCSP_check_validity OCSP_basic_verify OCSP_RESPONSE_free OCSP_single_get0_status OCSP_response_get1_basic OCSP_BASICRESP_free OCSP_crl_reason_str OCSP_resp_count OCSP_response_status OCSP_response_status_str OCSP_resp_get0 d2i_OCSP_RESPONSE SSL_CTX_set_next_proto_select_cb libssh: EVP_ripemd160 EVP_cast5_cbc Fixes: nodejs#23293 PR-URL: nodejs#23344 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Instead of getting a reference to the main `AliasedBuffer`, which would always unnecesarily allocate and destroy a `Persistent` handle, store and use a reference to the owning object. PR-URL: nodejs#23844 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Matheus Marchini <mat@mmarchini.me> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Instead of calling into C++ each time we need to check the value of a command line option, cache the option map in a new `internal/options` module for faster access to the values in JS land. PR-URL: nodejs#24091 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Refael Ackermann <refack@gmail.com>
Right now `node -p process.versions.openssl` always gets run in the Makefile even when it's not needed by the target (e.g. `make clean`, `make test-only`). This patch makes it a run time call instead of part of the global expansion. PR-URL: nodejs#24115 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
PR-URL: nodejs#24130 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
PR-URL: nodejs#24131 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Lazy load _http_common and HTTPParser so that the 'http_parser' binding can be monkey patched before any internal modules require it. This also probably improves startup performance minimally for programs that never require the HTTP stack. Fixes: nodejs#23716 Fixes: creationix/http-parser-js#57 PR-URL: nodejs#24006 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#24133 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Swapped the actual and expected arguments in `assert.strictEqual()` calls. Arguments are now in correct order. Literal value is now the second argument and the value returned by the function is the first argument. PR-URL: nodejs#24134 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: George Adams <george.adams@uk.ibm.com>
PR-URL: nodejs#24137 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
`assert.strictEqual` expects arguments in the following order: actual, expected[, message] PR-URL: nodejs#24138 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
PR-URL: nodejs#24142 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Switched arguments in assert.strictEqual() PR-URL: nodejs#24144 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs#24146 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
These tests ensure that OutgoingMessage setTimeout method will call setTimeout on its socket Co-authored-by: ZauberNerd <zaubernerd@zaubernerd.de> PR-URL: nodejs#24148 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#24152 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
File: test/parallel/test-http-client-upload-buf.js PR-URL: nodejs#24140 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
the actual and expected arguments in assert.strictEqual() calls are in the wrong order. Any literal value should be the second argument while the first argument should be the value returned by a function/be the calculated value. PR-URL: nodejs#24151 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
In the `test/parallel/test-vm-create-and-run-in-context.js` test the actual and expected arguments in the `assert.strictEqual()` call on line 32 are in the wrong order and they have to be switched around. PR-URL: nodejs#24141 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#24147 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#24155 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs#24213 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit adds http_parser_set_max_header_size() to the http-parser for overriding the compile time maximum HTTP header size. Backport-PR-URL: nodejs#25168 PR-URL: nodejs#24811 Fixes: nodejs#24692 Refs: nodejs/http-parser#453 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
This commit adds support for uint64_t option parsing. Backport-PR-URL: nodejs#25168 PR-URL: nodejs#24811 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Allow the maximum size of HTTP headers to be overridden from the command line. Backport-PR-URL: nodejs#25168 co-authored-by: Matteo Collina <hello@matteocollina.com> PR-URL: nodejs#24811 Fixes: nodejs#24692 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Remove magic numbers (500, 10, 100) from the test. Instead, detect when GC has started and stop sending requests at that point. On my laptop, this results in 16 or 20 requests per run instead of 500. Fixes: nodejs#23089 PR-URL: nodejs#24943 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Document that the limit was changed from 80KB to 8KB in 1860352. Fixes: nodejs#24693 PR-URL: nodejs#24700 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
This commit exposes the value of --max-http-header-size as a property of the http module. PR-URL: nodejs#24860 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
FreeBSD again: https://ci.nodejs.org/job/node-test-commit-freebsd/23054/ |
gireeshpunathil
approved these changes
Dec 26, 2018
addaleax
changed the title
addaleax
added a commit
to addaleax/node
that referenced
this pull request
Dec 26, 2018
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: nodejs#21451 Fixes: nodejs/build#1377 Refs: nodejs#25219
|
@MylesBorins Are these failures permanent (on a single machine) or is it more of a works-sometimes things? I think the changes 180f865 made to this particular file broke it, and reverting them should be a clean way to resolve this. I’ve opened #25226 to do that. |
MylesBorins
force-pushed
the
v10.x-staging
branch
from
17f4208
to
d04c3c2
Compare
Trott
pushed a commit
to Trott/io.js
that referenced
this pull request
Dec 27, 2018
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: nodejs#21451 Fixes: nodejs/build#1377 Refs: nodejs#25219 PR-URL: nodejs#25226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax
added a commit
that referenced
this pull request
Dec 30, 2018
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: #21451 Fixes: nodejs/build#1377 Refs: #25219 PR-URL: #25226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax
added a commit
that referenced
this pull request
Dec 30, 2018
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: #21451 Fixes: nodejs/build#1377 Refs: #25219 PR-URL: #25226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax
added a commit
that referenced
this pull request
Dec 30, 2018
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: #21451 Fixes: nodejs/build#1377 Refs: #25219 PR-URL: #25226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax
added a commit
that referenced
this pull request
Dec 30, 2018
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: #21451 Fixes: nodejs/build#1377 Refs: #25219 PR-URL: #25226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
gabrielschulhof
pushed a commit
to gabrielschulhof/node
that referenced
this pull request
Jan 2, 2019
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: nodejs#21451 Fixes: nodejs/build#1377 Refs: nodejs#25219 PR-URL: nodejs#25226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
|
Closing as it looks like test has been fixed on all branches |
refack
pushed a commit
to refack/node
that referenced
this pull request
Jan 14, 2019
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: nodejs#21451 Fixes: nodejs/build#1377 Refs: nodejs#25219 PR-URL: nodejs#25226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
rvagg
pushed a commit
that referenced
this pull request
Feb 28, 2019
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: #21451 Fixes: nodejs/build#1377 Refs: #25219 PR-URL: #25226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
rvagg
pushed a commit
that referenced
this pull request
Feb 28, 2019
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: #21451 Fixes: nodejs/build#1377 Refs: #25219 PR-URL: #25226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BethGriggs
pushed a commit
that referenced
this pull request
Mar 7, 2019
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: #21451 Fixes: nodejs/build#1377 Refs: #25219 PR-URL: #25226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refs: #21451
Refs: nodejs/build#1377
/cc @nodejs/platform-aix