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

Consider supporting <object> [not] to be empty #350

Closed
papandreou opened this issue Nov 18, 2016 · 4 comments
Closed

Consider supporting <object> [not] to be empty #350

papandreou opened this issue Nov 18, 2016 · 4 comments

Comments

@papandreou
Copy link
Member

papandreou commented Nov 18, 2016

Currently we only support <array-like> [not] to be empty and <string> [not] to be empty, which causes some confusion and inelegant code: #349 (review) #338

Implementing <object> [not] to be empty would be easy, of course. The only downside I can think of is that you probably always want to assert on the type as well so that a [] being replaced by {} will be detected by a test such as:

expect(foo, 'to be empty');

This is in the same territory as unexpected-set defining <Set> to have items satisfying..., of course. In that case an array can also be replaced by a set without a test failing, as long as the items/elements satisfy the criterion. I think we discussed this way back when we changed to be an array with items satisfying to to have items satisfying. If the type itself is important, you should assert that separately.

expect(foo, 'to be an array').and('to be empty');

I'm not totally sure, but it does seem better than including the type name in the assertion string, eg. to be an empty array or to be an empty object -- especially given the above linked use cases.

@sunesimonsen
Copy link
Member

sunesimonsen commented Nov 24, 2016

I'm actually okay with having all the options:
<array> [not] to be an empty array
<object> [not] to be an empty object
<string> [not] to be an empty string
<object|array|string> [not] to be empty

It seems like to be an empty array would we useful in a lot of user facing cases, where `to be empty would be useful for writing assertions.

@alexjeffburke
Copy link
Member

Quick note, I think this is quite a useful circumstance to be able to spell out.

@joelmukuthu
Copy link
Member

Perhaps this issue can be closed? This is currently supported (I just tested on v11.6.0)

@papandreou
Copy link
Member Author

Right! Thanks for noticing :)

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

4 participants