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.

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

WIP Test Framework #437

Merged
merged 13 commits into from
Jan 26, 2022
Merged

Conversation

JPercival
Copy link
Contributor

@JPercival JPercival commented Jan 24, 2022

In starting to build the submit-data tests and reviewing the work that's been done so far it became obvious that I hadn't provided quite enough guidance on testing or on building out shared components. This is a PR to fix a few of those things:

  • Reworked/streamlined utility classes, and guidance on how to create them

Read the documentation in the base README

A concrete example for usage of some of the new classes:

// Old way
private IBundleProvider performCodeSystemSearchByUrl(String rxNormUrl) {
  return this.myDaoRegistry.getResourceDao(CodeSystem.class)
		.search(SearchParameterMap.newSynchronous().add(CodeSystem.SP_URL, new UriParam(rxNormUrl)));
}

// New way
private IBundleProvider performCodeSystemSearchByUrl(String rxNormUrl) {
   return search(CodeSystem.class, Searches.byUrl(rxNormUrl));
}
  • OperationProvider base class that is set up to provide easy local CRUD
  • Base classes for Rest (meaning you need to spin the whole server up) and Dao (meaning you only need the database) Integration tests (WIP at the time of this writing, new to share spring properties so they aren't copy and pasted everywhere)
    • Dao only tests start about twice as fast, so nice to use them to validate operation provider behavior. They are not sufficient to validate that an operation provider is registered on a server
  • Reworked/streamlined test case loading
  • Code demonstrating how to check preconditions on public apis
  • Usage of the most specific assertion type for tests
// Yes
assertEquals(thisThing, thatThing)

// No
assertTrue(this == that)
  • Fixes for incorrect assertion ordering

Still to do:

  • Tests and docs for new code
  • Fixing broken tests

import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;

public class DaoRegistryOperationProvider implements OperationProvider, DaoRegistryUser, FhirContextUser {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a new class that provides CRUD helpers out of the box.


public static <CanonicalType extends IPrimitiveType<String>> String getId(CanonicalType theCanonicalType) {
checkNotNull(theCanonicalType);
checkArgument(theCanonicalType.getValue() != null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are examples of validating inputs to public APIs. Validate inputs as the very first step in a function.

/**
* Simulate FhirDal operations until such time as that is fully baked
*/
public interface DaoRegistryUser {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're working through an implementation of a common API for Java FHIR access. This interface approximates that so it's easier to get convergence when that finally happens.


public interface ResourceCreator extends FhirContextUser {
@SuppressWarnings("unchecked")
default <T extends IBaseResource, I extends IIdType> T newResource(I theId) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a helper that lets you create a new resource of the correct type and version based only on the id.

private Searches() {
}

public static SearchParameterMap all() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Find all resources (when combined with the Dao API above)

search(Measure.class, Searches.all())

import ca.uhn.fhir.rest.param.StringParam;
import ca.uhn.fhir.rest.param.UriParam;

public class Searches {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a set of utilities to construct commonly needed search parameter maps.

for (IBaseResource baseBundle : bundles) {
if (baseBundle instanceof Bundle) {
Bundle bundle = (Bundle) baseBundle;
List<Bundle> bundles = search(Bundle.class, Searches.all()).getAllResourcesTyped();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line combines the Search API with the new TypedBundleProvider. Compare to the previous code.

throw new RuntimeException("Could not find Group/" + subjectGroupRef);
}
Group group = (Group) baseGroup;
Group group = read(new IdType(subjectGroupRef));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's another example to compare to the previous code.

@@ -149,9 +132,9 @@ private void gatherPatientsFromReport(MeasureReport report, Map<String, Referenc
logger.debug("No subject results found.");
return results;
}
IBaseResource baseList = myDaoRegistry.getResourceDao(subjectResults.getReferenceElement().getResourceType()).read(subjectResults.getReferenceElement());
IBaseResource baseList = read(subjectResults.getReferenceElement());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

And one more example to compare to the old code.

if (baseList == null) {
logger.debug(String.format("No subject results found for: %s", subjectResults.getReference()));
logger.debug("No subject results found for: {}", subjectResults.getReference());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was quite a bit of this too. The loggers accept a string format and parameters, so no need to pre-format it.

"hapi.fhir.enforce_referential_integrity_on_write=false",
})
@EnableAutoConfiguration(exclude=QuartzAutoConfiguration.class)
public class SubmitDataProviderIT extends DaoIntegrationTest implements IdCreator, ResourceCreator {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm working on collapsing all the cruft from above into the base classes. Then the body of the tests will become the focus of the code.

SubmitDataProvider mySubmitDataProvider;

@Test
public void testSubmitData() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An example of using a few of the utilities to round-trip the database in just a few lines of code.

Copy link

@rob-reynolds rob-reynolds left a comment

Choose a reason for hiding this comment

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

There's a lot here to fully digest. Will adopt patterns in future implementations.

@JPercival JPercival marked this pull request as ready for review January 26, 2022 23:48
@JPercival JPercival merged commit cda1032 into feature-plugins Jan 26, 2022
@JPercival JPercival deleted the feature-plugins-test-framework branch January 26, 2022 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants