Upgrade Mockito to 3.x #73
Conversation
* Upgrade Mockito and PowerMock to the latest version * Fix deprecation warnings * Use withSettings().withoutAnnotations() in HelloActivityTest to prevent Mockito from copying annotations from activity interface to mock class * Re-enable HelloChildTest.testMockedChild() and HelloExceptionTest.testChildWorkflowTimeout() as they no longer fail with Mockito 3
|
@mfateev @vitarb I've noticed that Java SDK is getting Mockito 3 in the scope of temporalio/sdk-java#360. Do you think it's worth upgrading samples as well? |
The annotation is added for illustration purposes to show that Mockito's withSettings().withoutAnnotations() is required to mock an activity that has that annotation on a method.
|
@GreyTeardrop will need to be rebased. Also if you have time let's check if there are any examples tests that were added recently needs an update as well. Thanks! |
# Conflicts: # build.gradle
|
Thank you, @tsurdilo! I've merged the latest |
| // withoutAnnotations() is required to stop Mockito from copying | ||
| // method-level annotations from the GreetingActivities interface | ||
| GreetingActivities activities = | ||
| mock(GreetingActivities.class, withSettings().withoutAnnotations()); |
There was a problem hiding this comment.
What's the reason that you don't want Mockito to preserve the annotations?
Why if you do preserve them (testMockedActivityWithoutSettings) you expect the exception?
Just asking because if you have an Activity method with let's say
@ActivityMethod(name = "sayHello")
String composeGreeting(String greeting, String name);
The activity type is different if you remove the annotation, and you probably would want to test with the
actually registered type from the annotation name param? Same really with the @ActivityInterface annotation.
Just wondering.
There was a problem hiding this comment.
The annotation on the interface class GreetingActivities should not be affected. The withoutAnnotations() setting removes annotations that Mockito copies from the interface class to the dynamically generated mock. Without it, the worker.registerActivitiesImplementations(activities) call fails, as it explicitly checks that the activity implementation class should not have @ActivityMethod annotations.
Someone has reported that behavior as a temporalio/sdk-java#332 issue for the Java SDK.
There was a problem hiding this comment.
+1 thanks for the explanation
|
@vitarb please review as well when you can. thanks! |
Overview & motivation
Update test code to Mockito 3 and adapt when needed. This PR is inspired by temporalio/sdk-java#332 and should provide an example of how a recent Mockito can be used with Temporal SDK.
Details
withSettings().withoutAnnotations()inHelloActivityTestto prevent Mockito from copying annotations from activity interface to mock classHelloChildTest.testMockedChild()andHelloExceptionTest.testChildWorkflowTimeout()as they no longer fail with Mockito 3, presumably due to Mockito's switch to ByteBuddy