So we agreed that the approach we use now should be replaced, reasons can be found on the Unit Testing Conventions page - Now & The Future - New Style
we agreed on these first TODOs (which I almost completed)
- added Eclipse and IntelliJ templates.xml to openmrs-core/tools/src/main/resources for unit test snippets as a replacement for the Generate Test Case Plugin
- added a section on how to import and use the templates in the respective IDE setup wiki pages for Eclipse and IntelliJ
- Updated https://wiki.openmrs.org/display/docs/Unit+Testing+Conventions to show that a change in unit test convention is on its way and provided some background info (based on the infos posted by @darius here) (I still need to fill a proposition for “Naming Test Methods” which I want us to discuss then, please give me a bit more time , also I need to update the code examples, and resources on test driven development).
There some more items I would like to address:
@wyclif mentioned that using the generate test case plugin is convenient because you can write the sentence like a normal English sentence with whitespaces “should return an empty list given null” as opposed to just writing the method name directly as
myMethod_shouldReturnAnEmptyListGivenNull. Your also added that devs will not be used to it. But I think devs are very much used to writing camel cased variables, methods so I would not see this as a concern. But I get the convenience factor.
I therefore played a little with templates and IntelliJ has a camelCase() function which can turn a sentence as we wrote it before with whitespaces into what we want the method name to be. But and this defeats the purpose: this only works (or at least I found no other way) when you add a variable for example in a javadoc which contains the sentence which you take as an input to the template function
camelCase(MY_VARIABLE) and put that into another variable which feeds the method name. Also this does not work in Eclipse.
So I would not go this route of trying to replace the generate test case plugin. I would drop advocating the plugin, I dont think anyone maintains it anyway. I think we should agree on a common language (I write a proposal in “Naming Test Methods” in the wiki, tomorrow), use code templates that generate the boilerplate and just fill in the method name. Our devs will get used to it and I think other devs are already used to this.
Maybe @lluismf or @raff can also chime in on this, if I am wrong on how its done out there on other projects.
Other objections, thoughts?
We discussed that once we agreed on the templates, naming conventions and updated the wikis we would point devs to this new convention and only accept new PRs using this style.
Does anyone have an objection to this?
Would it make sense to only do this for the master branch?
We also discussed that it might be good to make a PR which removes all @should’s and @verifies from the javadocs.
This is important because the code serves as an example and currently we have two styles and after the change we would have 3 styles if we dont get rid of the old ones. It happens all the time that new devs use the old style. I also fell into this trap and changed all the tests in radiology to the old style when I started
I therefore strongly hope that once we agree and finished all above TODOs we can create a PR which cleans the openmrs-core master branch.
Other branches as well or just master?
That was a lot,
Thanks for reading!
I ask you to please try to import the templates and use them to see how it goes!!! We can of course tweak them!! I just need your feedback and if someone else has a better template, come forward and lets use it
I’ll continue working on the wiki and am waiting for your feedback.