Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#54724 new defect (bug)

PHPunit tests should not load remote patterns

Reported by: chouby's profile Chouby Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.8
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description

While running PHPunit tests for one of my plugins with PHP 8.1, I noticed deprecation notices in the Requests library when I am using set_current_screen(). These notices reveal that http requests are hooked to the current_screen action to load remote patterns.

add_action( 'current_screen', '_load_remote_block_patterns' );
add_action( 'current_screen', '_load_remote_featured_patterns' );

The first was added in 5.8 by #53246, the second was just added by #54623 for 5.9.
I propose to unhook these actions for PHPUnit tests.

Change History (4)

#1 @costdev
3 years ago

@Chouby Can you confirm if adding this line to the top of the test method(s) prevents the deprecation notice(s)?

add_filter( 'should_load_remote_block_patterns', '__return_false' );

If confirmed, this could be considered for addition either to the PHPUnit bootstrap file, or just in the test class, fixtures or methods as needed.

#2 @Chouby
3 years ago

I already had included

remove_action( 'current_screen', '_load_remote_block_patterns' );
remove_action( 'current_screen', '_load_remote_featured_patterns' );

to my test case. But I confirm that

add_filter( 'should_load_remote_block_patterns', '__return_false' );

is as efficient.
My proposal is to add this to the WordPress test framework as it should make sure that remote requests usually initiated by WordPress are never fired during PHPUnit tests.

#3 @audrasjb
3 years ago

  • Milestone changed from Awaiting Review to 5.9

Moving for 5.9 consideration.
Not a blocker, though, but worth considering.

#4 @hellofromTonya
3 years ago

  • Milestone changed from 5.9 to Awaiting Review
  • Version changed from trunk to 5.8

Currently there are no PHP unit or integration tests within Core that would be impacted by unhooking these callbacks. However, when tests are added, then the original issue returns.

That said, PHP 8.1 deprecation notices are not failures. Rather, they are alerting of incompatibilities that need to be fixed.

Some historical context:
The Requests library has a PHP 8.1 compatible version ready, i.e. version 2.0.0. Upgrading the library to that version is being tracked in #54504. It was planned for 5.9 but then reverted due to issues with Core's upgrader. It is planned for 6.0.

The root cause of the problem in this ticket is not the callbacks hooked into current_screen but rather the need to upgrade Request library.

Sounds like you have a solution in your plugin's test suite. That's good! Once the Requests library is upgrade in Core, those specific deprecation notices from Requests will be resolved.

Moving this ticket back to Awaiting Review. Let's revisit once Requests is upgraded to determine if unhooking these 2 specific callbacks in Core's test bootstrap makes sense or not. Also changing the version to 5.8 to denote when the first callback was added to Core.

Note: See TracTickets for help on using tickets.