mjackson / expect

Write better assertions
MIT License
2.29k stars 117 forks source link

False equality with Immutable Records #231

Closed dclowd9901 closed 6 years ago

dclowd9901 commented 6 years ago

Recently upgraded to Jest 21.0.2, and some equality tests relating to comparing Immutable Records started failing.

I was able to trace the problem back to this library, in the utils.js file for iterableEquality.

It turns out that iterable equality falsely identifies Immutable Records as iterables (likely because of this line:

  if (a.size !== undefined) { // Immutable Records also have a size prop

since IRs (Immutable Records) have a size property.

It then constructs a new object out of the properties of the compared objects (by combining them in an array and passing that array to the constructor of the a property), but merely compares the size of the newly constructed Record to the old a record and nothing else. It seems that when constructing a new Record out of the merged properties, Immutable uses the first reference listed in the array, not the second, to build the Record.

That means that if you have two Immutable Records of any kind that have the same properties, (nevermind the contents of those properties), this test will erroneously pass and not bother to look deeper in the records.

The real concern here is that the duck-typing utilized in iterableEquality is brittle, and subject to whether or not two constructed objects have a size param, and how they construct themselves.

Workaround: Convert Immutable Records to plain JSON objects and compare those instead, or stringify them and do the same.

ljharb commented 6 years ago

Issues with expect v21+ need to be filed in the jest repo.

dclowd9901 commented 6 years ago

Moved to https://github.com/facebook/jest/issues/4472