Skip to:
Content

BuddyPress.org

Opened 2 years ago

Closed 23 months ago

Last modified 2 months ago

#8649 closed defect (bug) (fixed)

Improves PHP 8.1 compatibility

Reported by: imath's profile imath Owned by: imath's profile imath
Milestone: 11.0.0 Priority: normal
Severity: normal Version:
Component: Core Keywords: has-patch has-unit-tests
Cc:

Description

There are a lot of deprecation notices in PHP 8.1 + we need to use PHPUnit 9.0 + we need to really enjoy Yoast PHPUnit polyfills editing our PHPUnit testcase methods and replace some deprecated Assertions.

Change History (22)

This ticket was mentioned in PR #13 on buddypress/buddypress by imath.


2 years ago
#2

  • Keywords has-unit-tests added

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


23 months ago

#4 @espellcaste
23 months ago

I left some code review suggestions in the pull request.

#5 @imath
23 months ago

Thanks a lot, I'll try to update the PR asap!

imath commented on PR #13:


23 months ago
#6

@renatonascalves Thanks a lot for your recommandations. I've applied them and ran tests locally using PHP 7.4.30 and PHP 8.1.9. In both cases everything went fine. Could you have another look at it?

renatonascalves commented on PR #13:


23 months ago
#7

@imath Could you also add support for the Github Action to run in pull requests?

imath commented on PR #13:


23 months ago
#8

@renatonascalves Just did, I've also included PHP 8.1 into the main PHP matrix.

renatonascalves commented on PR #13:


23 months ago
#9

@imath Cool! I think the last task is adding support for PHPUnit here: https://github.com/buddypress/buddypress/blob/master/.github/workflows/unit-tests.yml#L61-L62

"phpunit/phpunit": "^7.5",

renatonascalves commented on PR #13:


23 months ago
#10

Looks like there is an actual bug in the unit tests. 🤔

rafiahmedd commented on PR #13:


23 months ago
#11

https://i0.wp.com/user-images.githubusercontent.com/66135964/183944099-388cd326-ca03-4d7e-a632-b240c6e4bf3f.png
Hi,

I just run the test on the master branch, however, I couldn't find any issues on the test. Am I doing anything wrong here?

renatonascalves commented on PR #13:


23 months ago
#12

@rafiahmedd Are you using WordPress 5.7?

imath commented on PR #13:


23 months ago
#13

@renatonascalves yes there are bugs related to WordPress 5.7. I believe it's due to the fact wp-phpunit/wp-phpunit:5.7 doesn't include WordPress changes about using Yoast polifyier ((I believe these were included in 5.9).

I think @rafiahmedd tested on BuddyPress master, not this PR's branch 😉.

I'm wondering if it wouldn't fail if we were simply removing these lines into our GH action: https://github.com/buddypress/buddypress/blob/412b003469beef19af079e428f0e40ee5bd09887/.github/workflows/unit-tests.yml#L61|L63

It should use wp-phpunit/wp-phpunit:6.0 which includes Yoast polifyier... I'll try to give it a try locally.

renatonascalves commented on PR #13:


23 months ago
#14

I'd think it was going to fail since WordPress 5.7 won't won't work with "phpunit/phpunit": "^7.5". But maybe it will work with wp-phpunit/wp-phpunit:6.0.

It's worth a test.

rafiahmedd commented on PR #13:


23 months ago
#15

@imath yeah you are right I tested this with the master branch. I will check this from this branch.

imath commented on PR #13:


23 months ago
#16

Touchdown 🏈 @renatonascalves I was right about my intuition, using the wp-phpunit/wp-phpunit and PHPunit version for all tests fixes the failing tests under WP 5.7. I believe the risk of doing so is very limited as we only rely on WP Unit Tests class / factory and not on testcases. What's your opinion about it?

rafiahmedd commented on PR #13:


23 months ago
#17

The latest changes working perfectly. I think we can go with this one.

imath commented on PR #13:


23 months ago
#18

Awesome many thanks to you both @renatonascalves & @rafiahmedd I'll commit changes on trunk asap 🍾🥂

#19 @imath
23 months ago

In 13312:

Avoid various PHP 8.1 deprecated notices

Here are the treated deprecated notices:

  • ctype_digit() Argument of type int will be interpreted as string in the future.
  • Automatic conversion of false to array.
  • Passing null to strtotime() $datetime parameter.

Props renatonascalves, rafiahmedd

See #8649

#20 @imath
23 months ago

In 13313:

Remove composer.lock to avoid composer install issues in GH action

Props renatonascalves, rafiahmedd

See #8649

#21 @imath
23 months ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 13314:

Fully enjoy Yoast’s PHPUnit polyfills

Using these polyfills let us use PHPUnit v9.x for our tests and add PHP 8.1 to our testing matrix. Some additional edits to our PHP unit tests suite were needed:

  • Stop using PHPunit deprecated functions.
  • Rename some BP_UnitTestCase methods to use Yoast's polyfills.
  • Edit the PHP Unit test GH action and also run this action on pull requests.
  • Update some composer dependencies, remove the one about phpunit/phpunit:^7.5 and add a new composer script to use PHPUnit v9.x.

Props renatonascalves, rafiahmedd

Closes https://github.com/buddypress/buddypress/pull/13
Fixes #8649

#22 @imath
22 months ago

In 13323:

Make sure to run a type terms query when the types cache is empty

[13312] introduced a regression preventing group/member types stored as WP Terms to be retrieved.

Let's bring the right behavior back.

See #8649
Fixes #8736

#23 @espellcaste
2 months ago

In 13793:

Cast values used in the ctype_digit function to a string.

Argument of type int will be interpreted as string in the future.

Closes https://github.com/buddypress/buddypress/pull/265
See #8649
Fixes #9125

Note: See TracTickets for help on using tickets.