Improve ApiTestAssertionsTrait::assertJsonEquals with assertEquals insteads of assertEqualsCanonicalizing

[DaedalusDev]

Description

Dear ApiPlatform,

this issue is a development tools improvement to make functionnal tests easier and more effective.

I suggest to replace this code :

core/src/Symfony/Bundle/Test/ApiTestAssertionsTrait.php

Line 77 in 63697e7

static::assertEqualsCanonicalizing($json, self::getHttpResponse()->toArray(false), $message);

With :

        static::assertEqualsCanonicalizing($json, self::getHttpResponse()->toArray(false), $message);

Why ?

assertEqualsCanonicalizing can produce unexpected results

According to PhpUnit's doc, by canonicalizing comparison, keys are ignored and will produce non expected results.

Example

<?php

namespace App\Tests\Api;

use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;

class CanolicalizingTest extends ApiTestCase
{
    public function testCanonicalizing(): void
    {
        static::assertEqualsCanonicalizing(
            [
                'a' => 5,
                'b' => 10,
            ],
            [
                'c' => 5,
                'd' => 10,
            ],
            'assertEqualsCanonicalizing :-(',
        );
        static::assertEquals(
            [
                'a' => 5,
                'b' => 10,
            ],
            [
                'c' => 5,
                'd' => 10,
            ],
            'assertEquals :-(',
        );
    }
}

This exemple with produce :

assertEquals :-(
Failed asserting that two arrays are equal.

Object are supposed be an unordered collection of zero or more name/value pairs

According https://www.rfc-editor.org/rfc/rfc7159.html :

An object is an unordered collection of zero or more name/value
pairs, where a name is a string and a value is a string, number,
boolean, null, object, or array.

So, i think order of properties won't be used to assert a result. That mean this changed won't produce regression for existing projects (or will improve tests with previously false positive passing tests 😝).

Reference

This issues refer to another issue from api-platform/api-platform api-platform/api-platform#1372

Thanks for your great work.

[dunglas]

JSON objects aren't ordered but PHP arrays are. To prevent false positives we will have to order the keys of associative array keys.

A better fix would be to correct the underlying PHPUnit bug: sebastianbergmann/phpunit#5019

[DaedalusDev]

JSON objects aren't ordered but PHP arrays are. To prevent false positives we will have to order the keys of associative array keys.

A better fix would be to correct the underlying PHPUnit bug: sebastianbergmann/phpunit#5019

Sure ! But assertEquals compare elements recurcively as key paired value, so if i don't say a mistake, order is "not implicitly" compared, but value would not match.

Exemple :

<?php

namespace App\Tests\Api;

use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;

/**
 * @internal
 *
 * @coversNothing
 */
class CanolicalizingTest extends ApiTestCase
{
    public function testAssertEquals(): void
    {

        static::assertEquals(
            [
                'a' => 5,
                'b' => 10,
            ],
            [
                'b' => 10,
                'a' => 5,
            ],
            'It work',
        );
        static::assertEquals(
            [
                5,
                10,
            ],
            [
                10,
                5,
            ],
            'assertEquals :-(',
        );
    }
}

Result :

assertEquals :-(
Failed asserting that two arrays are equal.
<Click to see difference>

This exemple will also fail for deep arrays :

// [..]
        static::assertEquals(
            [
                [
                    'id' => 1,
                ],
                [
                    'id' => 2,
                ],
            ],
            [
                [
                    'id' => 2,
                ],
                [
                    'id' => 1,
                ],
            ],
            'assertEquals :-(',
        );
// [..]

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.