PhpUnit always returns true when comparing Ds\Map

[robincafolla]

php -v: PHP 7.2.9-1+ubuntu16.04.1+deb.sury.org+1

When PHPUnit\Framework\TestCase::assertEquals is used to compare two Maps it always returns true, regardless of contents.

Example:

use Ds\Map;
use PHPUnit\Framework\TestCase;

class MyTest extends TestCase

  public function testMaps():void {
    $mapA = new Map([
      1 => 'Foo',
      2 => 'Bar'
    ]);
    $mapB = new Map([
      3 => 'Cat',
      4 => 'Car'
    ]);

    $this->assertEquals($mapA, $mapB); // returns true
  }
}

Explanation:

This seems to happen because SebastianBergmann\Exporter::toArray uses (array) to cast objects for comparison. Since whenever you cast a Ds\Map to an array it becomes an empty array this means that assertEquals always returns true.

You might argue that this is a bug in PhpUnit, but I felt I should open it here first to see how you think a comparison should be done on Maps.

[robincafolla]

This btw is my hacky fix; add a custom comparator which compares the json versions of the two maps.

https://gist.github.com/robincafolla/739e3253a7c9f4b4363d4d1aa39e2af5

But it won't work if you're using objects as keys.

[rtheunissen]

Since whenever you cast a Ds\Map to an array it becomes an empty array

This is the bug right here.

[rtheunissen]

I think this is also a poor choice to convert objects to arrays for comparison. Any object that overrides comparison (ie. ==) will fail for assertEquals

[robincafolla]

I agree. I was originally going to open the issue on PHPUnit, but I didn't have a suggestion for them on how to improve it. I'll open an issue later for them to add support for Ds\Hashable.

[rtheunissen]

@robincafolla it's simple, they should use == for comparing objects.

[rtheunissen]

I'm going to open an issue on the comparator repo.

[rtheunissen]

See https://github.com/sebastianbergmann/comparator/pull/63

[jacek-foremski]

This issue happens for every Ds\Collection, not only Ds\Map.

Since issue on the comparator repo was closed and I needed something better than the fix posted here, I created the following comparator: https://gist.github.com/jacek-foremski/999af8e488efb1316ccbd0e52ead58b3

@robincafolla 's comparator result (for a empty Vector vs Vector with two empty objects):

There was 1 failure:

1) xxx::xxx with data set #1 (Ds\Vector Object ())
Objects are not equivalent
--- Expected
+++ Actual
@@ @@
-[]
+[
+    {},
+    {}
+]

My comparator result:

There was 1 failure:

1) xxx::xxx with data set #1 (Ds\Vector Object ())
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
 Ds\Vector Object (
+    0 => App\Example Object (...)
+    1 => App\Example Object (...)
 )

Hopefully someone will find that useful.

[rtheunissen]

@jacek-foremski I think you should also check that both arguments are an instance of the same class.

[jacek-foremski]

@rtheunissen This is done in the parent class (SebastianBergmann\Comparator\ObjectComparator)