-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Setting NODE_ENV to test has unintended side effects #3370
Comments
Hi folks, any thoughts on this? |
+1, user shouldn't be forced to pick ENV for tests.
|
This is a breaking change but I'm not opposed to it. |
Maybe there could be flag introduced, like:
but it doesn't look right to the person that don't know about this issue |
Also if we decide to make this change, we should introduce some new env variable that will indicate file is running in Jest. Something like |
So is there a way currently to stop Jest from setting the NODE_ENV environment variable? |
+1 For example, I found this issue, because we want to use different default env ( Btw, for reference, mocha and ava don't do that either: mochajs/mocha#185, avajs/ava#1470. |
From react index.js:
This means that the jest env variable will prevent react from loading the minified version. This might actually add a performance penalty during testing. |
You're free to set it to whatever you want. Jest only sets it to |
This is true if it is set as an OS variable. If you are setting it in .env file it is still being overwritten by jest. |
Does setting it to be a different value affect Jest in some way? If not, why does Jest do this at all? |
Answering my own question, from a quick scan of the code, this looks like the only reference to it, outside of a mention in the configuration docs: |
It makes e.g. |
Having |
Workaround:
Instead of using jest CLI, use Node and inject your own script which defaults You could even do process.env.NODE_ENV = process.env.NODE_ENV ?? 'dev';
// set by jest
if (process.env.NODE_ENV === 'test') {
require('@rbi-ctg/mocks/logger');
} |
Somewhat related to this. I spent quite a while trying to track down errors like this on a couple of projects:
It turned out that this was caused by having edit to add: this was with Jest 25.4.0 and 26.1.0 |
Had the same issue today. NODE_ENV=development is my default setting for development, to (de-) activate some optimizations. Im my opinion it feels weird if Jest just stops working in that case :/ |
Usually developers don't have |
For those using dotenv and wanting to have .env override variables which were set before (like jest's NODE_ENV=test):
|
Stumbled on this today trying to figure out how to make tests for the first time and honestly, though the feature is not without its merits, I feel like it causes more issues than intended. If not removing the feature or adding an option to opt out of NODE_ENV being set to "test" by default, at least a more descriptive entry on the "Environment Variables" documentation stating that the "if its not set already" part doesn't apply to dotenv would be nice. |
Text me 3379367844
…On Fri, May 13, 2022, 10:54 PM Andres Aldape ***@***.***> wrote:
Stumbled on this today trying to figure out how to make tests for the
first time and honestly, though the feature is not without its merits, I
feel like it causes more issues than intended.
If not removing the feature or adding an option to opt out of NODE_ENV
being set to "test" by default, at least a more descriptive entry on the
"Environment Variables" documentation stating that the "if its not set
already" part doesn't apply to dotenv would be nice.
—
Reply to this email directly, view it on GitHub
<#3370 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AXSL3Z5MJIXMLMX65G3LPLLVJ4PVZANCNFSM4DJBNXCA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days. |
This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one. |
In 3a38ddf, code was added to default the
NODE_ENV
variable totest
if one is not set (see the CHANGELOG entry). The rationale given was twofold: to be able to specify the Jest preset in Babel config based on theenv
, and to follow a convention used in Relay.The former is no longer specified in this way, and the latter is now specified explicitly by Relay so there's no dependency on Jest setting the default.
This came up in #1654 where Babel config for
development
was not read because Jest would change the environment away from the default — the workaround was to more clearly document this behavior in 968ca38 and b1e7af2.I believe that library code like Jest changing
NODE_ENV
(or indeed, any shared environment variables) is an anti-pattern and leads to unexpected behavior, like the Babel issue raised above. Essentially, it's modifying a global variable — see this great explanation on why doing so is considered harmful. If any other libraries are being used that check the value ofNODE_ENV
, their behavior could change simply by adding Jest — a strong violation of module-based encapsulation.At this point, there seems to be no requirement that it be defaulted in this manner, given that other
NODE_ENV
values can be explicitly set and Jest is expected to continue working properly.If an environment-type value needs to be explicitly set, and to be set to
test
if nothing else is provided, that should be set in a value local to Jest and not global to the environment — see, for example, this simpleenv
variable in express-js.I'm happy to create a PR to remove the modifying-
NODE_ENV
behavior, but I wanted to check that there isn't something intentional or required about this behavior first or if there are strong opinions from the maintainers that they want to keep it this way.The text was updated successfully, but these errors were encountered: