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

Assertions on error properties #123

Closed
dominykas opened this issue Jan 30, 2018 · 7 comments
Assignees
Labels
Milestone

Comments

@dominykas
Copy link

@dominykas dominykas commented Jan 30, 2018

Ran into a use case recently, where I'd like to assert on error codes and some more meta information.

I'm thinking about error([type], [message], [values]) / throw([type], [message], [values]), e.g.

expect(() => someMethod()).to.throw(CustomError, 'Oh no!', { code: 'E_NOENT' });

This would essentially do an extra expect(error).to.include(values); - there is no other way to check this when using to.throw() AFAIK?

Happy to PR if acceptable.

@mtharrison

This comment has been minimized.

Copy link
Member

@mtharrison mtharrison commented Jan 30, 2018

Sounds useful to me. Not sure if I'd be expecting includes() as you suggest or a deep equality check though.

@dominykas

This comment has been minimized.

Copy link
Author

@dominykas dominykas commented Jan 31, 2018

Yeah, I was thinking about deep vs includes(). But with error objects it may be hard to always assert the deep copy - there may be functions on it or other things you don't control, so a loose check feels more useful in practice. I couldn't come up with a nice way to make it optional - adding a fourth param would be pretty ugly? I also considered making the third param a function which receives the error thrown - but that feels overkill?

Another option would be to have something like error(type, [values], [options]), e.g.

expect(() => someMethod()).to.throw(CustomError, { message: 'Oh no!', code: 'E_NOENT' }, { strict: true });

Possibly leave out the { strict: true } as a future extension (if someone asks for it) and until then the second param is either a message or a values object for an includes() check?

@cjihrig

This comment has been minimized.

Copy link
Contributor

@cjihrig cjihrig commented Feb 4, 2018

I'll entertain a PR for this 😄

@dominykas

This comment has been minimized.

Copy link
Author

@dominykas dominykas commented Feb 4, 2018

@cjihrig any opinion on the API, before I jump into this? :)

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Feb 8, 2018

I'll replicate our reject() interface style and just return the error for further, separate assertions.

@hueniverse hueniverse self-assigned this Feb 8, 2018
@hueniverse hueniverse added the feature label Feb 8, 2018
hueniverse added a commit that referenced this issue Feb 8, 2018
@hueniverse hueniverse added this to the 5.1.3 milestone Feb 8, 2018
@cjihrig cjihrig closed this in 7417485 Feb 8, 2018
@mtharrison

This comment has been minimized.

Copy link
Member

@mtharrison mtharrison commented Feb 8, 2018

So one would do the following?

const err = expect(() => someMethod()).to.throw(CustomError);
expect(err).to.include({ code: 'E_NOENT' });
@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Feb 8, 2018

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.