Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Running tests with jest seems to trigger a change in environment variables #5322

Closed
janpieterz opened this issue Jan 15, 2018 · 8 comments · Fixed by #5465
Closed

Running tests with jest seems to trigger a change in environment variables #5322

janpieterz opened this issue Jan 15, 2018 · 8 comments · Fixed by #5465

Comments

@janpieterz
Copy link

Cross post for GoogleChrome/chrome-launcher#86

I've created a reproduction here: https://github.com/janpieterz/find-chrome-issue

npm start works, npm test fails.

I launch a Chromeless instance and do something there, this uses chrome-launcher internally to launch a chrome instance. This relies on certain environment variables to detect the Program files folders. These should be case insensitive on Windows + Node, but somehow running it with Jest seems to make them case sensitive.

This is present on:
OS Name: Microsoft Windows 10 Enterprise
OS Version: 10.0.16299 N/A Build 16299

@SimenB
Copy link
Member

SimenB commented Jan 15, 2018

It works correctly on mac, can you describe how it fails for you?

As far as I know we only set a single environment variable, are any missing?

https://github.com/facebook/jest/blob/7743c0970e974bca166bff67099d28462ddb3187/packages/jest-cli/bin/jest.js#L12-L14

@janpieterz
Copy link
Author

I've seen that one come through indeed. It certainly won't show up on a Mac as it's a Windows specific environment variable (and execution path).

The method called in chrome-finder.ts is here https://github.com/GoogleChrome/chrome-launcher/blob/master/src/chrome-finder.ts#L164. The error happens on the line https://github.com/GoogleChrome/chrome-launcher/blob/master/src/chrome-finder.ts#L171 where, for some reason the environment variables suddenly appear to be case sensitive when running through Jest. For example, process.env.PROGRAMFILES is not working, while process.env.ProgramFiles is.

On Windows, these are case-insensitive, and node respects this. See, for example, https://github.com/nodejs/node/blob/master/test/parallel/test-process-env.js#L93

I have no clue where to dig to find out why this changes while running in Jest. I've just verified (and pushed a fresh commit) that showcases this explicitly.

The code run is:

process.env.FOO = "bar";
console.log(process.env.FOO);
console.log(process.env.foo);

On my environment npm start outputs:

bar
bar

While npm test outputs:

  console.log random.test.js:5
    bar

  console.log random.test.js:6
    undefined

@janpieterz
Copy link
Author

Small addition, taken a look through the Jest code and noticed two Object.assign({}, process.env, {X}).
This is done only here https://github.com/facebook/jest/blob/master/integration-tests/runJest.js#L48 and here https://github.com/facebook/jest/blob/master/packages/jest-changed-files/src/hg.js#L16

I confirmed that using an assigned env like this showcases the same behavior as above running npm test.
From the sound of the files/packages it doesn't sound like it's in the main test execution path, but it could be hinting at another dependency somewhere lower, something that perhaps launches a process with a specific env set? Or a dependency called that specifically sets process.env to something assigned.

@SimenB
Copy link
Member

SimenB commented Jan 15, 2018

My guess is that we mess up when creating a process object to copy into the test process, see https://github.com/facebook/jest/blob/7743c0970e974bca166bff67099d28462ddb3187/packages/jest-util/src/create_process_object.js. See #4904.

Do you think you could look into a fix?

/cc @mjesun

@SimenB
Copy link
Member

SimenB commented Jan 15, 2018

Can confirm above theory, at least:
image

@mjesun
Copy link
Contributor

mjesun commented Jan 16, 2018

Hah, interesting! I was not aware of this behavior. It looks like env is a special object, since its constructor is neither Object nor null; and that's how it deals with this particular use cases.

The reason why this worked before is because the process.env object exposed for each test was the same one as the main process, which in turn, created leaks and unexpected behaviors (but since it's a native one, it worked as expected).

I will investigate what can we do here; but I guess a Proxy object is needed to achieve the exact same behavior.

I'm trying to get some clarification about how Node bootstraps the main context, and see if we can bootstrap any random context created via vm.createContext. This problem is, like many others (instanceof, native modules leaked...) derived from the fact that we cannot create Node contexts from scratch.

@janpieterz
Copy link
Author

Thanks a lot @mjesun !

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants