Reimplement t.throws() and t.notThrows() #1704

novemberborn · 2018-02-12T17:44:49Z

Implement t.throws() and t.notThrows() ourselves.

Remove core-assert dependency. When passed a function as the second argument, t.throws() now assumes it's a constructor. This removes support for a validation function, which may or may not have worked. Refs #1047.

t.throws() now fails if the exception is not an error. Fixes #1440.

Regular expressions are now matched against the error message, not the result of casting the error to a string. Fixes #1445.

Validate second argument to t.throws(). Refs #1676.

Assertion failures now display how AVA arrived at the exception. Constructors are printed when the error is not a correct instance. Fixes #1471.

Still need to support an expectation object. I'm planning to implement this alongside the current support for passing constructors, strings or regular expressions. The shorthand form seems useful and this way we won't have too many backward compatibility issues.

I'll be looking to support (a combination of) the following use cases:

t.throws(fn, {of: SyntaxError}) // err instanceof SyntaxError
t.throws(fn, {name: 'SyntaxError'}) // err.name === 'SyntaxError'
t.throws(fn, {is: expectedErrorInstance}) // err === expectedErrorInstance
t.throws(fn, {message: 'expected error message'}) // err.message === 'expected error message'
t.throws(fn, {message: /expected error message/}) // /expected error message/.test(err.message)

I'll try to post some screenshots too.

Move tests from test/promise.js and test/observable.js.

novemberborn · 2018-02-13T14:33:07Z

The expectation object works now.

The various examples run to hundreds of lines, so please expand the details below:

throws.js

import test from 'ava';
import Observable from 'zen-observable';

test.serial('fails, not a valid thrower', t => {
  t.throws(true);
});

test.serial('fails, not a valid expectation', t => {
  t.throws(() => {}, true);
});

test.serial('fails, not a valid expectation (of)', t => {
  t.throws(() => {}, {of: true});
});

test.serial('fails, not a valid expectation (message)', t => {
  t.throws(() => {}, {message: true});
});

test.serial('fails, not a valid expectation (name)', t => {
  t.throws(() => {}, {name: true});
});

test.serial('fails, not a valid expectation (unknown)', t => {
  t.throws(() => {}, {unknown: true});
});

test.serial('fails, does not throw', t => {
  t.throws(() => true);
});

test.serial('fails, not an error', t => {
  t.throws(() => {
    throw {foo: 'bar'};
  });
});

test.serial('fails, wrong instance', t => {
  const err = new Error();
  t.throws(() => {
    throw new Error();
  }, {is: err});
});

test.serial('fails, wrong name', t => {
  t.throws(() => {
    throw new Error();
  }, {name: 'TypeError'});
});

test.serial('fails, wrong message (string)', t => {
  t.throws(() => {
    throw new Error('foo');
  }, {message: 'bar'});
});

test.serial('fails, wrong message (regexp)', t => {
  t.throws(() => {
    throw new Error('foo');
  }, {message: /bar/});
});

test.serial('fails, does not reject', async t => {
  await t.throws(Promise.resolve(true));
});

test.serial('fails, does not error', async t => {
  await t.throws(Observable.of(true));
});

test.serial('fails, returned promise does not reject', async t => {
  await t.throws(() => Promise.resolve(true));
});

test.serial('fails, returned observable does not error', async t => {
  await t.throws(() => Observable.of(true));
});

test.serial('fails, returned promise does not reject with an error', async t => {
  await t.throws(() => Promise.reject(true));
});

test.serial('fails, returned observable does not error with an error', async t => {
  await t.throws(() => new Observable(observer => observer.error(true)));
});

output

$ npx ava throws.js -v

  ✖ fails, not a valid thrower `t.throws()` must be called with a function, observable or promise
  ✖ fails, not a valid expectation The second argument to `t.throws()` must be a function, string, regular expression, expectation object or `null`
  ✖ fails, not a valid expectation (of) The `of` property of the second argument to `t.throws()` must be a function
  ✖ fails, not a valid expectation (message) The `message` property of the second argument to `t.throws()` must be a string or regular expression
  ✖ fails, not a valid expectation (name) The `name` property of the second argument to `t.throws()` must be a string
  ✖ fails, not a valid expectation (unknown) The second argument to `t.throws()` contains unexpected properties
  ✖ fails, does not throw 
  ✖ fails, not an error 
  ✖ fails, wrong instance 
  ✖ fails, wrong name 
  ✖ fails, wrong message (string) 
  ✖ fails, wrong message (regexp) 
  ✖ fails, does not reject 
  ✖ fails, does not error 
  ✖ fails, returned promise does not reject 
  ✖ fails, returned observable does not error 
  ✖ fails, returned promise does not reject with an error 
  ✖ fails, returned observable does not error with an error 

  18 tests failed

  fails, not a valid thrower

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:5

   4: test.serial('fails, not a valid thrower', t => {
   5:   t.throws(true);                               
   6: });                                             

  `t.throws()` must be called with a function, observable or promise

  Called with:

  true

  Try wrapping the first argument to `t.throws()` in a function:

    t.throws(() => { /* your code here */ })

  Visit the following URL for more details:

    https://github.com/avajs/ava#throwsfunctionpromise-error-message



  fails, not a valid expectation

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:9

   8: test.serial('fails, not a valid expectation', t => {
   9:   t.throws(() => {}, true);                         
   10: });                                                 

  The second argument to `t.throws()` must be a function, string, regular expression, expectation object or `null`

  Called with:

  true



  fails, not a valid expectation (of)

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:13

   12: test.serial('fails, not a valid expectation (of)', t => {
   13:   t.throws(() => {}, {of: true});                        
   14: });                                                      

  The `of` property of the second argument to `t.throws()` must be a function

  Called with:

  {
    of: true,
  }



  fails, not a valid expectation (message)

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:17

   16: test.serial('fails, not a valid expectation (message)', t => {
   17:   t.throws(() => {}, {message: true});                        
   18: });                                                           

  The `message` property of the second argument to `t.throws()` must be a string or regular expression

  Called with:

  {
    message: true,
  }



  fails, not a valid expectation (name)

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:21

   20: test.serial('fails, not a valid expectation (name)', t => {
   21:   t.throws(() => {}, {name: true});                        
   22: });                                                        

  The `name` property of the second argument to `t.throws()` must be a string

  Called with:

  {
    name: true,
  }



  fails, not a valid expectation (unknown)

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:25

   24: test.serial('fails, not a valid expectation (unknown)', t => {
   25:   t.throws(() => {}, {unknown: true});                        
   26: });                                                           

  The second argument to `t.throws()` contains unexpected properties

  Called with:

  {
    unknown: true,
  }



  fails, does not throw

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:29

   28: test.serial('fails, does not throw', t => {
   29:   t.throws(() => true);                    
   30: });                                        

  Function returned:

  true



  fails, not an error

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:33

   32: test.serial('fails, not an error', t => {
   33:   t.throws(() => {                       
   34:     throw {foo: 'bar'};                  

  Function threw exception that is not an error:

  {
    foo: 'bar',
  }



  fails, wrong instance

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:40

   39:   const err = new Error();
   40:   t.throws(() => {        
   41:     throw new Error();    

  Function threw unexpected exception:

  Error {
    message: '',
  }

  Expected to be strictly equal to:

  Error {
    message: '',
  }



  fails, wrong name

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:46

   45: test.serial('fails, wrong name', t => {
   46:   t.throws(() => {                     
   47:     throw new Error();                 

  Function threw unexpected exception:

  Error {
    message: '',
  }

  Expected name to equal:

  'TypeError'



  fails, wrong message (string)

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:52

   51: test.serial('fails, wrong message (string)', t => {
   52:   t.throws(() => {                                 
   53:     throw new Error('foo');                        

  Function threw unexpected exception:

  Error {
    message: 'foo',
  }

  Expected message to equal:

  'bar'



  fails, wrong message (regexp)

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:58

   57: test.serial('fails, wrong message (regexp)', t => {
   58:   t.throws(() => {                                 
   59:     throw new Error('foo');                        

  Function threw unexpected exception:

  Error {
    message: 'foo',
  }

  Expected message to match:

  /bar/



  fails, does not reject

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:64

   63: test.serial('fails, does not reject', async t => {
   64:   await t.throws(Promise.resolve(true));          
   65: });                                               

  Promise resolved with:

  true



  fails, does not error

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:68

   67: test.serial('fails, does not error', async t => {
   68:   await t.throws(Observable.of(true));           
   69: });                                              

  Observable completed with:

  [
    true,
  ]



  fails, returned promise does not reject

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:72

   71: test.serial('fails, returned promise does not reject', async t => {
   72:   await t.throws(() => Promise.resolve(true));                     
   73: });                                                                

  Returned promise resolved with:

  true



  fails, returned observable does not error

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:76

   75: test.serial('fails, returned observable does not error', async t => {
   76:   await t.throws(() => Observable.of(true));                         
   77: });                                                                  

  Returned observable completed with:

  [
    true,
  ]



  fails, returned promise does not reject with an error

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:80

   79: test.serial('fails, returned promise does not reject with an error', asy…
   80:   await t.throws(() => Promise.reject(true));                            
   81: });                                                                      

  Returned promise rejected with exception that is not an error:

  true



  fails, returned observable does not error with an error

  /private/var/folders/2_/qczp184x76b2nl034sq5hvxw0000gn/T/tmp.toBbmRf1AV/throws.js:84

   83: test.serial('fails, returned observable does not error with an error', a…
   84:   await t.throws(() => new Observable(observer => observer.error(true)));
   85: });                                                                      

  Returned observable errored with exception that is not an error:

  true

sindresorhus · 2018-02-13T14:57:29Z

t.throws(fn, {of: SyntaxError}) // err instanceof SyntaxError

I don't think of is clear enough. Maybe name it instanceOf or constructor?

From the original issue:

We should also have much better validation logic to prevent mistakes. For example, using is-error-constructor on the constructor matcher.

Could you do this?

sindresorhus · 2018-02-13T15:00:27Z

What do you think about dropping support for t.throws(() => {}, TypeError);. It's not that commonly used and can now be done with t.throws(() => {}, {of: TypeError}); instead, which is only slightly longer. Just checking the type of the error is usually an anti-pattern, as it's too generic of a check.

novemberborn · 2018-02-13T15:11:31Z

From the original issue:

We should also have much better validation logic to prevent mistakes. For example, using is-error-constructor on the constructor matcher.

Could you do this?

I'm not sure it's necessary. We always check that the value is an error, so if it's not then you'd never get to the instanceof check. is-error checks the string tag, which works across realms, whereas is-error-constructor does not, so that wouldn't be consistent either.

t.throws(fn, {of: SyntaxError}) // err instanceof SyntaxError

I don't think of is clear enough. Maybe name it instanceOf or constructor?

I like the brevity of of but I've also had some doubts about it. instanceOf seems most appropriate, though the capital O is cumbersome to type. constructor would work but it doesn't quite signal how we run the assertion. instanceOf then?

What do you think about dropping support for t.throws(() => {}, TypeError);. It's not that commonly used and can now be done with t.throws(() => {}, {of: TypeError}); instead, which is only slightly longer. Just checking the type of the error is usually an anti-pattern, as it's too generic of a check.

That depends how tightly you want to couple your test to any current error message. E.g. when validating input type checks, looking for TypeError can be sufficient. Similarly if you're testing an API with custom errors then often the error class is the relevant bit and the message may be static.

{instanceOf: TypeError} is a fair bit longer, too.

sindresorhus · 2018-02-13T15:13:11Z

readme.md


-Returns the error thrown by `function` or a promise for the rejection reason of the specified `promise`.
+`expected` can be a constructor, in which case the thrown error must be an instance. It can be a string, which is compared against the thrown error's message, or a regular expression which is matched against this message. You can also specify a matcher object with one or more of the following properties:


must be an instance => must be an instance of the constructor

sindresorhus · 2018-02-13T15:16:05Z

readme.md

+* `name`: the expected `.name` value of the thrown error
+* `of`: a constructor, the thrown error must be an instance of
+
+`expected` does not need to be specified. If you don't need it but do want to set an assertion message you have to specify `null` or the empty object `{}`.


Since we're being strict, can we only support null? Doesn't make sense to specify an empty object.

sindresorhus · 2018-02-13T15:26:10Z

I like the brevity of of but I've also had some doubts about it. instanceOf seems most appropriate, though the capital O is cumbersome to type. constructor would work but it doesn't quite signal how we run the assertion. instanceOf then?

I like the brevity too, but I feel we're gonna get the same complaints as we got with t.same(), which is now t.deepEqual(). Out of constructor and instanceOf, I would pick the latter, even though the Of is ugly and annoying to type.

That depends how tightly you want to couple your test to any current error message. E.g. when validating input type checks, looking for TypeError can be sufficient.

But then you don't know whether the TypeError is coming directly from your function or some other function you're calling. I'm fine with leaving it, just thought I would mention how it can cause subtle issues.

Similarly if you're testing an API with custom errors then often the error class is the relevant bit and the message may be static.

That's when I would use it, yeah, but it's very rare that I create custom errors.

sindresorhus · 2018-02-13T15:28:38Z

I'm not sure it's necessary. We always check that the value is an error, so if it's not then you'd never get to the instanceof check. is-error checks the string tag, which works across realms, whereas is-error-constructor does not, so that wouldn't be consistent either.

Good point. I didn't think about that when commenting back then.

sindresorhus · 2018-02-13T15:52:10Z

I've been playing with this PR and seems to work perfectly.

novemberborn · 2018-02-13T16:23:46Z

@sindresorhus addressed feedback.

Remove `core-assert` dependency. When passed a function as the second argument, `t.throws()` now assumes its a constructor. This removes support for a validation function, which may or may not have worked. Refs #1047. `t.throws()` now fails if the exception is not an error. Fixes #1440. Regular expressions are now matched against the error message, not the result of casting the error to a string. Fixes #1445. Validate second argument to `t.throws()`. Refs #1676. Assertion failures now display how AVA arrived at the exception. Constructors are printed when the error is not a correct instance. Fixes #1471.

Fixes #1047. Fixes #1676. A combination of the following expectations is supported: ```js t.throws(fn, {of: SyntaxError}) // err instanceof SyntaxError t.throws(fn, {name: 'SyntaxError'}) // err.name === 'SyntaxError' t.throws(fn, {is: expectedErrorInstance}) // err === expectedErrorInstance t.throws(fn, {message: 'expected error message'}) // err.message === 'expected error message' t.throws(fn, {message: /expected error message/}) // /expected error message/.test(err.message) ```

shellscape · 2018-09-17T03:52:29Z

t.throws() now fails if the exception is not an error. Fixes #1440.

This is causing problems migrating an older module for me. [email protected] rejects with a CssSyntaxError that does not inherit from Error. Silly, but it's there. Would be keen on an option to tell this to allow unexpected behavior, and at the very least, and instance of a particular non-Error constructor.

novemberborn · 2018-09-17T08:15:50Z

@shellscape we don't have consensus in the Core team to change this behavior.

Would it be possible though to inherit CssSyntaxError from Error?

Place all throws/notThrows tests in test/assert.js

5da97cc

Move tests from test/promise.js and test/observable.js.

novemberborn force-pushed the new-throws branch from a4cd3ea to d467e72 Compare February 13, 2018 14:30

novemberborn changed the title ~~wip! Reimplement t.throws() and t.notThrows()~~ Reimplement t.throws() and t.notThrows() Feb 13, 2018

This was referenced Feb 13, 2018

No stack trace for exceptions in throws/notThrows #1372

Closed

Support code expectation in t.throws() #1708

Closed

sindresorhus requested changes Feb 13, 2018

View reviewed changes

sindresorhus approved these changes Feb 13, 2018

View reviewed changes

novemberborn added 2 commits February 13, 2018 18:00

novemberborn force-pushed the new-throws branch from 6d7c019 to 94ac094 Compare February 13, 2018 18:02

novemberborn merged commit 3c13c33 into master Feb 13, 2018

novemberborn deleted the new-throws branch February 13, 2018 18:31

sth mentioned this pull request Jun 13, 2018

Make exceptions/promise rejections testable with throw() even if they aren't instances of Error #1841

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement t.throws() and t.notThrows() #1704

Reimplement t.throws() and t.notThrows() #1704

novemberborn commented Feb 12, 2018

novemberborn commented Feb 13, 2018

sindresorhus commented Feb 13, 2018

sindresorhus commented Feb 13, 2018 •

edited

Loading

novemberborn commented Feb 13, 2018

sindresorhus Feb 13, 2018

sindresorhus Feb 13, 2018

sindresorhus commented Feb 13, 2018

sindresorhus commented Feb 13, 2018

sindresorhus commented Feb 13, 2018

novemberborn commented Feb 13, 2018

shellscape commented Sep 17, 2018

novemberborn commented Sep 17, 2018


		Returns the error thrown by `function` or a promise for the rejection reason of the specified `promise`.
		`expected` can be a constructor, in which case the thrown error must be an instance. It can be a string, which is compared against the thrown error's message, or a regular expression which is matched against this message. You can also specify a matcher object with one or more of the following properties:

Reimplement t.throws() and t.notThrows() #1704

Reimplement t.throws() and t.notThrows() #1704

Conversation

novemberborn commented Feb 12, 2018

novemberborn commented Feb 13, 2018

sindresorhus commented Feb 13, 2018

sindresorhus commented Feb 13, 2018 • edited Loading

novemberborn commented Feb 13, 2018

sindresorhus Feb 13, 2018

Choose a reason for hiding this comment

sindresorhus Feb 13, 2018

Choose a reason for hiding this comment

sindresorhus commented Feb 13, 2018

sindresorhus commented Feb 13, 2018

sindresorhus commented Feb 13, 2018

novemberborn commented Feb 13, 2018

shellscape commented Sep 17, 2018

novemberborn commented Sep 17, 2018

sindresorhus commented Feb 13, 2018 •

edited

Loading