Add "to have properties satisfying" and deprecate value in "to have property"

[alexjeffburke]

Part of my push for Unexpected 12 is to have consistent behaviour regarding ignoring undefined - so far this has centred primarily around making sure the "properties" assertions can take over from the "keys" assertions (which are sensitive to undefined for legacy reasons).

I discovered one last missing assertion for there to be feature parity between properties and keys (thus allowing us to consider deprecation of keys are previously discussed), namely: "to have properties satisfying". This PR implements the missing assertion - the most important difference being ignoring keys with undefined values (https://github.com/unexpectedjs/unexpected/compare/feature/toHavePropertiesSatisfying?expand=1#diff-0a56a2b979f6080486ec1909508cda1aR526).

[alexjeffburke]

If ignoring keys with undefined values is itself a little too special, I can return this to behaving like “to have keys satisfying” - but either way I think we need this so we can proceed.

[sunesimonsen]

This feature need to go hand in hand with deprecating to have properties that asserts on the values, otherwise it is too confusing.

@papandreou how to you feel about ignoring the undefined properties? It seems in line with other assertions, but is it surprising? You could argue that to satisfy allows undefined values:

expect({}, 'to satisfy', { foo: undefined });

[alexjeffburke]

@sunesimonsen thanks for reminding me about the optional value form - agreed that it needs deprecation alongside this. Will add my take on a warning and your test suggestions :)

[alexjeffburke]

@sunesimonsen I've gone through the tests and clarified the assertion cases from those requiring an expect.it. I've also added a nice deprecation notice for that value argument in "to have property", outputted once, that informs the user what to do:

[papandreou]

Explicitly undefined property values in the to satisfy rhs is very much a feature, and (by design) it doesn’t distinguish between absent properties and present properties with a value of undefined. I’d like to keep it that way, I find it very useful and concise.

[papandreou]

I’m not sure I like “to have properties satisfying” as an assertion name. It’s testing property names and not the value, so I don’t intuitively understand what it does from the name.

[alexjeffburke]

On further discussion a pretty compelling argument came up that this assertions might not be necessary at all - I first considered that adding this purely as a migration aid for anyone using "to have keys satisfying", but when prodded I realised if I needed to something like this I'd probably write:

expect(Object.keys(obj), 'to have items satisfying', 'to be a string');

It seems like the deprecation stuff has more legs than the assertion itself, and as such I've split it off as a separate PR.

[sunesimonsen]

Explicitly undefined property values in the to satisfy rhs is very much a feature, and (by design) it doesn’t distinguish between absent properties and present properties with a value of undefined. I’d like to keep it that way, I find it very useful and concise.

I was not suggesting that we should remove that feature, I also use that a lot. I was just saying that we actually handle undefined in to satisfy, we don't ignore it. So we probably should ignore it here either.

I’m not sure I like “to have properties satisfying” as an assertion name. It’s testing property names and not the value, so I don’t intuitively understand what it does from the name.

I had the same reaction, but we are going to limit the use of to have properties to only talk about the keys, so maybe it is because it previously talked about the values? I don't know, I would normally refer to does as keys. But to have own key also sounds a bit weird.

[sunesimonsen]

@alexjeffburke I'm not wild about us just removing random stuff. I think to have keys satisfying and to have values satisfying a makes sense. Maybe it is the to have properties that is the bad assertion? I never use to have properties and it's exotic flags, but I have used to have keys many times.

@alexjeffburke and @papandreou maybe we should make a issue about deprecating these things, it seems like it isn't fully solidified, if we are going to break compatibility, then it has to be with a pretty clear path.

[alexjeffburke]

@sunesimonsen @papandreou I very much apologise if it felt like suggesting random removals, certainly not me intention. It was totally the right call to write these up, if we intent to do something about this - hopefully the following will suffice: #690