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

Better output from to have properties #317

Open
msiebuhr opened this issue Jul 7, 2016 · 3 comments
Open

Better output from to have properties #317

msiebuhr opened this issue Jul 7, 2016 · 3 comments

Comments

@msiebuhr
Copy link
Contributor

msiebuhr commented Jul 7, 2016

> var expect = require('unexpected');
> expect({a:1, b:2}, 'to have properties', ['a', 'b', 'c'])
UnexpectedError:
expected { a: 1, b: 2 } to have properties [ 'a', 'b', 'c' ]

This gets rather pointless when the tested object has more than a few dozen properties. And it doesn't tell you if it considers the required properties as strict set equality or a subset.

To get around this, I've re-purposed the code from #255 (comment) to this bastard-thing:

> expect(Object.keys({a:1, b:2}), 'to be a subset of', ['b', 'c']);
expected [ 'a', 'b' ] to be a subset of [ 'b', 'c' ]
[
  'a', // should be removed
  'b'
]

(Yes, I know the assertion isn't the same, but it illustrates the difference in output quite well.)

@sunesimonsen
Copy link
Member

sunesimonsen commented Jul 9, 2016

Hi Morten, cool to get an issue from you :-)

The assertion you are suggesting have a different semantics than to have properties that will just check that the specified properties are present. The assertion you are making - if I understand it correctly - is asserting that there most not be any keys outside a recognized set. Which might be useful. I stopped to have properties when to satisfy was introduced. If I need to check the presence for a set of keys I use to have keys. But that does not mean we should make the output of to have properties as good a possible. So let's discuss that.

I would like the output to state the keys I'm missing when the assertion fails for both to have keys and to have properties. Something like:

expect({a:1, b:2}, 'to have properties', ['a', 'b', 'c'])

output:

expected { a: 1, b: 2 } to have properties [ 'a', 'b', 'c' ]
  property c is missing

@msiebuhr
Copy link
Contributor Author

Hep!

I'm not arguing for a new assertion here; I'm merely using it illustrate that the error message of to have properties isn't terribly informative on objects w. ~20+ properties. You suggestion seem quite reasonable to me.

I mostly implemented it as a diff as I could see multiple errors in one pass;

expect({a:1, b:2, x:3}, 'to only have keys', ['a', 'b', 'c', 'd']);

Should ideally give

expected {a:1, b:2, x:3} to have keys ['a', 'b', 'c', 'd']
  Key 'c' is missing
  Key 'd' is missing
  Key 'x' should not be there

(Also, hadn't noticed to have keys. I just went straight for to have properties...)

@sunesimonsen
Copy link
Member

Yes exactly, but maybe a bit more compact:

expected {a:1, b:2, x:3} to only have keys ['a', 'b', 'c', 'd']
  keys c, d is missing
  key x should be removed

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

2 participants