Compatibility with recent PHPUnit (8)

[DavidPrevot]

No description provided.

[Zegnat]

We use PHPUnit 7.5, which does not do any type definitions for setup. So is this a commit for PHPUnit 8 (8.4?) compatibility?

Here the same question arises as in #130: are we planning to drop PHP 7.1 support? That seems to be the requirement for upgrading to PHPUnit 8, which requires at least PHP 7.2 to run.

[DavidPrevot]

Hi,

is this a commit for PHPUnit 8 (8.4?) compatibility?

Indeed it is. This syntax is already accepted by PHPUnit 7 this project is currently using, hence this proposal: the tests still pass with the current setup, and won’t break when updating PHPUnit.

The PHP version used by this project is orthogonal to this PR, it may just make it a bit easier if it changes.

[Zegnat]

Would love to do this in addition to then adding || ^8.4 to the PHPUnit requirement, just so there would be some actual forward momentum. A change to return type just for change’s sake never sits well with me, though it is a perfectly valid commit.

Sadly, because of upstream php-http/psr7-integration-tests we aren’t compatible with PHPUnit 8 at all. Adding the void return type doesn’t change this.

Not sure how I’d personally progress on this, I’ll leave that for @Nyholm to decide.

[GrahamCampbell]

Why add this, when the composer.json file doesn't allow PHPUnit 8?

[Nyholm]

I don't really see any benefits for this at the moment.

Sure, we can do it as "why not". But we need to make sure the test dependencies also support PHPUnit 8

[Nyholm]

I've update composer.json to support phpunit 8.

I've also made sure php-http/psr7-integration-tests supports it too.

Thank you