[wp-trac] [WordPress Trac] #53651: unit test for wp_removable_query_args

[WordPress Trac]

#53651: unit test for wp_removable_query_args
------------------------------+------------------------------
 Reporter:  pbearne           |       Owner:  (none)
     Type:  enhancement       |      Status:  new
 Priority:  normal            |   Milestone:  Awaiting Review
Component:  Build/Test Tools  |     Version:
 Severity:  normal            |  Resolution:
 Keywords:  needs-unit-tests  |     Focuses:
------------------------------+------------------------------

Comment (by jrf):

 I've just had a look at the PR and at the function.

 The function does not actually contain any logic, so there isn't much to
 test in reality.

 I agree with @johnbillion that checking the exact content of the array in
 the first test does not add value.
 Just checking that the return value of the function is an array and that
 that array is not empty should be sufficient.
 The actual values within the array should not be tested like this.

 I also agree with @SergeyBiryukov that testing that the filter ''gets
 applied'' is a good idea, though I don't agree that the filter needs to be
 removed and the output checked again after. That is something which should
 be tested in the Hooks logic, not here.

 I also don't think we should care much about what the filter returns as
 that is defined within the test. We just need to check that the returned
 filtered value complies with what the test filter does. That confirms that
 the filter has been ''applied''.
 Testing whether the function doing the filtering is returning the expected
 type and is not empty and such, doesn't add any value as we don't control
 outside filters, so testing this doesn't actually yield any extra code
 security.

 I would suggest simplifying the filter test to this (also note the test
 function name change):
 {{{#!php
 <?php
 public function test_wp_removable_query_args_applies_filter() {
         add_filter( 'removable_query_args', static function( $args ) {
 return array(); } );

         $this->assertSame( array(), wp_removable_query_args() );
 }
 }}}

 All filter adjustments get automatically reverted via the abstract
 TestCase `tear_down()` method anyway, so we don't have to worry about
 using a closure which cannot be removed via `remove_filter()` in the
 tests.

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