fix: improved error messages

[microcood]

What:

• It fixes several errors which are thrown with type related messages, e.g. toBeInTheDOM fails with
  • str.match is not a function
  • Cannot read property 'match' of null
• Also fixes when received value is not html element: error stack was not captured and nice picture of the guilty line was not shown.
• Added snapshots to prevent similar errors in the future

Why:
User experience matters for developers too.

How:
Mostly by copying ideas from original jest/expect package.

Checklist:

• Documentation "N/A"
• Tests
• Ready to be merged

[gnapse]

This looks great! Thank you very much. Indeed developer experience matters a lot.

So far I just have a couple of questions/suggestions for your consideration. Let me know what you think.

[gnapse]

What was wrong with using getMessage? Isn't this more or less the common pattern of message that getMessage ensures?

I get why you removed its use from other cases (where the expect().to... line made the expected part evident already), but in this case it looks like you're replicating what getMessage already gives you.

[microcood]

This is lazy fix for the case when receivedAttribute is null. Out of the box stringify in getMessage will add quotes for already strings with escaping e.t.c.

Now i'm starting to think about removing 'Received:' part completely in such cases, but not so sure.

[gnapse]

Sorry for insisting, but what do you refer to with stringify in getMessage? I don't see stringify being used in it.

I insist because in every case we can get away with using getMessage is better, in case we change that format of messages we have a centralized way of doing it.

[microcood]

Yes, there is no stringify so it fails with Cannot read property 'match' of null. Ok i will look into making it work with non-strings.

[gnapse]

Thank you very much!

[microcood]

Fixed.You’re welcome!

[gnapse]

How about using getMessage here too? It could read something like this:

expect(element).toBeInTheDOM()

Expected:
  <red>element</> must be an HTMLElement
Received:
  object: <red>{\\"thisIsNot\\": \\"an html element\\"}</>"

[microcood]

This is clone of the value must be a Promise error message. 'Expected:' part is usually used when there are some arguments in the actual matcher e.g. as in toBeEqual. Also printWithType is not compatible with getMessage, but too handy in this case.

[gnapse]

Ok, fine then.

[gnapse]

Looks good now, thanks again. BTW, care to add yourself to the contributors list? It's ok if you don't want to.

[gnapse]

@microcood I merged your PR but the release is not working. Apparently there's a problem with the serialization of error messages into snapshots, because in environments such as Travis CI the terminal does not support the color codes, so the snapshots do not match. I found this issue jestjs/jest#4407 in jest's repo about it.

Can you take a look into how this could be solved? From what they mention there, there should exist a way to customize the output with a serializer that would omit all the special coding of colors, but they do not link to it.

If you can't I'll take a look at it later.

[gnapse]

I tentatively created #16, which I won't merge yet, but will probably merge it by the end of the day if I find no such serializer already provided or some other solution arises. If that's the case, you or someone else can happily provide a fix in a future PR supporting snapshots properly.

[microcood]

I don't think that is a serializer issue - jest uses the same. So maybe it will make sense to look into CI config itself

[gnapse]

🎉 This PR is included in version 1.3.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[gnapse]

I don't think that is a serializer issue - jest uses the same. So maybe it will make sense to look into CI config itself

I get it, will look into it, but in the mean time this PR fixes the "str.match is not a function" and "Cannot read property 'match' of null`" errors, so I prefer to have it out there. I'll try to bring back the snapshots one of these days, either with or without stripping out the ansi stuff. Will look into Travis' settings and possibly follow up open that travis issue you referred to.