search

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

`permutationOf` is not reliable for arrays containing non-primitive values #38

Open radekn opened 8 years ago

radekn commented 8 years ago

Example case:

a = function () {}
b = function () {}

demand([a, b]).be.permutationOf([a, b]) // this passes
demand([b, a]).be.permutationOf([a, b]) // this fails

The second assertion fails with AssertionError: [null,null] must be a permutation of [null,null]. This problem is caused by array sort function converting values to strings by default and using them for deciding what goes first.

[rant] Unfortunately, in JavaScript, for most types of values (like e.g. functions, objects, arrays, and recently symbols, maybe also regexps) there is no defined order. IMO, this is one of the reasons JS sucks, because it unnecessarily forces us into linear time complexity (or some dirty tricks), where otherwise logarithmic would be possible, or quadratic instead of linear. At least there is Map that helps alleviate the problem a bit. [/rant]

moll commented 8 years ago

Hey again. You're quite right. Thanks for bringing this up.

I agree wrt to JavaScript, the language. So many abstractions lacking that have been around for decades in the programming language space.

Anywho, due to lack of stable sort order, I think the permutationOf matcher needs to be redesigned to use Map or any of its polyfills to handle the case you bring up. Do you know what some other assertion libs do to solve this? Do they even solve this?

radekn commented 8 years ago

It looks like none of the other popular libs has a permutationOf or equivalent matcher - I glanced over the API docs of Should.js, Expect and Chai. Even Lodash doesn't have a helper for this. Maybe this issue is part of the reason for that. Achieving the same effect with the other libs would probably require something like this:

expect(a.length).to.be(b.length)
b.forEach(v => expect(a).to.contain(v))

Its main drawback is quadratic time complexity, assuming that contain has linear. Using Map would allow to reduce it to O(n) or O(n log n), depending on its (Map's) implementation.