Brain-WP/BrainMonkey

Mocking wp_die doesn't return and exit the method under test

dingo-d opened this issue · 5 comments

I am testing a method, used as an ajax callback, which has a validation check inside.

The validation part inside the method looks like this

/*
 * Validation check.
 */
if ( ! isset( $_POST['myKey'] ) ) {
  $response = 'Some error'; // Originally I have `esc_html__` here which is mocked.
  \wp_die( \wp_json_encode( $response ) ); // wp_json_encode is aliased with json_encode.
}

In my test I've mocked the wp_die function like this

Functions\when('wp_die')->alias(function($value) {
  return $value;
});

But the test fails because it seems that the execution of the callback method goes on (nonce verification and other functionality).

I'm puzzled as to why this happens.

Since my wp_die should return the first argument that is passed to it (\wp_json_encode( $response )), which should exit the method, and the test should pass. When I turn the coverage report, the part that checks if it's set (validation), and the wp_die is green, but then the rest fails, because I have an Undefined index: myKey (triggered by the nonce check).

Is there some extra functionality I'm missing in the userFunction?

Your (fake) function does return, correct, but the method under test does not return whatever it gets from wp_die(). So the execution just goes on.

Usually, you should just expect one call to wp_die(), or expect no call, depending on what it is that you are testing. But I understand your point. You would have to prepare stuff that you actually not need.

So, one (hack) way of doing this is make your stub throw an exception, and expect it in your test.

Maybe @gmazzap has a better suggestion, though. 🙂

I've tried putting throw new Exception( $value, 1 ); instead of just returning the value, but that just throws Exception and I think it stops the execution, like when I tried aliasing wp_die with die, which actually stopped all other tests from executing.

I could add return after wp_die in my method, but that kinda seems like an anti-pattern, since wp_die will stop the execution if encountered, so a return is not needed, right?

Yes, throw a real exception. But then you would have to expect it. PHPUnit has this built in.

Having your actual code contain return wp_die(); is another option, which I thought about too.
I wouldn't call this an anti-pattern, and it doesn't do anything bad for your production environment, as the return will never be reached. But your unit tests would be (much) simpler.

Hi @dingo-d,
sorry for late answer, just back from my vacation.

I agree with @tfrommen here, returning makes tests easier.

I tend to do something like:

wp_die( ... );
return;

This way the IDE does not complain for returning a function that returns nothing, and if the method/function where this happen is marked to return void this validates as well.

Thanks for the help guys! This helped a lot :)