Make WordPress Core

Opened 3 months ago

Closed 2 months ago

#39219 closed enhancement (fixed)

Add `assertNotFalse` method to `WP_UnitTestCase`.

Reported by: peterwilsoncc Owned by: peterwilsoncc
Milestone: 4.8 Priority: normal
Severity: normal Version: trunk
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:


The version of phpunit WP uses on older versions of PHP doesn't include the assertNotFalse method.

Attachments (2)

39219.diff (752 bytes) - added by peterwilsoncc 3 months ago.
39219-unit-tests-to-replace.diff (2.4 KB) - added by peterwilsoncc 3 months ago.

Download all attachments as: .zip

Change History (8)

3 months ago

#1 @peterwilsoncc
3 months ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to peterwilsoncc
  • Status changed from new to assigned

In 39219.diff.

I've tested locally that it passes with all falsy values except false. I'm not sure where I should put these.

$this->assertNotFalse( 0 );
$this->assertNotFalse( null );
$this->assertNotFalse( '' );
$this->assertNotFalse( array() );
$this->assertNotFalse( 0.0 );
$this->assertNotFalse( '0' );

#2 follow-up: @dd32
3 months ago

@peterwilsoncc Confirmed that the logic matches what is in PHPUnit: https://github.com/sebastianbergmann/phpunit/blob/9758eee6421efd997a42fe850f74292a490e9c70/src/Framework/Assert.php#L1185-L1196 so this seems fine.

Is it possible to find the occurrences in the current unit tests where this was to be used, but avoided?

#3 in reply to: ↑ 2 @peterwilsoncc
3 months ago

Replying to dd32:

Is it possible to find the occurrences in the current unit tests where this was to be used, but avoided?

Uploaded in 39219-unit-tests-to-replace.diff, it replaces assertNotSame( false, $actual ) with assertNotFalse( $actual ). There are a few assertNotEquals() calls that look like they could be replaced but I'd prefer to be conservative.

I've kept the tests seperate, do you think they can be committed as part of this ticket or should it be a follow up?

#4 @swissspidy
3 months ago

In addition to assertNotSame() there are a few instances of assertTrue( false === … ) instead of assertFalse() and assertTrue( false !== ) instead of assertNotFalse(). They should be updated as well.

#6 @SergeyBiryukov
2 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 39919:

Build/Test Tools: Add assertNotFalse() method to WP_UnitTestCase and use it where appropriate.

Props peterwilsoncc.
Fixes #39219.

Note: See TracTickets for help on using tickets.