Assertion helper for same properties but (strictly) different constructors

[j-ulrich]

There was already an issue targeting the same problem for arrays: #129
For arrays, this seems to be fixed (although there was no explicit fix mentioned in the issue) but the issue also applies for plain objects. This makes it complicated to move the test markup into an iframe for sandboxing.

I think one part of the "problem" is that deepEqual() suggests that only the properties of the objects are compared but in fact it also compares the functions (and they have to be identical).
It should really be document in the API documentation of deepEqual that the functions are compared because this can lead to very subtle failures. Consider this example:

function factory() {
    function F() { this.test = "foo"; }
    return new F();
}

deepEqual(factory(), factory(), "Similar objects fail!?!")

I would suggest to have two functions: keep deepEqual() as it is (just extend the documentation, see above) and add another function (something like deepPropertiesEqual()) which compares only the properties (array entries).

[jzaefferer]

Those two objects have different constructors, that's why they're not considered equal. I'd like to just close this as a duplicate of #279 - would that address your issue?

[j-ulrich]

Those two objects have different constructors, that's why they're not considered equal.

Exactly. But wouldn't it be reasonable to have an assertion which explicitly ignores this fact? Just to be clear: this is rather a feature request than a defect.

Ok, maybe I should describe the concrete problem that I am facing at the moment:
I am testing an application which uses jQuery Mobile. jQuery Mobile performs a lot of auto-initialization/auto-expansion (changing fonts, forming "divs" into "pages" etc.) and I don't want that auto-initialization to take place on my test suite markup. Therefore, I put that application into an iframe for the tests (sandboxing). Now I got the problem that when I compare (using deepEqual()) an object from the iframe with an object I created in the test code, the test always fails because the constructors are not equal:

test("check that object is correct", function() {
    var objectFromIFrame = window.frames[0].myObject,
        expectedObject = {
            foo: "bar",
            coordinates: {
                x: 17,
                y: -6
            }
        };
    deepEqual(objectFromIFrame, expectedObject); /* will ALWAYS fail, no matter how objectFromIFrame looks like */
});

However, I don't mind the constructors being different. I just want the object properties to be compared (recursively). Currently, QUnit does not provide an assertion which allows this kind of comparison.

I mean, I could write a helper method which iterates recursively over the properties of the objects (using hasOwnProperty()) and call strictEqual() for each property but this has several drawbacks:

1. Basically, it would be duplicating the functionality of deepEqual() with a slight variation. Since deepEqual()/equiv() is rather complicated (imho), I don't really want to duplicate/rewrite it.
2. It creates an assertion entry in the test output for every property of every object being compared.
3. I'd say that this is a very basic assertion for the case of sandbox testing, i.e. it is required in every test suite using sandboxing, and therefore, it should be part of the default assertions (well, maybe an extension would also suffice).

[jzaefferer]

#279 is related to this - that was about the output of constructor differences.

@prathe any ideas how to handle these issues?

[Krinkle]

I must say, when I first started using QUnit I always thought deepEqual would only look at (own) properties, not the constructor or prototype chain. So that one could e.g. do stuff like this:

var x = new X(123, 456);
x.doQuuxification();

assert.equal(x, {
    a: 123,
    b: 456,
    quuxified: true
}, 'Basic properties');

Its hard to show a good example for this as in general one shouldn't worry about properties like this, but hopefully it gets the point across.

[jzaefferer]

We should consider removing the constructor check from deepEquals. Would lead to a few cases of tests passing that were supposed to fail, though it wouldn't break any existing passing tests, I think.

@prathe @scottgonzalez @dmethvin any thoughts here?

[scottgonzalez]

This seems fine to me.

[dmethvin]

Sounds good to me as well.

[prathe]

We should consider removing the constructor check from deepEquals. Would lead to a few cases of tests passing that were supposed to fail.

That is why we cannot change the current behavior of deepEqual. It would be more acceptable to deprecate the assertion name deepEqual and use two new assertions instead.

deepEqual philosophy is to go the farther it can to detect non equality and that of course includes the constructor check. If new popular EcmaScript implementations allow for more accuracy, than deepEqual may honor it.

If someone is playing with constructors but is rather interested in JSON, then it is his job to produce the values he wants to provide deepEqual with.

Taking back @j-ulrich examples someone would need to do the following

function factory() {
    function F() { this.test = "foo"; }
    return new F();
}

 a = JSON.parse(JSON.stringify(factory()));
 b = JSON.parse(JSON.stringify(factory()));

deepEqual(a, b);

I don't think we should create a new assertion to skip constructor check. Because that would justify another assertion for another kind of behavior and then we would rather confuse a user having multiple kind of deepEquals. In this case I think it would be better to constrain the input then constraining and complexify deepEqual with multiple behaviors. Instead we could provide helpers and the first one could be the one that "jsonify" objects.

I agree that documentation about the behavior with regex, function and object is missing.

[Krinkle]

If new popular EcmaScript implementations allow for more accuracy

Exactly how are ECMAScript implementations relevant here?

 a = JSON.parse(JSON.stringify(factory()));
 b = JSON.parse(JSON.stringify(factory()));

[..] provide helpers and the first one could be the one that "jsonify" objects.

Instead of going through (recursive) stringification to JSON and then parsing it again, I think there are more sane methods of comparing objects by their (own) key/value properties. Surely there is no need to JSON-ify anything, that's absurd.

Something like this:

/* testrunner.js */

// Like Object.keys, but for values.
function values(obj) {
    var key, val, vals = QUnit.is('array', obj) ? [] : {}, hasOwn = vals.hasOwnProperty;
    for (key in obj) {
        if (hasOwn.call(obj, key)) {
            val = obj[key];
            vals[key] = val === Object(val) ? values(val) : val;
        }
    }
    return vals;
};
QUnit.assert.propEqual = function (actual, expected, message) {
    actual = values(actual);
    expected = values(expected);
    QUnit.push( QUnit.equiv(actual, expected), actual, expected, message );
};

/* factory.test.js */

QUnit.test('example', 2, function (assert) {
    function factory() {
        function F() {
            this.foo = 'bar';
            this.baz = ['quux'];
        }
        return new F();
    }

    assert.propEqual(factory(), factory(), 'Properties are the same (recursively)');

    assert.propEqual(factory(), {}, 'Failure to show that "Result" is a plain object');
});

[j-ulrich]

Because that would justify another assertion for another kind of behavior and then we would rather confuse a user having multiple kind of deepEqual.

@prathe: You should not think of that assertion (by the way, propEqual() sounds like a good name to me) as a "different kind of deepEqual()" but rather as the next grade of leniency:

strict                                                  lenient
<-------------------------------------------------------------->
equal()              deepEqual()                     propEqual()
object identity      "class identity" /              "state identity" /
                     object equality                 property equality

It doesn't feel confusing to me.

Instead we could provide helpers and the first one could be the one that "jsonify" objects.

This sounds more like a workaround than a solution (although an acceptable workaround).

I like the idea of @Krinkle's solution: build two objects with the same properties like actual and expected and use equiv() to compare them. However, @Krinkle's implementation ignores the inherited properties, meaning that the inherited properties might differ but the assertion would succeed, which shouldn't be the case, I'd say. (That might be an even more lenient assertion.)

[Krinkle]

@j-ulrich Interesting. Taking inheritance into account could be useful, though I'm personally not (yet) convinced. Should the following to fail the equality assertion?

function Foo() {}
Foo.prototype.x = 0;
Foo.prototype.doStuff = function () {};

function FooBar(x) {
    if (x !== undefined) {
        this.x = x;
    }
}
FooBar.prototype = Object.create(Foo.prototype /*, { constructor: .. } */ );
FooBar.prototype.doStuff = function () {
  this.x += this.x;
};

var f = new Foo(), fb = new FooBar(0);

assert.propEqual(f, fb);

f and fb both have the exact same properties. The only difference is the constructor, and that the inherited doStuff is a different function.

And then there is of course the convenient comparison to the object literal, where inheritance is also different:

assert.propEqual(new FooBar(0), {
  x: 0
});

We could implement assert.ownPropEqual for that instead, but I'm not sure we need propEqual and ownPropEqual. Mostly because I think it is rather unlikely. Imagine, what are the chances of comparing two objects that aren't deepEqual, but one does want them to equal each other. Most likely cause: Different constructor. Okay, but what are the odds of two different constructors implementing the exact same inheritance (or rather, there would have to be no inheritance at all. Any prototype function would fail the assertion).

[j-ulrich]

@Krinkle: I was talking about the inherited properties. The inherited functions should be ignored. So the answer to your question

Should the following to fail the equality assertion?

is "no, it should succeed".

But you implementation uses hasOwnProperty() and therefore, even objects with different properties would succeed the assertion. Consider this example:

function Base() {
    this.baseProp = "foo";
}

function Derived() {
    this.newProp = "bar";
}

Derived.prototype = new Base();
Derived.prototype.constructor = Derived;

var firstDerived = new Derived();
var secondDerived = new Derived();
secondDerived.baseProp = "test";

propEqual(firstDerived, secondDerived);

Your implementation of propEqual() would succeed in this case because it only compares the own properties and not the inherited properties. I think that propEqual() should fail in this case because the object properties are different:

// WARNING! This code is wrong! See the next comment of @Krinkle!!!
// firstDerived looks like this:             // secondDerived looks like this:
{                                            {
    baseProp: "foo";                             baseProp: "test";
    newProp: "bar";                              newProp: "bar";
}                                            }

So maybe the solution is to simply remove the if (hasOwn.call(obj, key)) check in your implementation.

Okay, but what are the odds of two different constructors implementing the exact same inheritance

Well, that brings me back to my initial example with the iframes: If you take two objects of the "same" class but created in different iframes, they have different constructors (in terms of object identity) but both implement the exact same inheritance (in terms of properties).

[Krinkle]

Prototype functions are just like any other properties in the prototype. Those functions are stored in properties, where else would they be?

Regarding the "looks like this" sections, that is an imaginary view, because that doesn't exist. They would actually look like this (not showing the methods as your example didn't have any):

// firstDerived looks like this:             // secondDerived looks like this:
{                                            {
    newProp: "bar"                                baseProp: "test"
    __proto__: Base                               newProp: "bar"
        baseProp: "foo"                           __proto__: Base 
        __proto__: Base                               baseProp: "foo" 
}                                                     __proto__: Base
                                             }

This is expected to fail because Derived would've been a very different kind of constructor than Base, as Derived would have odd properties in its prototype that aren't in Base's prototype (namely baseProp). It also puts an unneccecary step in the prototype chain as you can see.

Regarding the Base/Derived example, that looks like a problematic situation all together. Especially the line Derived.prototype = new Base();. Because that has the side effect of "randomly" instantiating Base while in the middle of unrelated code. Secondly, it puts properties that are intended as own of a Base instance (such as baseProp) in the prototype, of another function. It becomes even more problematic if the constructor would need certain arguments passed, which would become impossible because Base was already instantiated.

Why would you instantiate a constructor to make prototypal inheritance (as opposed to a plain object that inherits directly from the other prototype object). That's horrible, and not how javascript is supposed to be used (other than for workarounds in old browsers). Such a pattern is unreliable and unpredictable. In the Base class it clearly says this.baseProp, which is referring to a (in)direct instance of Base, it is not supposed to end up in Derived's prototype.

The sane version behaves as expected:

function Base() {
    this.baseProp = 'foo';
}

function Derived() {
    Base.call(this);

    this.newProp = 'bar';
}

Derived.prototype = Object.create(Base.prototype);
Derived.prototype.constructor = Derived;

var firstDerived = new Derived();
var secondDerived = new Derived();
secondDerived.baseProp = 'test';

var aBase = new Base();
aBase.newProp = 'bar';

assert.propEqual(firstDerived, secondDerived); // not equal
assert.propEqual(firstDerived, aBase); // yes equal

See, now the objects actually look like what you imagined earlier:

// firstDerived looks like this:             // secondDerived looks like this:
{                                            {
    baseProp: "foo"                               baseProp: "test"
    newProp: "bar"                                newProp: "bar"
    __proto__: Derived                            __proto__: Derived 
}                                            }

[j-ulrich]

Oh. Ok. Sorry if my example was poor. I'm not really familiar with inheritance in JS. I just did a quick google and adapted the code at http://phrogz.net/JS/classes/OOPinJS2.html (although I forgot to overwrite the constructor).

However, I think everyone got the idea. :-)

[jzaefferer]

Commits to branches close tickets, reopening until it lands in master.