WordPress.org

Make WordPress Core

Opened 7 weeks ago

Last modified 2 weeks ago

#39822 new defect (bug)

PHPUnit 6 compatibility

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

Download all attachments as: .zip

Change History (21)

@miyauchi
7 weeks ago

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

#5 follow-up: @miyauchi
7 weeks 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
7 weeks 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
4 weeks 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 weeks ago

#9 @johnbillion
3 weeks 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 weeks ago

In 40257:

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

See #39822, #40086

@gitlost
3 weeks ago

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

#11 @ocean90
3 weeks 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 weeks 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 weeks 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 weeks ago

#15 @johnbillion
3 weeks 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 weeks ago

Much nicer!

@gitlost
3 weeks ago

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

#17 @johnbillion
2 weeks ago

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

39822.4.patch Looks good.

Note: See TracTickets for help on using tickets.