Skip to content

Fix $_ENV/$_SERVER precedence in test framework #24686

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

Merged
merged 1 commit into from
Oct 26, 2017
Merged

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Oct 25, 2017

Q A
Branch? 3.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

When defining env vars values in phpunit.xml.dist, we are using <env ...>. PHPUnit registers those env vars in $_ENV, but not in $_SERVER. This means that those values might not be used by Symfony if env vars defined in .env are automatically registered (which is my case).

In any case, I think it makes sense to make $_ENV take precedence as this is how we register them in phpunit.xml.dist.

@nicolas-grekas
Copy link
Member

Shouldn't we also change the precedence in src/Symfony/Component/DependencyInjection/Container.php?

@nicolas-grekas nicolas-grekas added this to the 3.3 milestone Oct 25, 2017
@fabpot
Copy link
Member Author

fabpot commented Oct 26, 2017

I've made the change in Container.php for better consistency. Should be ready now.

@fabpot fabpot merged commit 6ed9919 into symfony:3.3 Oct 26, 2017
fabpot added a commit that referenced this pull request Oct 26, 2017
This PR was merged into the 3.3 branch.

Discussion
----------

Fix $_ENV/$_SERVER precedence in test framework

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

When defining env vars values in `phpunit.xml.dist`, we are using `<env ...>`. PHPUnit registers those env vars in `$_ENV`, but not in `$_SERVER`. This means that those values might not be used by Symfony if env vars defined in `.env` are automatically registered (which is my case).

In any case, I think it makes sense to make `$_ENV` take precedence as this is how we register them in `phpunit.xml.dist`.

Commits
-------

6ed9919 fixed $_ENV/$_SERVER precedence in test framework
This was referenced Oct 30, 2017
@fabpot fabpot mentioned this pull request Nov 10, 2017
@fabpot fabpot deleted the test-fix branch January 14, 2019 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants