Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Must can't handle ES6 (symbols) #36

Open
radekn opened this issue Dec 18, 2015 · 7 comments
Open

Must can't handle ES6 (symbols) #36

radekn opened this issue Dec 18, 2015 · 7 comments

Comments

@radekn
Copy link

radekn commented Dec 18, 2015

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
Copy link
Owner

moll commented Dec 18, 2015

Hey! Thanks for the catch! You're absolutely right. I fixed this quickly in a helper function (3fb59ad), 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
Copy link
Author

radekn commented Dec 18, 2015

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 radekn closed this as completed Dec 18, 2015
@radekn
Copy link
Author

radekn commented Dec 18, 2015

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]

@radekn radekn reopened this Dec 18, 2015
@moll
Copy link
Owner

moll commented Dec 18, 2015

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
Copy link
Owner

moll commented Dec 18, 2015

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

@moll
Copy link
Owner

moll commented Dec 18, 2015

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
Copy link
Author

radekn commented Jan 28, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants