FM2-652 - Update unit and integration tests to remove test conflicts and fix resulting failures #555
Conversation
…ts the static global property holder
| - 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 |
There was a problem hiding this comment.
Once I got the build to pass, the e2e tests failed as this syntax was now wrong. This fixes it.
| @@ -38,7 +37,7 @@ | |||
| import org.springframework.stereotype.Component; | |||
|
|
|||
| @Component | |||
| @Setter(AccessLevel.PACKAGE) | |||
| @Setter | |||
| @@ -26,7 +25,7 @@ | |||
|
|
|||
| @Component | |||
| @Transactional | |||
| @Setter(AccessLevel.PACKAGE) | |||
| @Setter | |||
| @@ -28,7 +27,7 @@ | |||
|
|
|||
| @Component | |||
| @Transactional | |||
| @Setter(AccessLevel.PACKAGE) | |||
| @Setter | |||
| @@ -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()); | |||
There was a problem hiding this comment.
This allows mocking out the obs.getValueBoolean() method in tests without having to introduce static mocks, which often cause issues dirtying the context.
| @@ -163,44 +155,6 @@ public void shouldOnlyCreateOneExtensionForExtensibleAttributes() { | |||
| assertThat(extension.getExtension().size(), greaterThan(1)); | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
So, I ended up getting rid of this test entirely, since it isn't actually testing anything happening intentionally in the FHIR2 module, but is just a symptom of what the OpenMRS getFullName method returns. This allowed me to not have to worry about mocking out the hacky static stuff in NameSupport
| import org.openmrs.module.fhir2.FhirConstants; | ||
| import org.openmrs.module.fhir2.api.translators.PractitionerReferenceTranslator; | ||
|
|
||
| public abstract class BaseFhirProvenanceResourceTest<T extends DomainResource> { | ||
|
|
There was a problem hiding this comment.
I don't think any of this was causing a problem in the end (not entirely sure, as I was just picking stuff off one by one), but none of it is needed / used, and removing it did not lead to any issues, so I removed it.
| @@ -86,6 +87,7 @@ public abstract class BaseFhirIntegrationTest<T extends IResourceProvider, U ext | |||
|
|
|||
| @Before | |||
| public void setup() throws Exception { | |||
| FhirGlobalPropertyHolder.reset(); | |||
There was a problem hiding this comment.
This was ultimately the key important fix, once I made it to the integration tests.
| @@ -45,8 +45,6 @@ public class GroupFhirResourceProviderIntegrationTest extends BaseFhirR4Integrat | |||
|
|
|||
| private static final String COHORT_DATA_XML = "org/openmrs/module/fhir2/api/dao/impl/FhirCohortDaoImplTest_initial_data.xml"; | |||
|
|
|||
| private static final String PATIENT_DATA_XML = "org/openmrs/module/fhir2/api/dao/impl/FhirPatientDaoImplTest_initial_data.xml"; | |||
There was a problem hiding this comment.
This integration test was actually failing. Not sure why this was never caught otherwise. But the tests are wrong - both the COHORT_DATA_XML and PATIENT_DATA_XML were contributing cohorts, but the tests were based only on those in COHORT_DATA_XML. I limited this to just the one file, and copied stuff from PATIENT_DATA_XML over that was needed.
| @@ -25,12 +25,15 @@ public class RequireAuthenticationInterceptor { | |||
| @Hook(Pointcut.SERVER_INCOMING_REQUEST_PRE_PROCESSED) | |||
| public boolean ensureUserAuthenticated(HttpServletRequest request, HttpServletResponse response) throws IOException { | |||
| if (!(request.getRequestURI().contains("/.well-known") || request.getRequestURI().endsWith("/metadata")) | |||
| && !Context.isAuthenticated()) { | |||
There was a problem hiding this comment.
This is another change I made to allow testing without static Context mocks.
mseaton
changed the title
See https://openmrs.atlassian.net/browse/FM2-652