fix: [kotest-assertions-json] shouldContainJsonKey should be true for keys with null values

[msewell]

We noticed that for a JSON object which contains property with null values like this:

{
  "nullable": null
}

the shouldContainJsonKey("nullable") matcher would throw even though the key was present, which was against our expectations.

The changes in this PR will make shouldContainJsonKey(:) not throw for keys with null values. This is a breaking change ⚠️, as tests relying on the previous behavior may break.

[sksamuel]

So the question is, should shouldContainJsonKey consider a null value as present or not.

[msewell]

So the question is, should shouldContainJsonKey consider a null value as present or not.

I think that's the wrong way to think about it, even if it does address the implementation issue. The question is (IMO): should someJsonString.shouldContainJsonKey(keyName) ever throw if keyName is present as a key in someJsonString? I don't think it should, regardless of the value belonging to that key; it is only the presence of the key itself that should be considered.

[sksamuel]

I think that is the right way to think about this. The name being jsonKey not jsonValue.

…

On Fri, 29 Jul 2022 at 00:26, Michael Sewell ***@***.***> wrote: So the question is, should shouldContainJsonKey consider a null value as present or not. I think that's the wrong way to think about it, even if it does address the implementation issue. The question is (IMO): should someJsonString.shouldContainJsonKey(keyName) *ever* throw if keyName is present as a key in someJsonString? I don't think it should, regardless of the *value* belonging to that key; it is only the presence of the key itself that should be considered. — Reply to this email directly, view it on GitHub <#3128 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFVSGWK4GNY2R4752RFLNTVWNTSBANCNFSM545AB25Q> . You are receiving this because you commented.Message ID: ***@***.***>