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

Backport support for the AssertObjectProperty polyfill #64

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Sep 23, 2023

PHPUnit 10.1.0 introduced two new assertions - assertObjectHasProperty() and assertObjectNotHasProperty() - to replace the assertObjectHasAttribute() and assertObjectNotHasAttribute() methods, which were deprecated in PHPUnit 9.6.0 and removed in PHPUnit 10.0.0.

These two new assertions have since been backported to PHPUnit 9.x in PHPUnit 9.6.11 to allow for getting rid of the deprecation notice and making preparing for PHPUnit 10 more smooth.

The PHPUnit Polyfills have followed suit and the 1.1.0 release contains polyfills for the backported assertObjectHasProperty() and assertObjectNotHasProperty() methods.

For the WP Test Utils package, this caused an interesting conundrum:

  • WP 6.4 - which enforces the use of PHPUnit Polyfills 1.1.0+ - will have the new polyfills available.
  • WP 5.9 - 6.3 will have the new polyfills available if the test dependencies are updated and PHPUnit Polyfills 1.1.0+ is used.
  • WP 5.2 - 5.9 versions, which include the backports, will not have the new polyfills available.
  • WP 5.2 - 5.9 versions, which don't include the backports, will not have the new polyfills available.
  • WP < 5.2 will not have the new polyfills available.

Previously the WP Test Utils would have two test cases available for integration tests:

  • One for use with WP 5.9+ and with WP versions < 5.9 which included the backports.
  • One for use with WP < 5.2 and with WP versions < 5.9 which do not include the backports.

To solve the conundrum, this commit:

  • Updates the minimum version of the PHPUnit Polyfills to 1.1.0.
  • Add the new polyfill to the test case which is used for WP < 5..2 and for WP versions < 5.9 which do not include the backports.
  • Introduces a third test case for integration tests against WP 5.2 - 5.8 versions, which do include the backports, but not the new polyfill. While this third polyfill might technically not be needed, I've seen too many issues with traits colliding on older PHP versions to be willing to take that risk.
  • Updates the autoloader to load the correct test case.

This should ensure that the new polyfill is available in all cases.

@jrfnl jrfnl added enhancement New feature or request yoastcs/qa labels Sep 23, 2023
@jrfnl jrfnl added this to the 1.x Next Release milestone Sep 23, 2023
@coveralls
Copy link

coveralls commented Sep 23, 2023

Coverage Status

coverage: 99.099%. remained the same when pulling b0a3817 on JRF/update-for-phpunit-polyfills-1.1.0 into 6de43fc on develop.

@jrfnl jrfnl force-pushed the JRF/update-for-phpunit-polyfills-1.1.0 branch 2 times, most recently from d74a1e1 to 1bc09eb Compare September 23, 2023 06:08
PHPUnit 10.1.0 introduced two new assertions - `assertObjectHasProperty()` and `assertObjectNotHasProperty()` - to replace the `assertObjectHasAttribute() and `assertObjectNotHasAttribute()` methods, which were deprecated in PHPUnit 9.6.0 and removed in PHPUnit 10.0.0.

These two new assertions have since been backported to PHPUnit 9.x in PHPUnit 9.6.11 to allow for getting rid of the deprecation notice and making preparing for PHPUnit 10 more smooth.

The PHPUnit Polyfills have followed suit and the 1.1.0 release contains polyfills for the backported `assertObjectHasProperty()` and `assertObjectNotHasProperty()` methods.

For the WP Test Utils package, this caused an interesting conundrum:
* WP 6.4 - which enforces the use of PHPUnit Polyfills 1.1.0+ - will have the new polyfills available.
* WP 5.9 - 6.3 will have the new polyfills available _if the test dependencies are updated and PHPUnit Polyfills 1.1.0+ is used_.
* WP 5.2 - 5.9 versions, which include the backports, will **not** have the new polyfills available.
* WP 5.2 - 5.9 versions, which **don't** include the backports, will **not** have the new polyfills available.
* WP < 5.2 will **not** have the new polyfills available.

Previously the WP Test Utils would have two test cases available for integration tests:
* One for use with WP 5.9+ and with WP versions < 5.9 which included the backports.
* One for use with WP < 5.2 and with WP versions < 5.9 which do not include the backports.

To solve the conundrum, this commit:
* Updates the minimum version of the PHPUnit Polyfills to 1.1.0.
* Add the new polyfill to the test case which is used for WP < 5..2 and for WP versions < 5.9 which do not include the backports.
* Introduces a third test case for integration tests against WP 5.2 - 5.8 versions, which _do_ include the backports, but not the new polyfill.
    While this third polyfill might technically not be needed, I've seen too many issues with traits colliding on older PHP versions to be willing to take that risk.
* Updates the autoloader to load the correct test case.

This should ensure that the new polyfill is available in all cases.
@jrfnl jrfnl force-pushed the JRF/update-for-phpunit-polyfills-1.1.0 branch from 1bc09eb to b0a3817 Compare September 24, 2023 06:35
Copy link

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

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

On local PHP 8.2, I:

  • updated composer.json to be dev-JRF/update-for-phpunit-polyfills-1.1.0
  • ran composer update
  • locally ran tests on a plugin - all OK.
  • updated $this->assertTrue( property_exists( $coauthor, 'ID' ) ); to be $this->assertObjectHasProperty( 'ID', $coauthor );
  • locally ran tests again - still all OK

(No surprises for PHP 8.2 and latest code.)

I then pushed the changes to GitHub which ran a workflow and ran tests on a selection of older WP/PHP including on WP 5.7 + PHP 7.1. They all passed.

Screenshot 2023-09-25 at 16 22 12

@jrfnl
Copy link
Collaborator Author

jrfnl commented Sep 27, 2023

Thank you @GaryJones for testing and confirming this works!

@jrfnl jrfnl merged commit b65370b into develop Sep 27, 2023
13 checks passed
@jrfnl jrfnl deleted the JRF/update-for-phpunit-polyfills-1.1.0 branch September 27, 2023 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request yoastcs/qa
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants