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

throw undefined #1395

Closed
disjukr opened this issue Oct 19, 2014 · 4 comments
Closed

throw undefined #1395

disjukr opened this issue Oct 19, 2014 · 4 comments
Labels
type: bug a defect, confirmed by a maintainer

Comments

@disjukr
Copy link

disjukr commented Oct 19, 2014

it('expected', function () {
    throw 'foo';
});
it('unexpected', function () {
    throw undefined;
});

then

  1) expected
  ✓ unexpected

in 1.21.5

@dasilvacontin
Copy link
Contributor

I'm working on this issue right now.

I agree that throwing undefined should make the test fail. What about throwing null?

Current behaviour is: a test passes if thrown either undefined or null. UNLESS you are throwing async on an async test, due to Runner.prototype.uncaught. It even generates an error with the message 'Caught undefined error, did you throw without specifying what?'. Only failing in that case must not be an intended behaviour.

Checking if the thrown exception is undefined can only be done in the catch block, since passing undefined to the callback means 'no error here'. (Well, you could pass to the callback a reference to a global object that means undefined, and then check the reference later, but that's quite ugly)

I made changes so that throwing undefined or null makes the test fail. However, it breaks the following project test case (line 228 at test/runnable.js):
Runnable(title, fn), .run(fn), when async, when an exception is thrown, should not throw its own exception if passed a non-object.

If I make it so that throwing null doesn't make a test fail whereas throwing undefined does make a test fail, the Runnable project test case does not break. But that's because it only checks throwing null, and not throwing undefined, which I believe is also a non-object.

I will send a PR with some changes and new tests. I'm marking as pending the Runnable test that breaks until we decide something.

IMO, if you are throwing, it should make the test fail.

@boneskull boneskull added the type: bug a defect, confirmed by a maintainer label Nov 7, 2014
@boneskull
Copy link
Contributor

Agreed; if anything is thrown, the test should fail.

@boneskull
Copy link
Contributor

@travisjeffery Looking for your feedback here if you have any.

@dasilvacontin
Copy link
Contributor

See #1410.

travisjeffery added a commit that referenced this issue Nov 27, 2014
* dasilvacontin-fix/throwUndefined:
  Fix test
  fix throwing undefined/null now makes tests fail, fixes #1395
tandrewnichols pushed a commit to tandrewnichols/mocha that referenced this issue Dec 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

3 participants