-
Notifications
You must be signed in to change notification settings - Fork 32
update phpunit #127
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
update phpunit #127
Conversation
Apologies for the delay in reviewing this. Will you summarize what is the value in updating this? It doesn't seem like we're taking advantage of any new functionality or bug fixes. |
PHPUnit 4.8 had hit its end of support in the year 2015 (as I remember). It should be possible to use newer versions of PHPUnit on newer versions of PHP. |
Thanks for explaining. Just to be clear, though, does this change affect users of the php test reporter? |
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.
👍
phpunit.xml.dist
Outdated
@@ -2,15 +2,19 @@ | |||
|
|||
<phpunit | |||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | |||
xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/4.8/phpunit.xsd" | |||
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/5.7/phpunit.xsd" |
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.
How about referencing the schema by a relative URL?
-xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/5.7/phpunit.xsd"
+xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
This way it will always be accurate, regardless of which version of phpunit/phpunit
is installed.
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 didn't know this is possible.
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.
done
@@ -4,7 +4,7 @@ | |||
use CodeClimate\PhpTestReporter\Application; | |||
use Symfony\Component\Console\Tester\ApplicationTester; | |||
|
|||
class ApplicationTest extends \PHPUnit_Framework_TestCase | |||
class ApplicationTest extends \PHPUnit\Framework\TestCase |
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.
Ha, nice, wasn't aware it's been backported to 4.8
, see https://github.com/sebastianbergmann/phpunit/blob/master/ChangeLog-4.8.md#4835---2017-02-06!
No, this only affects contributors, not users! |
composer.json
Outdated
@@ -28,7 +28,7 @@ | |||
}, | |||
"require-dev": { | |||
"friendsofphp/php-cs-fixer": "^2.0.0", | |||
"phpunit/phpunit": "^4.8.31" | |||
"phpunit/phpunit": "^4.8.35 || ^5.7 || ^6.0" |
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.
One more thing, how about making it more obvious which minor versions we are fine with?
-"phpunit/phpunit": "^4.8.35 || ^5.7 || ^6.0"
+"phpunit/phpunit": "^4.8.35 || ^5.7.0 || ^6.0.0"
This way it is very accurately documented.
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.
done
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.
Very nice, let's ship it!
👍 feel free to squash and merge @localheinz |
Thank you, @mimmi20! |
This PR tries to allow newer versions of PHPunit and changes to the namespaced version of the base test class.
This PR is a part of previous PR #124.