test-net-GH-5504.js is failing because environment variables are missing

[john-yan]

In test https://github.com/nodejs/node/blob/master/test/sequential/test-net-GH-5504.js

   var opt = {
     env: {
       NODE_DEBUG: 'net',
       NODE_COMMON_PORT: process.env.NODE_COMMON_PORT,
     }
   };
   ...
   var s = spawn(node, [__filename, 'server'], opt);

This code loses the original environment variables passing into the program, which causes failure for the spawn function. Specifically, the program could not start because the loader couldn't find the correct version of standard c++ library which is not in the default location. The loader is supposed to find the needed libraries thought LD_LIBRARY_PATH environment variable which is missing.

I suggests to change the above code to this instead:

   var opt = {
     env: process.env
   };
   opt.env.NODE_DEBUG = "net";
   opt.env.NODE_COMMON_PORT = process.env.NODE_COMMON_PORT;

[john-yan]

Same problem in these tests:
https://github.com/nodejs/node/blob/master/test/parallel/test-fs-readfile-error.js
https://github.com/nodejs/node/blob/master/test/sequential/test-stdin-script-child.js
https://github.com/nodejs/node/blob/master/test/sequential/test-util-debug.js

[rvagg]

good catch @john-yan, care to open a pull request against master to fix these? You don't need the NODE_COMMON_PORT line if we go with process.env copying though.

[john-yan]

@rvagg Thanks. The pull request is opened for the master branch, and I also back port it for v4.x.

[cjihrig]

We experienced this on joyent/node. I thought the appropriate commit was cherry picked over. You should just have to extend() env before creating a child process.

Does anyone think it would be useful to include an option to automatically extend the environment?