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
