unexpected icon indicating copy to clipboard operation
unexpected copied to clipboard

Alias 'to throw' as 'to throw ... satisfying'

Open joelmukuthu opened this issue 5 years ago • 10 comments

joelmukuthu avatar Dec 02 '18 21:12 joelmukuthu

Aliases for "to throw" are already supported - see the documentation for the assertion here.

The word "satisfying" is not something defined for "to throw", but if we added it that wouldn't really be an alias but an entirely different assertion. However, you can already achieve exactly the semantics you want by using expect.it, for example:

expect(() => { throw new Error() }, 'to throw', expect.it('to have property', 'message'));
expect(() => { throw new Error() }, 'to throw error', expect.it('to have property', 'message'));
expect(() => { throw new Error() }, 'to throw exception', expect.it('to have property', 'message'));

alexjeffburke avatar Dec 02 '18 23:12 alexjeffburke

The to throw... assertion already uses "to satisfy" semantics, though? In the light of that I think it would make sense to add the ...satisfying variant as an alias?

papandreou avatar Dec 03 '18 11:12 papandreou

My apologies, I think I misread the value type to mean it wouldn't compare the error using satisfy semantics - the assertion does something a little special for strings I believe (comparing the message against it) hence the expect.it suggestion.

alexjeffburke avatar Dec 03 '18 14:12 alexjeffburke

The issue title was also a bit confusing, I've updated it to make it a bit clearer. While I have your attention, what does <function> to throw (a|an) <function> do :)? https://github.com/unexpectedjs/unexpected/blob/9b30831c8365a531dce26af401ca5ebdc5d5fb7c/lib/assertions.js#L1030-L1044

joelmukuthu avatar Dec 03 '18 17:12 joelmukuthu

I've been thinking about this some more - I can definitely see the value in this alias but I'd like to raise one concern.

Using satisfying here would introduce something that behaves a little differently than for example "to have an item satisfying" primarily because of the matching of the error message to a string of its supplied. That would make this use of "satisfying" a little different to all the others. My concern is also that since (unfortunately) strings can be thrown at the language level this would create an annoying ambibugity. I might be over losing this though so keen to hear thoughts.

@joelmukuthu thanks for expanding on the description a bit :)

alexjeffburke avatar Dec 04 '18 09:12 alexjeffburke

@alexjeffburke I agree, but I've also always thought of to satisfy as an assertion that allows you to assert against different types which to me makes this an acceptable ambiguity aka "it's a feature" :D. Actually I think to throw error satisfying <string> reads better than to throw error <string>

joelmukuthu avatar Dec 04 '18 11:12 joelmukuthu

I agree about to throw error satisfying <string> reading better. And to throw error satisfying <object> even more so!

<function> to throw (a|an) <function> checks that the thrown value is an instance of a particular class, eg.:

expect(myFunction, 'to throw an', httpErrors.Conflict);

papandreou avatar Dec 04 '18 13:12 papandreou

Agreed about that spelling for which I think means we've reached some broad agreement - I can have a look next time I'm near core.

alexjeffburke avatar Dec 04 '18 16:12 alexjeffburke

the assertion does something a little special for strings I believe (comparing the message against it)

You're right, that's a bit of a wart in itself, but I think we can fix that so it reduces directly to <UnexpectedError> to satisfy...: #538.

papandreou avatar Dec 09 '18 20:12 papandreou

@joelmukuthu, personally I'd welcome a PR for this now that the other oddity got resolved.

papandreou avatar Jan 03 '19 22:01 papandreou