Skip to content

Let's make .throws failed test reporting more consistent with others #1363

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

Closed
revelt opened this issue Apr 20, 2017 · 2 comments
Closed

Let's make .throws failed test reporting more consistent with others #1363

revelt opened this issue Apr 20, 2017 · 2 comments

Comments

@revelt
Copy link

revelt commented Apr 20, 2017

Hi all,

When you use t.throws and the test is failing, you get a message "Missing expected exception (err)..":
screen shot 2017-04-20 at 07 20 30

but comparison assertions append the [message] argument on failing assertions, for example:
screen shot 2017-04-20 at 07 23 22
(obviously, we set that message string to whatever we want, but I kept it "message" for briefness).

My Pain

I tend to group multiple assertions under one test() and I find out which assertion is failing from that [message] argument, because I number my tests in a format feature.test.assertion. In example test #95 above, I actually would set message argument to the order number, in particular case, '95.01.03', so I know that it's third assertion failing.

screen shot 2017-04-20 at 07 30 41

Now since t.throws wants [error] argument as the actual error message, all throws within the same unit test show the same error message ("Missing expected exception (err)..") and it becomes impossible to find out which t.throws is failing within the same test().

Feature suggestion

Do you feel that t.throws is missing the second [message] argument? Something like:

.throws(function|promise, [error, [error's message]], [message if this assertion fails])

I'd like to put null as second argument, and have third argument [message] shown when throw assertion is failing.

To critics

I know some of critics out there will say don't group multiple assertions under one test and it's problem solved — the title of the test becomes the identifier.

I am aware of that and it's a valid point, albeit rather tenuous-one.

It doesn't feel DRY to have one assertion per-test. On the opposite, tests look scattered and messy with no organisation when put that way. That's why multiple assertions were allowed under test() at the first place, was it? To group them!

That's why I prefer to group assertions by tested feature, naming tests testedFeaturesIdNumber.unitTestNumber.assertionNumber. For example, in the screenshot above, number id 95 is reserved to utility function util.reclaimIntegerString (btw, for posterity, I'm naming my utilities' assertions backwards counting from 99 down, while normal features count from 01 up).

@novemberborn
Copy link
Member

This is much improved with the recent 0.19 release. There is one last issue in #1342, and a plan for improving t.throws() in #1047 that is waiting for somebody to actually do the work.

I'll close this now, but please reopen if the above issues or 0.19 don't improve your use case.

@revelt
Copy link
Author

revelt commented Apr 20, 2017

OK, I see, we can identify which test didn't threw by row number. That's acceptable then.

throw

Good job with 0.19.1 release!

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