◐ Shell
clean mode source ↗

test: fix test-repl-envvars by addaleax · Pull Request #25226 · nodejs/node

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

@nodejs-github-bot added the test

Issues and PRs related to the tests.

label

Dec 26, 2018

This was referenced

Dec 26, 2018

tniessen

benjamingr

lundibundi

@Trott Trott added the fast-track

PRs that do not need to wait for 72 hours to land.

label

Dec 27, 2018

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 addaleax deleted the test-repl-envvars-no-extend branch

December 30, 2018 18:31

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>

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>

This was referenced

Jan 29, 2019

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>