-
Notifications
You must be signed in to change notification settings - Fork 65
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
Fix the overriding of env variables for the phpunit config #117
Fix the overriding of env variables for the phpunit config #117
Conversation
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? |
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
Regarding #115, I agree on the use of an associative array. I will include this change and remove the duplicate Regarding #116, I directly answered in the issue. |
I was arguing about associative array vs old configuration: isn't this a BC? |
Normally no. I modified the code using the environment variables. |
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. |
Versions less than Symfony 3 do not use environment variables defined in the Phpunit config. However, the content of |
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. |
See this for instance |
Because the project in #114 load the envirnoment variables with Add By default, Symfony 4 Flex doesn't use 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 After, I may have misunderstood what is bothering you with #114 :-). |
No, ok, I got it, it should be good :) |
Thank you! |
Why this PR is closed ??? |
Sorry, wrong button! |
Ok, thank you. |
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 valuevariables_order=EGPCS
(GPCS
by default), but it is recommended to use theGPCS
value.Therefore, it is preferable to merge the variables in
$_SERVER
with$_ENV
like the Symfony WebServerBundle.