wp_die() doesn't accept a WP_Error during unit testing

[dd32]

As reported on the forums: ​https://wordpress.org/support/topic/passing-wp_error-instance-to-wp_die-when-unit-testing-doesnt-work?replies=2

I'm testing some code using PHPUnit, and when my code uses
wp_die(new WP_Error('errorCode', 'errorMessage'));
I get the error:
Error: Wrong parameters for WPDieException([string $message [, long $code [, Throwable $previous = NULL]]])
If I change my code to:
wp_die('errorMessage');
Then it works correctly (I get WPDieException: errorMessage).

The unit test wp_die() handler should accept the same things that the default wp_die() handler can.

[utkarshpatel]

I debug the issue and found when we call wp_die() from PHPunit it is going inside the default die handler of ajax _ajax_wp_die_handler and that function is not designed to handle WP_Error object.

I don't know if this is right way but here i've handled WP_Error in _ajax_wp_die_handler()

[utkarshpatel]

Handle WP_Error object in default _ajax_wp_die_handler

[utkarshpatel]

remove switch case from previous patch

[boonebgorges]

Could I get some more details on how to reproduce? It seems as if the issue being reported by @xoogu in the support forums is somewhat different from what's being proposed by @utkarshpatel. @dd32 suggests that the AJAX die handler for unit tests should match the src handler. But this should already be the case:

• https://core.trac.wordpress.org/browser/tags/4.4.2/tests/phpunit/includes/testcase-ajax.php#L134
• https://core.trac.wordpress.org/browser/tags/4.4.2/src/wp-includes/functions.php#L2719

In each case, a scalar value is handled one way (printing or passing to the Exception), while all other values die with '0'.

36166_1.patch​ proposes an enhancement that passes custom WP_Error messages to the die handler. This may be a viable enhancement, but it doesn't address the original report, which suggests that errors are handled differently in the context of unit tests.

Possibly related, AJAX test classes are responsible for providing their own die handlers. Core tests do this by extending WP_Ajax_UnitTestCase. If @utkarshpatel's tests are invoking the default _ajax_wp_die_handler(), I'm guessing that those tests are *not* extending WP_Ajax_UnitTestCase.

[xoogu]

Replying to boonebgorges:

Could I get some more details on how to reproduce? It seems as if the issue being reported by @xoogu in the support forums is somewhat different from what's being proposed by @utkarshpatel.

The issue I was having was when installing a plugin as part of the initial setup for testing. (The plugin install was called from setUpBeforeClass of my test). If the plugin couldn't be installed, then wp_die was called with a WP_Error object passed to it.

(I realise requiring another plugin to be installed rather than just mocking it isn't actually unit testing, but that's just how I like to do my tests).

[dd32]

To test it, I simply added that example to the functions.php unit tests:

+	function test_wp_die_with_wp_error() {
+		wp_die( new WP_Error( 'test', 'test' ) );
+	}

and got the fatal error as stated.

It turns out that the Ajax wp_die() handler handles it correctly, but the WP_UnitTest wp_die handler doesn't: https://core.trac.wordpress.org/browser/tags/4.4.2/tests/phpunit/includes/testcase.php#L274

[utkarshpatel]

Enhancement of wp_die_handler function in test case which accepts wp_error object as a param.

[utkarshpatel]

@boonebgorges yes, it was mistake but code will be same and we can also use enhancement of 36166_1.patch

Changes on https://core.trac.wordpress.org/browser/tags/4.4.2/tests/phpunit/includes/testcase.php#L274 function are attached in 36166_2.patch

[boonebgorges]

Replying to dd32:

To test it, I simply added that example to the functions.php unit tests:

+	function test_wp_die_with_wp_error() {
+		wp_die( new WP_Error( 'test', 'test' ) );
+	}

and got the fatal error as stated.

It turns out that the Ajax wp_die() handler handles it correctly, but the WP_UnitTest wp_die handler doesn't: https://core.trac.wordpress.org/browser/tags/4.4.2/tests/phpunit/includes/testcase.php#L274

Ah yes. Thanks for the pointer. This is what I was guessing at in this comment.

@utkarshpatel The proposal that wp_die() print WP_Error messages is worth considering, but it's beyond the scope of this ticket. The proposal raises a number of concerns related to backward compatibility, empty messages, and other potential problems. If you'd like to consider it, I encourage you to open an enhancement ticket.

[boonebgorges]

In 37071:

Tests: Ensure that the default wp_die() handler can handle a WP_Error object.

Props dd32, utkarshpatel.
Fixes #36166.