avajs / pretty-format

:sparkles: Stringify any JavaScript value
ISC License
1 stars 7 forks source link

Fix a bug on using `key` instead of `name = print(key,...)` #1

Closed caesarsol closed 7 years ago

caesarsol commented 7 years ago

This change broke all the tests!

I had major problems in testing an object with Symbol properties with AVA.

The worst part was the very unclear error message:

caesarsol $ ./node_modules/.bin/ava -v

  - testing object with symbol
Uncaught Exception: test.js
  TypeError: Cannot convert a Symbol value to a string
    printObject (node_modules/@ava/pretty-format/index.js:173:29)
    printComplexValue (node_modules/@ava/pretty-format/index.js:238:39)
    prettyFormat (node_modules/@ava/pretty-format/index.js:380:10)
    node_modules/clean-yaml-object/index.js:69:9
    cleanYamlObj (node_modules/clean-yaml-object/index.js:57:15)
    module.exports (node_modules/clean-yaml-object/index.js:3:9)

  ✖ Test results were not received from test.js

  0 tests passed [08:49:48]
  2 uncaught exceptions
caesarsol commented 7 years ago

But I agree that keys in Objects should be white and not red, should I open another PR?

novemberborn commented 7 years ago

@caesarsol turns out I wasn't watching this repo, hence #2 😄

@vadimdemedes why did you use key in the first place?

novemberborn commented 7 years ago

But I agree that keys in Objects should be white and not red, should I open another PR?

I agree as well. I've done some work to achieve this, unfortunately I didn't seem to be able to push it to your branch. I've pushed your commit to #2 instead. Let's continue the conversation there.

caesarsol commented 7 years ago

@novemberborn perfect, thank you!

I think @vadimdemedes did it by error, since all the tests were broken.

vadimdemedes commented 7 years ago

Yes, it was indeed my mistake, no idea why I replaced it.