-
-
Notifications
You must be signed in to change notification settings - Fork 492
Fix recipe for phpunit #952
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
Conversation
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.
Pull request passes validation.
@@ -25,12 +25,9 @@ | |||
</whitelist> | |||
</filter> | |||
|
|||
<!-- Run `composer require symfony/phpunit-bridge` before enabling this extension --> |
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 removes any diff between phpunit.xml.dist shipped by simple-phpunit vs phpunit recipes.
Without this change, what ppl get is random, leading to the deprecation report being unavailable.
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.
Pull request passes validation.
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.
Pull request passes validation.
if (is_file(dirname(__DIR__).'/vendor/phpunit/phpunit/phpunit')) { | ||
define('PHPUNIT_COMPOSER_INSTALL', dirname(__DIR__).'/vendor/autoload.php'); | ||
require PHPUNIT_COMPOSER_INSTALL; | ||
PHPUnit\TextUI\Command::main(); |
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.
Is this the real significant portion of this PR? Could you explain a bit further?
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 the only possible fix for #936
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.
I see how there is no PHP file in phpunit to directly require. And so, you've mimicked their phpunit executable.
<include> | ||
<directory suffix=".php">src</directory> | ||
</include> | ||
</coverage> |
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.
Note to others: changes made to the schema for PHPUnit 9.
@@ -6,7 +6,6 @@ | |||
"tests/": "tests/" | |||
}, | |||
"gitignore": [ | |||
".phpunit", |
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.
Why .phpunit
has been removed from .gitignore? I don't think that directory should be committed to the application source code, in my case it contains 2600+ files...
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.
Because now we prefer using the binary from PHPUnit (which does not create .phpunit
folder) instead the binary from Symfony PHPUnit bridge (simple-phpunit
).
So adding .phpunit
in .gitignore
is not useful anymore.
<server name="SYMFONY_PHPUNIT_REMOVE" value="" /> | ||
<server name="SYMFONY_PHPUNIT_VERSION" value="8.5" /> | ||
--> | ||
<server name="SYMFONY_PHPUNIT_VERSION" value="9.5" /> |
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.
Symfony 5.3 requires php 7.2. But phpunit 9.5 requires php 7.3. So PHP 7.2 users won't be able to run phpunit by default or am I misunderstanding this?
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.
PHP 7.2 is EOL so it shouldn't be maintained anymore IMHO
Fix #936
Fix symfony/symfony#41409 and symfony/symfony#41471