WordPress.org

Make WordPress Core

Opened 15 months ago

Closed 8 months ago

Last modified 4 weeks ago

#40109 closed task (blessed) (fixed)

Ensure PHP 7.2 compatibility in tests (and core)

Reported by: ocean90 Owned by: johnbillion
Milestone: 4.9 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: needs-patch
Focuses: Cc:

Description

The lastest Travis build for 7.2-dev has a few errors. I haven't found a roadmap but I think PHP 7.2 is planned for end of this year. So, while PHP 7.2 is still at the beginning we should start looking at the results and review each error. We did the same for PHP 7.1 and identified a bug in PHP itself, see #37295.


PHP 7.2 is going to deprecate

  • create_function()
  • __autoload()
  • each()
  • calling assert() with a string argument

Also, counting of non-countable objects will start throwing a warning in PHP 7.2.


  • create_function() is handled in #37082.
  • The assert() calls in WP_Import_UnitTestCase::_import_wp() should be replaced with assertTrue() and a custom message.
  • testXMLParser::parse(): sizeof() returns always 1 for a string, even if the string is empty. Same for false. Seems like we can just pass true as the third argument for xml_parse().

Attachments (7)

40109.patch (1.6 KB) - added by ocean90 15 months ago.
40109.2.patch (660 bytes) - added by desrosj 14 months ago.
Remove each() call.
40109.3.patch (6.0 KB) - added by desrosj 14 months ago.
Removes all each() calls, including those in Text_diff, Snoopy and in unit tests.
40109.4.patch (6.0 KB) - added by desrosj 14 months ago.
Same as previous, but removed an extra stray {
40109-string-asserts.patch (2.8 KB) - added by desrosj 14 months ago.
Fixes all assert() calls with strings passed.
41134.3.patch (5.9 KB) - added by ayeshrajans 11 months ago.
Patch to move the spl_autoload polyfils and autoload declaration
40109.5.patch (2.7 KB) - added by johnbillion 10 months ago.
Avoid counting some uncountable items

Download all attachments as: .zip

Change History (43)

@ocean90
15 months ago

#1 @ocean90
15 months ago

  • Type changed from defect (bug) to task (blessed)

@desrosj
14 months ago

Remove each() call.

#3 follow-up: @desrosj
14 months ago

Added a patch to #37082 for all create_function() calls in unit tests.

Just added a patch here for removing each() calls. There are 16 other occurrences, but all within Snoopy and the Text_diff library.

I believe Snoopy is an abandoned project (hasn't been updated since 2014). Is this maintained in WordPress as a fork? I can update the patch if so.

Text_Diff also appears to not be maintained.

#4 in reply to: ↑ 3 @netweb
14 months ago

Replying to desrosj:

I believe Snoopy is an abandoned project (hasn't been updated since 2014). Is this maintained in WordPress as a fork? I can update the patch if so.

A quick look at the history of wp-includes/class-snoopy.php shows it's been continuing to receive compatibility updates where and when required these past few years since being deprecated ~7 years ago r14052

@desrosj
14 months ago

Removes all each() calls, including those in Text_diff, Snoopy and in unit tests.

@desrosj
14 months ago

Same as previous, but removed an extra stray {

@desrosj
14 months ago

Fixes all assert() calls with strings passed.

#5 @johnbillion
13 months ago

  • Milestone changed from Future Release to 4.8

Moving to 4.8 for visibility, in case we don't get 4.9 out before PHP 7.2.

#6 @johnbillion
13 months ago

In 40555:

Feeds: Remove an incorrect usage of sizeof() in a helper class used during unit testing of XML element handling.

This also helps tidy up the test output when running PHP 7.2.

See #40109

#7 @ocean90
13 months ago

In 40649:

Import: Replace assert() calls with PHPUnit's assertTrue() method.

Calling assert() with a string argument will be deprecated in PHP 7.2.

See #40109.

#8 @swissspidy
13 months ago

Regarding a roadmap for PHP 7.2 I only found this: https://wiki.php.net/todo/php72

This ticket was mentioned in Slack in #core by obenland. View the logs.


12 months ago

#10 @obenland
12 months ago

@ocean90 Could you bring this over the finish line when you get a chance?

#11 @ocean90
12 months ago

  • Milestone changed from 4.8 to Future Release

Yep, but not for 4.8.

#12 @pento
11 months ago

  • Milestone changed from Future Release to 4.9

WordPress 4.9 will be out before PHP 7.2, WordPress 5.0 may or may not be.

#13 @swissspidy
11 months ago

#41134 was marked as a duplicate.

#14 @ayeshrajans
11 months ago

Excerpt from #41134, I submitted a patch to remove move the legacy autoloader code to a new file and include it only if SPL is not available, and that fixed the deprecation notice in PHP 7.2, without removing the SPL polyfill. I will attach that patch here as well.

@ayeshrajans
11 months ago

Patch to move the spl_autoload polyfils and autoload declaration

#15 @johnbillion
10 months ago

  • Priority changed from low to normal

FYI: PHP 7.2 alpha has been tagged and is now available on Travis as 7.2.

#16 @ayeshrajans
10 months ago

Also, PHP 7.2 is now branched off and has feature frozen (https://externals.io/message/99903).

#17 @johnbillion
10 months ago

Opened #41457 for the incorrect use of count() in WP_REST_Comments_Controller::check_read_post_permission().

@johnbillion
10 months ago

Avoid counting some uncountable items

#18 @johnbillion
10 months ago

In 41174:

General: Avoid counting uncountable values when reading theme directories, and in some unit tests.

See #40109

#19 @jorbin
10 months ago

In 41175:

Build/Test Tools: Add PHP 7.2 to test matrix

PHP has branched 7.2 and master is now 7.3. This change ensures that 7.2 is tested. As 7.2 is still in development and is not ready for production, failures are allowed.

See #40109.

#20 @johnbillion
10 months ago

In 41177:

Build/Test Tools: More PHP 7.2 tweaks for Travis CI.

See #40109

#21 @johnbillion
10 months ago

In 41178:

General: Move the __autoload() compat function into its own file to prevent deprecated notices being thrown by the compiler in PHP 7.2.

The __autoload() function is deprecated in PHP 7.2, which means WordPress' own __autoload() compat function for PHP 5.2 needs to be moved into a separate file to prevent the PHP 7.2 compiler from complaining.

Props ayeshrajans

See #40109

#22 @johnbillion
10 months ago

  • Keywords needs-patch added; has-patch removed

At least one of the fixes in 40109.4.patch for usage of each() is buggy because the array that's iterated over is altered during iteration. Needs a more comprehensive patch.

#23 @johnbillion
10 months ago

The each() issue is potentially complex, so I'm splitting each (heh) usage into its own ticket. First up: #41524

#24 @johnbillion
10 months ago

Two more:

Let's not bother trying to remove the usage of each() in the Snoopy class, because it's in a deprecated file.

This ticket was mentioned in Slack in #core by sergey. View the logs.


8 months ago

#26 @johnbillion
8 months ago

In 41636:

Build/Test tools: Update the WP_PHPUnit_Util_Getopt class for PHP 7.2 compatibility.

This removes usage of each() which is deprecated in PHP 7.2.

Props ayeshrajans

See #40109
Fixes #41525

#27 @johnbillion
8 months ago

In 41637:

Build/Test tools: Update the WP_PHPUnit_Util_Getopt class for PHP 7.2 compatibility.

This removes usage of each() which is deprecated in PHP 7.2.

See #40109, #41525

Merges [41636] to the 4.8 branch.

#28 @johnbillion
8 months ago

In 41730:

I18N: Improvements to the tests for plural forms.

  • Move the create_function() code into a file that's only loaded, and into a test that's only run, on PHP <= 7.2 to avoid deprecated warnings in 7.2+.
  • Convert the test skipping into a failure if the GlotPress locale file cannot be downloaded.
  • Ensure test_exceptions fails if an exception is not thrown.
  • Docs improvements

See #41562, #40109

#29 @johnbillion
8 months ago

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

In 41748:

Build/Test tools: Remove PHP 7.2 from the allowed failures list on Travis CI.

Fixes #40109

#30 @johnbillion
8 months ago

Whoops, the #41524 sub-ticket still needs fixing.

#32 @westonruter
7 months ago

In 42146:

Settings: Replace count() call with empty() in get_settings_errors() to prevent PHP 7.2 warnings when $wp_settings_errors is null.

Props pross, dd32, westonruter.
See #40109.
Fixes #42498 for trunk.

#33 @westonruter
7 months ago

In 42147:

Settings: Replace count() call with empty() in get_settings_errors() to prevent PHP 7.2 warnings when $wp_settings_errors is null.

Props pross, dd32, westonruter.
See #40109.
Fixes #42498 for 4.9.

This ticket was mentioned in Slack in #meta by otto42. View the logs.


6 months ago

#35 @SergeyBiryukov
4 weeks ago

In 43039:

Themes: Avoid a PHP 7.2 warning in get_theme_roots() when $wp_theme_directories is an uncountable value.

See [41174] for wp_get_themes() and get_raw_theme_root().

Props burlingtonbytes, teddytime, lbenicio, desrosj.
Fixes #43374. See #40109.

#36 @SergeyBiryukov
4 weeks ago

In 43040:

Themes: Avoid a PHP 7.2 warning in get_theme_roots() when $wp_theme_directories is an uncountable value.

See [41174] for wp_get_themes() and get_raw_theme_root().

Props burlingtonbytes, teddytime, lbenicio, desrosj.
Merges [43039] to the 4.9 branch.
Fixes #43374. See #40109.

Note: See TracTickets for help on using tickets.