WordPress PHPUnit Tests should run error free on PHP7 - 4.3 tracking ticket

[jorbin]

PHP7 is scheduled for release in October. In order to help facilitate that, we should get our PHPUnit tests in working order on PHP7.

Right now, there are a number of deprecated warnings and the tests don't run completely. We need to fix up the deprecated warnings and also ensure all tests run error free (while maintaining backwards compatability).

This ticket should be used for the small pieces. Larger changes that require there own discussion should be spun off into there own ticket.

This is a followup to #31454.

[johnbillion]

Here's a recent failing test for reference: ​https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/58632984 (not sure what's up with the WP_Import fatal).

[jdgrimes]

It looks like all of the deprecated warnings are caused by lingering PHP4 constructors:

Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP

Are we OK with removing PHP4 constructors? I don't know if there is any other way to avoid the notice, but we've been keeping these for backward compatibility.

[SergeyBiryukov]

The WP_Widget PHP4 constructor was kept for compatibility with plugins, see ticket:16768:9, #20801, #30482.

The others can probably be removed or changed to PHP5 constructors.

[jdgrimes]

Original RFC for this change in PHP: ​https://wiki.php.net/rfc/remove_php4_constructors

Of particular note:

// function filter is not used as constructor in PHP 5+
class Filter {
    function __construct() {}
 
    // PHP 5.0.0 - 5.2.13, 5.3.0 - 5.3.2: E_STRICT "Redefining already defined constructor"
    // PHP 5.2.14 - 5.2.17, 5.3.3 - 5.6: No error is raised
    // PHP 7: No error is raised
    // PHP 8: No error is raised
    function filter($a) {}
}

What this means is that we don't need to remove any of our PHP4 constructors (yay!). But we do need to make sure that all of the external libraries' classes have a __construct() defined before a PHP4 constructor. Some of them do not have a __construct() method in addition to their PHP4 constructor, and that is what is causing the errors. Note that the order in which the constructors appear is important, the PHP5 one must come before the PHP4 one.

[jdgrimes]

I'm compiling a list of classes which need PHP5 constructors added. Before I make up a patch, I'd like to know exactly how we want to handle this. We'll probably want to keep the PHP4 constructors for back-compat, but do we want to formally deprecate them? If not, should the new __construct() method call the PHP4 constructor, or should the logic be moved into __construct()?

[SergeyBiryukov]

Replying to jdgrimes:

If not, should the new __construct() method call the PHP4 constructor, or should the logic be moved into __construct()?

I'd suggest following the current WP_Widget approach:

• Move the logic to the new __construct() method.
• Call the new __construct() method in the PHP4 constructor.

[jdgrimes]

Replying to SergeyBiryukov:

I'd suggest following the current WP_Widget approach

OK, so the PHP4 constructors will just act as wrappers for the __construct() methods, and won't give any deprecation notices or anything.

I'll work on a patch.

[jdgrimes]

Replying to jdgrimes:

I'll work on a patch.

Now that trunk is 4.3, I'll try to get a patch up ASAP.

[netweb]

Now that we have a 4.3 milestone...

[jdgrimes]

PHP_CodeSniffer sniff used to generate the patch

[jdgrimes]

31982.diff​ is the first pass at a patch for the PHP5 constructors.

Notes:

• The ftp classes already had PHP5 constructors, but they were below the PHP4 constructors. This will give PHP Strict Standards: Redefining already defined constructor for class ftp_base, so I've moved the PHP5 constructors to above the PHP4 ones.
• I've added basic docblocks to most of the constructors, with just a short description ("PHP5 constructor"/"PHP4 constructor").
• I've update whitespace on most of the modified lines.
• I've mostly used tabs to indent things, even though a few of the files are space-indented. If anyone wants to clean this up, they are welcome to it.
• I've added the public keyword for many of the PHP4 constructors, some of which didn't have this before.
• FWIW PHP4ConstructorSniff.php​ is the PHPCS sniff I used to generate the initial patch (though I've modified both it and the patch since).

[netweb]

Patch 31982.1.diff is a refresh only of 31982.diff as it wouldn't apply /src/wp-includes/pomo/streams.php cleanly, tweaked the patch to include a new line at the end of file and appears to apply cleanly for me now.

[netweb]

I ran 31982.1.diff through Travis CI, the resulting job is -> ​https://travis-ci.org/ntwb/wordpress/jobs/60068715

All the deprecated warnings are gone, nice one @jdgrimes.

Just left with the WP_Import class not found error thus far.

...........................................SSSSSSSSSSS....... 1769 / 3348 ( 52%)
......EPHP Fatal error:  Class 'WP_Import' not found in /home/travis/build/ntwb/wordpress/tests/phpunit/tests/import/base.php on line 24
Fatal error: Class 'WP_Import' not found in /home/travis/build/ntwb/wordpress/tests/phpunit/tests/import/base.php on line 24
Warning:  Use --force to continue.
Aborted due to warnings.
The command "grunt $WP_TRAVISCI" exited with 3.
Done. Your build exited with 1.

[jdgrimes]

Fix deprecated warnings for WP_Import

[jdgrimes]

Thanks for testing it @netweb. WP_Import itself actually needs to be patched for deprecation errors (see wp-import.diff​). I'm not sure who needs to do that. Maybe that is causing the class not to be loaded for some reason? Probably not, but let's patch it and see if the error goes away.

[netweb]

The wp-import.diff patch should be added to #24373

Following that two discussions should be had:

1. In #24373 should this update create a new v0.6.2 release, with previous commits in that ticket still not merged into the current v0.6.1 release, or merged only into /trunk

2. Here on this ticket, should we change to using /trunk version of WordPress-Importer regardless of a tagged release in the plugin repo, the 1.0 beta release in /trunk is significantly improved, these improvements came from Frederick Ding's 2013 GSoC project but have never been deployed in a tagged release, only in /trunk

Edit: My bad, Frederick's enhancements are in the GSoC repo, none of these are in the plugins repo:

• ​https://gsoc.trac.wordpress.org/browser/2013/frederickding/importer
• ​https://plugins.trac.wordpress.org/browser/wordpress-importer/

[jdgrimes]

Replying to netweb:

The wp-import.diff patch should be added to #24373

Done.

Here on this ticket, should we change to using /trunk version of WordPress-Importer regardless of a tagged release in the plugin repo...

I think we already do. At least the previous changes from that ticket are in the version checked out with my core checkout.

[netweb]

Replying to jdgrimes:

I think we already do. At least the previous changes from that ticket are in the version checked out with my core checkout.

My bad, you are correct, we already do use /trunk :)

[netweb]

Related: #32480

[DrewAPicture]

Docs tweaks

[jorbin]

In 32989:

Add Deprecated Constructor Function

This function is one that can be called in core to indicate that a PHP4 style constructor is used. PHP4 style constructors are deprecated in PHP7.

Props jorbin, DrewAPicture for docs
See #31982

[jorbin]

In 32990:

Deprecate php4 style constructors

PHP7 is deprecating PHP4 style constructors, so we need to modify our code to have _construct methods that fire before the named PHP4 style constructors. The PHP4 style constructors will call the PHP5 style constructor in case it is being called directly (usually via parent::METHOD).

This modifies external libraries to add PHP5 style constructors, but doesn't add a notice for when they are used. In WordPress core code, PHP4 style constructors are being given a call to _deprecated_constructor. To the PHP4 style constructor I say "I know that I can't take no more | It ain't no lie | I wanna see you out that door | Baby, bye, bye, bye..."

Upstream: ​https://wiki.php.net/rfc/remove_php4_constructors

Props jdgrimes, netweb, jorbin
See #31982

[obenland]

In 33085:

Deprecate PHP4 style contructor in Twenty Eleven's Ephemera widget.

See #31982.

Props Frank Klein.
Fixes #32881.

[jorbin]

#32480 was marked as a duplicate.

[johnbillion]

There's no reason to retain the PHP4 constructors in our test suite. 31982.5.diff​ removes these.

[jorbin]

In 33123:

Remove PHP4 constructors from Unit Tests

If you are subclassing these classes in your own tests, you'll need to update your code.

Props johnbillion
See #31982

[jorbin]

I am going to mark this as completed for 4.3.

Our unit tests and code are in much better shape to support PHP7 (which just went to beta this week). There are some errors when running the tests that we can focus on fixing in 4.4 as we work to have better PHP7 support in time for it's official release.

If there are any specific regressions, please create a new ticket with as much detail as possible.

[ocean90]

Do we know the cause for the remaining failures related to nav menus?
​https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/72667233#L353-L414 is related to this line trunk/src/wp-includes/nav-menu.php@33256#L641​. Is it ​https://wiki.php.net/rfc/uniform_variable_syntax?

Edit:
The answer is yes, see the second example from the RFC.

                        // old meaning            // new meaning
$foo->$bar['baz']       $foo->{$bar['baz']}       ($foo->$bar)['baz']

[ocean90]

See ​https://wiki.php.net/rfc/uniform_variable_syntax

[ocean90]

31982.6.diff​ fixes the variable issue and tests are passing: ​https://travis-ci.org/ocean90/develop.wordpress/builds/72671062 \o/

@jorbin What do you think?

[jorbin]

I hadn't looked into it much, but that is the uniform variable syntax backwards incompatable change biting us. More info: ​https://wiki.php.net/rfc/uniform_variable_syntax#semantic_differences_in_existing_syntax. I'm going to commit the change so that we get fully green in php7.

[jorbin]

In 33427:

Use explicit variable variable syntax

PHP7 introduces a backwards compatable change to variable varibale syntax that requires us to use curly brackets to maintain the syntax between php5 and php7. For more info, see ​https://wiki.php.net/rfc/uniform_variable_syntax#semantic_differences_in_existing_syntax for the semantic differences.

Props ocean90
Fixes #31982

[netweb]

Awesome

[ocean90]

In 36599:

i18n-tools: Remove PHP4 constructor from add-textdomain.php.

_deprecated_constructor() isn't available in non-WordPress context.

See #31982.