Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

[Converge] tests: append instead of override environment #25

Closed
jasnell opened this issue May 22, 2015 · 4 comments
Closed

[Converge] tests: append instead of override environment #25

jasnell opened this issue May 22, 2015 · 4 comments

Comments

@jasnell
Copy link
Member

jasnell commented May 22, 2015

See: nodejs/node-v0.x-archive@e64ee2b
/cc @misterdjules @cjihrig @trevnorris

Original commit message: "Some tests that rely on some environment variables being passed to child processes would fail because they reset the child processes' environement instead of appending to it. This would break on test environments where some custom environment variables are needed to make node work properly."

@Fishrock123
Copy link
Member

I think that patch LGTM, although we haven't had any issues without it..

This would break on test environments where some custom environment variables are needed to make node work properly.

Does anyone know what platforms these are? Maybe cc @rvagg.

@cjihrig
Copy link
Contributor

cjihrig commented May 22, 2015

+1 to porting this, even if it hasn't caused any problems in io.js.

@rvagg
Copy link
Member

rvagg commented May 23, 2015

my only guess is NODE_COMMON_PORT and NODE_COMMON_PIPE but the latter is io.js-only afaik and we haven't had breakage

I'm +1 on merging this however I'd prefer to see this generalised into a ulity in common.js

jasnell referenced this issue in jasnell/node-1 May 27, 2015
From: nodejs/node-v0.x-archive@e64ee2b

Original commit message:
```
Some tests that rely on some environment variables being passed to child
processes would fail because they reset the child processes'
environement instead of appending to it. This would break on test
environments where some custom environment variables are needed to make
node work properly.
```

This port is modified to move the function into common.js per
nodejs/node#25 (comment)
jasnell referenced this issue in jasnell/node-1 May 27, 2015
From: nodejs/node-v0.x-archive@e64ee2b

Original commit message:
```
Some tests that rely on some environment variables being passed to child
processes would fail because they reset the child processes'
environement instead of appending to it. This would break on test
environments where some custom environment variables are needed to make
node work properly.
```

This port is modified to move the function into common.js per
nodejs/node#25 (comment)
jasnell referenced this issue in jasnell/node-1 Jun 4, 2015
From: nodejs/node-v0.x-archive@e64ee2b

Original commit message:
```
Some tests that rely on some environment variables being passed to child
processes would fail because they reset the child processes'
environement instead of appending to it. This would break on test
environments where some custom environment variables are needed to make
node work properly.
```

This port is modified to move the function into common.js per
nodejs/node#25 (comment)
jasnell referenced this issue in jasnell/node-1 Jun 4, 2015
From: nodejs/node-v0.x-archive@e64ee2b

Original commit message:
```
Some tests that rely on some environment variables being passed to child
processes would fail because they reset the child processes'
environement instead of appending to it. This would break on test
environments where some custom environment variables are needed to make
node work properly.
```

This port is modified to move the function into common.js per
nodejs/node#25 (comment)
jasnell referenced this issue Jun 18, 2015
Some tests that rely on some environment variables being passed to child
processes would fail because they reset the child processes'
environment instead of appending to it. This would break on test
environments where some custom environment variables are needed to make
node work properly.

PORT-FROM: joyent/node @ e64ee2b3f7b4067101b0291f1add842353cd6865
This port is modified to move the function into common.js per
nodejs/node#25 (comment)

PR-URL: nodejs/node#43
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
jasnell referenced this issue Jun 18, 2015
Some tests that rely on some environment variables being passed to child
processes would fail because they reset the child processes'
environment instead of appending to it. This would break on test
environments where some custom environment variables are needed to make
node work properly.

PORT-FROM: joyent/node @ e64ee2b3f7b4067101b0291f1add842353cd6865
This port is modified to move the function into common.js per
nodejs/node#25 (comment)

PR-URL: nodejs/node#43
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@Fishrock123
Copy link
Member

Landed as per nodejs/node#43

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants