@sgiehl opened this Pull Request on December 7th 2020 Member

Description:

Runs AllTests on PHP 8 instead of PHP 7.4

Review

  • [ ] Functional review done
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@sgiehl commented on December 7th 2020 Member

@tsteur the tests are running now on PHP 8. But I needed to update to PHPUnit 9 for that build, as PHPUnit 8.5 doesn't support PHP 8. Unfortunately PHPUnit 9 doesn't support PHP < 7.3, so we can't update in general.

Nevertheless there seem a lot tests failing. Would need some time to investigate that a bit further. Let me know if or when I should plan to do that.

@tsteur commented on December 7th 2020 Member

@sgiehl it seems to be mostly working? we could look into it as part of 4.1 if it's somewhat quick to do to fix these issues.

For now priority is 4.0.X and I think I pinged you on few other sec issues etc

@sgiehl commented on December 7th 2020 Member

@tsteur Actually might be a bit more. The second build that is shown as success actually silently fails. See https://travis-ci.com/github/matomo-org/matomo/jobs/455444024#L996
Not yet sure why the tests suddenly abort while running.

@sgiehl commented on December 10th 2020 Member

Some tests are failing due to adjustments required in TCPDF. Those should be fixed with https://github.com/tecnickcom/TCPDF/pull/293
Waiting for a new release to get that fixed...

@Findus23 commented on February 16th 2021 Member

@sgiehl I just saw that there were a few TCPDF releases last week:
https://github.com/tecnickcom/TCPDF/releases

@sgiehl commented on February 16th 2021 Member

@Findus23 correct. But the main branch still contains some extra fixes for php 8. See https://github.com/tecnickcom/TCPDF/compare/6.3.5...main

@Findus23 commented on February 16th 2021 Member

:facepalm: Okay, that was my mistake. I saw 14 Feb and totally missed that it was 2020 and not 2021.
So it seems like the status continues that there is no release including the PHP 8 fixes.

@flamisz commented on February 16th 2021 Contributor

Is this PR just for modifying the PHP8 version on travis or we want to do the same for local development?
If we want to make it work locally, we have to update phpunit in the composer.json as well.

And there is one more small issue running the test locally on PHP8:
Before we run the tests we have to migrate the tests db, by running ./console tests:setup-fixture OmniFixture. But during this process there is an issue on PHP8 in tests/PHPUnit/Fixtures/InvalidVisits.php file, because the json_decode throws a TypeError instead of Exception.

If this PR is for running the tests locally on PHP8 as well, we need to make those modifications.

@sgiehl commented on February 17th 2021 Member

@flamisz We can't update PHPUnit locally as the latest version doesn't run on PHP 7.2. But the currently used version doesn't work with PHP 8. So we actually need to wait until our requirement was raised to PHP 7.3, so we can use PHPUnit 9 globally.

For now this PR was only meant to run travis tests on PHP 8

@sgiehl commented on August 16th 2021 Member

This should now be ready for a review. Had to change a couple of tests in order to make them run on PHP 7 & PHP 8, but now the only remaining failing tests also fail on 4.x-dev...

Note: Once we raised the requirement for Matomo to PHP 7.3 or higher, we can remove the special handling for PHP 8 tests again and update PHPUnit globally, as newer version of PHPUnit are not compatible with PHP 7.2.

@tsteur commented on August 16th 2021 Member

👍 looks good. Great to have it working 🎉

This Pull Request was closed on August 17th 2021
Powered by GitHub Issue Mirror