percy / percy-js

[Deprecated] JavaScript API client library for Percy.
MIT License
31 stars 19 forks source link

.env.test file ignored when running tests with @percy/agent #199

Closed grzuy closed 4 years ago

grzuy commented 4 years ago

Hi,

Given a Rails app with dotenv-rails and a .env.test file. When running rails app tests preceded with yarn percy exec. .env.test is ignored.

I built a test app reproducing this, with test green on master, and showing the issue on a sample PR that adds percy-capybara and @percy/agent (as recommended in the doc): https://github.com/grzuy/visual_tested_app/pull/1.

Thank you!

wwilsman commented 4 years ago

Hi @grzuy! Thanks for opening an issue and thanks for putting together the example repo!

This was a tricky one to track down. When @percy/agent loads the .env file, it does so from a subdependency, percy-client. Both the @percy/agent and percy-client packages are Node packages and as such use Node's dotenv package, which does not support environment based .env files.

So what seems to be happening here is Node's dotenv library is loading your .env file and that file only. Then @percy/agent passes along Node's process.env to your test process. Since your process now already has the environment variable set, dotenv-rails will not override that variable.

I'm going to move this issue over to the percy-client repo and open up a PR shortly there to add support for environment based .env files. 👍

grzuy commented 4 years ago

Makes sense!

Thank you again!

grzuy commented 4 years ago

I'm going to move this issue over to the percy-client repo and open up a PR shortly there to add support for environment based .env files

@wwilsman In case trying to adapt to many other languages dotenv behaviour is too much/too complex/too brittle, having the possibility of optionally disabling dotenv-js for percy-js would be enough I guess.

I mean, something like...

$ DISABLE_DOTENV_JS=1 yarn percy exec bin/rails test

?

wwilsman commented 4 years ago

Just created the PR! Once it's merged, the NODE_ENV environment variable must be set for environment specific dotenv files. So to load your production dotenv files, you'll want to set NODE_ENV=production. I also made sure to mimic dotenv-rails exclusion of .env.local when NODE_ENV=test.

wwilsman commented 4 years ago

Just released 3.8.0 which should have the new behavior! You can also now disable Percy's dotenv loading by setting PERCY_DISABLE_DOTENV=1 (or to any value really).

Since this package is a transitive dependency, if you have trouble updating you'll have to uninstall and reinstall your SDK. You can also make sure you have the newest version by running npm ls percy-client.

Let me know if it works! 😃

grzuy commented 4 years ago

Both NODE_ENV=test and PERCY_DISABLE_DOTENV=1 fixes our use case.

Thank you for the quick turnaround.