test.equal fails on objects if one has a null prototype

[ephemer]

test.equal fails in unexpected ways when dealing with objects with a null prototype. I have summed this issue up in two simple test cases:

import { test } from "zora";

test("non-null prototype", (assert) => {
    const thing = {};
    thing.a = 1;
    thing.b = 2;
    thing.c = 3;

    assert.equal(thing, { a: 1, b: 2, c: 3 });
});

// result:
# Subtest: non-null prototype
    ok 1 - should be equivalent

test("null prototype", (assert) => {
    const thing = Object.create(null);
    thing.a = 1;
    thing.b = 2;
    thing.c = 3;

    assert.equal(thing, { a: 1, b: 2, c: 3 });
});

// result:
# Subtest: null prototype
    not ok 1 - should be equivalent
      ---
        wanted: {"a":1,"b":2,"c":3}
        found: {"a":1,"b":2,"c":3}   <---- very confusing
        at: " <stdout>:80:151"
        operator: "equal"
      ...
    1..1
not ok 3 - null prototype

I suspect this is an issue with the "deep equals" part of zora. From memory of glancing through the code zora is using an external library for this, so maybe the correct place for this bug report would be there, but maybe using a different library would also be an option.

To be clear: in our real use-case, the objects with null prototype are coming from external libraries (in our case, from graphql). I don't fully understand it but there has been a trend in recent years to use Object.create(null) wherever possible rather than {} for security reasons, so I don't think this case would be too uncommon.

[ephemer]

I did some more digging and found that the library zora uses for this has had an open issue about this topic for over a year: https://github.com/epoberezkin/fast-deep-equal/issues/49

[lorenzofox3]

Good catch, I wanted to try some other deep equal libraries anyway. this one seems a good candidate

[ephemer]

@lorenzofox3 looks fast! Hope it works with this test case

[lorenzofox3]

I had a chance to have a look your issue this week end. I start to doubt that the assertion library should make the assumption that Object.create(null, foo) == Object.create({}, foo). That is a strong move the library you mentioned made which causes all sort of problems looking at the issues in their repo.

So I am not keen in modifying the equal(and aliases) assertion method. Maybe we could add a new one propsEql or something similar. Meanwhile you can try to decorate the assertion to check the behaviour or even easier, mutate the AssertPrototype.

// run this code before the test suite
AssertPrototype.propsEq = function (actual, expected, ...rest) {
  return this.equal(Object.assign(Object.create({}),actual), expected, ...rest);
};

// and use t.propsEq instead of t.eq 

// or if you prefer to overwrite the default behaviour

const equal = AssertPrototype.equal;

AssertPrototype.equal = function(actual, expected, ...rest){
    return equal.call(this, Object.assign(Object.create({}),actual), expected, ...rest)
}

Then after we have your feedback on the experience, we can think about the next move

Please have a look at that pen

[ephemer]

@lorenzofox3 sorry for the late reply here, my focus has been elsewhere for a while.

I totally agree that test.equal(Object.create(SomePrototype, foo), Object.create(AnotherPrototype, foo)) should for sure not pass, because the prototype of objects that are set explicitly – an extremely common use-case being via the class syntax – can have significant differences. But I can't think of a case where you would not want null and Object.prototype to be equal for the purposes of testing. I think the main reason for that is that testers have other opportunities to test an object's prototype (test.is(true, foo instanceof MyType)), which in my opinion would be clearer. Maybe this is just our use case, but judging from the issue linked on the other repo, it seems like others have similar concerns.

I think for now we will override equal (and its aliases) with the following:

const equal = AssertPrototype.equal;
AssertPrototype.eq = function (actual, expected) {
  return equal({ ...actual ), { ...expected });
};

Thanks for the tip and for zora generally! We're using it to migrate away from jest and are able to speed up our tests with it by a factor of 10x!

Edit: I just realised after looking at our code that the issue goes deeper than this. Using graphql on the server could produce an entire tree of objects that each have a null prototype, so a simple workaround is not possible. We can use the same concept and call convertNullPrototypeObjectTreeToNormalObject (our code, which "deeply" switches out the prototypes) instead of using an object assign / spread, but something about that workaround feels quite unsatisfying.