tj / should.js

BDD style assertions for node.js -- test framework agnostic
MIT License
2.75k stars 194 forks source link

Problem comparing object to array #148

Closed ARAtlas closed 10 years ago

ARAtlas commented 10 years ago

I would expect the following unit test to pass:

var obj = { '0': 'hello' };
var array = ['hello'];

obj.should.be.instanceOf(Object);
array.should.be.instanceOf(Array);

obj.should.not.eql(array);

However, it fails with:

AssertionError: expected { '0': 'hello' } to not equal [ 'hello' ]
btd commented 10 years ago

In your case it is equal because enumerable properties are the same:

> Object.keys(obj)
[ '0' ]
> Object.keys(array)
[ '0' ]
> 

.eql not strictly equal (as i know there is no right way to know that 2 objects was produced with the same constructor function) and you need to combine this with other checks.

L8D commented 10 years ago

Would there be a sane way of checking whether they're both instances of or both not instances of array? On Nov 12, 2013 3:00 AM, "Denis Bardadym" notifications@github.com wrote:

In your case it is equal because enumerable properties are the same:

Object.keys(obj) [ '0' ] Object.keys(array) [ '0' ]

.eql not strictly equal (as i know there is no right way to know that 2 objects was produced with different constructor functions) and you need to combine this with other checks.

— Reply to this email directly or view it on GitHubhttps://github.com/visionmedia/should.js/issues/148#issuecomment-28277579 .

L8D commented 10 years ago

Perhaps a check like: obj1 instanceof Array === obj2 instanceof Array

btd commented 10 years ago

As i know one way it is using .constructor:

$ node
> var a = [];
undefined
> var b = {};
undefined
> a.constructor
[Function: Array]
> b.constructor
[Function: Object]
> 

But this is only convention and not an standard.

ghost commented 10 years ago

Wait, .constructor is convention? Are you sure about this? The V8 and TraceMonkey runtimes seem to behave this way.

btd commented 10 years ago

ecma-262 said about .constructor only in native objects. Correct me if i am wrong (i would like to be wrong there). Node team do not use it in assert module (they maybe better know then I). I know that ff and chrome add this in Object.create, but still as i said it is not a standard thing.

$ node
> function Test() {}
undefined
> (new Test).constructor
[Function: Test]
> 
L8D commented 10 years ago

Yeah, comparing .constructor can potentially cause issues if there is a class which extends Objectsomehow.

btd commented 10 years ago

I will close this, as we all know dynamic types nature of js do not allow us to check that references have the same time and user is responsible to check types (as usually he knows types better than abstract library) before comparing object of different types. If will be any good enough way to check this i will add it.