Phpunit 12 and 13 compatibility#23
Conversation
Bilge
left a comment
Bilge
left a comment
There was a problem hiding this comment.
Please share what you learned whilst making this PR so we can understand the reasons for each change. I cannot merge this without commentary explaining why the changes were necessary.
composer.json
Outdated
| "require": { | ||
| "php": ">=8.1", | ||
| "phpunit/phpunit": "^10.5.28|^11.2.8" | ||
| "php": ">=8.3", |
There was a problem hiding this comment.
PHP 8.1 is EOL, so I don't think is a good option to have a new release with an obsolete PHP version... As I said in the previous comment, maybe we can have here >=8.2
There was a problem hiding this comment.
That may be so, but this PR claims to be about adding PHPUnit 12 and 13 compatibility. Nowhere previously have you mentioned that a secondary goal is to change the project's PHP compatibility alignment, nor do I think such a change is necessary to accomplish the (noble) goal of improving PHPUnit upwards compatibility. Please continue adding the invalid combinations to the exclusion matrix and keep PHP compatibility unchanged at this time.
There was a problem hiding this comment.
ok. I put back PHP 8.1 version
|
|
||
| 50% . [32;1mScriptFUSIONTest\Pip\CapabilitiesTest::testDataProvider with data set "foo"[0m [32m(%d ms)[0m | ||
| 100% . [32;1mScriptFUSIONTest\Pip\CapabilitiesTest::testDataProvider with data set #0[0m [32m(%d ms)[0m | ||
| 50% . [32;1mScriptFUSIONTest\Pip\CapabilitiesTest::testDataProvider%swith data%s[0m [32m(%d ms)[0m |
There was a problem hiding this comment.
Tests should be as precise as possible. This is very lax, having now completely omitted the data set names.
- Why is the first
%sneeded? - Why were data set names replaced with the second
%s?
There was a problem hiding this comment.
By using phpunit 10/11 the data set and data is detailed always after the test: "with data set '@name or data set'|'#index of dataset' and then the ('data used'), but from Phpunit 12, the dataset is specified just after the test name: testDataProvider@foo|testDataProvider#0 with data ('data'), so, if I wanted to support both I tried by using EXPECTREGEX, but I can't found a way to have it working
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #23 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 70 70
===========================================
Files 6 6
Lines 157 157
===========================================
Hits 157 157 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@ireneperezddc1 Thank you for your contribution. |
2 participants
adjusts on actions and composer to upgrade compatibility with PHP 8.5 and PHPUnit 12 and 13