Skip to content
This repository was archived by the owner on Mar 25, 2022. It is now read-only.

FM-172. Added Tests for radiology handler #203

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

haripriya999
Copy link

FM-172: Added Tests for Radiology Handler

Description of what I changed

Added tests in RadiologyHandlerTest.java

Issue I worked on

see https://issues.openmrs.org/browse/FM-172

Checklist: I completed these to help reviewers :)

  • My pull request only contains ONE single commit
    (the number above, next to the 'Commits' tab is 1).

    No? -> read here on how to squash multiple commits into one

  • My IDE is configured to follow the code style of this project.

    No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend

  • I have added tests to cover my changes. (If you refactored
    existing code that was well tested you do not have to add tests)

    No? -> write tests and add them to this commit git add . && git commit --amend

  • I ran mvn clean package right before creating this pull request and
    added all formatting changes to my commit.

    No? -> execute above command

  • All new and existing tests passed.

    No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.

  • My pull request is based on the latest changes of the master branch.

    No? Unsure? -> execute command git pull --rebase upstream master

assertNotNull(radiologyHandler.getFHIRDiagnosticReportBySubjectName(name));
}
catch(Exception e)
{
Copy link
Member

Choose a reason for hiding this comment

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

@haripriya999 , thank you very much for the great work. Quick question, why would you need to catch an Exception ?

Copy link
Author

Choose a reason for hiding this comment

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

@Ruhanga Thank you for reviewing my code :) . There seems to be some problem with the variable, serverBase in RadiologyHandler, beacause of which a null pointer exception was being throw. I understood the root cause of the problem but I'm unsure of what the variable value should be set to. Hope this Talk would provide further details- https://talk.openmrs.org/t/error-variable-serverbase-in-radiologyhandler-is-returning-null/22511 . Also, I think we can create a new issue for this.

catch(Exception e)
{
e.printStackTrace();
}
Copy link
Member

Choose a reason for hiding this comment

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

Also can you consider https://wiki.openmrs.org/display/docs/Java+Conventions#JavaConventions-loggingLogging in case you want to printout anything though it would be unnecessary for this use case.

<artifactId>openmrs-api</artifactId>
<version>${openMRSVersion}</version>
<type>test-jar</type>
<scope>test</scope>
Copy link
Member

Choose a reason for hiding this comment

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

For what use-case is this change made?

Copy link
Author

Choose a reason for hiding this comment

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants