Adding test bootstrap.php file so .env vars are read #366

weaverryan · 2018-02-11T17:41:50Z

Q	A
License	MIT

Fixes symfony/flex#251

The problem is that currently, environment variables must be completely duplicated between .env and phpunit.xml.dist. That's just unacceptable DX to put in the user... which includes me - I don't want to do that :).

This is an easy win:

.env is read in the same way as bin/console and public/index.php
Environment variables in phpunit.xml.dist WIN over .env. This is used to set the environment to test.

This works particularly well, because any env vars set in phpunit.xml.dist WIN over .env. This means that .env can contain default values, but can then be easily overridden for the test environment.

ghost

Pull request passes validation.

quentinus95 · 2018-02-11T18:01:38Z

phpunit/phpunit/4.7/tests/bootstrap.php

+ * Environment variables can also be specified in phpunit.xml.dist.
+ * Those variables will override any defined in .env.
+ */
+if (!isset($_SERVER['APP_ENV'])) {


Just by curiosity, why do we use this condition, also in the console? This disallow from having a generic bash script with a conditional behavior depending of the APP_ENV, and then launch specific commands based on this. It does not make sense in my opinion that APP_ENV=dev ./bin/console s:r launches the server without reading the .env file while ./bin/console s:r does. I will have the same issue here as I may set the APP_ENV var to test before launching the tests (I actually do) ; it won't load the .env file consequently...

I would expect that the .env file is always loaded (except if APP_ENV is defined and set to prod so we can prevent misuses of .env files), if it exists. The DotEnv component already takes care to avoid overriding already defined variables.

fabpot · 2018-02-11T18:09:43Z

@weaverryan You should also create a companion PR on symfony/flex to remove the addition of env vars in phpunit.xml.dist.

ogizanagi · 2018-02-11T19:05:06Z

@weaverryan: That's already a great improvement, but this doesn't solve another linked issue that should also be considered to me: running a command in another env, including test one, which is sometimes handy to debug your test suite, or even simply to init the database/other stuff (despite it can also be done from the test suite bootstrap file).

If it can be of any help, here is what I'm using currently on flex projects: A DotEnvLoader class allows to load an extra .env.[env] file (in addition to the .env one), where "[env]" is the symfony env. Other patches inside the gist shows how to use it.

This is tough: we're getting WTF moments when someone tries to set, for example, APP_ENV=test or some other environment, and suddenly the .env file isn't loaded. This is to hopefully make things work in a more predictable way: .env is always loaded, except in the prod environment. This works because DotEnv does not override any existing variables, so there's no risk in loading it.

weaverryan · 2018-02-11T20:43:35Z

Hey guys!

I've just made this PR more complicated :). I've change to load the .env file even if the APP_ENV. The only time we don't load it is in the prod environment.

I think this is important: we continue to see users confused about why the .env variable suddenly isn't read if they set APP_ENV. The previous situation also made this PR strange: the phpunit.xml.dist env vars are loaded FIRST, and then bootstrap.php is executed. But, the phpunit.xml.dist env vars are ONLY added to $_ENV, not $_SERVER. So, .env was still loaded (which I liked). But if you suddenly started setting true env vars, you'd get a different behavior.

So the goal is simple: load the .env file in the least WTF kind of way. Because it doesn't override existing env vars, I think this makes sense.

Also, @ogizanagi brings up another point in his comment. There's not an easy way, for example, to set a different DATABASE_URL in test, and be able to use it in your tests and in bin/console. He suggests the possibility of having a .env.[env] file that would override the normal one, and I agree. It's also what Laravel does, which I think is important, because they've been using the .env idea longer so have probably learned a few things.

So.... I think we need to make several changes to our .env handling.... and so possibly some sort of bootstrap.php file that's used by all entry points - index.php, console, testing.

Cheers!

ghost

Pull request passes validation.

quentinus95 · 2018-02-11T21:43:28Z

I guess the possibility to use .env.[env] files is mitigated by the fact that we can now load the .env file even if some environment files are already loaded. One could easily create custom .env.test file (for instance) and load those using a source + allexport in a bash script and then run the tests normally.

ogizanagi · 2018-02-12T08:36:50Z

What about symfony/flex providing something like the DotEnvLoader class mentioned earlier (or whatever name we use for it) and use it here ?

SergeC · 2018-02-16T11:47:57Z

Maybe we should have a .env.[env].dist which will be updated by SF Flex the same way as .env.dist? The .env.[env].dist will be pushed to GIT while .env.[env] will be ignored. If env=prod the use .env.

bocharsky-bw · 2018-02-22T11:16:52Z

Probably this file also should be updated: https://github.com/symfony/recipes/blob/master/behat/symfony2-extension/2.1/features/bootstrap/bootstrap.php#L6

bocharsky-bw · 2018-02-22T11:33:48Z

phpunit/phpunit/4.7/tests/bootstrap.php

+ * Those variables will override any defined in .env.
+ */
+
+if (!isset($_SERVER['APP_ENV']) || 'prod' !== $_SERVER['APP_ENV']) {


Why don't we use getenv('APP_ENV') instead of $_SERVER['APP_ENV']. Are there any reasons behind? Symfony recommends to use getenv() in docs but has $_SERVER[] in code which is not consistent as for me.

See #156 about that. It should probably be updated in the docs then.

getenv is an issue because of not being thread safe (probably less an issue in the PHPUnit bootstrap as this is used only in the CLI)

Nyholm

Great. I like this. Just had a minor question

Nyholm · 2018-02-25T10:13:09Z

symfony/phpunit-bridge/3.3/tests/bootstrap.php

+ * Environment variables can also be specified in phpunit.xml.dist.
+ * Those variables will override any defined in .env.
+ */
+if (!isset($_SERVER['APP_ENV'])) {


Shouln't this be the same as for version 4.7?

if (!isset($_SERVER['APP_ENV']) || 'prod' !== $_SERVER['APP_ENV']) {

Nyholm · 2018-02-25T10:17:03Z

symfony/phpunit-bridge/3.3/tests/bootstrap.php

+
+/*
+ * Environment variables can also be specified in phpunit.xml.dist.
+ * Those variables will override any defined in .env.


B-Galati · 2018-03-08T16:18:10Z

symfony/framework-bundle/3.3/public/index.php

@@ -8,7 +8,7 @@
 require __DIR__.'/../vendor/autoload.php';

 // The check is to ensure we don't use .env in production
-if (!isset($_SERVER['APP_ENV'])) {
+if (!isset($_SERVER['APP_ENV']) || 'prod' !== $_SERVER['APP_ENV']) {


I just had an issue trying to warm-up my cache on the CI.
My APP_ENV was test and I was overriding the env with the --env option like so :

bin/console --env=prod cache:clear --no-warmup [08-Mar-2018 16:54:49 Europe/Paris] PHP Fatal error: Uncaught RuntimeException: APP_ENV environment variable is not defined. You need to define environment variables for configuration or add "symfony/dotenv" as a Composer dependency to load variables from a .env file. in /app/bin/console:20

I can see that my case is kind of weird and I am going to change the way I ran the command but we could do something to improve DX in that case perhaps ?

sstok · 2018-03-11T12:30:06Z

To be clear phpunit follows the same convention as the DotEnv component, if an env is already set it will not overwrite, unless you provide force="true".

joshlopes · 2018-05-11T12:54:10Z

A DotEnvLoader from @ogizanagi seems what SF needs - why isn't this shown any love? Is there a workaround for this already implemented?

weaverryan · 2018-05-12T01:08:19Z

It’s just waiting on me to find some time to finish it. I have some agreement that this is probably the right approach

weaverryan · 2018-05-15T15:23:48Z

Closing in favor of #408

Adding a new test bootstrap.php file so .env vars are read

d49d21b

This works particularly well, because any env vars set in phpunit.xml.dist WIN over .env. This means that .env can contain default values, but can then be easily overridden for the test environment.

ghost deployed to staging February 11, 2018 17:41 View deployment

ghost approved these changes Feb 11, 2018

View reviewed changes

weaverryan mentioned this pull request Feb 11, 2018

Reading .env in PHPUnit versus env variables in phpunit.xml.dist symfony/flex#251

Closed

quentinus95 reviewed Feb 11, 2018

View reviewed changes

javiereguiluz approved these changes Feb 11, 2018

View reviewed changes

ghost deployed to staging February 11, 2018 20:44 View deployment

ghost approved these changes Feb 11, 2018

View reviewed changes

bocharsky-bw reviewed Feb 22, 2018

View reviewed changes

Nyholm reviewed Feb 25, 2018

View reviewed changes

B-Galati reviewed Mar 8, 2018

View reviewed changes

ogizanagi mentioned this pull request Mar 19, 2018

[feature] Add APP_ENV_FILE to select env file #385

Closed

weaverryan mentioned this pull request Mar 20, 2018

Removing all env configuration logic for phpunit.xml(.dist) files symfony/flex#318

Closed

joshlopes mentioned this pull request May 15, 2018

Bootstrap environment #408

Closed

weaverryan closed this May 15, 2018

weaverryan deleted the test-env-vars branch May 16, 2018 14:27

ogizanagi mentioned this pull request Sep 20, 2018

PHPUnit: use a .env file #465

Closed

dunglas mentioned this pull request Sep 21, 2018

Allow to override .env files for specific environments #466

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

Adding test bootstrap.php file so .env vars are read #366

Adding test bootstrap.php file so .env vars are read #366

weaverryan commented Feb 11, 2018

ghost left a comment

quentinus95 Feb 11, 2018

fabpot commented Feb 11, 2018

ogizanagi commented Feb 11, 2018

weaverryan commented Feb 11, 2018

ghost left a comment

quentinus95 commented Feb 11, 2018

ogizanagi commented Feb 12, 2018 •

edited

Loading

SergeC commented Feb 16, 2018

bocharsky-bw commented Feb 22, 2018

bocharsky-bw Feb 22, 2018

ogizanagi Feb 22, 2018

stof May 16, 2018

Nyholm left a comment

Nyholm Feb 25, 2018

Nyholm Feb 25, 2018

B-Galati Mar 8, 2018

sstok commented Mar 11, 2018

joshlopes commented May 11, 2018

weaverryan commented May 12, 2018

weaverryan commented May 15, 2018

Adding test bootstrap.php file so .env vars are read #366

Adding test bootstrap.php file so .env vars are read #366

Conversation

weaverryan commented Feb 11, 2018

ghost left a comment

Choose a reason for hiding this comment

quentinus95 Feb 11, 2018

Choose a reason for hiding this comment

fabpot commented Feb 11, 2018

ogizanagi commented Feb 11, 2018

weaverryan commented Feb 11, 2018

ghost left a comment

Choose a reason for hiding this comment

quentinus95 commented Feb 11, 2018

ogizanagi commented Feb 12, 2018 • edited Loading

SergeC commented Feb 16, 2018

bocharsky-bw commented Feb 22, 2018

bocharsky-bw Feb 22, 2018

Choose a reason for hiding this comment

ogizanagi Feb 22, 2018

Choose a reason for hiding this comment

stof May 16, 2018

Choose a reason for hiding this comment

Nyholm left a comment

Choose a reason for hiding this comment

Nyholm Feb 25, 2018

Choose a reason for hiding this comment

Nyholm Feb 25, 2018

Choose a reason for hiding this comment

B-Galati Mar 8, 2018

Choose a reason for hiding this comment

sstok commented Mar 11, 2018

joshlopes commented May 11, 2018

weaverryan commented May 12, 2018

weaverryan commented May 15, 2018

ogizanagi commented Feb 12, 2018 •

edited

Loading