moll / js-must

An assertion library for JavaScript and Node.js with a friendly BDD syntax (awesome.must.be.true()). It ships with many expressive matchers and is test runner and framework agnostic. Follows RFC 2119 with its use of MUST. Good stuff and well tested.
Other
336 stars 35 forks source link

`eql` and `equal` too similar words #21

Open creynders opened 9 years ago

creynders commented 9 years ago

Is there a specific reason you didn't want to use deepEqual or deep.equal? All the other assertions are pretty readable, but I constantly need to double-check eql or equal. Also, IMO, it's a weird discrepancy in the API. Intuitively I'd think eql is an alias for equal, like gt et cetera.

moll commented 9 years ago

Hey!

Thanks for caring!

The inspiration for eql and equal came from a few sources: Ruby's RSpec, Should.js and most of all, the commonness of shorter equivalence operators to be less strict. Like in JavaScript: == vs ===. I believe tests should never do type-coercion therefore eql is still type-safe while being recursive.

Having said that, I get what your liking of the "deep" prefix. deep.equal would clash with Must.js's goal of decoupled matchers (sans the not link of course :), but I wouldn't mind deepEqual or deepEql to be aliases to all.

What do you think? Should deepEqual be merely a recursive version of equal (comparing object-identity otherwise) and deepEql then the same as eql now?

Just to clarify, the looseness of eql comes from the ability to compare value types like Date or your own types.

As Must.js hasn't hit v1.0.0 yet (but it should), we can still do a few breaking changes. And add a migration-helper sed script, of course. ^_^. Ideas, @koresar?

creynders commented 9 years ago

Should deepEqual be merely a recursive version of equal (comparing object-identity otherwise) and deepEql then the same as eql now?

:+1: Yeah that would be great!

moll commented 9 years ago

Would you agree that recursiveness should only apply to plain objects and arrays? The rest, in deepEqual's case, should be compared strictly (===)?

What, do you reckon, should eql then end up doing? Be a non-recursive version of loose checking? E.g:

{}.must.eql({}) // fail
[].must.eql([]) // fail
5..must.eql(5) // pass
new Date(2000, 5).must.eql(new Date(2000, 5)) // pass
creynders commented 9 years ago

Good question. Ok, when you start to think about this stuff, it suddenly gets pretty complicated. So, based on my gut feeling, I'd say equal is strict, as it is now. Two objects are deepEqual if they have strictly equal values for their members recursively AND a strictly equal prototype. eql and deepEql reflect this, but with loose equality.

E.g.

9.must.equal("9") //fail
9.must.deepEqual("9") // fail
9.must.eql("9") // pass
9.must.deepEql("9") // pass

{}.must.equal({}) // fail
{}.must.deepEqual({}) // pass
{}.must.eql({}) // fail
{}.must.deepEql({}) // pass

{foo:"9"}.must.equal({foo:"9"}) // fail
{foo:"9"}.must.deepEqual({foo:"9"}) //pass
{foo:"9"}.must.eql({foo:"9"}) // fail
{foo:"9"}.must.deepEql({foo:"9"}) //pass

{foo:9}.must.equal({foo:"9"}) // fail
{foo:9}.must.deepEqual({foo:"9"}) //fail
{foo:9}.must.eql({foo:"9"}) // fail
{foo:9}.must.deepEql({foo:"9"}) //pass

o1 = new Foo("whatever");
o2 = new Foo("whatever");
o1.must.equal(o2) // fail
o1.must.deepEqual(o2) //pass
o1.must.eql(o2) // fail
o1.must.deepEql(o2) // pass

This is what seems most logical to me. However, it will be pretty nightmarish to migrate tests I think.

So, the easiest solution would be to alias eql with deepEqual and forget about deepEql.

koresar commented 9 years ago

The naming of functions in the must.js is somewhat confusing (see #6). I agree with @creynders on that.

Must.prototype.equal(expected)

Assert object strict equality or identity (===).

I'd rename equal to strictEqual.

Must.prototype.eql(expected)

Assert object equality by content and if possible, recursively.

I'd rename eql to objectEqual.

rightaway commented 9 years ago

+1 for deepEqual to alias eql