Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#39822 closed defect (bug) (fixed)

PHPUnit 6 compatibility

Reported by: miyauchi's profile miyauchi Owned by: johnbillion's profile johnbillion
Milestone: 4.8 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
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 8 years ago.
39822-2.patch (2.4 KB) - added by miyauchi 8 years ago.
39822.3.patch (11.7 KB) - added by gitlost 8 years ago.
Iteration of 39822-2.patch with shim for setExpectedException().
39822.4.patch (3.3 KB) - added by gitlost 8 years ago.
Use is_callable( 'parent::setExpectedException' ) instead of _setExpectedException().

Download all attachments as: .zip

Change History (41)

@miyauchi
8 years ago

#1 @jdgrimes
8 years 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
8 years 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
8 years 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
8 years ago

#5 follow-up: @miyauchi
8 years 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
8 years 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
8 years 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.


8 years ago

#9 @johnbillion
8 years 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
8 years ago

In 40257:

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

See #39822, #40086

@gitlost
8 years ago

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

#11 @ocean90
8 years 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
8 years 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
8 years 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.


8 years ago

#15 @johnbillion
8 years 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
8 years ago

Much nicer!

@gitlost
8 years ago

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

#17 follow-up: @johnbillion
8 years ago

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

39822.4.patch Looks good.

#18 @robert.peake
8 years 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.


8 years ago

#21 @johnbillion
8 years ago

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

#22 @johnbillion
8 years 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
8 years 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
8 years 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
8 years 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
8 years 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
8 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#28 @johnbillion
8 years ago

In 40853:

Build/Test Tools: Add a missing class to the PHPUnit 6 back compat.

See #39822

#29 @johnbillion
8 years ago

In 40854:

Build/Test Tools: Add a missing class to the PHPUnit 6 back compat.

See #39822

Merges [40853] to the 4.7 branch.

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


8 years ago

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


8 years ago

#32 @johnbillion
8 years ago

In 40872:

Build/Test Tools: Fix PHP 5.2 compatibility for grandchild methods which expect exceptions to be raised.

This is due to is_callable( 'parent::setExpectedException' ) not being supported on PHP 5.2 when the method being checked only exists on the grandparent class.

Props jdgrimes

See #39822

#33 @johnbillion
8 years ago

In 40873:

Build/Test Tools: I don't know how this happened.

See #39822

#34 @johnbillion
8 years ago

  • Keywords needs-patch removed

This is fixed in trunk. Needs back-porting to 4.7 branch and side-porting to 4.8 branch.

#35 @johnbillion
8 years ago

In 40875:

Build/Test Tools: Fix PHP 5.2 compatibility for grandchild methods which expect exceptions to be raised.

This is due to is_callable( 'parent::setExpectedException' ) not being supported on PHP 5.2 when the method being checked only exists on the grandparent class.

See #39822

Merges [40872] and [40873] to the 4.8 branch.

#36 @johnbillion
8 years ago

In 40876:

Build/Test Tools: Fix PHP 5.2 compatibility for grandchild methods which expect exceptions to be raised.

This is due to is_callable( 'parent::setExpectedException' ) not being supported on PHP 5.2 when the method being checked only exists on the grandparent class.

See #39822

Merges [40872] and [40873] to the 4.7 branch.

#37 @johnbillion
8 years ago

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