Chai: expect .to.throw does not work with new Errors

Created on 23 Apr 2015  路  16Comments  路  Source: chaijs/chai

This is my test:

  it('should throw a RangeError when the end date is before the start date', function () {
    var dateRange = {
      start: '2015-04-01',
      end: '2014-04-29',
    };
    var error = new RangeError('Invalid date range');
    var test = function () {
      throw error;
    };
    expect(test).to.throw(new RangeError('Invalid date range'));
  });

which fails with the message:

    AssertionError: expected [Function] to throw 'RangeError: Invalid date range' but 'RangeError: Invalid date range' was thrown
        at Context.<anonymous> (test/widgets/staff-planning-projection.spec.js:90:1)

I couldn't find any explanations / solutions / workarounds for this, so I _think_ it's a bug with the expect(x).to.throw assertion. As a redundancy check, I tried:

expect(test).to.throw(error);

and that passed. Any thoughts?

bug medium-size

Most helpful comment

Hi @suissa. What you're running into is something different, and not a bug. You need to pass a function to expect instead of calling the function and passing its return value to expect. The way you have it now, your function is being called and throwing an error before expect is called, so expect doesn't have a chance to catch the error.

Working example:

  describe('An Immutable',  () => {
    it(' cannot accept a value different than OBJECT', () => {
      expect(() => require('../lib/iammutable')(1)).to.throw(TypeError, 'You need to send an OBJECT!')
    })
  })

All 16 comments

Looks like the docs explicitly mention that you can pass in new errors and expect them to work - so yes I'd say this is a bug.

The Throw method is quite a big one, its in assertions.js#L1195-1310. Upon some inspection it looks like when you pass in a new *Error() it sets desiredError to that reference (see L1211-1214), and then does an === on desiredError (see L1228-1239).

I think the solution would be to check if desiredError matches both type and message, so String(err) === String(desiredError) would suffice - however this could be a breaking change, so maybe we should play it safe and when you make a Pull Request, put it in the 3.x.x branch instead of against master.

If you need any more help, please let me know and I'll be happy to assist.

In fact, looking through the code, it might be worthwhile ditching the whole desiredError check, and in the L1211-1214 if, just set the constructor and message to the error instance's constructor and message. This would significantly reduce the complexity of the assertion too :smile:

Maybe as well, add some documentation so that if people really want to test the error by value then they can do expect(err).to.throw(new Error('foo')).and.equal(myErrorReference).

@keithamus I was looking into this issue, and I attempted to implement the change you suggested. I checked if constructor and errMsg were null, and if they were, I set them to those of the err. The actual changes I made can be found here in my fork.

However, I am getting this test failure:

  1) assert doesNotThrow:

      AssertionError: expected 'expected [Function] to not throw null but \'Error: foo\' was thrown' to equal 'expected [Function] to not throw an error but \'Error: foo\' was thrown'
      + expected - actual

      -expected [Function] to not throw null but 'Error: foo' was thrown
      +expected [Function] to not throw an error but 'Error: foo' was thrown

      at global.err (test/bootstrap/index.js:17:35)
      at Context.<anonymous> (test/assert.js:568:5)

I speculate that it's because the name is null, but I don't seem to see under what conditions the error message would display an error. The best I could do is to make it display 'Error', which still fails the test. Any idea how can I proceed from here? I'd appreciate any help with this.

@jluchiji, this issue predates our efforts to pull out the error checking code into a new module, and so I'd tread carefully when implementing code here. Have a read of chai-as-promised/issues#47 and #470 and #457.

I'd say for now, the best place to move the error assertions forward is in those areas - then we can take a look at behaviours like this.

Removing the pull-request-wanted label - because as mentioned, we need to refactor the error check code out into its own lib.

Any update on this issue? is there a workaround for tests like this?

Same bug.

i have a sinon.spy on a function then execute expect(myfn).to.throw it pass.
but the sinon.spy.callCount doesn't increase.
sinon-chai also not work in this case?
Is this the way it designed or this just my fault to write like that?
sorry i just new :smile:
@keithamus any advice sir?

Hey @brutalcrozt. You would be better off asking in our chat forums, either on Gitter or Slack. Alternatively you could try raising an issue with sinon-chai - but I'd recommend making sure you have a reduced test case before filing an issue.

@keithamus The behavior of checking for both the error and the message when they exist has been fixed, but we took the design decision of doing strict (===) comparisons when an Error instance is passed. So the correct assertion here would be expect(test).to.throw(RangeError, 'Invalid date range');, and not the one with the new operator (since passing an error instance triggers strict comparison).

We did this in order to enable users to choose between both behaviors (strict comparisons and a comparison using the constructor and/or message).

Same bug here.

My code:

'use strict'

module.exports = (obj) => {
  if(obj instanceof Object)
    if(Object.isFrozen(obj)) return obj
    else return Object.freeze(obj)
  else throw new TypeError('You need to send an OBJECT!')
}

test:

  describe('An Immutable',  () => {
    it(' cannot accept a value different than OBJECT', () => {
      expect(require('./iammutable')(1)).to.throw(TypeError, 'You need to send an OBJECT!')
    })
  })

output:

 An Immutable
      1)  cannot accept a value different than OBJECT


  1 failing

  1) I am Mutable, but not!  An Immutable  cannot accept a value different than OBJECT:
     TypeError: You need to send an OBJECT!
      at module.exports (iammutable.js:7:14)
      at Context.it (iammutable.test.js:30:37)

Hi @suissa. What you're running into is something different, and not a bug. You need to pass a function to expect instead of calling the function and passing its return value to expect. The way you have it now, your function is being called and throwing an error before expect is called, so expect doesn't have a chance to catch the error.

Working example:

  describe('An Immutable',  () => {
    it(' cannot accept a value different than OBJECT', () => {
      expect(() => require('../lib/iammutable')(1)).to.throw(TypeError, 'You need to send an OBJECT!')
    })
  })

this is my test:
var {
generateMessage
} = require('./message');
describe('generateMessage',()=>{
it('it should create correct obj',()=>{
var from='farkhanda';
var text='some message';
var message=generateMessage(from,text);
expect(message.createdat).toBeA('number');
});
});
fail with this error:
generateMessage
it should create correct obj:
TypeError: expect(...).toBeA is not a function
at Context.it (serverutilsmessage.test.js:12:31)

@farkhandaAbbas .toBeA isn't part of the Chai API. Try this instead: .to.be.a('number');.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

leifhanack picture leifhanack  路  4Comments

huaguzheng picture huaguzheng  路  3Comments

zzzgit picture zzzgit  路  3Comments

corybill picture corybill  路  4Comments

andipavllo picture andipavllo  路  3Comments