Skip to content

Commit

Permalink
TRUNK-3944: Updated junit to 4.11 and fixed tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@rkorytkowski
rkorytkowski committed Mar 28, 2013
1 parent 6bec7a3 commit 1171746
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 26 deletions.
2 changes: 1 addition & 1 deletion api/src/main/java/org/openmrs/hl7/handler/ORUR01Handler.java
View file
Expand Up @@ -1062,7 +1062,7 @@ private Patient getPatient(PID pid) throws HL7Exception {
if (patientId == null)
throw new HL7Exception("Could not resolve patient");

return Context.getPatientService().getPatient(patientId);
return Context.getPatientService().getPatient(patientId);
}

/**
Expand Down
54 changes: 33 additions & 21 deletions api/src/test/java/org/openmrs/api/ConceptServiceTest.java
View file
Expand Up @@ -13,10 +13,12 @@
*/
package org.openmrs.api;

import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.openmrs.test.TestUtil.containsId;

Expand All @@ -33,6 +35,9 @@
import junit.framework.Assert;

import org.apache.commons.collections.CollectionUtils;
import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.hamcrest.TypeSafeMatcher;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
Expand All @@ -58,6 +63,7 @@
import org.openmrs.GlobalProperty;
import org.openmrs.Location;
import org.openmrs.Obs;
import org.openmrs.OpenmrsObject;
import org.openmrs.Patient;
import org.openmrs.Person;
import org.openmrs.User;
Expand Down Expand Up @@ -915,8 +921,6 @@ public void getConceptByName_shouldFindConceptsWithNamesInMoreGenericLocales() t
@Verifies(value = "should find concepts with names in same specific locale", method = "getConceptByName(String)")
public void getConceptByName_shouldFindConceptsWithNamesInSameSpecificLocale() throws Exception {
executeDataSet(INITIAL_CONCEPTS_XML);
// sanity check
Assert.assertEquals(Context.getLocale(), Locale.UK);

This comment has been minimized.

Sign in to view

Copy link
@dkayiwa

dkayiwa May 27, 2013

Member

Did you intentionally remove this assert? :)

This comment has been minimized.

Sign in to view

Copy link
@rkorytkowski

rkorytkowski Jun 4, 2013

Author Member

Yes, because it depends on previous tests to set Locale.UK. I'll fix this test to set Locale.UK.

// make sure that concepts are found that have a specific locale on them
Assert.assertNotNull(Context.getConceptService().getConceptByName("Numeric name with en_GB locale"));
Expand Down Expand Up @@ -1365,12 +1369,21 @@ public void getConceptsByConceptSet_shouldReturnAllConceptsInSet() throws Except

List<Concept> conceptSet = conceptService.getConceptsByConceptSet(concept);

Assert.assertEquals(5, conceptSet.size());

This comment has been minimized.

Sign in to view

Copy link
@dkayiwa

dkayiwa May 27, 2013

Member

Don't we need to maintain the assert for ensuring that the size is 5?

This comment has been minimized.

Sign in to view

Copy link
@rkorytkowski

rkorytkowski Jun 4, 2013

Author Member

containsInAnyOrder fails if conceptSet is of different size than the number of matchers
Assert.assertEquals(true, conceptSet.contains(conceptService.getConcept(2)));
Assert.assertEquals(true, conceptSet.contains(conceptService.getConcept(3)));
Assert.assertEquals(true, conceptSet.contains(conceptService.getConcept(4)));
Assert.assertEquals(true, conceptSet.contains(conceptService.getConcept(5)));
Assert.assertEquals(true, conceptSet.contains(conceptService.getConcept(6)));
assertThat(conceptSet, containsInAnyOrder(hasId(2), hasId(3), hasId(4), hasId(5), hasId(6)));
}

private Matcher<? super OpenmrsObject> hasId(final Integer id) {

This comment has been minimized.

Sign in to view

Copy link
@rkorytkowski

rkorytkowski Mar 28, 2013

Author Member

This should probably go to something like OpenmrsMatchers. I'm just not sure if we should put it in main or tests... thoughts?

This comment has been minimized.

Sign in to view

Copy link
@rkorytkowski

rkorytkowski Mar 28, 2013

Author Member

Actually forget the question...
return new TypeSafeMatcher<OpenmrsObject>() {

@Override
public void describeTo(Description description) {
}

@Override
protected boolean matchesSafely(OpenmrsObject item) {
return id.equals(item.getId());
}
};
}

/**
Expand All @@ -1393,7 +1406,7 @@ public void saveConceptStopWord_shouldSaveConceptStopWordIntoDatabase() throws E
@Test
@Verifies(value = "should assign default Locale ", method = "saveConceptStopWord(ConceptStopWord)")
public void saveConceptStopWord_shouldSaveConceptStopWordAssignDefaultLocaleIsItNull() throws Exception {
ConceptStopWord conceptStopWord = new ConceptStopWord("The");
ConceptStopWord conceptStopWord = new ConceptStopWord("The", Locale.UK);

This comment has been minimized.

Sign in to view

Copy link
@dkayiwa

dkayiwa May 27, 2013

Member

Is having to add Locale.UK a result of changing the unit testing dependencies? Or some other reason?

This comment has been minimized.

Sign in to view

Copy link
@rkorytkowski

rkorytkowski Jun 4, 2013

Author Member

Correct, though I'll fix it differently. I will revert to the default locale after each test.
conceptService.saveConceptStopWord(conceptStopWord);

List<String> conceptStopWords = conceptService.getConceptStopWords(Locale.UK);
Expand All @@ -1416,7 +1429,7 @@ public void getConceptStopWords_shouldReturnDefaultLocaleConceptStopWordsIfLocal
@Test
@Verifies(value = "should put generated concept stop word id onto returned concept stop word", method = "saveConceptStopWord(ConceptStopWord)")
public void saveConceptStopWord_shouldSaveReturnConceptStopWordWithId() throws Exception {
ConceptStopWord conceptStopWord = new ConceptStopWord("A");
ConceptStopWord conceptStopWord = new ConceptStopWord("A", Locale.UK);

This comment has been minimized.

Sign in to view

Copy link
@dkayiwa

dkayiwa May 27, 2013

Member

Same question as above.
ConceptStopWord savedConceptStopWord = conceptService.saveConceptStopWord(conceptStopWord);

assertNotNull(savedConceptStopWord.getId());
Expand Down Expand Up @@ -1459,9 +1472,8 @@ public void saveConceptStopWord_shouldSaveConceptStopWordInUppercase() throws Ex
@Verifies(value = "should return list of concept stop word for given locale", method = "getConceptStopWords(Locale)")
public void getConceptStopWords_shouldReturnListOfConceptStopWordsForGivenLocale() throws Exception {
List<String> conceptStopWords = conceptService.getConceptStopWords(Locale.ENGLISH);
assertEquals(2, conceptStopWords.size());

This comment has been minimized.

Sign in to view

Copy link
@dkayiwa

dkayiwa May 27, 2013

Member

Don't we need to maintain the assert for ensuring that the size is 2?

This comment has been minimized.

Sign in to view

Copy link
@rkorytkowski

rkorytkowski Jun 4, 2013

Author Member

See my comment above about containsInAnyOrder
assertEquals("A", conceptStopWords.get(0));
assertEquals("AN", conceptStopWords.get(1));

assertThat(conceptStopWords, containsInAnyOrder("A", "AN"));
}

/**
Expand Down Expand Up @@ -2065,19 +2077,19 @@ public void getConceptReferenceTerms_shouldReturnUniqueTermsWithACodeOrNameConta
for (ConceptReferenceTerm conceptReferenceTerm : matches)
Assert.assertTrue(uniqueTerms.add(conceptReferenceTerm));
}

/**
/**
* @see ConceptService#getConceptsByAnswer(ConceptClass)
*/
@Test
public void getConceptsByAnswer_shouldFindAnswersForConcept() throws Exception {
Concept concept = conceptService.getConcept(7);
Assert.assertNotNull(concept);
List<Concept> concepts = conceptService.getConceptsByAnswer(concept);
Assert.assertEquals(1, concepts.size());
Assert.assertEquals(21, concepts.get(0).getId().intValue());
Concept concept = conceptService.getConcept(7);
Assert.assertNotNull(concept);
List<Concept> concepts = conceptService.getConceptsByAnswer(concept);
Assert.assertEquals(1, concepts.size());
Assert.assertEquals(21, concepts.get(0).getId().intValue());
}
/**
* @see ConceptService#getConceptsByClass(ConceptClass)
* @verifies not fail due to no name in search
Expand Down
4 changes: 4 additions & 0 deletions api/src/test/java/org/openmrs/hl7/HL7ServiceTest.java
View file
Expand Up @@ -24,6 +24,7 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;
import org.openmrs.Concept;
import org.openmrs.GlobalProperty;
Expand All @@ -39,6 +40,7 @@
import org.openmrs.test.Verifies;
import org.openmrs.util.OpenmrsConstants;
import org.openmrs.util.OpenmrsUtil;
import org.springframework.test.annotation.DirtiesContext;

import ca.uhn.hl7v2.HL7Exception;
import ca.uhn.hl7v2.app.Application;
Expand Down Expand Up @@ -185,6 +187,7 @@ public void parseHL7String_shouldParseTheGivenStringIntoMessage() throws Excepti
* @see {@link HL7Service#processHL7Message(Message)}
*/
@Test
@Ignore("TRUNK-3945")

