Hi Michael, I don't have a preference between the two mocking libraries, but creating unit tests for MacroFormRenderer necessitates mocking static methods. Mockito doesn't offer static method mocking, but JMockit does.
There are work arounds, but I think will be too cumbersome and brittle to be effective. Quoting myself [1]: The second PR extends the first PR and introduces unit tests. The potentially controversial issue is that to implement the tests I felt it was necessary to mock out static methods, functionality that's not available from Mockito, and therefore brought in a test dependency on JMockit. To provide unit tests for MacroFormRenderer without using a mocking library like JMockit will require the class to be refactored which I deemed a bit of a risky change at this point, particularly without existing tests to prove correct behaviour is maintained. Choosing JMockit here was about using the right tool for the job. As far as I know other mocking libraries, such as PowerMock, offer static mocking too but my experience is with JMockit. [1] https://issues.apache.org/jira/browse/OFBIZ-4035?focusedCommentId=17042669&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17042669 Thanks, Dan. On Mon, 2 Mar 2020 at 13:02, Michael Brohl <michael.br...@ecomify.de> wrote: > Hi Dan, > > why would you prefer JMockit over Mockito? > > Thanks, > > Michael Brohl > > ecomify GmbH - www.ecomify.de > > > Am 02.03.20 um 11:23 schrieb Daniel Watford: > > Hello, > > > > There are two outstanding PRs on OFBIZ-4035. > > > > https://github.com/apache/ofbiz-framework/pull/26 should be considered > low > > risk as it completes a minor missing piece of functionality for > generating > > the DOM element id for container fields created in form widgets. > > > > https://github.com/apache/ofbiz-framework/pull/27 introduces JMockit in > > order to unit test MacroFormRenderer which I mentioned in my first email > in > > this thread. I'd like to use JMockit for these hard-to-test cases where > > code design doesn't lend well to use of libraries like Mockito. > > > > We could consider use of JMockit as an indicator that a class might > benefit > > from some architectural refactoring, but at least we'd have some tests in > > place to help us ensure behaviour is not broken. > > > > Thanks, > > > > Dan. > > > > On Sun, 23 Feb 2020 at 18:54, Daniel Watford <d...@foomoo.co.uk> wrote: > > > >> Hello, > >> > >> I've replaced the original PRs with a few more focused ones: > >> > >> https://github.com/apache/ofbiz-framework/pull/24 > >> https://github.com/apache/ofbiz-framework/pull/25 > >> https://github.com/apache/ofbiz-framework/pull/26 > >> > >> Hopefully the above are considered low risk and/or minor improvements > and > >> can be committed. > >> > >> The 4th PR: > >> https://github.com/apache/ofbiz-framework/pull/27 > >> covers the introduction of JMockit needed to test the MacroFormRenderer > as > >> described in my previous email. > >> > >> Thanks, > >> > >> Dan. > >> > >> On Sat, 22 Feb 2020 at 18:32, Daniel Watford <d...@foomoo.co.uk> wrote: > >> > >>> Hello, > >>> > >>> Two PRs have been created to address this ticket - please see comments > in > >>> jira. > >>> > >>> The first PR is mostly a documentation change as it turned out the > >>> required functionality already existed. > >>> > >>> The second PR introduces some unit testing, but to be able to test > >>> MacroFormRenderer without an explosion of mockito mocks, and lots of > >>> careful setup in the environment, which would lead to brittle tests, I > felt > >>> it appropriate to introduce JMockit. > >>> > >>> How does the development community feel about the trade-off of > >>> introducing another test library versus the ability to create tests > without > >>> a massive amount of, in my opinion, brittle code. > >>> > >>> Possibly the preferred approach would have been to apply some > refactoring > >>> to MacroFormRender to allow easier testing. Specifically MFR could be > split > >>> into two or more classes to: > >>> - convert from the field type to the rendered FTL macro string > >>> - execute a rendered FTL macro string > >>> > >>> The refactoring is probably preferable in the long run, but felt too > >>> large a task for this particular ticket. > >>> > >>> What do you think? Can we permit the JMockit tests until some > refactoring > >>> is carried out? > >>> > >>> Thanks, > >>> > >>> Dan. > >>> > >>> -- > >>> Daniel Watford > >>> > >> > >> -- > >> Daniel Watford > >> > > > > -- Daniel Watford
