[wp-trac] [WordPress Trac] #54662: The `test_get_terms_post_args_paging()` test performs no assertions

[WordPress Trac]

#54662: The `test_get_terms_post_args_paging()` test performs no assertions
-------------------------------+---------------------
 Reporter:  johnbillion        |       Owner:  (none)
     Type:  defect (bug)       |      Status:  new
 Priority:  normal             |   Milestone:  6.1
Component:  REST API           |     Version:
 Severity:  normal             |  Resolution:
 Keywords:  reporter-feedback  |     Focuses:
-------------------------------+---------------------
Changes (by costdev):

 * keywords:   => reporter-feedback
 * milestone:  Awaiting Review => 6.1


Comment:

 @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:

 {{{#!php
 // 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
 [https://core.trac.wordpress.org/attachment/ticket/54662/54662.diff
 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
 [https://make.wordpress.org/core/handbook/testing/automated-testing
 /writing-phpunit-tests/#using-assertions 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.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/54662#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform