rockettheme / toolbox

RocketTheme\Toolbox package contains a set of reusable PHP interfaces, classes and traits.
MIT License
21 stars 16 forks source link

Failing tests on fresh install and missing CI #31

Closed lcharette closed 2 years ago

lcharette commented 2 years ago

After a fresh install (git clone ... + composer install), PHPUnit produce some error on PHP 8.0 :

$ vendor/bin/phpunit 
PHP Fatal error:  Declaration of UniformResourceLocatorTest::setUpBeforeClass() must be compatible with PHPUnit\Framework\TestCase::setUpBeforeClass(): void in /Users/xxx/toolbox/ResourceLocator/tests/UniformResourceLocatorTest.php on line 15

Fatal error: Declaration of UniformResourceLocatorTest::setUpBeforeClass() must be compatible with PHPUnit\Framework\TestCase::setUpBeforeClass(): void in /Users/xxx/toolbox/ResourceLocator/tests/UniformResourceLocatorTest.php on line 15

Plus the Travis implementation doesn't work anymore : https://travis-ci.org/github/rockettheme/toolbox/

This is a big red flags, as CI doesn't works and testing is not properly ran with each new releases. I would suggest replacing Travis with GitHub action ASAP. It's not inviting to submit a pull request currently since we can't know if a change breaks something else.

lcharette commented 2 years ago

Note that the current composer.json is flawed. PHP 5.6 is the minimum requirement, as is PHPunit 8.0. But PHPunit 8.0 has PHP 7.2 as the minimum requirement. So it’s technically impossible to test PHP version older than 7.2 right now.

Plus, to fix the above error, you would lose compatibility with older versions of PHPUnit, rendering testing of older version even more difficult as you can’t simply rely on an older version of PHPUnit.

So the minimum version of PHP would need be raised to provide consistent testing. If older php version are still being supported (which shouldn’t be advised…) by this repo, I would recommend splitting it in a new major version (2.0) supporting only PHP 8.0 and up (with proper testing).

mahagr commented 2 years ago

Fixed in cdac559d7f92c6346d3a0915c23889dc6faae66c

I removed the tests for now, they will be back in 2.0.