-
Notifications
You must be signed in to change notification settings - Fork 546
Allow PHPUnit 12 #4624
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 PHPUnit 12 #4624
Conversation
|
I'd prefer to have just 12 in composer.json, and then downgrade in action yamls in lower unsupported PHP versions. So ideally PHPUnit 12 on PHP 8.3+ in the composer.json files, downgrade to 9 on PHP < 8.3. Infection could only run on PHP 8.3+. To have a lock file and then multiple version ranges for a package in composer.json doesn't make sense. Also don't forget to commit the tests/ lock file 😊 |
|
hm this would mean to update most nette/ packages in the same step, as we would also raise phpstan-src/composer.json to php 8.3? |
|
Updating the Nette packages is hard, they probably drop PHP 7.4 in versions that support 8.3. And some packages we will probably never be able to upgrade, like nette/di. There's a longer story, but 3.1.15 (the one that is copied in composer.json in its entirety) is the one that works for me, newer versions break stuff. |
|
so maybe use PHPUnit 9 on old php versions, PHPUnit 11 on 8.1,8.2 and PHPUnit 12+ on 8.3+ ? alternatively: use PHPUnit12 only in the mutation testing job and leave everything else on the current state of things |
I'm a bit slow, what would it solve in regards to Nette packages? |
if I declare PHPUnit12 on top of PHPUnit11, the php runtime version will decide which phpunit version it will run on. if I update to PHPUnit12 only, I need to raise the min php version to 8.3 and this means I need to raise nette/* packages |
|
|
|
Alright, so there will be PHPUnit 11 in composer.json, and actions will sometimes downgrade to PHPUnit 9, and sometimes upgrade to PHPUnit 12? So why doesn't having PHPUnit 12 in composer.json work? (Why |
it doesn't work. |
|
Add |
|
doesn't work either. but I don't get your point. do you want to have settings in composer.json which only work when invoked with |
It wouldn't be committed to composer.json. It'd only be in GitHub Actions in places where we want PHPUnit v12. |
|
yeah, but what you describe is what this PR is already doing? |
|
That's good news, I assumed it doesn't work, didn't check the log. I'd like to have PHPUnit 12 on 8.3+ in normal tests run too. |
|
The failures about mocks vs. stubs should be fixed in a separate PR merged before this one 😊 |
phpunit warnings are fixed in #4625 will do another PR for the stubs |
2f4c833 to
23c53ac
Compare
| run: "vendor/bin/phpunit --check-php-configuration" | ||
|
|
||
| - name: "Tests" | ||
| run: "php tests/vendor/bin/paratest --runner WrapperRunner --no-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.
I need to call paratest directly, instead of using make tests which would trigger composer install --working-dir tests, and this errors because of platform-requirement problems with:
composer install --working-dir tests
Installing dependencies from lock file (including require-dev)
Verifying lock file contents can be installed on current platform.
Your lock file does not contain a compatible set of packages. Please run composer update.
Problem 1
- brianium/paratest is locked to version v7.16.0 and an update of this package was not requested.
- brianium/paratest v7.16.0 requires php ~8.3.0 || ~8.4.0 || ~8.5.0 -> your php version (8.2.99; overridden via config.platform, actual: 8.3.28) does not satisfy that requirement.
Problem 2
- phpunit/php-code-coverage is locked to version 12.5.1 and an update of this package was not requested.
- phpunit/php-code-coverage 12.5.1 requires php >=8.3 -> your php version (8.2.99; overridden via config.platform, actual: 8.3.28) does not satisfy that requirement.
see https://github.com/phpstan/phpstan-src/actions/runs/20089728006/job/57634537245?pr=4624
in case this is fine, I think I can remove the symfony/* packages again from
composer require --dev phpunit/phpunit:^12 brianium/paratest:^7.16 symfony/process:^7 symfony/string:^7 symfony/console:^7 --update-with-dependencies --ignore-platform-reqs --working-dir=tests
| @@ -1,5 +1,5 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd" bootstrap="tests/bootstrap.php" cacheResult="false" colors="true" executionOrder="random" failOnRisky="true" failOnWarning="true" failOnEmptyTestSuite="true" beStrictAboutChangesToGlobalState="true" beStrictAboutOutputDuringTests="true" cacheDirectory="tmp/.phpunit.cache" beStrictAboutCoverageMetadata="true" displayDetailsOnPhpunitDeprecations="true" displayDetailsOnIncompleteTests="true"> | |||
| <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd" bootstrap="tests/bootstrap.php" cacheResult="false" colors="true" executionOrder="random" failOnRisky="true" failOnWarning="true" failOnPhpunitWarning="false" failOnEmptyTestSuite="true" beStrictAboutChangesToGlobalState="true" beStrictAboutOutputDuringTests="true" cacheDirectory="tmp/.phpunit.cache" beStrictAboutCoverageMetadata="true" displayDetailsOnPhpunitDeprecations="true" displayDetailsOnIncompleteTests="true"> | |||
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 disabled exit-code=1 on phpunit warnings because we currently get warnings like
1) PHPStan\Analyser\LegacyNodeScopeResolverTest::testTypeFromMethodPhpDocs
* Data set #5 provided by PHPStan\Analyser\LegacyNodeScopeResolverTest::dataTypeFromMethodPhpDocs has more arguments (3) than the test method accepts (2)
* Data set #9 provided by PHPStan\Analyser\LegacyNodeScopeResolverTest::dataTypeFromMethodPhpDocs has more arguments (3) than the test method accepts (2)
* Data set #18 provided by PHPStan\Analyser\LegacyNodeScopeResolverTest::dataTypeFromMethodPhpDocs has more arguments (3) than the test method accepts (2)
* Data set #19 provided by PHPStan\Analyser\LegacyNodeScopeResolverTest::dataTypeFromMethodPhpDocs has more arguments (3) than the test method accepts (2)
* Data set #5 provided by PHPStan\Analyser\LegacyNodeScopeResolverTest::dataTypeFromMethodPhpDocs has more arguments (3) than the test method accepts (2)
* Data set #9 provided by PHPStan\Analyser\LegacyNodeScopeResolverTest::dataTypeFromMethodPhpDocs has more arguments (3) than the test method accepts (2)
* Data set #18 provided by PHPStan\Analyser\LegacyNodeScopeResolverTest::dataTypeFromMethodPhpDocs has more arguments (3) than the test method accepts (2)
* Data set #19 provided by PHPStan\Analyser\LegacyNodeScopeResolverTest::dataTypeFromMethodPhpDocs has more arguments (3) than the test method accepts (2)
/home/runner/work/phpstan-src/phpstan-src/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php:3764
2) PHPStan\Rules\Methods\OverridingMethodRuleTest::testOverridingFinalMethod
* Data set #0 provided by PHPStan\Rules\Methods\OverridingMethodRuleTest::dataOverridingFinalMethod has more arguments (3) than the test method accepts (2)
* Data set #1 provided by PHPStan\Rules\Methods\OverridingMethodRuleTest::dataOverridingFinalMethod has more arguments (3) than the test method accepts (2)
* Data set #0 provided by PHPStan\Rules\Methods\OverridingMethodRuleTest::dataOverridingFinalMethod has more arguments (3) than the test method accepts (2)
* Data set #1 provided by PHPStan\Rules\Methods\OverridingMethodRuleTest::dataOverridingFinalMethod has more arguments (3) than the test method accepts (2)
/home/runner/work/phpstan-src/phpstan-src/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php:59
3) PHPStan\Rules\Methods\OverridingMethodRuleTest::testBug3403
* Data set #0 provided by PHPStan\Rules\Methods\OverridingMethodRuleTest::dataOverridingFinalMethod has more arguments (3) than the test method accepts (1)
* Data set #1 provided by PHPStan\Rules\Methods\OverridingMethodRuleTest::dataOverridingFinalMethod has more arguments (3) than the test method accepts (1)
* Data set #0 provided by PHPStan\Rules\Methods\OverridingMethodRuleTest::dataOverridingFinalMethod has more arguments (3) than the test method accepts (1)
* Data set #1 provided by PHPStan\Rules\Methods\OverridingMethodRuleTest::dataOverridingFinalMethod has more arguments (3) than the test method accepts (1)
/home/runner/work/phpstan-src/phpstan-src/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php:325
which we cannot ignore using validateArgumentCount #[DataProvider] parameter because this parameter doesn't exist on PHPUnit 11.x:
XDEBUG_MODE=off php tests/vendor/bin/paratest --runner WrapperRunner --no-coverage
ParaTest v7.8.4 upon PHPUnit 11.5.45 by Sebastian Bergmann and contributors.
In AttributeParser.php line 421:
Invalid attribute PHPUnit\Framework\Attributes\DataProvider for method PHPS
tan\Rules\Methods\OverridingMethodRuleTest::testOverridingFinalMethod() in
/home/runner/work/phpstan-src/phpstan-src/tests/PHPStan/Rules/Methods/Overr
idingMethodRuleTest.php:59
Unknown named parameter $validateArgumentCount
paratest [--functional] [-m|--max-batch-size MAX-BATCH-SIZE] [--no-test-tokens] [--passthru-php PASSTHRU-PHP] [-p|--processes PROCESSES] [--runner RUNNER] [--tmp-dir TMP-DIR] [-v|--verbose] [--bootstrap BOOTSTRAP] [-c|--configuration CONFIGURATION] [--no-configuration] [--cache-directory CACHE-DIRECTORY] [--testsuite TESTSUITE] [--exclude-testsuite EXCLUDE-TESTSUITE] [--group GROUP] [--exclude-group EXCLUDE-GROUP] [--filter FILTER] [--process-isolation] [--strict-coverage] [--strict-global-state] [--disallow-test-output] [--dont-report-useless-tests] [--stop-on-defect] [--stop-on-error] [--stop-on-failure] [--stop-on-warning] [--stop-on-risky] [--stop-on-skipped] [--stop-on-incomplete] [--fail-on-incomplete] [--fail-on-risky] [--fail-on-skipped] [--fail-on-warning] [--fail-on-deprecation] [--order-by ORDER-BY] [--random-order-seed RANDOM-ORDER-SEED] [--colors [COLORS]] [--no-progress] [--display-incomplete] [--display-skipped] [--display-deprecations] [--display-errors] [--display-notices] [--display-warnings
make: *** [Makefile:6: tests] Error 1
Error: Process completed with exit code 2.
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.
alternatively we could drop the validateArgumentCount parameter in the downgrading process
|
Yeah, maintaining this is getting more and more complicated. At least we can look forward to dropping some of the complexity when PHPStan 3.0 will only support PHP 8.1+. |
|
This pull request has been marked as ready for review. |
|
Thank you! |
Before
After upgrading to PHPUnit 12
After adding
--exclude-source-from-xml-coverage