Fix the overriding of env variables for the phpunit config

[francoispluchino]

Q	A
Branch?	master
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	~
License	MIT
Doc PR	~

With Symfony 4 Flex, it is now recommended to configure the environment variables in the phpunit XML configuration. Phpunit does not overwrite the existing environment variables with the default variables set in the XML configuration.

It works fine using Phpunit directly, however this is not the case using Fastest on linux (and therefore also Docker), but works for Windows. The PR #98 already made it possible to correct a problem of environment variable for Windows, but in this case, the problem is also present for all the platforms.

We could set in the php.ini file the value variables_order=EGPCS (GPCS by default), but it is recommended to use the GPCS value.

Therefore, it is preferable to merge the variables in $_SERVER with $_ENV like the Symfony WebServerBundle.

[DonCallisto]

LGTM, btw we still trying to understand what to do with symfony 4 and fastest: need we to release a new major version? Don't know what semver says in this cases.

Could you take a look at #116 and leave your opinion? Moreover we have this case in #115 (that is currently closed) and that we can manage directly with this PR.

[francoispluchino]

It is not necessary to switch to a major or minor version. It's just that now the Symfony team has chosen the use of a Phpunit feature (configuration of environment variables in the Phpunit config file), which simply highlights this feature. Given that this is a bug, a patch version is sufficient.

To make quick about semver version MAJOR.MINOR.PATH:

• PATCH: version when you make backwards-compatible bug fixes.
• MINOR: version when you add functionality in a backwards-compatible manner
• MAJOR: version when you make incompatible API changes

Regarding #115, I agree on the use of an associative array. I will include this change and remove the duplicate PATH variable which is also in $_SERVER.

Regarding #116, I directly answered in the issue.

[DonCallisto]

I was arguing about associative array vs old configuration: isn't this a BC?

[francoispluchino]

Normally no. I modified the code using the environment variables.

[DonCallisto]

Yes but see #114. Don't know if in symfony versions < 4 it is still good. That is what I'm trying to explain here.

[francoispluchino]

Versions less than Symfony 3 do not use environment variables defined in the Phpunit config. However, the content of $_ENV must be an associative array and not a list. With Symfony 4, you just notice the problem. Unit tests pass with versions lower than version 4.

[DonCallisto]

I suppose unit tests we have here does not cover this section as other tests I've made with symfony 4 didn't pointed what is explained in #114

This is the main reason about my question on BC.

[DonCallisto]

See this for instance

[francoispluchino]

Because the project in #114 load the envirnoment variables with DotEnv in the bootstrap.

Add DoEnv populate the $_ENV, $_SERVER variables and use the putenv() method.

By default, Symfony 4 Flex doesn't use DotEnv in tests, And many code in Symfony 4 check the $_ENV variable and not the getenv() method. In this case, you have the variables in array keyes, and variable in list with string VAR=VAL.

Your tests will not be able to reproduce this case, there is no BC for Symfony 2.x, 3.x and 4.x, your new matrix tests should pass correctly.

With this PR, the ENV_TEST_CHANNEL_READABLE env variable is defined for Symfony 3 and 4.

After, I may have misunderstood what is bothering you with #114 :-).

[DonCallisto]

No, ok, I got it, it should be good :)

[DonCallisto]

Thank you!

[francoispluchino]

Why this PR is closed ???

[DonCallisto]

Sorry, wrong button! Thank you!

[francoispluchino]

Ok, thank you.