Skip to content
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

Allow for PHPUnit 11 #623

Merged
merged 7 commits into from
Sep 8, 2024
Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Sep 7, 2024

Composer: update the PHPUnit Polyfills to v 3.0

A PHPUnit 11 compatible version of the PHPUnit Polyfills has been released, so let's start using it.

Mind: PHPUnit 11 throws deprecation notices about the use of annotations instead of attributes.
On PHPUnit < 10.5.32 and PHPUnit < 11.3.3, these deprecation notices would fail the test run.

PHPUnit 10.5.32 and 11.3.3 introduce two new options for both the configuration file and as CLI arguments: displayDetailsOnPhpunitDeprecations and failOnPhpunitDeprecation, both of which will now default to false.

These options have been added to the PHPUnit configuration for PHPUnit 10/11 to safeguard them against changes in the default value.
This does mean that the configuration is no longer compatible with PHPUnit 10.0 - 10.5.31 and 11.0 - 11.3.2 and would fail the test run on seeing those config options.

With this in mind, it could be considered to set an explicit PHPUnit dependency to ensure the test suite isn't run on lower PHPUnit 10/11 versions, but as that is unlikely anyway, as Composer should always install the most recent version of PHPUnit, I deem this unnecessary.

Ref:

GH Actions/PHPUnit config: deal with removed --coverage-cache CLI option

The --coverage-cache flag was silently deprecated in PHPUnit 10 and has been removed in PHPUnit 11.

The --cache-directory flag replaces it and can also be set in the XML file.

This commit removes the outdated flag from the commands used in GH Actions and adds the attribute to the PHPUnit config file.

Ref:

UtilityMethodTestCase: add Before/After[Class] attributes

PHPUnit 10 introduced attributes as replacements for docblock annotations.
PHPUnit 11 deprecates the use of docblock annotations in favour of attributes.

If both an attribute as well as an annotation are found, no PHPUnit deprecation warning will be thrown.

This commit adds the Before/After* attributes in the user-facing UtilityMethodTestCase class to prevent such deprecation notices.

The "annotations to attributes" conversion for the PHPCSUtils native test suite is not yet necessary and is left for the update to PHPUnit 12.

The @before/after* annotations remain as the tests also still need to run on PHP 5.4 - 8.0 using PHPUnit 4.x - 9.x.
These can be removed once the codebase has a PHP 8.1/PHPUnit 10 minimum requirement.

Note: due to the syntax for attributes, these can be safely added as they are ignored as comments on PHP < 8.0.
Along the same line, if there is no "listener" for the attributes (PHP 8.0/PHPUnit 9.x), they are ignored by PHP as well.

Also note that the addition of the attributes, now adds an explicit runtime dependency on PHPUnit for the UtilityMethodTestCase, while previously this dependency was implicit.
By rights, this means that the phpunit/phpunit package dependency should be moved in the composer.json file from require-dev to require. I deem this undesirable though, as for most end-users, who use external standards which use PHPCSUtils, it won't be a dependency. The dependency is realistically only for the tests of external standards using the UtilityMethodTestCase, in which case, the external standard is expected to have a dependency on PHPUnit anyway.

Tests/PolyfilledTestCase: make compatible with PHPUnit Polyfills 3.x

As the available polyfill traits are different between the 1.x, 2.x and 3.x versions of the PHPUnit Polyfills, a different class definition is needed for PHPUnit Polyfills 3, which removes two traits (one used in this class) and introduces three new traits.

This commit adds the version-conditional class declaration for PHPUnit Polyfills 3.0.

Ref:

Tests/AbstractArrayDeclarationSniffTest: work around for MockBuilder::getMockForAbstractClass() deprecation

The MockBuilder::getMockForAbstractClass() is soft deprecated in PHPUnit 10, hard deprecated in PHPUnit 11 and will be removed in PHPUnit 12.

While it is not strictly necessary to address this deprecation now, I've chosen to do so anyway.

Please note:

  • The getMockBuilder() class is also deprecated now, so replacing with another method call on the MockBuilder class will only work in the short/medium term and in a future iteration, this will have to be refactored again.

Refs:

Tests/AbstractArrayDeclarationSniffTest: work around for TestCase::onConsecutiveCalls() deprecation

The TestCase::onConsecutiveCalls() is soft deprecated in PHPUnit 10, hard deprecated in PHPUnit 11 and will be removed in PHPUnit 12.

While it is not strictly necessary to address this deprecation now, I've chosen to do so anyway.

Note: the method call is not directly changed from ->will($this->onConsecutiveCalls(...)) to ->willReturn(...) - which is the PHPUnit native recommendation -, as this would make the test incompatible with older PHPUnit versions.

Additionally, a previous fix to work around the deprecation of InvocationMocker->withConsecutive() already uses the willReturnCallback() method, so the return value expectations will now need to be set via the ExpectWithConsecutiveArgs::setExpectationWithConsecutiveArgs() method.

Refs:

Docs: minor fixes

... picked up along the way.

A PHPUnit 11 compatible version of the PHPUnit Polyfills has been released, so let's start using it.

Mind: PHPUnit 11 throws deprecation notices about the use of annotations instead of attributes.
On PHPUnit < 10.5.32 and PHPUnit < 11.3.3, these deprecation notices would fail the test run.

PHPUnit 10.5.32 and 11.3.3 introduce two new options for both the configuration file and as CLI arguments: `displayDetailsOnPhpunitDeprecations` and `failOnPhpunitDeprecation`, both of which will now default to `false`.

These options have been added to the PHPUnit configuration for PHPUnit 10/11 to safeguard them against changes in the default value.
This does mean that the configuration is no longer compatible with PHPUnit 10.0 - 10.5.31 and 11.0 - 11.3.2 and would fail the test run on seeing those config options.

With this in mind, it could be considered to set an explicit PHPUnit dependency to ensure the test suite isn't run on lower PHPUnit 10/11 versions, but as that is unlikely anyway, as Composer should always install the most recent version of PHPUnit, I deem this unnecessary.

Ref:
* https://github.com/Yoast/PHPUnit-Polyfills/releases/tag/3.0.0
…ption

The `--coverage-cache` flag was silently deprecated in PHPUnit 10 and has been removed in PHPUnit 11.

The `--cache-directory` flag replaces it and can also be set in the XML file.

This commit removes the outdated flag from the commands used in GH Actions and adds the attribute to the PHPUnit config file.

Ref:
* sebastianbergmann/phpunit 4599
PHPUnit 10 introduced attributes as replacements for docblock annotations.
PHPUnit 11 deprecates the use of docblock annotations in favour of attributes.

If both an attribute as well as an annotation are found, no PHPUnit deprecation warning will be thrown.

This commit adds the `Before/After*` attributes in the user-facing `UtilityMethodTestCase` class to prevent such deprecation notices.

_The "annotations to attributes" conversion for the PHPCSUtils native test suite is not yet necessary and is left for the update to PHPUnit 12._

The `@before/after*` annotations remain as the tests also still need to run on PHP 5.4 - 8.0 using PHPUnit 4.x - 9.x.
These can be removed once the codebase has a PHP 8.1/PHPUnit 10 minimum requirement.

Note: due to the syntax for attributes, these can be safely added as they are ignored as comments on PHP < 8.0.
Along the same line, if there is no "listener" for the attributes (PHP 8.0/PHPUnit 9.x), they are ignored by PHP as well.

Also note that the addition of the attributes, now adds an _explicit_ runtime dependency on PHPUnit for the `UtilityMethodTestCase`, while previously this dependency was _implicit_.
By rights, this means that the `phpunit/phpunit` package dependency should be moved in the `composer.json` file from `require-dev` to `require`. I deem this undesirable though, as for most _end-users_, who use external standards which use PHPCSUtils, it won't be a dependency. The dependency is realistically only for the _tests_ of external standards using the `UtilityMethodTestCase`, in which case, the external standard is expected to have a dependency on PHPUnit anyway.
As the available polyfill traits are different between the 1.x, 2.x and 3.x versions of the PHPUnit Polyfills, a different class definition is needed for PHPUnit Polyfills 3, which removes two traits (one used in this class) and introduces three new traits.

This commit adds the version-conditional class declaration for PHPUnit Polyfills 3.0.

Ref:
* https://github.com/Yoast/PHPUnit-Polyfills/releases/tag/3.0.0
…::getMockForAbstractClass()` deprecation

The `MockBuilder::getMockForAbstractClass()` is soft deprecated in PHPUnit 10, hard deprecated in PHPUnit 11 and will be removed in PHPUnit 12.

While it is not strictly necessary to address this deprecation now, I've chosen to do so anyway.

Please note:
* The `getMockBuilder()` class is also deprecated now, so replacing with another method call on the `MockBuilder` class will only work in the short/medium term and in a future iteration, this will have to be refactored again.

Refs:
* sebastianbergmann/phpunit 5241
* sebastianbergmann/phpunit 5247
* sebastianbergmann/phpunit 5252
…nConsecutiveCalls()` deprecation

The `TestCase::onConsecutiveCalls()` is soft deprecated in PHPUnit 10, hard deprecated in PHPUnit 11 and will be removed in PHPUnit 12.

While it is not strictly necessary to address this deprecation now, I've chosen to do so anyway.

Note: the method call is not directly changed from `->will($this->onConsecutiveCalls(...))` to `->willReturn(...)` - which is the PHPUnit native recommendation -, as this would make the test incompatible with older PHPUnit versions.

Additionally, a previous fix to work around the deprecation of `InvocationMocker->withConsecutive()` already uses the `willReturnCallback()` method, so the return value expectations will now need to be set via the `ExpectWithConsecutiveArgs::setExpectationWithConsecutiveArgs()` method.

Refs:
* sebastianbergmann/phpunit 5423
* sebastianbergmann/phpunit 5424
... picked up along the way.
@jrfnl jrfnl force-pushed the feature/tests-compatibility-with-phpunit-11 branch from 5981d50 to 00721ba Compare September 8, 2024 01:53
@jrfnl jrfnl merged commit daea84b into develop Sep 8, 2024
56 checks passed
@jrfnl jrfnl deleted the feature/tests-compatibility-with-phpunit-11 branch September 8, 2024 02:00
@fredden
Copy link
Member

fredden commented Oct 12, 2024

This does mean that the configuration is no longer compatible with PHPUnit 10.0 - 10.5.31 and 11.0 - 11.3.2 and would fail the test run on seeing those config options.

This sounds like a good use of the conflict section of composer.json.

@jrfnl
Copy link
Member Author

jrfnl commented Oct 12, 2024

This does mean that the configuration is no longer compatible with PHPUnit 10.0 - 10.5.31 and 11.0 - 11.3.2 and would fail the test run on seeing those config options.

This sounds like a good use of the conflict section of composer.json.

@fredden I don't think it is. This is an issue which only affects this package. Adding the conflict directive would affect packages which depend on this package, while, whether their test setup is compatible with those PHPUnit versions is not influenced by this package.
As said in the original commit message: if I'd want to address this, it should be done by setting an explicit require-dev for PHPUnit (for this package only).

Am I missing something ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants