Skip to content

Commit

Permalink
TRUNK-5816 Use MockitoExtension
Browse files Browse the repository at this point in the history 
Extensions are the Junit 5 way of extending its capabilities.
MockitoExtension is setup strict by default which is very useful in that
it shows us unnecessary stubs like

org.mockito.exceptions.misusing.UnnecessaryStubbingException:
Unnecessary stubbings detected.
Clean & maintainable test code requires zero unnecessary code.
Following stubbings are unnecessary (click to navigate to relevant line of code):
  1. -> at org.openmrs.api.impl.PatientServiceImplTest.checkPatientIdentifiers_shouldThrowDuplicateIdentifierGivenDuplicateIdentifiers(PatientServiceImplTest.java:165)
Please remove unnecessary stubbings or use 'lenient' strictness. More info: javadoc for UnnecessaryStubbingException class.

this helps us clean up our tests that have had mocks added over time but
not removed once not needed anymore.

MockitoExtension also allows us to inject mocks in constructor/test
methods

Note that we did setup the mocked
userContext.getAuthenticatedUser() in the ContextHelper as lenient so
that this mock that all

BaseContextSensitiveTest
BaseContextMockTest
BaseContextMockJunit5Test

get does not cause such errors if not used
  • Loading branch information
@teleivo
teleivo committed Jul 11, 2020
1 parent 0a9f185 commit 68f17d6
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 73 deletions.
6 changes: 6 additions & 0 deletions api/pom.xml
View file
Expand Up @@ -296,6 +296,12 @@
<version>${junitVersion}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-junit-jupiter</artifactId>
<version>3.3.3</version>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<resources>
Expand Down
72 changes: 26 additions & 46 deletions api/src/test/java/org/openmrs/aop/RequiredDataAdviceTest.java
View file
Expand Up @@ -24,7 +24,6 @@
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
Expand All @@ -46,15 +45,10 @@
import org.openmrs.Location;
import org.openmrs.OpenmrsObject;
import org.openmrs.Person;
import org.openmrs.PersonName;
import org.openmrs.Role;
import org.openmrs.User;
import org.openmrs.annotation.AllowDirectAccess;
import org.openmrs.annotation.DisableHandlers;
import org.openmrs.api.APIException;
import org.openmrs.api.AdministrationService;
import org.openmrs.api.context.Context;
import org.openmrs.api.context.ServiceContext;
import org.openmrs.api.handler.BaseVoidHandler;
import org.openmrs.api.handler.OpenmrsObjectSaveHandler;
import org.openmrs.api.handler.RequiredDataHandler;
Expand All @@ -67,26 +61,19 @@
import org.openmrs.test.BaseContextMockJunit5Test;
import org.openmrs.util.HandlerUtil;
import org.openmrs.util.Reflect;
import org.openmrs.util.RoleConstants;
import org.springframework.context.ApplicationContext;

/**
* Tests the {@link RequiredDataAdvice} class.
*/
public class RequiredDataAdviceTest extends BaseContextMockJunit5Test {

@Mock
AdministrationService administrationService;

@Mock
ApplicationContext applicationContext;

@Mock
Context context;

@Mock
ServiceContext serviceContext;

@Spy
OpenmrsObjectSaveHandler saveHandler;

Expand All @@ -97,37 +84,6 @@ public class RequiredDataAdviceTest extends BaseContextMockJunit5Test {

@BeforeEach
public void setUp() {

Context.setUserContext(userContext);
context.setServiceContext(serviceContext);
Context.setContext(serviceContext);
serviceContext.setApplicationContext(applicationContext);

User user = new User();
user.setUuid("1010d442-e134-11de-babe-001e378eb67e");
user.setUserId(1);
user.setUsername("admin");
user.addRole(new Role(RoleConstants.SUPERUSER));
Person person = new Person();
person.setUuid("6adb7c42-cfd2-4301-b53b-ff17c5654ff7");
person.setId(1);
person.addName(new PersonName("Bob", "", "Smith"));
Calendar calendar = Calendar.getInstance();
calendar.set(1980, 01, 01);
person.setBirthdate(calendar.getTime());
person.setGender("M");
user.setPerson(person);
when(userContext.getAuthenticatedUser()).thenReturn(user);
when(userContext.isAuthenticated()).thenReturn(true);

Map<String, SaveHandler> saveHandlers = new HashMap<>();
saveHandlers.put("saveHandler", saveHandler);
when(applicationContext.getBeansOfType(SaveHandler.class)).thenReturn(saveHandlers);

Map<String, VoidHandler> voidHandlers = new HashMap<>();
voidHandlers.put("voidHandler", voidHandler);
when(applicationContext.getBeansOfType(VoidHandler.class)).thenReturn(voidHandlers);

//Clear cache since handlers are updated
HandlerUtil.clearCachedHandlers();
}
Expand Down Expand Up @@ -544,6 +500,10 @@ public void before_shouldNotCallHandlerOnSaveWithNullOrNoArguments() throws Thro
@Test
public void before_shouldCallHandlerOnSaveWithOpenmrsObjectArgument() throws Throwable {

Map<String, SaveHandler> saveHandlers = new HashMap<>();
saveHandlers.put("saveHandler", saveHandler);
when(applicationContext.getBeansOfType(SaveHandler.class)).thenReturn(saveHandlers);

Method m = WithAppropriatelyNamedMethod.class.getMethod("saveSomeOpenmrsData", SomeOpenmrsData.class);
SomeOpenmrsData openmrsObject = new SomeOpenmrsData();
requiredDataAdvice.before(m, new Object[] { openmrsObject }, new WithAppropriatelyNamedMethod());
Expand All @@ -564,6 +524,10 @@ public void before_shouldNotCallHandlerOnSaveMethodNameNotMatchingDomainObject()
@Test
public void before_shouldCallHandlerOnSaveMethodNameWithCollectionArgument() throws Throwable {

Map<String, SaveHandler> saveHandlers = new HashMap<>();
saveHandlers.put("saveHandler", saveHandler);
when(applicationContext.getBeansOfType(SaveHandler.class)).thenReturn(saveHandlers);

Method m = WithAppropriatelyNamedMethod.class.getMethod("saveSomeOpenmrsDatas", List.class);
List<SomeOpenmrsData> openmrsObjects = Arrays.asList(new SomeOpenmrsData(), new SomeOpenmrsData());
requiredDataAdvice.before(m, new Object[] { openmrsObjects }, new WithAppropriatelyNamedMethod());
Expand All @@ -585,6 +549,10 @@ public void before_shouldNotCallHandlerOnVoidWithNullOrNoArguments() throws Thro
@Test
public void before_shouldCallHandlerOnVoidMethodNameMatchingDomainObject() throws Throwable {

Map<String, VoidHandler> voidHandlers = new HashMap<>();
voidHandlers.put("voidHandler", voidHandler);
when(applicationContext.getBeansOfType(VoidHandler.class)).thenReturn(voidHandlers);

Method m = WithAppropriatelyNamedMethod.class.getMethod("voidSomeOpenmrsData", SomeOpenmrsData.class);
SomeOpenmrsData openmrsObject = new SomeOpenmrsData();
requiredDataAdvice.before(m, new Object[] { openmrsObject, "void reason" }, new WithAppropriatelyNamedMethod());
Expand All @@ -595,6 +563,10 @@ public void before_shouldCallHandlerOnVoidMethodNameMatchingDomainObject() throw
@Test
public void before_shouldCallHandlerOnVoidMethodWhenDomainObjectIsAssignableFromMethodNameObject() throws Throwable {

Map<String, VoidHandler> voidHandlers = new HashMap<>();
voidHandlers.put("voidHandler", voidHandler);
when(applicationContext.getBeansOfType(VoidHandler.class)).thenReturn(voidHandlers);

Method m = WithAppropriatelyNamedMethod.class.getMethod("voidSomeOpenmrsData", SomeOpenmrsData.class);
SomeOpenmrsData openmrsObjectSubClass = new SomeOpenmrsDataSubClass();
requiredDataAdvice.before(m, new Object[] { openmrsObjectSubClass, "void reason" },
Expand All @@ -616,6 +588,10 @@ public void before_shouldNotCallHandlerOnVoidMethodNameNotMatchingDomainObject()
@Test
public void before_shouldNotCallHandlersAnnotatedAsDisabled() throws Throwable {

Map<String, VoidHandler> voidHandlers = new HashMap<>();
voidHandlers.put("voidHandler", voidHandler);
when(applicationContext.getBeansOfType(VoidHandler.class)).thenReturn(voidHandlers);

Method m = WithAppropriatelyNamedMethod.class.getMethod("voidClassWithDisableHandlersAnnotation",
ClassWithDisableHandlersAnnotation.class);

Expand All @@ -638,6 +614,10 @@ public void before_shouldNotCallHandlersAnnotatedAsDisabled() throws Throwable {
@Test
public void before_shouldCallHandlersNotAnnotatedAsDisabled() throws Throwable {

Map<String, VoidHandler> voidHandlers = new HashMap<>();
voidHandlers.put("voidHandler", voidHandler);
when(applicationContext.getBeansOfType(VoidHandler.class)).thenReturn(voidHandlers);

Method m = WithAppropriatelyNamedMethod.class.getMethod("voidClassWithDisableHandlersAnnotation",
ClassWithDisableHandlersAnnotation.class);

Expand Down
20 changes: 6 additions & 14 deletions api/src/test/java/org/openmrs/api/impl/PatientServiceImplTest.java
View file
Expand Up @@ -10,6 +10,8 @@
package org.openmrs.api.impl;

import static junit.framework.TestCase.assertTrue;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -162,9 +164,6 @@ public void checkPatientIdentifiers_shouldThrowDuplicateIdentifierGivenDuplicate
final PatientIdentifierType sameIdentifierType = new PatientIdentifierType(equalIdentifierTypeId);
sameIdentifierType.setName(equalIdentifierTypeName);

when(patientDaoMock.getPatientIdentifierTypes(any(), any(), any(), any()))
.thenReturn(new ArrayList<>());

final Patient patientWithIdentifiers = new Patient();
final PatientIdentifier patientIdentifier = new PatientIdentifier("some identifier", identifierType,
mock(Location.class));
Expand All @@ -176,17 +175,10 @@ public void checkPatientIdentifiers_shouldThrowDuplicateIdentifierGivenDuplicate
patientIdentifierSameType.setIdentifier(equalIdentifier);
patientWithIdentifiers.addIdentifier(patientIdentifierSameType);

// when
try {
patientService.checkPatientIdentifiers(patientWithIdentifiers);
// then
fail();
}
catch (DuplicateIdentifierException e) {
assertNotNull(e.getPatientIdentifier());
assertTrue(e.getMessage().contains("Identifier1 id type #: 12345"));
}

DuplicateIdentifierException thrown = assertThrows(DuplicateIdentifierException.class,
() -> patientService.checkPatientIdentifiers(patientWithIdentifiers));
assertNotNull(thrown.getPatientIdentifier());
assertThat(thrown.getMessage(), containsString("Identifier1 id type #: 12345"));
}

@Test
Expand Down
17 changes: 4 additions & 13 deletions api/src/test/java/org/openmrs/test/BaseContextMockJunit5Test.java
View file
Expand Up @@ -10,21 +10,20 @@
package org.openmrs.test;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.junit.jupiter.MockitoExtension;
import org.openmrs.api.context.ContextMockHelper;
import org.openmrs.api.context.UserContext;
import org.openmrs.module.ModuleUtilTest;

/**
* Tests extending this class have a mocked authenticated UserContext. In addition you can mock
* Context.get...Service() calls by annotating fields with {@link Mock}.
*
* @see ModuleUtilTest
* @since 1.11, 1.10, 1.9.9
* @since 2.4.0
*/
@ExtendWith(MockitoExtension.class)
public abstract class BaseContextMockJunit5Test {

@Mock
Expand All @@ -33,14 +32,6 @@ public abstract class BaseContextMockJunit5Test {
@InjectMocks
protected ContextMockHelper contextMockHelper;

/**
* Initializes fields annotated with {@link Mock}. Sets userContext and authenticatedUser.
*/
@BeforeEach
public void initMocks() {
MockitoAnnotations.initMocks(this);
}

@AfterEach
public void revertContextMocks() {
contextMockHelper.revertMocks();
Expand Down

0 comments on commit 68f17d6

Please sign in to comment.