WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 2 hours ago

#39822 reopened defect (bug)

PHPUnit 6 compatibility

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

Description

PHPUnit no longer supports PHPUnit_Framework_TestCase as the base class.
https://github.com/sebastianbergmann/phpunit/blob/6.0/ChangeLog-6.0.md#600---2017-02-03

$ phpunit 
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
PHP Fatal error:  Class 'PHPUnit_Framework_TestCase' not found in /Users/miyauchi/wp-core/trunk/tests/phpunit/includes/testcase.php on line 15

I wrote aliases for it.
It looks woking fine with phpunit 5.6 and phpunit 6.

Related:
https://github.com/wp-cli/wp-cli/issues/3802

Attachments (4)

39822.patch (2.1 KB) - added by miyauchi 4 months ago.
39822-2.patch (2.4 KB) - added by miyauchi 4 months ago.
39822.3.patch (11.7 KB) - added by gitlost 3 months ago.
Iteration of 39822-2.patch with shim for setExpectedException().
39822.4.patch (3.3 KB) - added by gitlost 3 months ago.
Use is_callable( 'parent::setExpectedException' ) instead of _setExpectedException().

Download all attachments as: .zip

Change History (31)

@miyauchi
4 months ago

#1 @jdgrimes
4 months ago

  • Keywords needs-refresh has-patch added

Won't the use of the namespace separator in that code cause a parse an error on PHP 5.2? I know that it won't actually be executed, because an older version of PHPUnit will be running on that old of a version of PHP. But WordPress still runs its tests against PHP 5.2, so this solution needs to be PHP 5.2 compatible. And because the code is a part of the file, even if it isn't executed, it will still have to get parsed.

I would say the best way to fix this would be to move the migration code to a separate file, that would only be included (and thus parsed), if ( class_exists( 'PHPUnit\Runner\Version' ) {}. That way the main file should still parse on PHP 5.2, and the migration file that requires a newer PHP version to parse won't be loaded then.

#2 follow-up: @miyauchi
4 months ago

Oh, that is exactly right!
Do you know any solution to include it for PHP 5.2.

if ( class_exists( 'PHPUnit\Runner\Version' ) {} doesn't work on PHP 5.2 right?

Thanks for your response.

#3 in reply to: ↑ 2 @jdgrimes
4 months ago

Replying to miyauchi:

I think that this should work fine on PHP 5.2:

<?php
if ( class_exists( 'PHPUnit\Runner\Version' ) {
    require_once dirname( __FILE__ ) . '/phpunit-6-compat.php';
}

In the phpunit-6-compat.php file would be the rest of the code, of course.

@miyauchi
4 months ago

#5 follow-up: @miyauchi
4 months ago

Thanks @jdgrimes , @jorbin

I updated the bootstrap.php and phpunit-6-compat.php.

But there are some errors on /usr/local/bin/phpunit --group ajax with PHPUnit 6 for now.
(Sorry I tried only /usr/local/bin/phpunit (without option) on my local machine.)

https://travis-ci.org/miya0001/WordPress/builds/200084016

I'll try to fix it tomorrow.

#6 @netweb
4 months ago

There is also a deprecation error for the HHVM job: https://travis-ci.org/miya0001/WordPress/jobs/200084031

Fatal error: Class already declared: PHPUnit_Framework_Error_Deprecated in phar://phpunit-5.7.7.phar/phpunit/Framework/Error/Deprecated.php on line 21

#7 in reply to: ↑ 5 @ocean90
3 months ago

Replying to miyauchi:

But there are some errors on /usr/local/bin/phpunit --group ajax with PHPUnit 6 for now.

setExpectedException() has been removed in PHPUnit 6: https://github.com/sebastianbergmann/phpunit/issues/2074

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


3 months ago

#9 @johnbillion
3 months ago

In 40255:

Build/Test tools: Explicitly use PHPUnit 5.7 for the PHP 7 builds on Travis.

This avoids fatal errors while the tests remain incompatible with PHPUnit 6, which was recently introduced on Travis.

See #39822, #40086

#10 @johnbillion
3 months ago

In 40257:

Build/Test tools: Account for PHP 5.2 when using Composer to install PHPUnit.

See #39822, #40086

@gitlost
3 months ago

Iteration of 39822-2.patch with shim for setExpectedException().

#11 @ocean90
3 months ago

In 40260:

Build/Test Tools: Update .travis.yml to include latest improvements from trunk.

  • Explicitly use PHPUnit 5.7 for the PHP 7 builds on Travis.
  • On Travis CI install and use the node version which is specified in package.json.
  • Add some more debugging to Travis and bring the format of the Xdebug fix inline with branches.

Merge of [40255] and [40257-40259] to the 4.7 branch.

Props netweb, johnbillion.
See #35105, #39822, #40086.

#12 @ocean90
3 months ago

In 40261:

Build/Test Tools: Update .travis.yml to include latest improvements from trunk.

  • Explicitly use PHPUnit 5.7 for the PHP 7 builds on Travis.
  • On Travis CI install and use the node version which is specified in package.json.
  • Add some more debugging to Travis and bring the format of the Xdebug fix inline with branches.

Merge of [40255] and [40257-40259] to the 4.6 branch.

Props netweb, johnbillion.
See #35105, #39822, #40086.

#13 @ocean90
3 months ago

In 40262:

Build/Test Tools: Update .travis.yml to include latest improvements from trunk.

  • Explicitly use PHPUnit 5.7 for the PHP 7 builds on Travis.
  • On Travis CI install and use the node version which is specified in package.json.
  • Add some more debugging to Travis and bring the format of the Xdebug fix inline with branches.

Merge of [40255] and [40257-40259] to the 4.5 branch.

Props netweb, johnbillion.
See #35105, #39822, #40086.

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


3 months ago

#15 @johnbillion
3 months ago

@gitlost In 39822.3.patch rather than replacing every instance of setExpectedException() with _setExpectedException(), you should be able to do something like this:

protected function setExpectedException( ... ) {
	if ( is_callable( 'parent::setExpectedException' ) ) {
		parent::setExpectedException( ... );
	} else {
		// shim code
	}
}

#16 @gitlost
3 months ago

Much nicer!

@gitlost
3 months ago

Use is_callable( 'parent::setExpectedException' ) instead of _setExpectedException().

#17 follow-up: @johnbillion
3 months ago

  • Keywords needs-refresh removed
  • Milestone changed from Awaiting Review to 4.8
  • Version trunk deleted

39822.4.patch Looks good.

#18 @robert.peake
6 weeks ago

Any ETA on when this will show up in https://develop.svn.wordpress.org/trunk/ ?

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


6 weeks ago

#21 @johnbillion
5 weeks ago

  • Owner set to johnbillion
  • Status changed from new to reviewing

#22 @johnbillion
5 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 40536:

Build/Test Tools: Add support for PHPUnit 6+.

This adds a compatibility shim for the new namespaced structure of PHPUnit and the removed setExpectedException() method. In addition, this updates the Travis config so PHPUnit 6.1 is used where appropriate.

Props miyauchi, gitlost.

Fixes #39822

#23 @johnbillion
5 weeks ago

  • Keywords needs-patch added; has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

This broke the PHP 5.2 build: https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/224813063

I'll look into it tomorrow.

#24 follow-up: @johnbillion
5 weeks ago

In 40539:

Build/Test Tools: Move the setExpectedException() method into the WP_Ajax_UnitTestCase class to avoid a fatal error when PHPUnit 3.6 is in use.

This needs more investigation but it gets the test suite on PHP 5.2 passing again.

See #39822

#25 @johnbillion
5 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 40547:

Build/Test Tools: Backport various recent changes to the 4.7 branch.

  • Add support for PHPUnit 6+.
  • Add Composer files to the cache on Travis.
  • Remove HHVM from the test infrastructure on Travis.

Merges [40536], [40538], [40539], and [40546] to the 4.7 branch.

See #40539
Fixes #39822, #40548

#26 in reply to: ↑ 24 @jdgrimes
28 hours ago

Replying to johnbillion:

In 40539:

Build/Test Tools: Move the setExpectedException() method into the WP_Ajax_UnitTestCase class to avoid a fatal error when PHPUnit 3.6 is in use.

This needs more investigation but it gets the test suite on PHP 5.2 passing again.

See #39822

I'm now having issues with this in a plugin, due to the fact that is_callable( 'parent::method' ) is apparently not supported on PHP 5.2 when the method is actually on the grandparent.

When using setExpectedException() in a testcase extending WP_Ajax_UnitTestCase, I now get this error:

Fatal error: Call to undefined method WordPoints_Breaking_Module_Check_Ajax_Test::expectException() in /tmp/wordpress/tests/phpunit/includes/testcase-ajax.php on line 215

However, method_exists( 'PHPUnit_Framework_TestCase', 'setExpectedException' ) would appear to work. See https://3v4l.org/it5tc#v512

#27 @johnbillion
2 hours ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
Note: See TracTickets for help on using tickets.