Reading .env in PHPUnit versus env variables in phpunit.xml.dist

[weaverryan]

Hi guys!

I noticed that the EnvConfiguration adds environment variables to .env.dist and also phpunit.xml.dist. This is because phpunit does not read .env. Instead, you should define all of your environment variables in phpunit.xml.dist (or set real env variables).

I think this is confusing (I just got bit by it). It means that my environment variables now need to be duplicated in the .env files and also phpunit.xml.dist. It also makes it difficult to run commands in the test environment, as using --env=test isn't enough: the incorrect environment variables will be used (those from .env instead of phpunit.xml.dist). I also didn't know to look in phpunit.xml.dist: I was looking in .env and the app wasn't seeing my changes.

Obviously, this was done intentionally. But, are we sure about this behavior? I would prefer consistency with console and index.php: that .env is read (assuming there is no APP_ENV) and then any environment variables in phpunit.xml.dist override those (admittedly, I'm not exactly sure how to accomplish this).

Cheers!

[javiereguiluz]

I agree. Let's avoid repetition and let's make phpunit reuse .env contents. If you want to override the value of one or some env vars, then you can use the current idea of defining the new values in the phpunit.xml.dist file.

[sstok]

FYI. PHPUnit doesn't (by default) overwrite env variables that are already set. You can use the force="true" option to force this (requires PHPUnit v6.3 - sebastianbergmann/phpunit#2723)

[nicolas-grekas]

"dev" and "test" need very different settings to properly work. I don't think they should be shared between the envs... That could even be dangerous (eg. running tests and destroying "dev" db)
phpunit.xml.dist is the native way to set these, not sure we should create yet-another non-standard way...

[Pierstoval]

I agree. For example, I'm used to change the database config in test to Sqlite instead of mysql, for faster tests, then I must set different env vars in the phpunit.xml.dist file, which is awesome, because the old parameters.yml system needed me to write everything directly in the config_test.yml, to make sure the test env wouldn't use parameters.yml.

With env vars directly in phpunit.xml.dist, the test env will never use the .env file, so it's perfectly isolated 😄

[weaverryan]

Hmm, ok. So if we keep the current situation, we have bad DX. I'm cool with solving that however we can :). Specifically:

A) There is not way to run bin/console commands easily in the test environment (e.g. to initialize the database). That's because the environment variables you set in phpunit.xml.dist are not available for bin/console.

B) There is duplication between .env and phpunit.xml.dist. Usually, 90% of my "dev" values for machine-specific config will be the same for dev and test (e.g. an API key to the sandbox of some service we use). Currently, I need to duplicate this config in 2 places.

C) The behavior is surprising: the kernel/container in the tests works different than all other kernels, including running bin/console --env=test.

What would I suggestion? What if we:

1. Read .env in the tests. This could be done in KernelTestCase or possibly add code to hook into PHPUnit (which could be activated in phpunit.xml.dist). To be consistent with everything else, this would only be read if APP_ENV is not set.

2. If users want to override any environment variables in the test environment, they can just do what they always do: set true environment variables or put the values in phpunit.xml.dist themselves. This will prevent .env from being read if you want that control.

This would allow me to do what I've always done: use the same environment variables between dev and test environments, and leverage a committed test-specific config file (e.g. config_test.yml in 3.x) to override the database connection to use a hardcoded SQLite path. No need to have a different set of parameters.

[Pierstoval]

Let's find solutions:

A) Yeah, we do not often run bin/console in tests, because test env is different. To solve this, I simply call this, either in my tests, or in my bootstrap.php file. Create an Application object, inject a Kernel instanciated in test mode, run commands, debug... This has the big advantage to be blocking when dealing with tests. Even though it needs to write code, I think it's much better, because we can debug this in a much easier way if the test env is broken at compile-time (because it's harder to debug external processes when doing something like exec('php bin/console ...4);

B) Yes, there is duplication. And there may also be some duplicates when dealing with your CI (because you set env vars in your CI, not in versioned files), as well as production env. That doesn't seem like a problem to me. You don't need to duplicate it: flex recipes will add the env vars automatically in your test env, and if it's similar to your dev one, then you will probably never update it, unless you change this API key.

C) Why would the kernel and container working differently than all other kernels be an issue? It's obvious: dev, test and prod are different envs, therefore kernels created with these envs shouldn't work the same way...

For your 1. proposal, I'm not sure it's the best option. It would need a wrapper for the .env-reading part, then we would need to create a bootstrap.php file for PHPUnit that would read the env file, and even then I'm not sure about the precedence (when running PHPUnit, which one is interpreted first: env vars in phpunit.xml or in bootstrap? My intuition would say "the first one", so the xml file, but I don't know 😕 ).
This becomes a problem because we can't use DotEnv (it doesn't overwrite existing env vars), so we need something else... Or we can use the .env file only for vars that are not set in the phpunit.xml files.

For your proposal 2, that's a bit better, but still needs a bootstrap file. Even though I think it's a nice idea to ship such file because we almost always need to setup a project before testing (database, fixtures, etc.) (unless we only do unit tests), I'm not sure implementing a new env system that would read in three places is the best for DX. Then, to debug an incorrect env vars you would have to lookup three types: xml file, env file, global vars. Too much IMO. Only using the xml & global env are enough to me, because xml is for dev, global is for CI. Much simpler, WDYT?

[quentinus95]

On my side, the way env variables are managed by Symfony is the main issue I have since I started working with Symfony 4 + flex.

First, I don't completely get why DotEnv variables are only loaded if APP_ENV is not defined. In a normal workflow, I assume env vars will only be loaded from the .env file if we are in dev/test mode because in production, .env files should not be used at all. So we could assume these variables can be loaded if DotEnv is available (i.e. installed) and APP_ENV is not set to prod (to avoid misusages).

Second, I was used to run several commands in test env before running tests (to initiate database schema, for instance) in several projects. The current implementation does not allow this ; I need to run the commands in dev mode or change the conditions in the bin/console (this is the solution I chose). Adding application specific environment variables in the phpunit dist file is not convinent as we should duplicate the vars from the .env file (as stated before). I guess it would be more logical to have only phpunit/testing related env vars (such as SYMFONY_DEPRECATIONS_HELPER) and load application specific env vars from DotEnv if available. This could be done in the bin/phpunit script generated from simple-phpunit (edit: I'm not so sure about this...). We would also keep the current default env vars in the phpunit file (even if I'm not completely convinced by the APP_SECRET variable).

[emmanuelballery]

I'm currently using a bash script to solve this "ENV vars in test" problem:

set -o allexport # export all defined variables

source .env # load dev parameters
source .env.test # override with test parameters (only those needed)

php bin/console cache:clear -e "test" --no-debug
php bin/console doctrine:database:drop -e "test" --force --no-debug
php bin/console doctrine:database:create -e "test" --no-debug
php bin/console doctrine:migration:migrate -e "test" -n --no-debug
php bin/console doctrine:fixtures:load -e "test" -n --no-debug
# ... and more

php vendor/bin/simple-phpunit | tee tests.txt

This way :

• APP_ENV is defined and Dotenv is not used in "bin/console"
• you can keep your "dev" configuration and just override it with what needs to change in "test"
• you don't need to set any env key in phpunit.xml or phpunit.xml.dist
• you don't expose those env vars outside this script

[mhujer]

I just got bitten by this. Consider the following use-case:

In tests, I'm importing the fixtures by running doctrine:fixtures:load with the kernel available in KernelTestCase, so it works as expected (it uses the database defined in phpunit.xml.dist).

Now I was a debugging an issue with fixtures loading (caused by incorrect Doctrine mapping), so I tried to run the command directly as I did before Flex (note the --env=test):

bin/console doctrine:fixtures:load --purge-with-truncate --env=test

And it truncated tables and imported the fixtures into the other database (defined in .env).

I'm perfectly aware why this happens (I've read this issue few days ago), but it is still quite unexpected and surprising. It feels like a BC break (radical change in behaviour).

[quentinus95]

@emmanuelballery

I have adapted my testing script to source the .env file instead of changing the console source code. However using source is sometimes a problem, such as when running tests in a CI pipeline, where you may need to redefine some env variables (for instance, database connection URI) before running your tests.

EDIT

And another strange behavior: it looks like behat is running in test env but still loads the .env file, which is the opposite behavior of the console file... However, if the APP_ENV var is set to test before running behat, the .env file is not loaded anymore (which is the same behavior as the console file).

The behavior I'm expecting is:

• Never load a .env file in "prod" env.
• Always load the .env file in any other env, without overriding existing env vars.
• Override the env vars with script-specific vars (for instance, vars defined in the phpunit.xml file). I'm not sure about this one, but I guess it would be convenient so we can have some general env vars defined for the test environment and tweak those before running a specific script, depending on which kind of tests we want to run (phpunit/behat/...).

[weaverryan]

Fix at symfony/recipes#366 - please review it and make sure it satisfies all of your requirements! :)

[mhujer]

@weaverryan Great improvement! 👏

I was thinking about how to solve the use-case I mentioned three comments above. I came up with following options so far:

1. create .env.test (maybe better env.test.dist to allow local overriding) and load in the tests bootstrap and also when the bin/console is ran in --env=test

2. load env vars from phpunit.xml.dist (or phpunit.xml) in when the bin/console is ran in --env=test (very hacky one)

[weaverryan]

Yea, we’re discussing the environment-specific .env file idea on the linked PR too. You can share your support there if you agree (there’s no code for this in the PR yet, just a comment of mine)