throwException() improvement #9

Closed
satazor opened this Issue Feb 3, 2012 · 3 comments

Projects

None yet

2 participants

@satazor

I have a project that does a lot of validations, and if something wrong happens an error is thrown.

If I got a test that asserts an exception being throw and if I accidentally call an undefined method inside the code being tested, the assert passes because it was expected to throw an error. To solve this, we could somehow let users specify the expected type of error (ReferenceError, TypeError, etc). But again, this won't be enough, because one could want to test against the error.code or error.message. This test could be direct or using a regular expression to test for dynamic error messages like 'Method foo() is incomplete ..'.

What you guys think?

@satazor

Maybe something like:

throwError() <- standard, will catch Error
throwError(ReferenceError) <- will catch only ReferenceErrors
throwError({ type: ReferenceError, message: 'Some Message', code: 'some_code' });

When using an object literal, everything is optional:

  • type defaults to Error
  • message defaults no null (all messages will be caught)
  • code defaults to null (all codes will be caught)

Both message and code can be a string or a regex, so it gives us the possibility to catch more dynamic errors as mentioned above.

If you guys agree with this syntax, I will be glad to implement it myself and make a pull request.

@rauchg

How about:

expect(fn).to.throwException(function (e) {
  expect(e.message).to.match(/my message/);
});

The idea is to be able to now get access to the exception object, but leverage the full set of expect.js utilities. The main reason behind this is that you don't need to learn lots of different possible syntaxes for a specific utility, in this case throwException.

For example, in addition to supporting the object/property validation like you suggested, we would also need to support regular expressions:

expect(fn).to.throwException({ message: /test/ });

But then again you might want to check both the constructor and the message with a regular expression, and calling the function twice could mean changing state that renders the test useless.

expect(fn).to.throwException(function (e) {
  expect(e).to.be.an(RandomException);
  expect(e.message).to.match(/test/);
});

addresses both use cases.

Finally, since in my opinion checking message is what's most useful (in fact, we do that a lot in the should.js/expect.js tests), there's value to supporting:

expect(fn).to.throwException(/test/);
@satazor

Totally agree on that one.

@rauchg rauchg added a commit that closed this issue Feb 4, 2012
@rauchg rauchg Fixes #9 e73b50c
@rauchg rauchg closed this in e73b50c Feb 4, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment