[wp-trac] [WordPress Trac] #33929: Wrong data type for several variables holding a wpdb query result

WordPress Trac noreply at wordpress.org
Mon Nov 16 18:39:10 UTC 2015


#33929: Wrong data type for several variables holding a wpdb query result
----------------------------------------+-----------------------------
 Reporter:  tfrommen                    |       Owner:  boonebgorges
     Type:  defect (bug)                |      Status:  assigned
 Priority:  normal                      |   Milestone:  Future Release
Component:  General                     |     Version:
 Severity:  normal                      |  Resolution:
 Keywords:  has-patch needs-unit-tests  |     Focuses:
----------------------------------------+-----------------------------

Comment (by boonebgorges):

 >  Or would individual patches in this ticket be good already?

 Individual patches on this ticket should be fine.

 > I guess @boonebgorges (or someone else) will slurp the plugins and check
 for usage of the changed action/filter parameters and function return
 values.

 Or you! I should note that this is not an exact science, and I understand
 that collateral damage may be unavoidable. The purpose of the plugin
 search is to get a sense - often, an informed intuition - about whether
 the amount of potential breakage is acceptable, given the gains.

 The reasoning, in each case, should work like this:
 - Is anyone actually doing strict comparison here? If no one is, then we
 can feel more comfortable making the change.
 - In cases where we break strict checks, how bad will the break be? Some
 functions are likely to be used in mission-critical ways, while others are
 just for display, etc. If there's a possibility that we could, say,
 introduce a security vulnerability because of a type change, then we
 should probably not make the change.

 > I don't really understand what (kind of) unit tests you would like to
 have here.

 I want tests that accurately describe behavior, so that these kinds of
 type-related ambiguities don't reoccur. So, for `count_user_posts()`,
 there'd be tests that show that both 0 and non-zero results are returned
 as ints. Your technique will work; a more PHPUnit-native way is:

 {{{
 $this->assertInternalType( 'int', count_user_posts( $user_id ) );
 }}}

 Alternatively, you could go through any existing `count_user_posts()`
 tests - in this case, there are some - and change `assertEquals()` to
 `assertSame()`. This has the same effect.

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


More information about the wp-trac mailing list