Skip to content

Unexpected throw in .throws() #1439

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
StoneCypher opened this issue Jul 5, 2017 · 9 comments
Closed

Unexpected throw in .throws() #1439

StoneCypher opened this issue Jul 5, 2017 · 9 comments

Comments

@StoneCypher
Copy link
Contributor

You can replicate by pulling my repo and uncommenting the commented-out test.

Test Source

  it('probable exits for throws on non-state', t => t.throws(() => {
    const machine = new jssm.machine({
      initial_state: '1',
      transitions:[
        { name:'id1', from:'1', to:'2', action:'identical' }
      ]
    });
    // why does this test fail Threw unexpected exception: 'No such state "3" in probable_exits_for' >:(
    machine.probable_exits_for('3');
  }, Error));

Error Message & Stack Trace

  general › Illegal machines probable exits for throws on non-state
  /Users/johnhaugeland/projects/jssm/src/js/tests/general.js:780

   779:                                                                     
   780:   it('probable exits for throws on non-state', t => t.throws(() => {
   781:                                                                     

  Threw unexpected exception:

  'No such state "3" in probable_exits_for'

The exception is expected, and that is the correct exception message.

This test is a .throws(). I don't understand why this is treated as a failure. Near-verbatim identical tests cover my other throws and work as expected.

Config

I do not configure Ava. No such block or .rc exists in my project.

Command-Line Arguments

My build is trivial.

    "test": "ava src/js/tests/*.js",

Relevant Links

This is the repo.

Environment

Replicates under MacOS current and Windows 10 Pro current. Both tested under node 8 current.

Replicates on travis-ci under node-6, node-7, and node-8, all current, all under whatever linux they use.

All are using ava 0.20.0, installed as local node modules.

@sindresorhus
Copy link
Member

You're throwing a string and not an Error object: https://github.com/StoneCypher/jssm/blob/6f22cc476a2a24476a10f6d5c8f75bead619f48e/src/js/jssm.js#L265

@StoneCypher
Copy link
Contributor Author

Oh.

May I please recommend that the error message here point that out as a possibility? It would not have occurred to me that throwing a string wouldn't be caught by .throws .

@StoneCypher
Copy link
Contributor Author

And/or should strings thrown be caught by .throws ?

@kevva
Copy link
Contributor

kevva commented Jul 5, 2017

You should use throw new Error(string) in your code. Read more about the Error object here https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Error#Throwing_a_generic_error.

@StoneCypher
Copy link
Contributor Author

@kevva - sorry, no, throw can and always has legally accepted strings, as the first example on your source shows.

@kevva
Copy link
Contributor

kevva commented Jul 5, 2017

I'm aware of that, but it's a bad practice.

And/or should strings thrown be caught by .throws ?

There has been some discussion around it in #661 and a new proposal for throws in #1047.

Will close this as a duplicate.

@kevva kevva closed this as completed Jul 5, 2017
@StoneCypher
Copy link
Contributor Author

this is not a duplicate of 661; the behavior is entirely different, and does not appear to be defective in presentation

this is not a duplicate of 1047, which is about simplifying throws, and barely touches on this in passing

i wish this had not been closed, and generally it's polite to ask the person opening the ticket before closing

i retain the belief that throws should accept all valid throws, which it currently does not, without mentioning that in the documentation

@sindresorhus
Copy link
Member

@StoneCypher It's because you specified it to expect an Error object in the assertion, but you threw a string. I've opened #1440 instead to improve the experience.

@StoneCypher
Copy link
Contributor Author

Thank you.

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

3 participants