[
https://issues.apache.org/jira/browse/OFBIZ-3748?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12863559#action_12863559
]
Scott Gray commented on OFBIZ-3748:
-----------------------------------
Is it correct to have two separate Factory implementations? I thought it was
the job of the factory to decide which delegator implementation to provide?
About the "base delegator name", the question you aren't asking yourself is why
was the delegator being cloned in the first place? I could have (and
originally did) just turned on the test mode in the base delegator. Here's the
original jira for reference: https://issues.apache.org/jira/browse/OFBIZ-2259
Although I would argue that it's impossible to run multiple test suites in
parallel on the same database.
I like the approach you've taken, but let's be clear, all we're really
achieving here is a little cleaner code by moving some methods up one level.
We're still intruding on the GenericDelegator implementation by forcing it to
notify us of any changes.
> 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.
