https://github.com/matomo-org/matomo/pull/16897 - Mouve

Site d'origine

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

Run AllTests on PHP8 #16897

Draft
wants to merge 15 commits into
base: 4.x-dev
Choose a base branch
from
Draft

Run AllTests on PHP8 #16897

wants to merge 15 commits into from

Conversation

@sgiehl
Copy link
Member

@sgiehl sgiehl commented Dec 7, 2020

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 sgiehl added this to the 4.1.0 milestone Dec 7, 2020
@sgiehl sgiehl force-pushed the php8tests branch 8 times, most recently from a4f0a32 to 0869f11 Dec 7, 2020
@sgiehl
Copy link
Member Author

@sgiehl sgiehl commented Dec 7, 2020

@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.

@sgiehl sgiehl force-pushed the php8tests branch 2 times, most recently from ade2b73 to f9482e3 Dec 7, 2020
@tsteur
Copy link
Member

@tsteur tsteur commented Dec 7, 2020

@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
Copy link
Member Author

@sgiehl sgiehl commented Dec 7, 2020

@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 sgiehl mentioned this pull request Dec 8, 2020
0 of 9 tasks complete
@sgiehl sgiehl force-pushed the php8tests branch 6 times, most recently from 4bde232 to df86948 Dec 8, 2020
@sgiehl sgiehl mentioned this pull request Dec 9, 2020
0 of 9 tasks complete
@sgiehl sgiehl force-pushed the php8tests branch 2 times, most recently from cc3a2f6 to 3769f1f Dec 9, 2020
@sgiehl
Copy link
Member Author

@sgiehl sgiehl commented Dec 10, 2020

Some tests are failing due to adjustments required in TCPDF. Those should be fixed with tecnickcom/TCPDF#293

Waiting for a new release to get that fixed...

@sgiehl sgiehl force-pushed the php8tests branch 4 times, most recently from 109b850 to e8d49b3 Dec 10, 2020
@sgiehl
Copy link
Member Author

@sgiehl sgiehl commented Feb 16, 2021

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

@Findus23
Copy link
Member

@Findus23 Findus23 commented Feb 16, 2021

🤦 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
Copy link
Contributor

@flamisz flamisz commented Feb 16, 2021

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
Copy link
Member Author

@sgiehl sgiehl commented Feb 17, 2021

@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

@mattab mattab modified the milestones: 4.2.0, 4.3.0 Feb 22, 2021
@sgiehl sgiehl force-pushed the php8tests branch from 515dda7 to a3e0f1d Mar 15, 2021
@sgiehl sgiehl force-pushed the php8tests branch from 7a2769b to 62f8269 Mar 23, 2021
@sgiehl sgiehl mentioned this pull request Mar 29, 2021
0 of 10 tasks complete
@sgiehl sgiehl force-pushed the php8tests branch from 62f8269 to 6b8f3a7 Mar 29, 2021
@sgiehl sgiehl force-pushed the php8tests branch from 6b8f3a7 to 40cc41b Apr 7, 2021
@sgiehl sgiehl force-pushed the php8tests branch from 40cc41b to 439ad3b Apr 14, 2021
@sgiehl sgiehl force-pushed the php8tests branch from 439ad3b to bc7d78f Apr 21, 2021
@sgiehl sgiehl force-pushed the php8tests branch from bc7d78f to 08abf20 May 10, 2021
sgiehl added 15 commits Dec 6, 2020
…known named parameters
@sgiehl sgiehl force-pushed the php8tests branch from 08abf20 to 919010c May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants

level de log mis à 1

Récupération du document https://github.com/matomo-org/matomo/pull/16897.

Récupération de l'URL https://github.com/matomo-org/matomo/pull/16897...

Raccourcis

Commandes

Fermer