Tests: rest-posts-controller hooks blow under certain circumstances

[SirLouen]

In this revision [39079] it was introduced some code

In this file tests/phpunit/tests/rest-api/rest-posts-controller.php:

<?php
        add_filter( 'rest_pre_dispatch', array( $this, 'wpSetUpBeforeRequest' ), 10, 3 );
        add_filter( 'posts_clauses', array( $this, 'save_posts_clauses' ), 10, 2 );
}

public function wpSetUpBeforeRequest( $result, $server, $request ) {
        $this->posts_clauses = array();
        return $result;
}
public function save_posts_clauses( $orderby, $query ) {
        if ( 'revision' !== $query->query_vars['post_type'] ) {
                array_push( $this->posts_clauses, $orderby );
        }
        return $orderby;
}

If we introduce the following test in this file, it will blow:

public function test_blowing_example() {
        $this->go_to( get_permalink( self::$post_id ) );
}

The reason is that rest_pre_dispatch is running after posts_clauses, $this->posts_clauses = array(); is not being set on time and the array_push blows because $this->posts_clauses is null

Why severity blocker? Because I cannot continue working on another report until this is solved, so technically is a blocker report.

Possible solutions that come to my mind:

1. Changing the filter to another that runs earlier for wpSetUpBeforeRequest (or later save_posts_clauses). I can't think of which filter would be the most adequate
2. Adding a $this->posts_clauses empty() conditional check in save_posts_clauses to ignore array_push in case it's empty (a possible safe option)

I'm adding this PR to showcase the solution and see if it passes the tests suite.

Trac ticket: https://core.trac.wordpress.org/ticket/63635

[SirLouen]

This is more convoluted than I expected

I've tested all 16 failing tests individually and they all pass
Same for running the whole class WP_Test_REST_Posts_Controller

npm run test:php -- --filter WP_Test_REST_Posts_Controller

This only fails when running the whole restapi group npm run test:php -- --group restapi

So it seems debugging this will be not as straightforward was I thought. Lets see where to start from.

[SirLouen]

After some debugging I found and sorted the problem

For some reason, some tests need to array_push their empty array to $this->post_clauses; otherwise they will fail. I did not consider that wpSetUpBeforeRequest was setting an empty array that was not passing this. So in this case, instead of checking for not empty, I shall be checking just for isset so they can add their empty array, this way, now this is fully working.

[SirLouen]

As per today's bugscrub, the idea is to Milestone (and if possible, also merge). ​More info

This is a very little fix, but its blocking future Unit Tests, maybe @SergeyBiryukov could lend a hand with this patch?

Remember to remove the showcase example test test_blowing_example before committing(!)