search

mockito / mockito-kotlin

Using Mockito with Kotlin
MIT License
3.09k stars 198 forks source link

#478 Upgrade Mockito #482

Closed lukas-krecan closed 1 year ago

lukas-krecan commented 1 year ago

It's not compatible with older Mockito versions so it should be a major version release.

emartynov commented 1 year ago

Is it for #474 ?

lukas-krecan commented 1 year ago

Is it for #474 ?

Yes, and https://github.com/mockito/mockito-kotlin/issues/478

lukas-krecan commented 1 year ago

I just bumped Java version in CI as Mockito 5 requires Java 11

TimvdLippe commented 1 year ago

@lukas-krecan Can you please answer the above question? Then this is ready to merge

lukas-krecan commented 1 year ago

Sorry, I do not understand. Which tests do you mean?

TimvdLippe commented 1 year ago

Mockito 5 made breaking changes to varargs matchers. I expected that to have consequences for Kotlin as well. However, I see little to no changes in the regression test suite of mockito-kotlin in this PR. Does that mean that these breaking changes were not relevant for Kotlin folks, or that we are missing test suite coverage?

lukas-krecan commented 1 year ago

Does that mean that these breaking changes were not relevant for Kotlin folks, or that we are missing test suite coverage?

I guess both. The only impacted method here is anyVararg, I had to add a tests as I did not trust the current coverage. Moreover, in Kotlin you can't just use an array in vararg, you have to use the method(*array) syntax. Maybe it helped.

But I am a first-time contributor to this library, so taky me with a huge grain of salt.

lukas-krecan commented 1 year ago

Just for completenes, changing the signature to fun <T : Any> anyVararg(clazz: KClass<T>): Array<T> and using it like this method(*anyVararg()) might be cleaner, but would complicate the migration path without much additional value.