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

Possible confusion between shouldContainExactlyInAnyOrder overloads #2587

Closed
Tracked by #2226
nicopico-dev opened this issue Oct 28, 2021 · 5 comments
Closed
Tracked by #2226
Labels
assertions 🔍 Related to the assertion mechanisms within the testing framework. bug 🐛 Issues that report a problem or error in the code.
Milestone

Comments

@nicopico-dev
Copy link

nicopico-dev commented Oct 28, 2021

Using Kotest 4.6.3 with Kotlin 1.5.31

There is a possible confusion for user between these overloads of shouldContainExactlyInAnyOrder() assertion:

infix fun <T, C : Collection<T>> C?.shouldContainExactlyInAnyOrder(expected: C): C?
fun <T, C : Collection<T>> C?.shouldContainExactlyInAnyOrder(vararg expected: T): C? 

When using an optional collection for expected, Kotlin will use the vararg parameter implementation, which will fails as it checks if expected is an item in the actual collection instead of comparing both collections.

Allowing optional collection would likely prevent this issue

infix fun <T, C : Collection<T>> C?.shouldContainExactlyInAnyOrder(expected: C?): C?

Example

val actual: List<String> = listOf("A", "B", "C")
val expected: List<String>? = listOf("A", "B", "C")
actual.shouldContainExactlyInAnyOrder(expected)
// => ERROR: Collection should contain [["A", "B", "C"]] in any order, but was ["A", "B", "C"]
val actual: List<String> = listOf("A", "B", "C")
val expected: List<String> = listOf("A", "B", "C")
actual.shouldContainExactlyInAnyOrder(expected)
// => OK
@jschneidereit
Copy link
Member

Might need to do the ...(first: T, second: T, vararg rest: T) trick here if we add that collection overload

@jschneidereit
Copy link
Member

Now that I'm more awake, what is the use case for having an optional expected? That seems kinda unlikely 🤔

@nicopico-dev
Copy link
Author

I was bitten by this in a test that is more or less like this:

val expectedParameters: Map<String, List<String>>
val actualParameterValues: List<String>

actualParameterValues.shouldContainExactlyInAnyOrder(expectedParameters["SomeParameter"])

The issue is that expectedParameters["SomeParameter"] is of type List<String>?, so the assertion did not compare the lists as I would have expected, and I couldn't understand what was happening for some time.

It just seems confusing to me that the behavior can change so much just by making a type optional

@sksamuel
Copy link
Member

The real solution would be to inform the compiler not to promote T to Any but alas we can't do that yet so we'll need a work around.
Thanks for the bug report, this is indeed a gotcha.

@sksamuel sksamuel added this to the 5.0 milestone Oct 30, 2021
@sksamuel sksamuel added assertions 🔍 Related to the assertion mechanisms within the testing framework. bug 🐛 Issues that report a problem or error in the code. labels Oct 30, 2021
@sksamuel
Copy link
Member

I created this a while ago, please upvote: https://youtrack.jetbrains.com/issue/KT-49194

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assertions 🔍 Related to the assertion mechanisms within the testing framework. bug 🐛 Issues that report a problem or error in the code.
Projects
None yet
Development

No branches or pull requests

3 participants