Equality testing for iterables broken

[U1F984]

Which version of Kotest are you using
4.2.5

Due to the recent changes from 6bbbfec equality testing for iterables is broken:

listOf("hello") shouldNotBe emptyList<String>() // java.lang.AssertionError: [] should not equal ["hello"]

this is caused by a missing check in IterableEq#checkIterableEquality that the given iterables are the same size. Also, the implementation is not fast because it will create a list in the background (which is then prompty discarded).

A (untested) alternative implementation could look something like this:

    private fun checkIterableEquality(actual: Iterable<*>, expected: Iterable<*>): Throwable? {
        var index = 0
        val actualIterator = actual.iterator()
        val expectedIterator = expected.iterator()
        while (actualIterator.hasNext() && expectedIterator.hasNext()) {
            val t = eq(actualIterator.next(), expectedIterator.next())
            if (t != null) return failure(
                Expected(expected.show()),
                Actual(actual.show()),
                "Elements differ at index $index: "
            )
            index++
        }
        if(actualIterator.hasNext()) {
            return failure(
                Expected(expected.show()),
                Actual(actual.show()),
                "Expected fewer elements"
            )
        }
        if(expectedIterator.hasNext()) {
            return failure(
                Expected(expected.show()),
                Actual(actual.show()),
                "Expected more elements"
            )
        }
        return null
    }

[daviddenton]

Possibly related, but equality of Jackson JsonNode objects are also broken. A JsonNode itself is iterable of its own child nodes, but in the case of something like a TextNode there are no children. So a text value of "value1" should NOT be equal to "value2", but kotest reports it is because it just compares the children.

By the same token any object which is iterable but has state other than its contents will also be inaccurately reported as always equal.

[sksamuel]

Thanks for the bug reports guys. So I believe @Quicksteve your issue is already fixed in master, if you look at the latest implementation, it does basically what you pasted. I'll add another set of test cases for shouldNotBe just to be sure.

@daviddenton that is an excellent observation and something that happened with Path already. I think the safest course of action is to have an accepted list of iterable types that use the IterableEq typeclass - so list, set, etc. And if it's not in that list, then it uses the DefaultEq implementation.

[sksamuel]

Fixed in master, will relaese 4.2.6 with these changes.

[sksamuel]

4.2.6 is released.

[TheSench]

Still seeing JsonNode comparisons not properly detect different values in TextNodes in 5.3.1.

[TheSench]

Here's a simple test that reproduces this for me:

@Test
fun `it should return detect differences in values`() {
  val map1 = mapOf("test" to 1)
  val map2 = mapOf("test" to 2)
  val mapper = ObjectMapper()
  val jsonNode1 = mapper.valueToTree<JsonNode>(map1)
  val jsonNode2 = mapper.valueToTree<JsonNode>(map2)

  // Passes
  jsonNode1 shouldBe jsonNode2
  // Fails with:
  // expected:<"{"test":2}"> but was:<"{"test":1}">
  jsonNode1.toString() shouldBe jsonNode2.toString()
}