Test on PHP 8

[mfn]

Evaluating what it takes to make things run on PHP 8.

Turns out most dev dependencies, which are not used in the actual unit test matrix, are not needed and by removing them there, we can at run the test suite on PHP 7.1 to PHP 8.0

[simPod]

@mfn AFAIK GA does not allow something in matrix to fail yet

[coveralls]

Coverage remained the same at 86.185% when pulling 290eb18 on mfn:mfn-gha into 8795c03 on webonyx:master.

[mfn]

@simPod

AFAIK GA does not allow something in matrix to fail yet

Not sure what you mean exactly, fail-fast is true by default and will abort the whole matrix if one job fails.

[simPod]

Yup, I remember that in travis you could specify allowed failures. So I guess that's the reason 8.0 was not tested yet as it could not be allowed to fail. Just a note, keep going :D

[mfn]

keep going

Well yes, that's it for now :)

Depends on whether this is acceptable but I figured since composer.json supports php8 but you can't even install things locally until the dependencies are fixed, I figured it's also worth running the tests and we later remove the composer modifications once the issues are solved?

[simPod]

You can run --ignore-platform-req=php to install those.

[mfn]

Doesn't seem to want to play together with 7.1 => https://github.com/webonyx/graphql-php/pull/745/checks?check_run_id=1478696970#step:7:8

[simPod]

You should ignore platform php req only for php8. Now it installs incompatible phpunit (8.5) for php7.1

[mfn]

I thought so.

I'm just not sure this is the better approach?

In fact, I like my approach of removing dependencies not necessary for the tests, as it will also speed them up because they neither need to be resolved/installed/cached.

What do you think?

[simPod]

Just mentioned the possibility in reaction to 👇🏾 😄

but you can't even install things locally until the dependencies are fixed

But yes, the deps might not be compatible, eg. phpunit in v8.5 had few days ago extends Match in its code so that would crash anyway. Removing them is IMO fine, at least until we stop supporting 7.1.

[mfn]

Ok, activated git time travel and we're back to the composer remove approach 😅

[mfn]

I'll leave the git mess until we know what solution we want to merge (or not) and I'll clean them up then.

[mfn]

FTR, none of those dependencies are used anyway in this jobs matrix so the argument could be made we always want to remove them, speeds up composer (no resolving, no installing, no downloading, no caching, etc.).

But otherwise it could be deemed as temporarily solution until the dependency issues can be resolved.

[simPod]

I'd keep it as a temporary solution as it would introduce more complexity into CI. As for speed, it's micro optimalization and therefore not significant IMO.

[mfn]

Roger that!

[spawnia]

Nicely done

@vladar can we just merge PRs like this?

[vladar]

@spawnia Feel free to merge any PR when you feel confident enough! I trust you here 👍 In the worst-case, we can always revert.

[spawnia]

Thank you @vladar, will do!

[mfn]

Thank you @spawnia , @vladar and @simPod !