Wiki Spaces
Documentation
Projects
Resources
Get Help from Others
Q&A: Ask OpenMRS
Discussion: OpenMRS Talk
Real-Time: IRC Chat | Slack
Our unit tests are written using the testing framework JUnit version 4. (Currently Migrating to JUnit version 5)
When writing unit tests you should make sure that they
Take for example this method
public static Boolean isInNormalNumericRange(Float value, ConceptNumeric concept) { if (concept.getHiNormal() == null || concept.getLowNormal() == null) { return false; } return (value <= concept.getHiNormal() && value >= concept.getLowNormal()); }
It lets the user know if the given value is in the range of values (high and low) set in the given concept. How does the method behave when you pass in a concept that has no high or low range set? It will return false. How does the method behave when you pass in a value that is within the range of the given concept? It will return true.
How do we test these behaviors? The following shows 2 example tests that pass in a Concept with different values set for the range by its setters setHiNormal and setLowNormal. Finally we assert on the output of the method we are testing to ensure the result is what we expect. This shows that we try to think of the methods we test as black boxes where we tweak the input to exercise a certain path in the code and assert on its output.
@Test public void isInNormalNumericRange_shouldReturnFalseIfHiNormalIsNull() { ConceptNumeric concept = new ConceptNumeric(); concept.setHiNormal(null); assertFalse(OpenmrsUtil.isInNormalNumericRange(5.67f, concept)); } @Test public void isInNormalNumericRange_shouldReturnTrueIfValueIsInRange() { ConceptNumeric concept = new ConceptNumeric(); concept.setHiNormal(10.34d); concept.setLowNormal(3.67d); assertTrue(OpenmrsUtil.isInNormalNumericRange(5.67f, concept)); }
Make sure to also test edge cases like in this example when the value is equal to the low or high bound of the range. You can see all of the test cases for isInNormalNumericRange at https://github.com/openmrs/openmrs-core/blob/master/api/src/test/java/org/openmrs/util/OpenmrsUtilTest.java.
The following sections describe our requirements and the new style/convention we follow.
We encourage you to use Test Driven Development but you are free to use the development methodology you want to use.
The only thing we do not negotiate is tests. We want to write code to save lives. It is out of question that new contributions come without tests.
These are the basic requirements:
public int add(int a, int b)
: you pass in two values and expect the method to return you the sum, assert (check) that the return value is as you expectednull
, an empty List
, ...)First start by importing the code templates we provide with the OpenMRS core repository (Eclipse or IntelliJ) which will make it easy for you to insert tests according to our conventions and save you from typing a lot.
Then read on the next sections on how to name test methods, what to test and have a look at the example code.
We want to help newcomers with simple rules with good explanations as to why we think these rules (be it naming or testing) are good. These rules also help a community of many to write code in a similar way which helps when reading code someone else wrote. However, the conventions should only be seen as a guideline not as a law and they will also evolve. What we think of as a good approach today might not be so good tomorrow. Please speak up, give us your input, help us to improve the way we work
We follow a naming approach described by Dan North as Behavior Driven Development, he states: "What to call your test is easy – it’s a sentence describing the next behaviour in which you are interested."
We name our test methods in this scheme:
shouldDoThisGivenThat()
where the keyword should
is followed by a camel cased sentence of what you expect the method you are testing to do or return.
Lets say you want to write a test that asserts that
public Patient savePatient(Patient patient)
throws a NullPointerException
when called with null
.
shouldThrowNullPointerExceptionGivenNull or shouldFailIfGivenNullAsPatient
shouldFailGivenNull
Reasons:
null
Method
public User authenticate(String username, String password)
authenticates a user if the given user and password are correct.
These are the test method names testing
shouldAuthenticateGivenUsernameAndPassword()
shouldNotAuthenticateGivenUsernameAndIncorrectPassword()
see how this reads nicely about what behavior is expected and the case of failure is simply shown by Not
and the parameter that caused the failure highlighted with the word Incorrect
You ensure your code behaves as you intended it to by checking that the method you test either returns what you expect or checking that an object is changed in the way you expect.
To do that you will use whats called an assertion.
Assertions are checks that ensure what you expect is actually happening. For example in
assertTrue(isAuthenticated(user));
the assertion assertTrue from JUnit will make sure that the call to isAuthenticated will evaluate to true for the given user. At least one assertion has to be present in every one of your tests. The testing framework JUnit already comes with a set of assertions which we extend with the assertion library hamcrest. Hamcrest helps make these checks read more natural and gives better outputs in case the tests fail. You will most likely choose a Hamcrest assertion when working with collections like Lists or Maps, Strings or Objects. Hamcrest also lets you combine matchers.
Hamcrest has a very valuable short tutorial showing you why it exists and how you can get started using it http://hamcrest.org/JavaHamcrest/tutorial. Just note that the tutorial uses the latest Junit version 5 and OpenMRS still relies on the version 4. Once you are comfortable with the basics you can find all the matchers you can use at http://hamcrest.org/JavaHamcrest/javadoc/2.2/
Use the assertThat
from Hamcrest instead of the one coming from Junit as the Junit one is deprecated! For details see
The following shows a test using Hamcrest matchers
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.is; import org.junit.Test; @Test public void parse_shouldParseMappingFiles() throws IOException { Document config = buildOnValidConfigXml() .withTextNode("mappingFiles", "\n ReportDesign.hbm.xml ReportDesign.hbm.xml\n \t\tReportRequest.hbm.xml \t") .build(); Module module = parser.parse(writeConfigXmlToFile(config)); assertThat(module.getMappingFiles().size(), is(3)); assertThat(module.getMappingFiles(), hasItems("ReportDesign.hbm.xml", "ReportRequest.hbm.xml")); }
The test is checking that if we parse an XML configuration for an OpenMRS module we will get a module that has all of the mapping files set. We do that by looking at the getMappingFiles() which gives us a List<String>. We use the org.hamcrest.MatcherAssert.assertThat combined with matchers like is and hasItems. hasItems makes sure that the list contains these 2 items. Hamcrest provides many more matchers that you can find at http://hamcrest.org/JavaHamcrest/javadoc/2.2/
Always use the assertThrows that was added in JUnit 4.13. Do not use the ExpectedException Rule or the expected field in the "@Test" annotation! Both will not be available in JUnit 5 to which we are currently migrating - TRUNK-5813Getting issue details... STATUS https://github.com/openmrs/openmrs-core/ ! For details on how to use assertThrows see
https://github.com/junit-team/junit4/wiki/Exception-testing#using-assertthrows-method
Lets say you wanted to test that the method Order.isDiscontinued throws an APIException in case the dateStopped is after the autoExpireDate as in the code below
public boolean isDiscontinued(Date aCheckDate) { if (dateStopped != null && autoExpireDate != null && dateStopped.after(autoExpireDate)) { throw new APIException("Order.error.invalidDateStoppedAndAutoExpireDate", (Object[]) null); } if (getVoided()) { return false; } Date checkDate = aCheckDate == null ? new Date() : aCheckDate; if (!isActivated(checkDate) || dateStopped == null) { return false; } return checkDate.after(dateStopped); }
This is how you can ensure an APIException is thrown using the JUnit assertThrows method.
@Test public void isDiscontinued_shouldFailIfDateStoppedIsAfterAutoExpireDate() throws Exception { Order order = new Order(); order.setDateActivated(DateUtils.parseDate("2014-11-01 11:11:10", DATE_FORMAT)); order.setAutoExpireDate(DateUtils.parseDate("2014-11-01 11:11:11", DATE_FORMAT)); OrderUtilTest.setDateStopped(order, DateUtils.parseDate("2014-11-01 11:11:12", DATE_FORMAT)); APIException exception = assertThrows(APIException.class, () -> order.isDiscontinued(DateUtils.parseDate("2014-11-01 11:11:13", DATE_FORMAT))); assertThat(exception.getMessage(), is(Context.getMessageSourceService().getMessage("Order.error.invalidDateStoppedAndAutoExpireDate"))); }
assertThrows does return the Exception so you can assert on its message if you want. In this case it might not be absolutely necessary since isDiscontinued only throws one APIException. So if our test passes due to assertThrows making sure an APIException was thrown we know we hit the exact code path we wanted to test. You will find occasions where a method throws more than one Exception of the same type like the APIException. In such cases its better to also assert on at least parts of the message. This makes sure your tests cover different cases. Otherwise you might end up with 2 tests passing because the expected Exception is thrown but since you did not assert on the message it could well be the same in both tests.
The getters and setters do not have to be tested unless they are doing something more than getting/setting an internal variable
Like for example
Patient.getIdentifiers() which returns an empty set if the field identifiers
is null, see test PatientTest
All methods in the ___Service classes should be testing with proper input, improper input, and improper data in between.
Most database methods are testing in the process of testing the aforementioned Service Layer Testing. However, some database layer methods are not exposed through Service layer methods. These methods need to be tested.
All set and get methods in the editor require testing.
See DrugEditorTest.java http:--dev.openmrs.org-browser-openmrs-trunk-test-api-org-openmrs-test-propertyeditor-DrugEditorTest.java
Make use of JUnits @Before
to setup variables (like for example encounter
) that you need in multiple tests so that your tests ideally only contain a few changes to the variables that make this test special. Also assert the exceptions you throw using JUnits assertThrows
. Note that you will find existing tests that use the ExpectedException Rule but that one is deprecated so please do not copy this approach.
We have been changing the way we write our tests twice in the past which you can see in the code of the OpenMRS core and maybe also some OpenMRS modules.
It started with tests looking like this
@Test @Verifies(value = "return null low and high if accessors are not called", method = "DoubleRange ()") public void DoubleRange_shouldReturnNullLowAndHighIfAccessorsAreNotCalled() { DoubleRange dr = new DoubleRange(); assertNull(dr.getHigh()); assertNull(dr.getLow()); }
with the org.openmrs.test.Verifies annotation
And moved on to
/** * @see org.openmrs.web.WebUtil#getContextPath() * @verifies should return empty string if webappname is null */ @Test public void getContextPath_shouldReturnEmptyStringWhenWebAppNameIsNull() throws Exception { WebConstants.WEBAPP_NAME = null; Assert.assertEquals("", WebUtil.getContextPath()); }
where verifies was an annotation in the javadoc stating what was tested.
Both styles had in common that you were writing a sentence about what case you were trying to test using the should annotation in an interface or above a concrete method
/** * Gets a allergy matching the given allergyId * * @param allergyId of allergy to return * @return the allergy matching the given allergyId * @should return allergy given valid allergyId * @should return null if no object found with given allergyId */ public Allergy getAllergy(Integer allergyId);
and then use the IDE plugin - Generate Test Case Plugin to generate the test boilerplate (javadoc, test annotation and test method with no test implemented) in a test class.
The reason why these styles came into existence were, back then
Going forward we are discouraging new contributions from using these old styles for the following reasons:
If you want to read the community discussion we had about changing the style see https://talk.openmrs.org/t/rethink-unit-test-style-to-remove-duplication/10200