Chai: support .withArgs() on expect(fn)

Created on 22 Jul 2016  路  14Comments  路  Source: chaijs/chai

expect.js has a feature to pass arguments to a function using the following syntax

expect(fn).withArgs(some, args).to.throw();

Right now I'm having to write the following

expect(() => fn(some, args)).to.throw();
feature-requests more-discussion-needed

Most helpful comment

fn.should.throw.with.args(some, args);

Would probably fit more with the rest of the code. Having args as the assertion actually makes for some interesting developments:

addNumbers.should
  .throw.with.args(1, 'a')
  .and.args('a', 2)
  .but.not.throw.with.args(1, 2)

It becomes challenging though - because we can't assert on the error that is being thrown in a sensible way:

addNumbers.should.throw/* a typeerror, but I can't assert this */.with.args(1, 'a')

Whereas if the args was in the chain, then we can:

addNumbers.should.with.args(1, 'a').throw(TypeError)

All 14 comments

Hi @blockloop, thanks for your issue!

That makes total sense to me and looks like a relevant feature (I'm not sure we can call that a "feature" since it may be implemented just as syntax sugar, but let's keep going). Its syntax is clear and concise enough for me.

I'm just wondering how we would implement that on the should interface if everyone agreed upon this feature. Maybe we could change the order of the assertion words, for example:

fn.should.throw.withArgs(some, args);

What do you guys think?
cc @meeber @keithamus

fn.should.throw.with.args(some, args);

Would probably fit more with the rest of the code. Having args as the assertion actually makes for some interesting developments:

addNumbers.should
  .throw.with.args(1, 'a')
  .and.args('a', 2)
  .but.not.throw.with.args(1, 2)

It becomes challenging though - because we can't assert on the error that is being thrown in a sensible way:

addNumbers.should.throw/* a typeerror, but I can't assert this */.with.args(1, 'a')

Whereas if the args was in the chain, then we can:

addNumbers.should.with.args(1, 'a').throw(TypeError)

@keithamus Whoa!
That's what I call heavy code ninjitsu. That looks awesome.
It's like writing in plain english and having your computer to understand it.
I totally agree with this proposal.

Which one? .throw.with.args() or .with.args().throw()?

We could also have args() propagate the error like throw() does, which loses flexibility on the first example I had, but you get to assert on the error which is probably more preferable:

addNumbers.should
  .throw.with.args(1, 'a')
  .and.be.an.instanceof(TypeError)

Oh I have seen a different version of this, probably before you edited it.
Anyway, considering your new ideas I guess that having args to propagate the error seems more useful, however it is not as semantically correct as this:

addNumbers.should
  .throw.with.args(1, 'a')
  .and.args('a', 2)
  .but.not.throw.with.args(1, 2)

If I was a user I would not expect args() to change the assertion's subject, even if it is useful it doesn't seem to be much obvious.

Yeah sorry, was attempting to ninja edit my comment as I realised it was more complex than I thought.

I agree with your comment, that's our challenge - finding a good balance between useful and expected. My guess is that users will want to more precisely specify why those calls were throwing - and so having .args() change the subject to the error is useful, even if it makes less sense.

We should wait for more people to chime in, like @blockloop and @meeber

I agree with the verbiage. Maybe it's just a style thing but I don't like this

addNumbers.should
  .throw.with.args(1, 'a')
  .and.args('a', 2)
  .but.not.throw.with.args(1, 2)

Not because of the verbiage but because it's multiple assertions in one statement.

Not because of the verbiage but because it's multiple assertions in one statement.

Yeah, most of our chained assertions just drill down into the same assertion, e.g. expect(fn).to.throw().and.be.an('error').that.has.property('message').that.matches(/foo/), whereas the chaining of multiple args would be multiple individual assertions.

What about this

fn.calledWith(1, 'a').should.throw()

@blockloop IMO that is not a viable solution because it adds another property to the Object.prototype.
I think that having should is already enough. If we did that it would be like a new Assertion interface.

True. That's actually one of the reasons I prefer the expect() interface :). That being said, addNumbers.should.throw.with.args(1, 'a') appears to be the most semantic for the should interface.

This is a great feature request. With proper documentation, it should help cut down on instances of this problem.

I prefer styles that allow the arguments to be positioned as close as possible to the function, instead of having them be separated by the throw keyword.

An alternative or alias to args in that spirit:

expect(myFn).given(arg1, arg2).to.throw(TypeError);
myFn.should.given(arg1, arg2).throw(TypeError);

Hi @blockloop, thanks again for opening this issue!

This is indeed a great feature request. Due to the house-cleaning we've been doing, I added it to our roadmap project board so that we know what to tackle for v5 and therefore I'll be closing this just so that we can get a clean slate to work on.

You can see the card on the roadmap at this link.

Thank you very much!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

endymion00 picture endymion00  路  3Comments

corybill picture corybill  路  4Comments

leifhanack picture leifhanack  路  4Comments

jockster picture jockster  路  4Comments

AnAppAMonth picture AnAppAMonth  路  3Comments