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

PHPUnit 11.3.1 | AssertIsList: sync error message with upstream #195

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Sep 6, 2024

PHPUnit 11.3.1 improved the type information in select assertion failure messages.

For the polyfills, this only affects the AssertIsList polyfill.

Now the choice was between the following:

  • Just update the test(s) to have a different message expectation for PHPUnit 11.3.1+.
  • Update the polyfills to mirror the improvement made in PHPUnit upstream.

I've chosen to implement the latter as it makes the message significantly more informative.

This means that on PHPUnit 6.x to 9.x and 11.3.1+, the new, improved message is shown. While on PHPUnit 10.0 - 11.3.0, the "old" message is shown as, in that case, the PHPUnit native assertIsList() method is used without the message improvements.

Includes:

  • Renaming the assertIsListFailureDescription() method parameter from $other to $value to be more descriptive.
  • Updating the tests, including adding an extra test with an anonymous class as that has separate handling for the error message in the polyfill.

Refs:

PHPUnit 11.3.1 improved the type information in select assertion failure messages.

For the polyfills, this only affects the `AssertIsList` polyfill.

Now the choice was between the following:
* Just update the test(s) to have a different message expectation for PHPUnit 11.3.1+.
* Update the polyfills to mirror the improvement made in PHPUnit upstream.

I've chosen to implement the latter as it makes the message significantly more informative.

This means that on PHPUnit 6.x to 9.x and 11.3.1+, the new, improved message is shown. While on PHPUnit 10.0 - 11.3.0, the "old" message is shown as, in that case, the PHPUnit native `assertIsList()` method is used without the message improvements.

Includes:
* Renaming the `assertIsListFailureDescription()` method parameter from `$other` to `$value` to be more descriptive.
* Updating the tests, including adding an extra test with an anonymous class as that has separate handling for the error message in the polyfill.

Refs:
* sebastianbergmann/phpunit@5e8b79e
* sebastianbergmann/phpunit@42cfc7d
@coveralls
Copy link

Coverage Status

coverage: 97.152% (+0.05%) from 97.104%
when pulling a0766bb on feature/3.x/assertislist-sync-error-message-with-upstream
into f0cd55e on 3.x.

Copy link
Collaborator

@hellofromtonya hellofromtonya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@hellofromtonya hellofromtonya merged commit e7fbd67 into 3.x Sep 6, 2024
215 checks passed
@hellofromtonya hellofromtonya deleted the feature/3.x/assertislist-sync-error-message-with-upstream branch September 6, 2024 19:41
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.

3 participants