Control proxy implementation via env #10282
Conversation
derrabus
force-pushed
the
improvement/proxy-tests
branch
from
5babe01 to
d7f71fc
Compare
tests/Doctrine/Tests/TestUtil.php
Outdated
| $configuration->setProxyNamespace('Doctrine\Tests\Proxies'); | ||
|
|
||
| $proxyImplementation = $_ENV['ORM_PROXY_IMPLEMENTATION'] | ||
| ?? $_SERVER['ORM_PROXY_IMPLEMENTATION'] |
There was a problem hiding this comment.
Why is this fallback to $_SERVER needed?
There was a problem hiding this comment.
Some SAPIs store environment variables in $_ENV, some in $_SERVER. But since we're not dealing with any SAPI other than CLI, I can probably remove it.
There was a problem hiding this comment.
In fact, on CLI, environment variables are stored in $_SERVER. I think I might just switch to getenv(). 🤦🏻
There was a problem hiding this comment.
I remember stuggling with that myself in the past, and it seems to be quite a mess: https://www.php.net/manual/en/ini.core.php#ini.variables-order
getenv() seems like a good idea 👍
tests/Doctrine/Tests/TestUtil.php
Outdated
| ?? null; | ||
|
|
||
| if (! $proxyImplementation) { | ||
| $proxyImplementation = trait_exists(LazyGhostTrait::class) ? 'lazy-ghost' : 'common'; |
There was a problem hiding this comment.
Why not add an new ?? line with that ternary?
There was a problem hiding this comment.
Because I also want the fallback to kick in if the environment variable has been set to an empty string.
There was a problem hiding this comment.
Because unsetting environment variables is hard. If you're dealing with enviroment variables, you should always treat the empty string as if the variable wasn't set.
Fun fact: I did not take the empty string into account at first which made the CI fail: https://github.com/doctrine/orm/actions/runs/3664887468/jobs/6195591631
There was a problem hiding this comment.
I'm sorry, where are we unsetting that environment variable? And is that not as simple as putenv('MYVAR')?
There was a problem hiding this comment.
I'm sorry, where are we unsetting that environment variable?
We aren't, but e.g. Github Actions needs to do that.
derrabus
force-pushed
the
improvement/proxy-tests
branch
from
d7f71fc to
94823fe
Compare
* 2.14.x: Control proxy implementation via env (doctrine#10282)
* 2.14.x: Control proxy implementation via env (doctrine#10282) Fix association mapping with enum fields Correct spelling errors
Our test suite is currently run with the lazy ghost implementation if the var-exporter component is installed. In our CI, we install the component if we want to test the lazy ghost implementation.
This is a bit brittle given that Symfony Cache depends on VarExporter, so the component will be installed no matter what. Also, if I want to run tests locally, it's a bit annoying that I have to install the component and live with local changes in my composer.json.
With this PR, I'm adding VarExporter to the dev requirements. I'm introducing an environment variable
ORM_PROXY_IMPLEMENTATIONwhich allows us to control the proxy implementation that should be used by the tests.