The `test_get_terms_post_args_paging()` test performs no assertions

[johnbillion]

The WP_Test_REST_Tags_Controller::test_get_terms_post_args_paging() test is faulty and performs no assertions. The response from both of the calls to WP_REST_Request are empty, therefore nothing is iterated and the calls to $this->assertSame() within both of the foreach() loops are never called.

Ref: ​https://github.com/WordPress/wordpress-develop/runs/4574808918?check_suite_focus=true#step:20:329

Needs investigating.

[johnregan3]

New Tests.

[johnregan3]

@costdev Polite bump for visibility.

[costdev]

@johnregan3 Thanks for the ping!

I believe to make WP_Test_REST_Tags_Controller::test_get_terms_post_args_paging() perform assertions:

1. The only necessary fix is:

// Line 57

// Change
$tag_ids[] = $factory->tag->create(

// To
self::$tag_ids[] = $factory->tag->create(

2. Additionally, using $this->assertNotEmpty( $tags ) before each foreach loop would stabilise the tests by ensuring they fail if the response returns empty.

---

@johnbillion What is the scope of this ticket? Is it to make the minimum number of changes to ensure that this test method always performs assertions, or is there scope for improving this test method at the same time?

If so:

• While 54662.diff improves the tests, it doesn't verify that the orderby argument was successful.
• The test method, and patch, lacks the message argument, which is required for test methods with multiple assertions per ​Writing PHPUnit Tests - Using Assertions.
• I believe this test method only needs two assertions: $this->assertNotEmpty() to make sure something was returned, and $this->assertSameSetsWithIndex() to make sure the results for the given page are as expected. The rest can be handled with an additional fixture, $tag_names, that can be set up before the class, and a data provider that allows setting the page and per_page arguments for more thorough, but stable, testing.
• I can attach a patch that demonstrates these improvements if this ticket has the scope for it. Otherwise, the two changes noted in the first section should be enough to ensure the method performs assertions.

---

P.S. Milestoning this for 6.1 as it's easily possible to resolve this during the current cycle.

[SergeyBiryukov]

In 53909:

Tests: Assign created fixtures to the dedicated class properties in some test classes.

This affects:

• WP_Test_REST_Categories_Controller
• WP_Test_REST_Comments_Controller
• WP_Test_REST_Tags_Controller

and brings consistency with:

• WP_Test_REST_Posts_Controller
• WP_Test_REST_Users_Controller

These test classes were previously updated to improve performance by creating less fixtures and reusing them where possible. While the pagination tests for categories and comments still passed due to enough items being created, the pagination test for tags did not work as expected and did not perform any assertions due to trying to iterate over an empty array of results.

This is now corrected by assigning the properties as intended and adding more assertions to the affected test.

Follow-up to [46657].

Props johnregan3, costdev, johnbillion.
See #54662.

[SergeyBiryukov]

Replying to costdev:

I believe to make WP_Test_REST_Tags_Controller::test_get_terms_post_args_paging() perform assertions:

1. The only necessary fix is:

// Line 57

// Change
$tag_ids[] = $factory->tag->create(

// To
self::$tag_ids[] = $factory->tag->create(

2. Additionally, using $this->assertNotEmpty( $tags ) before each foreach loop would stabilise the tests by ensuring they fail if the response returns empty.

Good catch! This needed to be fixed either way and should now be resolved in [53909].

Keeping the ticket open for now for any additional improvements.

[desrosj]

Since this was only left open for any additional improvements that came up, I'm going to close this one out. It can be reopened if anything is spotted prior to 6.1 release.