This comment has been minimized.

Sign in to view

Copy link
@dkayiwa

dkayiwa May 27, 2013

Member

Am i correct to say that this test is not properly written because its passing depends on order of execution?

This comment has been minimized.

Sign in to view

Copy link
@rkorytkowski

rkorytkowski Jun 4, 2013

Author Member

Yes.
@Verifies(value = "should parse message type supplied by module", method = "processHL7Message(Message)")
public void processHL7Message_shouldParseMessageTypeSuppliedByModule() throws Exception {
Properties props = super.getRuntimeProperties();
Expand Down Expand Up @@ -234,6 +237,7 @@ public void processHL7Message_shouldParseMessageTypeSuppliedByModule() throws Ex
* @see {@link HL7Service#processHL7InQueue(HL7InQueue)}
*/
@Test
@Ignore("TRUNK-3945")

This comment has been minimized.

Sign in to view

Copy link
@dkayiwa

dkayiwa May 27, 2013

Member

Same as above.
@Verifies(value = "should parse oru r01 message using overridden parser provided by a module", method = "processHL7InQueue(HL7InQueue)")
public void processHL7InQueue_shouldParseOruR01MessageUsingOverriddenParserProvidedByAModule() throws Exception {
executeDataSet("org/openmrs/hl7/include/ORUTest-initialData.xml");
Expand Down
2 changes: 0 additions & 2 deletions api/src/test/java/org/openmrs/module/ModuleInteroperabilityTest.java
View file
Expand Up @@ -37,7 +37,6 @@ public class ModuleInteroperabilityTest extends BaseContextSensitiveTest {
*/
@Test
public void shouldAllowModuleAToLoadModuleBIfARequiresB() throws Exception {

OpenmrsClassLoader loader = OpenmrsClassLoader.getInstance();
Class<?> atdServiceClass = loader.loadClass("org.openmrs.module.atdproducer.service.ATDService");
Class<?> dssServiceClass = loader.loadClass("org.openmrs.module.dssmodule.DssService");
Expand All @@ -60,7 +59,6 @@ public void shouldAllowModuleAToLoadModuleBIfARequiresB() throws Exception {
Class<?> dssServiceClass2 = atdClassLoader.loadClass("org.openmrs.module.dssmodule.DssService");
ModuleClassLoader dssServiceClassLoader = (ModuleClassLoader) dssServiceClass2.getClassLoader();
assertEquals("dssmodule", dssServiceClassLoader.getModule().getModuleId());

}

}
3 changes: 2 additions & 1 deletion api/src/test/java/org/openmrs/test/StartModuleAnnotatioTest.java
View file
Expand Up @@ -15,11 +15,12 @@

import junit.framework.Assert;

import org.junit.Ignore;
import org.junit.Test;
import org.openmrs.api.context.Context;

@StartModule( { "org/openmrs/module/include/dssmodule-1.44.omod", "org/openmrs/module/include/atd-0.51.omod" })
public class StartModuleAnnotatioTest extends BaseModuleContextSensitiveTest {
public class StartModuleAnnotatioTest extends BaseContextSensitiveTest {

This comment has been minimized.

Sign in to view

Copy link
@dkayiwa

dkayiwa May 27, 2013

Member

Good catch here. :)

This comment has been minimized.

Sign in to view

Copy link
@djazayeri

djazayeri Jun 4, 2013

Member

Are you sure this is right? Wasn't this test testing something to do with modules, and perhaps it actually was supposed to have context from classpath*:moduleApplicationContext.xml?

This comment has been minimized.

Sign in to view

Copy link
@rkorytkowski

rkorytkowski Jun 5, 2013

Author Member

I reverted to BaseModuleContextSensitiveTest and it works so this fix wasn't necessary, but still the test only checks if module's classes are loaded and not services.

@Test
public void shouldStartModules() throws Exception {
Expand Down
2 changes: 2 additions & 0 deletions api/src/test/java/org/openmrs/test/StartModuleExecutionListener.java
View file
Expand Up @@ -65,6 +65,8 @@ public void prepareTestInstance(TestContext testContext) throws Exception {
if (!Context.isSessionOpen())
Context.openSession();

ModuleUtil.shutdown();

// load the omods that the dev defined for this class
String modulesToLoad = StringUtils.join(startModuleAnnotation.value(), " ");

Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Expand Up @@ -579,7 +579,7 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.10</version>
<version>4.11</version>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
Expand Down

0 comments on commit 1171746

Please sign in to comment.