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

Must can't handle ES6 (symbols) #36

Open radekn opened 8 years ago

radekn commented 8 years ago

Using Must in Node.js 5.3, the following code:

demand(Symbol("Hi there!")).be.undefined()

correctly throws, but the error message is highly confusing:

AssertionError: undefined must be undefined

Seems like Must uses a custom formatter for displaying values in situations like this, and it doesn't account for the possibility of encountering an object of a type unknown to it.

It should be fairly easy to at least add a fallback to String(value) for such cases in order to have forward compatibility. What do you think?

moll commented 8 years ago

Hey! Thanks for the catch! You're absolutely right. I fixed this quickly in a helper function (https://github.com/moll/js-must/commit/3fb59ada5346881aed5325fd41543f8f002446a2), but see stringifying in general could use a little refactoring.

Please give that commit a try and if you give it a green light, I'll release a new patch bump with it. ;) Thanks!

radekn commented 8 years ago

Thanks, I just tried that commit and it works fine for me, so I'm closing the issue.

BTW, I really like the library. I migrated my hobby project from Chai to Must just a few days ago - I started looking for alternatives precisely because of the inconsistency of chai/should API (with regards to asserting on access vs call), and it's nice to know I'm not the only one seeing a problem there. As for Must, It can sometimes be a little rough on the edges, but shows great promise and it's already very usable. Keep up the good work :)

radekn commented 8 years ago

Oh wait, I closed it too soon. Seems like there are still cases where symbols are reported as null, e.g. when testing arrays containing them for equivalence. This code:

demand([Symbol("hello!")]).be.eql([Symbol("bye!")])

throws with the following message:

AssertionError: [null] must be equivalent to [null]

moll commented 8 years ago

Ah, you stumbled upon exactly the case I hinted at with refactoring. :-) Thanks. I'll do that then after dinner today. ;-)

Thanks for the love, too. Mind sharing what rough edges you noticed that I could perhaps get at with the same stick? :)

moll commented 8 years ago

Refactored stringifying to stringify both Symbols and also RegExps in nested objects. Please give it a try now and let me know. Thanks!

moll commented 8 years ago

While I was there, I threw in Must.prototype.symbol. If you have ideas of other matchers you'd like to see, do share. ;)

radekn commented 8 years ago

Hi there, and sorry for late reply. I tried the latest commit, and it works for the above cases, but now I discovered another one where it doesn't. When I use a permutationOf matcher, it always fails with the following stack trace:

TypeError: Cannot convert a Symbol value to a string
      at Array.sort (native)
      at isPermutationOf (node_modules/must/must.js:614:27)
      at Must.permutationOf (node_modules/must/must.js:606:12)

Edit: I think the problem with permutationOf is not exclusive to Symbols, so I opened a new issue: #38.