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

Restws 674 - Include More Test cases for ModuleController1_8 in REST Module #292

Merged
merged 1 commit into from
Aug 16, 2017

Conversation

suthagar23
Copy link
Member

Included test cases for Full, Default, and Ref representations.

@mention-bot
Copy link

@suthagar23, thanks for your PR! By analyzing the history of the files in this pull request, we identified @pgutkowski, @teleivo and @AdamGrzybkowski to be potential reviewers.

}

@Test
public void shouldTestDefaultRepresentation() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

shouldTest is not a good name. Can you think of an alternative?

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay I will change it

@coveralls
Copy link

Coverage Status

Coverage remained the same at 46.181% when pulling 3bbf96b on suthagar23:RESTWS-674 into 7a39d69 on openmrs:master.

@suthagar23
Copy link
Member Author

@dkayiwa changed to ShouldGetA() format

@coveralls
Copy link

Coverage Status

Coverage remained the same at 46.181% when pulling e26e3d8 on suthagar23:RESTWS-674 into 7a39d69 on openmrs:master.


@Test
public void shouldGetADefaultRepresentation() throws Exception {
MockHttpServletRequest req = request(RequestMethod.GET, getURI() + "/" + getUuid());
Copy link
Member

@dkayiwa dkayiwa Aug 9, 2017

Choose a reason for hiding this comment

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

The name does not still reflect what is being tested. For instance, if the default representation was returned without a uuid or any other expected property, would this test name reflect that?

@dkayiwa
Copy link
Member

dkayiwa commented Aug 14, 2017

@suthagar23 ping

@suthagar23
Copy link
Member Author

@dkayiwa Okay, I got your point right now
I will modify the test part and sorry for the delay, I missed it :-(

@coveralls
Copy link

Coverage Status

Coverage remained the same at 46.181% when pulling cb767c2 on suthagar23:RESTWS-674 into 7a39d69 on openmrs:master.

@dkayiwa
Copy link
Member

dkayiwa commented Aug 15, 2017

@suthagar23 the test was fine. My comment was simply for you to change the test name to reflect what is being tested.

Added some test cases for moduleController1_8

Added some test cases for moduleController1_8

Included test cases for Full, Default, and Ref representations.

Added some test cases for moduleController1_8
@suthagar23
Copy link
Member Author

@dkayiwa Is it good for the name?

@dkayiwa dkayiwa merged commit e12f8c7 into openmrs:master Aug 16, 2017
@dkayiwa
Copy link
Member

dkayiwa commented Aug 16, 2017

@suthagar23 i think you already know my response to your question. 😊

@suthagar23
Copy link
Member Author

@dkayiwa absolutely 😊

@coveralls
Copy link

Coverage Status

Coverage remained the same at 46.181% when pulling 1635a26 on suthagar23:RESTWS-674 into 7a39d69 on openmrs:master.

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.

4 participants