Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#53123 closed task (blessed) (fixed)

Tests: review of assertTrue or assertFalse with in_array

Reported by: hellofromtonya's profile hellofromTonya Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.9 Priority: normal
Severity: minor Version:
Component: Build/Test Tools Keywords:
Focuses: docs Cc:

Description

While looking at the tests in the context of another ticket, @jrf and I came across tests checking if a value is in an array by using assertTrue or assertFalse with in_array(). For example:

$this->assertTrue( in_array( $function, $defined, true ), $msg );

PHPUnit provides the following assertions:

  • assertContains: checks if a value exists in an array
  • assertNotContains: checks the opposite, i.e. if a value does not exist in an array

See: - https://phpunit.readthedocs.io/en/7.0/assertions.html#assertcontains

Using the example from above, it could be changed to:

$this->assertContains( $function, $defined, $msg );

This ticket proposes to review the test suite to verify each occurrence and then replace:

  • assertTrue( in_array() ) with assertContains()
  • assertFalse( in_array() ) with assertNotContains()

Change History (2)

#1 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 5.9
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

I was going to do this as part of a similar work on #53363 (specifically [51335], [51337], [51367], [51397], and [51403]), but luckily found this ticket :)

IIRC, assertContains() only performs strict comparison for array elements in PHPUnit 9+. So technically this might seem like a step backwards from switching to strict comparisons using assertSame() in #38266, #52482, and #53364, as WordPress test suite still uses PHPUnit 7.5.x at the moment.

However, as the plan is to be able to use PHPUnit 9+ via #46149 soon, this concern would be resolved then.

#2 @SergeyBiryukov
3 years ago

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

In 51404:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( in_array( ... ) ) with assertContains() to use native PHPUnit functionality.

Follow-up to [51335], [51337], [51367], [51397], [51403].

Props hellofromTonya, jrf, SergeyBiryukov.
Fixes #53123. See #53363.

Note: See TracTickets for help on using tickets.