[
https://issues.apache.org/jira/browse/OFBIZ-3748?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12863643#action_12863643
]
Bob Morley commented on OFBIZ-3748:
-----------------------------------
I think separate Factory implementations is appropriate. Ultimately, one would
not have the test objects in the production jar at all so there should not be
reference to them in the "DelegatorFactoryImpl".
I have read that thread and it does make sense. The trouble is the amount of
lengths you had to go through to solve what (at that time and at this current
point in time) is still a theoretical requirement. My approach is usually to
attempt to create a flexible solution that does its best not to hamstring
someone in the future, but does not try to solve problems that currently do not
exist. To the best of my knowledge, that probably came out of a lot of years
with agile practices. I would have put in your first solution and then if we
have a need to execute in parallel solve that problem and the raft of other
problems you would encounter. When you go to tackle the problem (test
framework execution) you may solve it with something other than parallel
execution.
Agree we are intruding on the GenericDelegator implementation; but that is
really not my intent. What I would typically do is override just the pieces I
wanted to make changes; the trouble is that the implementation of these methods
are while not monolithic, they are still rather lengthy. Would rather gut it
up a bit more to make it more modular that would allow sub-classes more degrees
of freedom. Perhaps it would be better to have the database operations moved
into their own methods -- performUpdate, performDelete, performInsert kinda
deal and then override in the test delegator and add our test behaviours?
> Remove test specific code in the GenericDelegator
> -------------------------------------------------
>
> Key: OFBIZ-3748
> URL: https://issues.apache.org/jira/browse/OFBIZ-3748
> Project: OFBiz
> Issue Type: Bug
> Components: framework
> Affects Versions: SVN trunk
> Reporter: Bob Morley
> Attachments: OFBIZ-3748_TestGenericDelegator.patch
>
>
> Adam -- This is the results from our conversation on consistent rolling back
> of unit testers. We talked about moving the logic that is in the
> GenericDelegator that is specific to testing into a sub-class. This patch is
> NOT meant to be merged at this time, I wanted to put it up for review before
> I continue down this path ...
> Here are the key pieces:
> - TestGenericDelegator - test version of a generic delegator that contains
> the ability to record the list of database operations and then
> programmatically roll them back in reverse order. This was from existing
> code in GenericDelegator.
> - TestDelegatorFactoryImpl - a new service implementation of DelegatoryFactor
> that will construct instances of TestGenericDelegator.
> Things to consider:
> - Should rollback be on the Delegator interface? I sort of field it should
> not be there; but I left it there for now with GenericDelegator reporting an
> error if it is called.
> - Since there are two implementations of the DelegatorFactory there needs to
> be a way to determine which one to use; the way I have done this in the past
> is through configuration. Usually something like ...
> service.org.ofbiz.entity.DelegatorFactory=org.ofbiz.entity.DelegatorFactoryImpl
> that could (for Ofbiz) be placed in the start.properties or test.properties
> file. However, looking at the factory unit tester it looks like each factory
> should be able to determine if it is applicable based on the incoming
> parameters. As a result (until more discussion) I have made a choice based
> on the delegator name -- I know this is clearly NOT the go forward method.
> But would like some suggestions here ... was considering a new attribute on
> the entityengine.xml delegator definition, but there should be some mechanism
> to be able to provide control over which implementation is used I would think
> ...
> - I got an inkling that "base delegator name" may not be required anymore.
> This is because I no longer create the standard delegator and then "clone" to
> a test version. I simply instantiate the proper version right up front ...
> Moreover, I let the delegator / dispatcher names be as they are (not adding a
> random alpha-numeric suffix). Not sure about this, did not research further.
> Go forward plan --
> - If there are agreement on these changes and a resolution for things to
> consider point #2 above, I would then re-code my standalone rollback base
> class for unit tests to leverage this functionality. This would ensure we
> consistently rollback regardless of executing the test directly or through
> the test infrastructure.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
