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

TRUNK-4166: added another unit test and @should annotation in BaseAttrib... #832

Closed
wants to merge 1 commit into from

Conversation

lbat
Copy link
Contributor

@lbat lbat commented Mar 28, 2014

@dkayiwa
Copy link
Member

dkayiwa commented Mar 28, 2014

@lbat i can reopen this when you are done with the above comments.

@dkayiwa dkayiwa closed this Mar 28, 2014
@lbat
Copy link
Contributor Author

lbat commented Mar 28, 2014

hi daniel,
thanks for your feedback!
I adjusted my code according to your suggestions and used the unit test tool. Unfortunately I couldn't find the comment regarding the "@should", but I hope everything is okay now.
Can you reopen the pull request?

test method name:
initially had a more meaningful name :). Then I assumed that there is a naming convention, because all the tests are named like the @should annotation [1]. I thought that the new testcase belongs to the "@should require DatatypeConfiguration..." and therefore named the method ...2(). I was aware that this name is far from ideal and added a description in the method comment.

[1] https://wiki.openmrs.org/display/docs/Unit+Testing+With+at-should+Annotation

@dkayiwa
Copy link
Member

dkayiwa commented Mar 29, 2014

@lbat can u comment on the ticket and change its status?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants