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
Allow Infection test suite to be executed in parallel using Paratest #1544
Conversation
2 goals here: 1. This is much faster (5 seconds for `vendor/bin/paratest` instead of 24 seconds for `vendor/bin/phpunit`) 2. This is the first step for supporting new Test Framework Adapter - Paratest, to run initial tests suite run using `paratest` instead of `phpunit` For now, just for the local usage
@@ -485,7 +485,7 @@ private function startUp( | |||
): void { | |||
$locator = $container->getRootsFileOrDirectoryLocator(); | |||
|
|||
if ($customConfigPath = (string) $io->getInput()->getOption(self::OPTION_CONFIGURATION)) { | |||
if (($customConfigPath = (string) $io->getInput()->getOption(self::OPTION_CONFIGURATION)) !== '') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is because of the upgraded symfony/console
, PHPStan complained
(new Filesystem())->remove( | ||
normalizePath( | ||
realpath(sys_get_temp_dir()) . '/' . self::TMP_DIR_NAME | ||
sprintf( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when tests are executed in parallel, different threads started to remove the temp folders of another thread. We need to have different folders for each thread, which is done here
…nd is not deprecated Otherwise, it does not make sense because starting from `syfmony/console` 4.4 environment variable are inhereted automatically, and calling deprecated methods fails the tests
@@ -77,7 +79,7 @@ public function createProcess( | |||
|
|||
$process->setTimeout(null); // Ignore the default timeout of 60 seconds | |||
|
|||
if (method_exists($process, 'inheritEnvironmentVariables')) { | |||
if (method_exists($process, 'inheritEnvironmentVariables') && version_compare(Versions::getVersion('symfony/console'), 'v4.4', '<')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maks-rafalko why do we need to have an additional condition which uses ocramius/package-versions here? does method_exists
not cover our case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to explain it in the commit message:
Call
$process->inheritEnvironmentVariables();
only when it exists AND is not deprecated
Otherwise, it does not make sense because starting fromsyfmony/console
4.4 environment variable are inhereted automatically, and calling deprecated methods fails the tests
We have a job that does composer install ... --prefer-lowest
. After some dependencies upgrade, this job started installing symfony/console:4.4.0
instead of symfony/console:3.*
.
In symfony/console:4.4.*
the method inheritEnvironmentVariables()
exists, but calling it produces a deprecation notice, which failed the build
So, technically we need to call this method when it exists and it does something, but in 4.4.*
it does nothing (see the deprecation message: ...env variables are always inherited
).
The final result
< 4.4.0
-inheritEnvironmentVariables()
method exists and we should call it= 4.4.*
-inheritEnvironmentVariables()
method exists but we should not call it, it does nothing and produces deprecation notice> 4.4.*
-inheritEnvironmentVariables()
method does not exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah. I've forgot that we still support symfony packages < 4.4.
2 goals here:
vendor/bin/paratest
instead of 24 seconds forvendor/bin/phpunit
)paratest
instead ofphpunit
For now, just for the local usage