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

WIP: Format failed external asserts nicer #2230

Closed
wants to merge 4 commits into from

Conversation

tryzniak
Copy link
Contributor

Hello. This for #1277 . I'd love to show you what I've been up to, here are the new output
image
testfile

I don't really like the way supertest errors are shown, I have no idea how we could detect what line of the test its expect failed.
Other errors have inconsistent formatting, which is not very nice.
I'm thinking of capturing and detecting a type of an error earlier.
Any input would be useful for me! Thanks

External asserts like:
- chai
- expect

Closes avajs#1277
Copy link
Member

@novemberborn novemberborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's an interesting approach. You're looking at the thrown error to see if it may have come from an assertion library? And then the hope is that its message is useful?

lib/test.js Show resolved Hide resolved
@tryzniak
Copy link
Contributor Author

tryzniak commented Sep 6, 2019

Oh that's an interesting approach. You're looking at the thrown error to see if it may have come from an assertion library? And then the hope is that its message is useful?

Yeah! Judging by a quick look, they usually have an already formatted message.

@novemberborn
Copy link
Member

Oh that's an interesting approach. You're looking at the thrown error to see if it may have come from an assertion library? And then the hope is that its message is useful?

Yeah! Judging by a quick look, they usually have an already formatted message.

Sounds great!

Copy link
Member

@novemberborn novemberborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tryzniak could you add some tests for this? Let me know if you'd like some pointers.

lib/test.js Show resolved Hide resolved
@novemberborn
Copy link
Member

Closing due to inactivity. We'd be happy to pick this up again when you have the time to get back to this.

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

Successfully merging this pull request may close these issues.

2 participants