RAD-382 Create RadiologyReportServiceTest

[manuela-grindei]

Description

Created unit tests for RadiologyReportServiceTest and removed unnecessary integration tests.

Related Issue

see https://issues.openmrs.org/browse/RAD-382

Checklist:

• My pull request only contains one single commit.
• My pull request is based on the latest master branch
  git pull --rebase upstream master.
• I ran mvn clean package right before creating this pull request and
  added all formatting changes to my commit.
• My code follows the code style of this project.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[mention-bot]

@manuela-grindei, thanks for your PR! By analyzing the history of the files in this pull request, we identified @teleivo, @durac and @bgeVam to be potential reviewers.

[coveralls]

Coverage decreased (-0.1%) to 94.079% when pulling 08560e4 on manuela-grindei:master into 2d98c5a on openmrs:master.

[dkayiwa]

@teleivo do you think this is fit to merge?

[dkayiwa]

@ivange94 do you think you can review this? :)

[ivange94]

@manuela-grindei looks like you missed this https://github.com/openmrs/openmrs-module-radiology/blob/master/api/src/test/java/org/openmrs/module/radiology/report/RadiologyReportServiceComponentTest.java#L478-L498.

Aside from that, @dkayiwa this is good to go.

[ivange94]

That was suppose to be a comment instead of suggestion of changes. :D

[manuela-grindei]

@ivange94 Hello, thank you for reviewing this!
I skipped that one on purpose, as the class under test is not doing actual validation, but calling the validator. In these conditions, I thought it would be best to leave it as an integration. I could add a unit test using doThrow(Exception), but I'm not sure it would be that valuable.

[ivange94]

@manuela-grindei yes you are right. Just forget about my previous comment(I deleted it). Just stepped through the code and the saveRadiologyReport does not actually do the validation. I think this is a testing behavior vs testing implementation problem. To my knowledge we test for behavior, thats why we use mocks and in this case what that test is doing is testing how that method will behave when an invalid report is passed. Its not testing who is actually doing the validation(which I think is testing the implementation). So I still think you should extract that test method out of the integration test. @dkayiwa what do you think?

[dkayiwa]

@teleivo now that you are back, do you think we can merge this? :)

[dkayiwa]

@teleivo should i help you merge this? :)

[teleivo]

@manuela-grindei this is not needed. was there a reason you added it?

you did not in
https://github.com/openmrs/openmrs-module-radiology/blob/master/api/src/test/java/org/openmrs/module/radiology/order/RadiologyOrderServiceTest.java#L25

[manuela-grindei]

Good point, removed it!

[coveralls]

Coverage decreased (-0.1%) to 94.079% when pulling 8665885 on manuela-grindei:master into da4c9d2 on openmrs:master.

[dkayiwa]

@teleivo what do you think of this? 😊

[teleivo]

@manuela-grindei thank you very much and sorry for taking so long! 😉 @dkayiwa