Skip to content

Conversation

@jselbo
Copy link
Contributor

@jselbo jselbo commented Dec 8, 2025

Fixes the first problem mentioned in #555
"anyOrNull with nullable primitive value class matcher throws IllegalStateException"

createInstance value class logic assumed the value class can be constructed with a null value, but this doesn't work when the wrapped type is a primitive.

@TimvdLippe
Copy link
Contributor

@m-koops do you mind reviewing this PR? Thanks!

@m-koops
Copy link
Contributor

m-koops commented Dec 8, 2025

@m-koops do you mind reviewing this PR? Thanks!

Sure, NP.

@m-koops
Copy link
Contributor

m-koops commented Dec 9, 2025

Hi Joshua, many thanks for providing this PR.

The extra tests that you have added, but which you had to disable with @ignore as they fail at this moment, shed an interesting light on how Kotlin hands over value classes to Java code. So, apart from your direct code improvement, #555 and this PR have provided valueable additional input on the weak spots of value class handling in Mockito-Kotlin.

So far, my impression was that value class are always unboxed when handed over to Java code. But the new test cases proof that in case of primitives as inner value, the value classes are handed over to the Mockito Java code as-is, say boxed.

@TimvdLippe, apart from the unused import that I marked with a comment, this PR is ready to be accepted.

As follow-up, I will have a look into the boxing and unboxing behavior of value classes in general and hopefully I can soon provide a fix that will make the new ignored test cases succeed.

Cheers, Mark

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR and the review!

@TimvdLippe TimvdLippe merged commit ae8905b into mockito:main Dec 9, 2025
4 checks passed
@jselbo jselbo deleted the value1 branch December 9, 2025 22:25
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.

3 participants