PHP 8.0 support

[boesing]

Feature Request

Q	A
New Feature	yes

Summary

To be prepared for the december release of PHP 8.0, this repository has some additional TODOs to be tested against the new major version.

In order to make this repository compatible, one has to follow these steps:

• Modify composer.json to provide support for PHP 8.0 by adding the constraint ~8.0.0
• Modify composer.json to drop support for PHP less than 7.3
• Modify composer.json to implement phpunit 9.3 which supports PHP 7.3+
• Modify .travis.yml to ignore platform requirements when installing composer dependencies (simply add --ignore-platform-reqs to COMPOSER_ARGS env variable)
• Modify .travis.yml to add PHP 8.0 to the matrix (NOTE: Do not allow failures as PHP 8.0 has a feature freeze since 2020-08-04!)
• Modify source code in case there are incompatibilities with PHP 8.0

[jensdenies]

Hi @boesing, regarding the PHPUnit dependency upgrade: the assertions assertAttribute* have been removed #(sebastianbergmann/phpunit#3338). This was primarily done because it makes it too easy to test implementation details.

This repository seems to use these assertions quite a lot, what's the recommended upgrade path for the tests which use these assertions?

[lcobucci]

@jensdenies that really depends on the context in which those assertions are used.

Some can be addressed by comparing objects (assertEquals not assertSame, sure). However, there might be cases where the test method is just useless and needs to be removed or adjusted to use the public API.

Perhaps PHPUnit upgrade and dropping PHP 7.3 should be done in a separate PR, so that we can more easily contain the scope of changes?

[jensdenies]

Hi @lcobucci, several of these assertions are used to read private properties, which are indeed implementation details. Those properties do not always have an associated getter, so IMO it would be best to remove those and only test the public API of those classes.

Regarding your last comment, that would certainly make thing easier because the assertions are still available in PHPUnit 8. I think it would make the upgrade path a lot easier.

[jensdenies]

@lcobucci I've created a draft PR here: #55.

[boesing]

@jensdenies Imho, the current tests can be rewritten with reflection. Feel free to create some trait within the tests directory and share it with those tests which are using the removed assertAttribute* methods.

I wont recommend rewriting the whole testsuite as this would take too much time.
We can still consider this for the next major but for now, just use a self-written method which uses reflection.

We've done so in many other components aswell.

[jensdenies]

@boesing I will create a PR with a trait to polyfill those traits, like you proposed.

When looking at other Laminas repositories, I see that in some cases, tests which test implementation details are removed.

https://github.com/laminas/laminas-cache/pull/48/files#diff-5934f6347f914a564aa00ffada1a80a1ad425c8df17ff34de7e0dbe5efd921c0L25

Is that something you'd like to see in the PR as well?

[boesing]

@jensdenies Depends. The tests which were removed were tests to verify if backwards compatibility to servicemanager v2 works.
As we dropped support for the servicemanager v2 in laminas-cache, the tests were removed accordingly along with the implementation.

[jensdenies]

@boesing Okay, I will create a PR working on PHP 8.0 with a polyfill trait and let's see from there.

[Ocramius]

Handled in #59, #60, #62.