WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#37295 closed defect (bug) (fixed)

PHP 7.1.0: Tests_Query_IsTerm::test_cat_uncat_action_tax() is failing

Reported by: ocean90 Owned by:
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: General Keywords:
Focuses: Cc:
PR Number:

Description

Tested with PHP 7.1.0alpha3 (cli) (built: Jul 6 2016 11:50:33).

See https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/142322444#L387-L396 and

1) Tests_Query_IsTerm::test_cat_uncat_action_tax
Parameter 1 to Tests_Query_IsTerm::_cat_uncat_action_tax() expected to be a reference, value given

/src/wp-includes/plugin.php:600
/src/wp-includes/query.php:2503
/src/wp-includes/query.php:4050
/src/wp-includes/class-wp.php:609
/src/wp-includes/class-wp.php:727
/tests/phpunit/includes/testcase.php:462
/tests/phpunit/tests/query/isTerm.php:123

I haven't found something in the changelog (https://github.com/php/php-src/blob/PHP-7.1.0/NEWS) which could cause this behaviour. I only know that removing the filter for pre_get_posts makes the test passing.

Attachments (2)

37295.diff (578 bytes) - added by netweb 3 years ago.
phpunit.output (34.2 KB) - added by ocean90 3 years ago.

Download all attachments as: .zip

Change History (17)

@netweb
3 years ago

#1 follow-up: @netweb
3 years ago

#2 @ocean90
3 years ago

There is also Tests_Query_IsTerm::pre_get_posts_tax_category_tax_query() and Tests_Query_Conditionals::pre_get_posts_with_tax_query, why is there no error for them?

#3 @netweb
3 years ago

I've no idea ¯\_(ツ)_/¯

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


3 years ago

#5 in reply to: ↑ 1 ; follow-up: @SergeyBiryukov
3 years ago

Replying to netweb:

I also couldn't find anything in https://github.com/php/php-src/blob/PHP-7.1.0/NEWS that matched up

https://bugs.php.net/bug.php?id=72508 seems to be related.

#8 follow-up: @ocean90
3 years ago

  • Keywords needs-testing removed

This is going to be fun...

From the ticket:

PHP-7.1 doesn't allow modification of $this through references on purpose.
See https://wiki.php.net/rfc/this_var#disable_ability_to_re-assign_this_indirectly_through_reference

The simplest workaround is replacing $this with another variable.

- do_action_ref_array( 'pre_get_posts', [ &$this ] );
+ $obj = $this;
+ do_action_ref_array( 'pre_get_posts', [ &$obj ] );

#9 in reply to: ↑ 8 @netweb
3 years ago

Replying to ocean90:

From the ticket:

PHP-7.1 doesn't allow modification of $this through references on purpose.
See https://wiki.php.net/rfc/this_var#disable_ability_to_re-assign_this_indirectly_through_reference

The simplest workaround is replacing $this with another variable.

- do_action_ref_array( 'pre_get_posts', [ &$this ] );
+ $obj = $this;
+ do_action_ref_array( 'pre_get_posts', [ &$obj ] );

A new reply to that reply:

... that's a workaround, but it's only a workaround for the bug… the bug needs to be fixed somehow though before 7.1.0. I don't find this broken behavior acceptable to be shipped in the release.

Thanks for creating the PHP ticket @ocean90, so at least there is now some progress and more fun to come for sure

This ticket was mentioned in Slack in #buddypress by netweb. View the logs.


3 years ago

@ocean90
3 years ago

#11 @ocean90
3 years ago

PHP 7.1.0 beta 2 was released a few days ago. phpunit.output is the output for

$ php -v
PHP 7.1.0beta2 (cli) (built: Aug  9 2016 22:03:47) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.1.0-dev, Copyright (c) 1998-2016 Zend Technologies
    with Xdebug v2.4.0, Copyright (c) 2002-2016, by Derick Rethans
$ phpunit --version
PHPUnit 5.5.0 by Sebastian Bergmann and contributors.

It's not only one failing test anymore:

Tests: 6363, Assertions: 20176, Errors: 42, Failures: 22, Skipped: 73.

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


3 years ago

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


3 years ago

#14 @ocean90
3 years ago

  • Component changed from Build/Test Tools to General
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.7

A fix for this was committed the other day: https://github.com/php/php-src/commit/906456c4106ac74da0688b2481e91e27de3b176b

The warning will remain but the call is no longer aborted.

Related: #37772

#15 @ocean90
3 years ago

  • Keywords needs-patch removed
  • Resolution set to fixed
  • Status changed from new to closed

Fixed in [38571].

Note: See TracTickets for help on using tickets.