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

Support for ignoring unknown keys in JSON asserts #2303

Merged
merged 11 commits into from
Sep 18, 2021

Conversation

Kantis
Copy link
Member

@Kantis Kantis commented May 31, 2021

Closes #2298

Added new matchers shouldEqualJsonIgnoringUnknown and
shouldNotEqualJsonIgnoringUnknown. Supports for specifying all
other flags that the previous json matcher supported.

The existing compare method already supported the use-case quite easily,
and had good error messages for possible validation errors.

Unknown keys can only exist in object nodes. The scenarios currently
covered by tests are:

  • Actual contains a key not specified in expected - key is ignored
  • Actual contains a key specified in expected - key is compared
  • Actual is missing a key specified in expected - matcher error occurs

Since comparison happens recursively, I also added tests to make sure
the ignoring is in effect deeper in the structure as well.

Todo

  • add in changelog
  • update docs

@Kantis
Copy link
Member Author

Kantis commented May 31, 2021

Will add documentation as well.

Do you think it would make sense to put it between shouldEqualJson and shouldContainJsonKey at https://kotest.io/docs/assertions/json-matchers.html? It feels closely related to shouldEqualJson.

@sksamuel
Copy link
Member

Is this what Lenient mode is meant to do ?

@Kantis
Copy link
Member Author

Kantis commented Jun 14, 2021

Is this what Lenient mode is meant to do ?

As a user, I would prefer to have strict types while comparing if possible, but I agree it would feel natural and this change might make the API too verbose? Perhaps it might be worth introducing something like this:

class CompareOptions(
   build: CompareOptions.() -> Unit
) {
   var ignoreKeyOrder: Boolean = false
   var coerceTypes: Boolean = false
   var ignoreUnknownKeys: Boolean = false

   init {
       build()
   }
}

fun String.shouldEqualJson(expected: String, options: CompareOptions.() -> Unit): Unit =
   TODO()

fun x() {
   "actual".shouldEqualJson("expected") { ignoreKeyOrder = true }
}

it loses out on the infix fun though, which is a shame..

@sksamuel
Copy link
Member

sksamuel commented Jun 14, 2021 via email

@Kantis
Copy link
Member Author

Kantis commented Jun 16, 2021

Something like this? And we replace the ignoreUnknownKeys boolean with this?

enum class FieldComparison {
   /** Verifies all expected key-values match, and that no extra keys exist in the actual */
   ComparingAll,
   /** Verifies all expected key-values match, ignoring keys in actual that are not defined in expected*/
   ComparingExpected
}

@Kantis Kantis force-pushed the feature/kantis/2298-json-ignore-unknown branch from d4ad7c2 to 873011d Compare June 21, 2021 21:51
Added new matchers `shouldEqualJsonIgnoringUnknown` and
`shouldNotEqualJsonIgnoringUnknown`. Supports for specifying all
other flags that the previous json matcher supported.

The existing compare method already supported the use-case quite easily,
and had good error messages for possible validation errors.

Unknown keys can only exist in object nodes. The scenarios currently
covered by tests are:

* Actual contains a key not specified in expected - key is ignored
* Actual contains a key specified in expected - key is compared
* Actual is missing a key specified in expected - matcher error occurs

Since comparison happens recursively, I also added tests to make sure
the ignoring is in effect deeper in the structure as well.
@Kantis Kantis force-pushed the feature/kantis/2298-json-ignore-unknown branch from 873011d to 835755a Compare June 21, 2021 21:53
@sksamuel
Copy link
Member

Something like this? And we replace the ignoreUnknownKeys boolean with this?

enum class FieldComparison {
   /** Verifies all expected key-values match, and that no extra keys exist in the actual */
   ComparingAll,
   /** Verifies all expected key-values match, ignoring keys in actual that are not defined in expected*/
   ComparingExpected
}
enum class FieldComparison {
   /** Verifies all expected key-values match, and that no extra keys exist in the actual */
   Exact,
   /** Verifies all expected key-values match, ignoring keys in actual that are not defined in expected*/
   IgnoreExtra
}

Maybe something like that ?

@sksamuel
Copy link
Member

sksamuel commented Aug 1, 2021

Is this ready for final review ?

@Kantis
Copy link
Member Author

Kantis commented Aug 3, 2021

@sksamuel aye, it should be.

@sksamuel
Copy link
Member

Sorry is this still waiting on me? If so can we fix the conflicts and I will review.

@Kantis
Copy link
Member Author

Kantis commented Sep 18, 2021

@sksamuel should be ready now, I think the failing test is flaky.

@sksamuel sksamuel merged commit 4ae2869 into kotest:master Sep 18, 2021
@Kantis Kantis deleted the feature/kantis/2298-json-ignore-unknown branch September 18, 2021 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to ignore unspecified keys when using shouldEqualJson matcher
3 participants