a few exception related issues

142 views
Skip to first unread message

Martijn Faassen

unread,
Aug 27, 2012, 12:42:56 PM8/27/12
to Buster.JS development
Hi there,

Here is some general feedback about buster's assert.exception. More
detail can be found in the issues:

https://github.com/busterjs/buster/issues/253

Buster expects exceptions to have a 'name' and 'message'. This is what
Error supplies so is reasonable, but toString() (and any other object
introspection?) should be tried if the exception object fails to
provide either, so we don't see too much 'undefined' in the output.

https://github.com/busterjs/buster/issues/272

I'd like to be able to pass an exception constructor function to the
'exception' argument of 'assert.exception' instead of a string. The
exception code can then use instanceof to check whether the exception
is of the right type. This one I was familiar with from qunit and
missed it so much (as I had a huge amount of tests using this) so I
hacked up my own variety of assert.exception to make it work). If this
sounds like a good idea, I'll try to fork buster assertions and add it
in.

https://github.com/busterjs/buster/issues/273

I realized today that assert.exception shouldn't treat it as a failure
if an unexpected error is thrown in code under test, but should treat
this as an error, just like when such an error is thrown elsewhere.
Could this be added by re-throwing the exception object if it's not
the expected one? Would we still have a correct traceback in that
case? If so, I can try adding this as well.

Regards,

Martijn

August Lilleaas

unread,
Aug 27, 2012, 4:32:56 PM8/27/12
to Martijn Faassen, Buster.JS development
There's been a number of discussions on the lack of features in assert.exceptions, we certainly want to improve it. I basically agree with everything you're saying.

Christian Johansen

unread,
Aug 27, 2012, 5:07:17 PM8/27/12
to August Lilleaas, Martijn Faassen, Buster.JS development
Actually, assert.exception already has been improved :) https://github.com/busterjs/referee/blob/master/lib/referee.js#L464

The new API looks like this:

assert.exception(fn[, matcher[, message]]);

assert.exception(function () { ... }); // Passes if the function throws an exception
assert.exception(function () { ... }, "Some message"); // Like above, with custom message
assert.exception(function () { ... }, { name: "TypeError", message: /meh/ }); // Passes if the function throws an error, AND assert.match(error, match); passes.

I guess it would make sense to also support

assert.exception(function () { ... }, Error); // Passes if the function throws an error that is instanceof Error

Christian
--
MVH
Christian

Martijn Faassen

unread,
Aug 28, 2012, 7:15:01 AM8/28/12
to Christian Johansen, August Lilleaas, Buster.JS development
Hello,

On Mon, Aug 27, 2012 at 11:07 PM, Christian Johansen
<chri...@cjohansen.no> wrote:
> Actually, assert.exception already has been improved :)
> https://github.com/busterjs/referee/blob/master/lib/referee.js#L464

Does the improved implementation also treat other exceptions as
errors, not as test failures?

Regards,

Martijn

Christian Johansen

unread,
Aug 31, 2012, 4:20:24 AM8/31/12
to Martijn Faassen, August Lilleaas, Buster.JS development
No, it still treats other exceptions as an assertion failure. I'm not really convinced it should cause test errors?

Martijn Faassen

unread,
Aug 31, 2012, 5:59:28 AM8/31/12
to Christian Johansen, August Lilleaas, Buster.JS development
Hi there,

On Fri, Aug 31, 2012 at 10:20 AM, Christian Johansen
<chri...@cjohansen.no> wrote:
> No, it still treats other exceptions as an assertion failure. I'm not really
> convinced it should cause test errors?

I figured that might not be clear. What is important is not the
designation error or failure so much as debugging information or not
(which is associated with failure and error). Consider this scenario:

assert.exception(f, e);

There are three possibilities:

f throws e, all is as expected, test passes

f doesn't throw any exception, which isn't our expectation, test fails

and the possibility under discussion:

f throws a different exception

now if the test fails, I'll get a notice that I get a different
exception than was expected.

Now as the developer of 'f()' I need to debug this scenario. So I need
to find out why this wrong exception was thrown. But I only get a
failure report from buster now, which doesn't give me any clue as to
where the problem lies. It quite possibly has absolutely nothing to do
with the exception I *do* expect to be thrown and might be in a
different part of the code.

So what is my recourse? I can change the test temporarily to do this:

f()

and then I'll get a proper error traceback I can debug.

I wouldn't have to do this if buster considered this an error in the
first place and thus reported with a traceback. The exception I'm
getting quite possibly has nothing to do with the exception I'm
expecting. It isn't that I, developer of f(), accidentally threw the
*wrong* exception (that's certainly a possible scenario, but is it the
likely one?) it's that some other part of f() is broken and this
brokenness is revealed by an exception.

JavaScript is perhaps a bit misleading in this area in that "catch" is
catch-all always (ignoring for the moment the catch conditionals that
aren't universally supported). Let's consider a language where
specific catch is the recommended practice, such as Python. Its
default unit test framework has an 'assertRaises' like this
(considering the wrong exception an error):

http://docs.python.org/library/unittest.html#unittest.TestCase.assertRaises

One of the more advanced testing frameworks for the language, py.test,
has a slightly different behavior that at first sight looks like
buster's:

http://docs.python.org/library/unittest.html#unittest.TestCase.assertRaises

but it is redeemed for the debugging scenario because it actually does
give traceback information for the unexpected exception. So this could
also be resolved by changing the 'failure' handling to include
traceback information in case of an exception-triggered failure
instead of making this an error.

Regards,

Martijn

Christian Johansen

unread,
Sep 3, 2012, 2:20:33 AM9/3/12
to Martijn Faassen, August Lilleaas, Buster.JS development
Hi,

http://docs.python.org/library/unittest.html#unittest.TestCase.assertRaises

One of the more advanced testing frameworks for the language, py.test,
has a slightly different behavior that at first sight looks like
buster's:

http://docs.python.org/library/unittest.html#unittest.TestCase.assertRaises

but it is redeemed for the debugging scenario because it actually does
give traceback information for the unexpected exception. So this could
also be resolved by changing the 'failure' handling to include
traceback information in case of an exception-triggered failure
instead of making this an error.

Including the stack trace with the failure seems like the best of both worlds
to me. Fixed in referee, will be part of the next Buster release.

Thanks!

Christian


Martijn Faassen

unread,
Sep 3, 2012, 12:19:02 PM9/3/12
to Christian Johansen, August Lilleaas, Buster.JS development
Hey,

On Mon, Sep 3, 2012 at 8:20 AM, Christian Johansen
<chri...@cjohansen.no> wrote:

> Including the stack trace with the failure seems like the best of both
> worlds to me. Fixed in referee, will be part of the next Buster release.

Cool, thanks!

Regards,

Martijn
Reply all
Reply to author
Forward
0 new messages