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

BahmniPatientProfileResourceTest#updatePatient() fails when actually adding relationships #68

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Andu033
Copy link

@Andu033 Andu033 commented Apr 7, 2020

No description provided.

@Andu033
Copy link
Author

Andu033 commented Apr 7, 2020

@rbuisson

@Andu033 Andu033 changed the title issue Update Patient test fail Apr 8, 2020
@Andu033
Copy link
Author

Andu033 commented Apr 8, 2020

Update patient test fail just by adding a relationship in patient.json. The reason for failing is that PatientResource1_8 is null. Doesnt return that return from context.

@rbuisson rbuisson changed the title Update Patient test fail BahmniPatientProfileResourceTest#updatePatient() test fails when actually adding relationships Apr 9, 2020
@rbuisson rbuisson changed the title BahmniPatientProfileResourceTest#updatePatient() test fails when actually adding relationships BahmniPatientProfileResourceTest#updatePatient() fails when actually adding relationships Apr 9, 2020
@@ -63,6 +64,7 @@
@Mock
PatientResource1_8 patientResource1_8;


Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this as well.

@mks-d mks-d self-requested a review April 9, 2020 20:21
Copy link
Member

@mks-d mks-d left a comment

Choose a reason for hiding this comment

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

@Andu033 @rbuisson isn't there a ticket linked to this PR?

Copy link
Member

@mks-d mks-d left a comment

Choose a reason for hiding this comment

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

@Andu033 you will have to add something that returns a mock for PersonResource1_8 like it is done here for PatientResource1_8, something like that:

PowerMockito.when(restService.getResourceBySupportedClass(Person.class))
  .thenReturn(personResource1_8);

... and make sure that the mock does what it should for the tests to make sense.

Cc @rbuisson

@Andu033 Andu033 requested a review from rbuisson April 10, 2020 12:23
Copy link
Collaborator

@rbuisson rbuisson left a comment

Choose a reason for hiding this comment

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

Not straighftoward to make this work. In fact the tests weren't testing anything.
I have also renamed some of the variables to make it clearer.

This is now fixed but I am still unsure why the BahmniPatientProfileResource#update method is calling twice the patientProfile.setRelationships().

delegate.setRelationships(getRelationships(propertiesToUpdate, delegate.getPatient()));

https://github.com/Bahmni/bahmni-core/blob/master/bahmnicore-omod/src/main/java/org/bahmni/module/bahmnicore/web/v1_0/controller/BahmniPatientProfileResource.java#L185

@mks-d mks-d requested review from mks-d and removed request for mks-d April 15, 2020 13:37
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.

3 participants