chobie / jira-api-restclient

php JIRA REST API
MIT License
218 stars 123 forks source link

Travis failures due phpunit strictness #187

Open glensc opened 5 years ago

glensc commented 5 years ago

What's up with the phpunit setup on travis? What is the purpose of such setup?

install:
  - if [ "$TRAVIS_PHP_VERSION" = "5.5" ]; then composer require "phpunit/phpunit:^4.8" --dev; fi;
  - if [ "$TRAVIS_PHP_VERSION" != "5.5" ]; then composer install --prefer-dist; fi;

script:
  - if [ "$TRAVIS_PHP_VERSION" = "5.5" ]; then ./vendor/bin/phpunit; fi;
  - if [ "$TRAVIS_PHP_VERSION" != "5.5" ]; then phpunit; fi;

Currently travis is failing on 7.2, because it's using travis preinstalled version, that requires :void type hints for unit tests.

Example:

PHP Fatal error:  Declaration of Tests\chobie\Jira\Api\Client\AbstractClientTestCase::setUp() must be compatible with PHPUnit\Framework\TestCase::setUp(): void in /home/travis/build/chobie/jira-api-restclient/tests/Jira/Api/Client/AbstractClientTestCase.php on line 312

I've recently tackled with travis and phpunit, and ended up using composer global require only one specific version, so the phpunit deprecations and new methods don't end up conflicting.

https://github.com/eventum/eventum/pull/468

If you describe goal (phpunit version) i'd try to submit PR with that goal

cc @aik099

glensc commented 5 years ago

the 5.5 compare is from https://github.com/chobie/jira-api-restclient/pull/169

aik099 commented 5 years ago

What is the purpose of such setup?

On Travis CI the preinstalled version of PHPUnit works well (has forward compatible namespaced PHPUnit class names). Unfortunately that's not the case with PHPUnit installed on PHP 5.5 version (that PHPUnit version is fixed in the container used to install PHP 5.5 VM). For that reason alone we've installing latest PHP 5.5.x compatible PHPUnit version and using it.

Not sure if the issue was fixed on Travis CI. I'd rather prefer to use built-in PHPUnit version to speed up build time.

PHP Fatal error: Declaration of Tests\chobie\Jira\Api\Client\AbstractClientTestCase::setUp() must be compatible with PHPUnit\Framework\TestCase::setUp(): void in /home/travis/build/chobie/jira-api-restclient/tests/Jira/Api/Client/AbstractClientTestCase.php on line 312

That is tricky situation. Likely preinstalled on Travis version of PHPUnit is the one, where strict types are used and that breaks all out test case classes, because due to BC break introduced in PHPUnit we can't have same test case that would comply with all PHPUnit versions anymore.

What we need to use PHPUnit version corresponding to used PHP version and a single copy of a test case class to work on them. We can try limiting used PHPUnit version to which where strict types weren't introduced and use same .travis.yml trick to install that.

glensc commented 5 years ago

Speed up pipeline can be done with caching, so that shouldn't be an issue.

Do you have phpunit versions you wish to support? I know that need to have 4.8.36 for PHP 5.3, but what about the upper bound?

aik099 commented 5 years ago

I don't care much what PHPUnit version is used on a particular PHP version during build as long as tests can run. We can pick PHPUnit version, that runs across all current PHP version and use it consistently in all builds as well. I'm not sure, that such version exists considering that PHPUnit is eager to increase minimal supported PHP version with every major release :(

I think, that for fixing PHP 7.2 failed build it's enough to lock PHPUnit version used there to PHPUnit 7 (good if it works) or PHPUnit 6 at least.