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

Using expect(...).to.throw(...) with custom assertion function #655

Closed
Turbo87 opened this issue Mar 31, 2016 · 12 comments
Closed

Using expect(...).to.throw(...) with custom assertion function #655

Turbo87 opened this issue Mar 31, 2016 · 12 comments

Comments

@Turbo87
Copy link
Contributor

Turbo87 commented Mar 31, 2016

I would like to be able to run more detailed assertions against a thrown exception than what currently seems possible. It would be awesome if expect(...).to.throw(...) supported something like the following pattern:

expect(function() {
  someFunctionThatThrows();
}).to.throw(function(err) {
  expect(err.toString()).to.equal('Error: something is wrong here');
  expect(err.actual).to.not.exist;
  expect(err.expected).to.not.exist;
});

I currently have this implemented in a local helper, but I think that supporting this might make sense in chai itself:

Assertion.overwriteMethod('throw', function (_super) {
  return function(fn) {
    if (typeof fn === 'function') {
      var obj = this._obj;

      try {
        obj();
      } catch (err) {
        return fn(err);
      }

      throw new chai.AssertionError('expected [Function] to throw an exception');

    } else {
      _super.apply(this, arguments);
    }
  };
});

Note that this doesn't support using it with .not and I'm not sure if that would actually make sense. That case should probably be handled though and fail if the 'negate' flag is encountered.

@lucasfcosta
Copy link
Member

Hi @Turbo87, thanks for the issue!

Currently you can already do this in a more 'chai-like' manner.
Our throws assertion changes the assertion subject to the thrown error here, here, here, here and here.

Because of this you can write, for example:

expect(functionThatThrows).to.throw(ErrorConstructor, 'a message').that.has.property('thisPropExists').that.is.a('string');

// Maybe having `.does` as a language chain would be nice
expect(functionThatThrows).to.throw(CustomError, 'foo').that.does.not.have.property('thisDoesNotExist');

I thinks this suits your case 😄

@Turbo87
Copy link
Contributor Author

Turbo87 commented Apr 1, 2016

@lucasfcosta but how would you run multiple assertions against that exception like described above?

@lucasfcosta
Copy link
Member

@Turbo87 when you use the throw assertion, every other assertion chained after it will run against that exception.
If you use, for example .has.property after the throw assertion it will run against the exception and not against the function that has thrown it.

You can chain how many assertions you want to after using throw as long as they don't change the value of the 'object' flag.

@Turbo87
Copy link
Contributor Author

Turbo87 commented Apr 1, 2016

You can chain how many assertions you want to after using throw as long as they don't change the value of the 'object' flag.

that is exactly the problem. if I use your example:

expect(functionThatThrows).to.throw(ErrorConstructor, 'a message')
  .that.has.property('thisPropExists').that.is.a('string');

I can't check multiple properties for their value, because the following doesn't work:

expect(functionThatThrows).to.throw(ErrorConstructor, 'a message')
  .that.has.property('firstProp').that.is.a('string')
  .that.has.property('secondProp').that.is.a('string');

@keithamus
Copy link
Member

@Turbo87 There's a few solutions here, let me go through them:

If you want to assert on all of the properties for equality, .deep.equal will work:

expect(functionThatThrows).to.throw(ErrorConstructor, 'a message')
  .that.deep.equals({
    'firstProp': 'foo',
    'secondProp': 'bar',
  });

If you want to assert on some of the properties for equality, .members will work:

expect(functionThatThrows).to.throw(ErrorConstructor, 'a message')
  .which.has.members({
    'firstProp': 'foo',
    'secondProp': 'bar',
  });

However, if you want to assert some semantics over individual properties, we don't have a way to do that just yet. You could use satisfy for now:

expect(functionThatThrows).to.throw(ErrorConstructor, 'a message')
  .that.satisfies(function (error) {
    return typeof error.firstProp === 'string' && typeof error.secondProp === 'string';
  });

Alternatively, you can subscribe to issue #644 - which is working out how to implement a matcher API for this kind of thing.

Hopefully that gives you some ideas to work with @Turbo87. Let us know if that's helpful, or if you want to discuss things further 😄

@Turbo87
Copy link
Contributor Author

Turbo87 commented Apr 1, 2016

@keithamus thanks for the detailed answer. I guess the last part might work.

in case you're wondering what my use-case is: https://github.com/Turbo87/chai-files/blob/master/test/test.js#L20-L29

@keithamus
Copy link
Member

I see. Thanks for showing us that @Turbo87. I think I'd prefer to see matchers as part of the API, rather than using functions for selected methods like that; but if you disagree I'd love to hear your opinions over in #644 - this kind of stuff needs to be properly worked out before we go forward with it.

@RahmanM
Copy link

RahmanM commented May 22, 2018

Hi,
What is wrong with this? I get this error:

  AssertionError: expected [Function] to throw 
  'Error: Divide by zero is not permitted.' but 
  'Error: Divide by zero is not permitted.' was thrown
  + expected - actual

Test:

describe('divide', function() {
      
        it('2/0 expect exception', ()=>{
          var calc = new index.Calculator;
          expect(()=> calc.divide(2,0)).to.throw(new Error('Divide by zero is not permitted.'));   
        });
  
      })

function:

   divide(number1, number2){
        if(number2 === 0) throw new Error('Divide by zero is not permitted.')
        
        return number1 / number2;
    }

@keithamus
Copy link
Member

@RahmanM

You'd have much more success posting your question to Stack Overflow - https://stackoverflow.com/. It's a great resource full of knowledgable people who will happily take the time you help you with this. This issue tracker, however, is only monitored by less than a handful of people whose time is stretched thin.

@RahmanM
Copy link

RahmanM commented May 23, 2018

Ok, this error message was very confusing. For anyone else who might be having the same issue the solution was this:

expect(()=> calc.divide(2,0)).to.throw('Divide by zero is not permitted.');

Instead of:

expect(()=> calc.divide(2,0)).to.throw(new Error('Divide by zero is not permitted.'));

@rulatir
Copy link

rulatir commented Jan 10, 2021

I'd like something like .that.passes(subject => { /* perform chai assertions here */ }), where the body of the callback is basically nested testcase code that can perform chai assertions.

Alternatively it would be nice if the whole assertion handling logic could be publicly invokeable:

expect(promiseMeSomething()).to.eventually.be.rejectedWith(BuildError).that.satisfies(err => {

    try {
        expect(err.reason).to.be.instanceOf(CommandError);
        expect(err.reason.getBuildTraceAsString()).to.match(/\bbecause pipeFail recipe exited with code 173\b/)
    }
    catch(e) {
        mocha.handleAssertionFailuresLikeYouOtherwiseWouldPrettyPlease(e);
    }
});

@rulatir
Copy link

rulatir commented Jan 10, 2021

This issue is a feature request. The feature is not implemented. The workarounds offered are unsatisfactory. Neither a consensus was reached to the effect that the feature should not be implemented, nor was a czar's ukaz to that effect issued. Why is the issue closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants