Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FM2-652 - Update unit and integration tests to remove test conflicts and fix resulting failures #555

Merged
merged 10 commits into from
Jan 6, 2025
Merged

FM2-652 - Update unit and integration tests to remove test conflicts and fix resulting failures #555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
01ba800
Remove unnecessary and invalid mocks
mseaton Dec 20, 2024
d703738
Additional fixes to mocked context sensitive tests
mseaton Dec 20, 2024
d063290
Additional fixes to mocked context sensitive tests
mseaton Dec 20, 2024
7f58602
Additional fixes to mocked context sensitive tests
mseaton Dec 20, 2024
66f7a5f
Refactor all context sensitive tests to extend a base class that rese…
mseaton Dec 20, 2024
d736f9d
Fix issues with mocking and test conflicts in omod project
mseaton Dec 20, 2024
4bd8690
Fix incorrect integration test
mseaton Dec 20, 2024
53aa3c2
Reset cached GPs in base fhir integration test
mseaton Dec 20, 2024
d1f6f11
Attempt to fix e2e tests. Temporarily disable integration tests.
mseaton Dec 20, 2024
acc9584
Re-enable integration tests
mseaton Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ jobs:
path: ${{ github.workspace }}/omod/target
- name: run db and web containers
run: |
docker-compose -f docker/docker-compose-refqa.yml up -d
docker compose -f docker/docker-compose-refqa.yml up -d
- name: wait for openmrs instance to start
run: while [[ "$(curl -s -o /dev/null -w ''%{http_code}'' http://localhost:8080/openmrs/login.htm)" != "200" ]]; do sleep 1; done
- name: Run End to End tests
run: mvn verify --batch-mode -P e2e-test --file pom.xml
- name: Stop db and web containers
if: always()
run: docker-compose -f "docker/docker-compose-refqa.yml" down
run: docker compose -f "docker/docker-compose-refqa.yml" down
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once I got the build to pass, the e2e tests failed as this syntax was now wrong. This fixes it.

3 changes: 1 addition & 2 deletions api/src/main/java/org/openmrs/module/fhir2/api/dao/impl/FhirConceptDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.Optional;

import ca.uhn.fhir.rest.param.StringAndListParam;
import lombok.AccessLevel;
import lombok.Setter;
import org.hibernate.Criteria;
import org.hibernate.transform.DistinctRootEntityResultTransformer;
Expand All @@ -38,7 +37,7 @@
import org.springframework.stereotype.Component;

@Component
@Setter(AccessLevel.PACKAGE)
@Setter
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For mocking

public class FhirConceptDaoImpl extends BaseFhirDao<Concept> implements FhirConceptDao {

@Autowired
Expand Down
3 changes: 1 addition & 2 deletions api/src/main/java/org/openmrs/module/fhir2/api/impl/FhirConceptServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import java.util.List;
import java.util.Optional;

import lombok.AccessLevel;
import lombok.Setter;
import org.openmrs.Concept;
import org.openmrs.ConceptMap;
Expand All @@ -26,7 +25,7 @@

@Component
@Transactional
@Setter(AccessLevel.PACKAGE)
@Setter
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For mocking

public class FhirConceptServiceImpl implements FhirConceptService {

@Autowired
Expand Down
3 changes: 1 addition & 2 deletions api/src/main/java/org/openmrs/module/fhir2/api/impl/FhirConceptSourceServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import java.util.Collection;
import java.util.Optional;

import lombok.AccessLevel;
import lombok.Setter;
import org.openmrs.ConceptSource;
import org.openmrs.Duration;
Expand All @@ -28,7 +27,7 @@

@Component
@Transactional
@Setter(AccessLevel.PACKAGE)
@Setter
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For mocking

public class FhirConceptSourceServiceImpl implements FhirConceptSourceService {

@Autowired
Expand Down
21 changes: 18 additions & 3 deletions ...in/java/org/openmrs/module/fhir2/api/translators/impl/ObservationValueTranslatorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ public Type toFhirResource(@Nonnull Obs obs) {

// IMPORTANT boolean values are stored as a coded value, so for this to
// work, we must check for a boolean value before a general coded value
if (obs.getValueBoolean() != null) {
return new BooleanType(obs.getValueBoolean());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows mocking out the obs.getValueBoolean() method in tests without having to introduce static mocks, which often cause issues dirtying the context.

Boolean valueBoolean = getValueBoolean(obs);
if (valueBoolean != null) {
return new BooleanType(valueBoolean);
} else if (obs.getValueCoded() != null) {
return conceptTranslator.toFhirResource(obs.getValueCoded());
} else if (obs.getValueDrug() != null) {
Expand Down Expand Up @@ -99,11 +100,25 @@ public Obs toOpenmrsType(@Nonnull Obs obs, @Nonnull Type resource) {
} else if (resource instanceof Quantity) {
obs.setValueNumeric(((Quantity) resource).getValue().doubleValue());
} else if (resource instanceof BooleanType) {
obs.setValueBoolean(((BooleanType) resource).getValue());
setValueBoolean(obs, ((BooleanType) resource).getValue());
} else if (resource instanceof StringType) {
obs.setValueText(((StringType) resource).getValue());
}

return obs;
}

/**
* @return the valueBoolean of the given obs
*/
protected Boolean getValueBoolean(Obs obs) {
return obs.getValueBoolean();
}

/**
* sets the valueBoolean property of the given obs to the given value
*/
protected void setValueBoolean(Obs obs, Boolean valueBoolean) {
obs.setValueBoolean(valueBoolean);
}
}
21 changes: 9 additions & 12 deletions ...2/MockedCalendarFactoryConfiguration.java → ...e/fhir2/BaseFhirContextSensitiveTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,16 @@
*/
package org.openmrs.module.fhir2;

import org.mockito.Mockito;
import org.openmrs.module.fhir2.api.util.LocalDateTimeFactory;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Primary;
import org.junit.Before;
import org.openmrs.module.fhir2.api.util.FhirGlobalPropertyHolder;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.springframework.test.context.ContextConfiguration;

@Configuration
public class MockedCalendarFactoryConfiguration {
@ContextConfiguration(classes = { TestFhirSpringConfiguration.class }, inheritLocations = false)
public abstract class BaseFhirContextSensitiveTest extends BaseModuleContextSensitiveTest {

@Bean
@Primary
public LocalDateTimeFactory getCalendarFactory() {
return Mockito.mock(LocalDateTimeFactory.class);
@Before
public void setupBaseFhirContextSensitive() {
FhirGlobalPropertyHolder.reset();
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This came through odd, but I removed the MockedCalendarFactoryConfiguration class and took a different approach in the tests that required it.

And then I set up a base class for all of the context-sensitive tests in order to ensure the FhirGlobalPropertyHolder is reset before each test.

I believe this FhirGlobalPropertyHolder, in the end, was likely the primary cause of all of the issues, but I still think it's worth cleaning out any mocks from context sensitive tests, and trying to avoid static mocks if at all possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's very likely the case...


}
30 changes: 0 additions & 30 deletions api/src/test/java/org/openmrs/module/fhir2/MockedGlobalPropertyServiceConfiguration.java
View file
Open in desktop

This file was deleted.

23 changes: 4 additions & 19 deletions ...rc/test/java/org/openmrs/module/fhir2/api/dao/impl/FhirAllergyIntoleranceDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,19 @@
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;

import org.hibernate.SessionFactory;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.openmrs.Allergen;
import org.openmrs.AllergenType;
import org.openmrs.Allergy;
import org.openmrs.AllergyReaction;
import org.openmrs.Concept;
import org.openmrs.module.fhir2.TestFhirSpringConfiguration;
import org.openmrs.module.fhir2.api.FhirGlobalPropertyService;
import org.openmrs.module.fhir2.BaseFhirContextSensitiveTest;
import org.openmrs.module.fhir2.api.dao.FhirAllergyIntoleranceDao;
import org.openmrs.module.fhir2.api.dao.FhirConceptDao;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.test.context.ContextConfiguration;

@ContextConfiguration(classes = TestFhirSpringConfiguration.class, inheritLocations = false)
public class FhirAllergyIntoleranceDaoImplTest extends BaseModuleContextSensitiveTest {
public class FhirAllergyIntoleranceDaoImplTest extends BaseFhirContextSensitiveTest {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many of the changes that follow will have similar change to this - remove the context configuration and set the base class to the new BaseFhirContextSensitiveTest (which has the context configuration), and update imports.


private static final String ALLERGY_INTOLERANCE_INITIAL_DATA_XML = "org/openmrs/module/fhir2/api/dao/impl/FhirAllergyIntoleranceDaoImplTest_initial_data.xml";

Expand All @@ -43,22 +37,13 @@ public class FhirAllergyIntoleranceDaoImplTest extends BaseModuleContextSensitiv
private static final String CODED_REACTION_UUID = "5087AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";

@Autowired
@Qualifier("sessionFactory")
private SessionFactory sessionFactory;

@Mock
private FhirGlobalPropertyService globalPropertyService;

private FhirAllergyIntoleranceDaoImpl allergyDao;
private FhirAllergyIntoleranceDao allergyDao;

@Autowired
private FhirConceptDao conceptDao;

@Before
public void setup() throws Exception {
allergyDao = new FhirAllergyIntoleranceDaoImpl();
allergyDao.setSessionFactory(sessionFactory);
allergyDao.setGlobalPropertyService(globalPropertyService);
executeDataSet(ALLERGY_INTOLERANCE_INITIAL_DATA_XML);
}

Expand Down
7 changes: 2 additions & 5 deletions api/src/test/java/org/openmrs/module/fhir2/api/dao/impl/FhirConceptDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,12 @@
import org.openmrs.ConceptSource;
import org.openmrs.api.ConceptService;
import org.openmrs.api.context.Context;
import org.openmrs.module.fhir2.TestFhirSpringConfiguration;
import org.openmrs.module.fhir2.BaseFhirContextSensitiveTest;
import org.openmrs.module.fhir2.api.dao.FhirConceptDao;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.test.context.ContextConfiguration;

@ContextConfiguration(classes = TestFhirSpringConfiguration.class, inheritLocations = false)
public class FhirConceptDaoImplTest extends BaseModuleContextSensitiveTest {
public class FhirConceptDaoImplTest extends BaseFhirContextSensitiveTest {

private static final String CONCEPT_DATA_XML = "org/openmrs/api/include/ConceptServiceTest-initialConcepts.xml";

Expand Down
7 changes: 2 additions & 5 deletions api/src/test/java/org/openmrs/module/fhir2/api/dao/impl/FhirConceptSourceDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,12 @@
import org.junit.Test;
import org.openmrs.ConceptSource;
import org.openmrs.api.ConceptService;
import org.openmrs.module.fhir2.BaseFhirContextSensitiveTest;
import org.openmrs.module.fhir2.FhirTestConstants;
import org.openmrs.module.fhir2.TestFhirSpringConfiguration;
import org.openmrs.module.fhir2.model.FhirConceptSource;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.test.context.ContextConfiguration;

@ContextConfiguration(classes = TestFhirSpringConfiguration.class, inheritLocations = false)
public class FhirConceptSourceDaoImplTest extends BaseModuleContextSensitiveTest {
public class FhirConceptSourceDaoImplTest extends BaseFhirContextSensitiveTest {

private static final String CONCEPT_SOURCE_FHIR_DATA = "org/openmrs/module/fhir2/api/dao/impl/FhirConceptSourceDaoImplTest_initial_data.xml";

Expand Down
7 changes: 2 additions & 5 deletions api/src/test/java/org/openmrs/module/fhir2/api/dao/impl/FhirConditionDaoImpl_Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,11 @@
import org.openmrs.ConditionVerificationStatus;
import org.openmrs.api.ConceptService;
import org.openmrs.api.PatientService;
import org.openmrs.module.fhir2.TestFhirSpringConfiguration;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.openmrs.module.fhir2.BaseFhirContextSensitiveTest;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.test.context.ContextConfiguration;

@ContextConfiguration(classes = TestFhirSpringConfiguration.class, inheritLocations = false)
public class FhirConditionDaoImpl_Test extends BaseModuleContextSensitiveTest {
public class FhirConditionDaoImpl_Test extends BaseFhirContextSensitiveTest {

private static final String CONDITION_UUID = "2cc6880e-2c46-15e4-9038-a6c5e4d22fb7";

Expand Down
7 changes: 2 additions & 5 deletions api/src/test/java/org/openmrs/module/fhir2/api/dao/impl/FhirContactPointMapDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,12 @@
import org.openmrs.api.LocationService;
import org.openmrs.api.PersonService;
import org.openmrs.api.ProviderService;
import org.openmrs.module.fhir2.TestFhirSpringConfiguration;
import org.openmrs.module.fhir2.BaseFhirContextSensitiveTest;
import org.openmrs.module.fhir2.api.FhirContactPointMapService;
import org.openmrs.module.fhir2.model.FhirContactPointMap;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.test.context.ContextConfiguration;

@ContextConfiguration(classes = TestFhirSpringConfiguration.class, inheritLocations = false)
public class FhirContactPointMapDaoImplTest extends BaseModuleContextSensitiveTest {
public class FhirContactPointMapDaoImplTest extends BaseFhirContextSensitiveTest {

@Autowired
private SessionFactory sessionFactory;
Expand Down
7 changes: 2 additions & 5 deletions api/src/test/java/org/openmrs/module/fhir2/api/dao/impl/FhirDiagnosticReportDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,12 @@
import org.openmrs.api.ConceptService;
import org.openmrs.api.ObsService;
import org.openmrs.api.PatientService;
import org.openmrs.module.fhir2.TestFhirSpringConfiguration;
import org.openmrs.module.fhir2.BaseFhirContextSensitiveTest;
import org.openmrs.module.fhir2.model.FhirDiagnosticReport;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.test.context.ContextConfiguration;

@ContextConfiguration(classes = TestFhirSpringConfiguration.class, inheritLocations = false)
public class FhirDiagnosticReportDaoImplTest extends BaseModuleContextSensitiveTest {
public class FhirDiagnosticReportDaoImplTest extends BaseFhirContextSensitiveTest {

private static final String DATA_XML = "org/openmrs/module/fhir2/api/dao/impl/FhirDiagnosticReportDaoImplTest_initial_data.xml";

Expand Down
7 changes: 2 additions & 5 deletions api/src/test/java/org/openmrs/module/fhir2/api/dao/impl/FhirEncounterDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,13 @@
import org.openmrs.Encounter;
import org.openmrs.Order;
import org.openmrs.api.context.Context;
import org.openmrs.module.fhir2.BaseFhirContextSensitiveTest;
import org.openmrs.module.fhir2.FhirConstants;
import org.openmrs.module.fhir2.TestFhirSpringConfiguration;
import org.openmrs.module.fhir2.api.search.param.SearchParameterMap;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.test.context.ContextConfiguration;

@ContextConfiguration(classes = TestFhirSpringConfiguration.class, inheritLocations = false)
public class FhirEncounterDaoImplTest extends BaseModuleContextSensitiveTest {
public class FhirEncounterDaoImplTest extends BaseFhirContextSensitiveTest {

private static final String ENCOUNTER_UUID = "430bbb70-6a9c-4e1e-badb-9d1034b1b5e9";

Expand Down
7 changes: 2 additions & 5 deletions api/src/test/java/org/openmrs/module/fhir2/api/dao/impl/FhirEpisodeOfCareDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,11 @@
import org.junit.Before;
import org.junit.Test;
import org.openmrs.PatientProgram;
import org.openmrs.module.fhir2.TestFhirSpringConfiguration;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.openmrs.module.fhir2.BaseFhirContextSensitiveTest;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.test.context.ContextConfiguration;

@ContextConfiguration(classes = TestFhirSpringConfiguration.class, inheritLocations = false)
public class FhirEpisodeOfCareDaoImplTest extends BaseModuleContextSensitiveTest {
public class FhirEpisodeOfCareDaoImplTest extends BaseFhirContextSensitiveTest {

private static final String ENCOUNTER_INITIAL_DATA_XML = "org/openmrs/module/fhir2/api/dao/impl/FhirEpisodeOfCareDaoImpl_2_2Test_initial_data.xml";

Expand Down
7 changes: 2 additions & 5 deletions api/src/test/java/org/openmrs/module/fhir2/api/dao/impl/FhirGroupDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@
import org.junit.Before;
import org.junit.Test;
import org.openmrs.Cohort;
import org.openmrs.module.fhir2.TestFhirSpringConfiguration;
import org.openmrs.test.BaseModuleContextSensitiveTest;
import org.openmrs.module.fhir2.BaseFhirContextSensitiveTest;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.test.context.ContextConfiguration;

@ContextConfiguration(classes = TestFhirSpringConfiguration.class, inheritLocations = false)
public class FhirGroupDaoImplTest extends BaseModuleContextSensitiveTest {
public class FhirGroupDaoImplTest extends BaseFhirContextSensitiveTest {

private static final String COHORT_UUID = "1d64befb-3b2e-48e5-85f5-353d43e23e46";

Expand Down
Loading
Loading