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
