Add PHP version constraint to Composer #10
Conversation
Codecov Report
@@ Coverage Diff @@ ## master #10 +/- ## ============================================= + Coverage 51.09% 83.33% +32.23% Complexity 32 32 ============================================= Files 4 4 Lines 137 84 -53 ============================================= Hits 70 70 + Misses 67 14 -53
Continue to review full report at Codecov.
|
composer.json Outdated
| }, | ||
| "scripts": { | ||
| "test": "phpunit -c test" | ||
| "test": "php vendor/bin/phpunit -c test/phpunit.xml" |
There was a problem hiding this comment.
This line should be functionally equivalent to the previous one. How does this fix code coverage?
There was a problem hiding this comment.
phpunit command option -c means --configuration.
And I just specify the current path of phpunit.xml.
There was a problem hiding this comment.
PHPUnit looks in the specified directory for phpunit.xml, so it should do the same thing. This pattern is used in other projects and works fine: https://github.com/ScriptFUSION/Porter/blob/master/composer.json#L39. For some reason, (probably because the tests are PHPT format,) it just doesn't work in this project. I cannot deny your change seems to fix the issue, but I suspect there's still a bug in PHPUnit itself.
If specifying the config file explicitly fixes the issue, does that mean the php vendor/bin/ change isn't needed?
composer.json Outdated
| "require": { | ||
| "phpunit/phpunit": "^5.5|^6" | ||
| "php": ">=5.6", | ||
| "phpunit/phpunit": "^5.5|^6.5" |
There was a problem hiding this comment.
It seems that the PHPUnit require 5.5+ or 6.5+.
So I add the required PHP version >=5.6 to check whether the environment is the correct PHP version.
There was a problem hiding this comment.
The environment check is fine, but why does the ^6 constraint need changing to ^6.5?
There was a problem hiding this comment.
Because I want to let PHPUnit be the latest version.
There was a problem hiding this comment.
That doesn't make sense. ^6 already permits the latest version; ^6.5 just prevents earlier versions, which is not desirable unless you can prove it's necessary to fix this issue.
There was a problem hiding this comment.
All right, I will add the PHPUnit version back.
| As I just proved in #11, these changes do not improve coverage. The coverage issue that used to exist has since been fixed by external changes in PHPUnit, Codecov or something else. The only change I'd be willing to merge here is the PHP version constraint in Composer. |
3 participants
It's just for this issue.