#30522: WP_UnitTestCase::assertEqualSets can pass incorrectly
------------------------------+------------------------------
Reporter: dd32 | Owner:
Type: defect (bug) | Status: new
Priority: normal | Milestone: Awaiting Review
Component: Build/Test Tools | Version:
Severity: normal | Resolution:
Keywords: | Focuses:
------------------------------+------------------------------
Description changed by dd32:
Old description:
> WP_UnitTestCase::assertEqualSets currently allows different sized sets to
> be passed, which can hide errors in unit tests.
>
> Take for example the following unit test, I can't see a reason for why
> this should pass, and yet, it currently does:
> {{{
> $this->assertEqualSets( array( 1, 2, 3 ), array( 1, 2, 2, 3, 3, 3 ) );
> }}}
>
> There's a few options, including asserting the count is the same, however
> that doesn't work as something like this would then also pass:
> {{{
> $expected = array( 1, 2, 2 );
> $actual = array( 1, 1, 2 );
> $this->assertEqualSets( $expected, $actual );
> }}}
>
> The function currently only cares about the values (as it uses
> `array_diff()`), so we could simply perform a sort and then
> {{{assertEquals}}} the resulting array, that could account for the above
> cases.
>
> However, we also have some tests (Terms for example) which are testing
> `id=>slug`, those tests could have the incorrect `id` (array key) and
> still pass (both now, and with the sorting option), eg:
> {{{
> $expected = array(
> 1 => 'one',
> 100 => 'one hundred'
> );
> $actual = array(
> 50 => 'one',
> 500 => 'one hundred'
> );
> $this->assertEqualSets( $expected, $actual );
> }}}
>
> `assertEqualSets` was introduced in [1127/tests].
New description:
WP_UnitTestCase::assertEqualSets currently allows different sized sets to
be passed, which can hide errors in unit tests.
Take for example the following unit test, I can't see a reason for why
this should pass, and yet, it currently does:
{{{
$this->assertEqualSets( array( 1, 2, 3 ), array( 1, 2, 2, 3, 3, 3 ) );
}}}
There's a few options, including asserting the count is the same, however
that doesn't work as something like this would then still pass:
{{{
$expected = array( 1, 2, 2 );
$actual = array( 1, 1, 2 );
$this->assertEqualSets( $expected, $actual );
}}}
The function currently only cares about the values (as it uses
`array_diff()`), so we could simply perform a sort and then
{{{assertEquals}}} the resulting array, that would account for the above
cases.
However, we also have some tests (Terms for example) which are testing
`id=>slug`, those tests could have the incorrect `id` (array key) and
still pass (both now, and with the sorting option), eg:
{{{
$expected = array(
1 => 'one',
100 => 'one hundred'
);
$actual = array(
50 => 'one',
500 => 'one hundred'
);
$this->assertEqualSets( $expected, $actual );
}}}
`assertEqualSets` was introduced in [1127/tests].
--
--
Ticket URL: <https://core.trac.wordpress.org/ticket/30522#comment:2>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform