Company model factory improvements to test persistent interfaces

• Key: JDO-377
• Type: Improvement
• Status: Open
• Priority: Minor
• Assignee: Craig Russell
• Reporter: Craig Russell
• Component/s: tck2
• Affects Version/s: JDO 2 rc1

Description

Please review this patch.

In order to test persistent interfaces better, a method was added to the CompanyFactory interface to allow the test program to get the persistent instances.

List getAllInstances();

To implement this new API, changes were made to the factory implementation classes to keep track of instances as they were created.

Additionally, only classes (or interfaces) actually instantiated were put into the tearDown class array. So Person.class and Employee.class were removed from the array.

Finally, a bug was fixed in CompanyFactoryPMClass where the interface IMedicalInsurance.class was incorrectly added to the tearDown class array.

A new test class was used for testing. This class might be added to the TCK if suitable.

And a new configuration was used for testing.

Comments

[Craig Russell]

Please review these changes.

In addition to the changes mentioned, a bug was fixed in CompletenessTest where the tearDown classes are obtained always from the default reader, not the reader used for persistent instances.

[Michael Bouschen]

The patch looks good! Please note, the CompletenessTest changes and the tearDownList modifications are already checked in as as part of JDO-375.

About the new test class LifecycleCompleteness.java:
I vote for adding this new new test class. Maybe we can add three new configurations: one for each CompanyFactory implementation (CompanyFactoryPMInterface, CompanyFactoryPMClass, CompanyFactoryConcreteClass). Please remove the last method in comments, it seems to be outdated.

What do the others think?

[Michael Bouschen]

Attached you find a new patch JDO-377-mbo.patch fixing a bug in the LifecycleCompletetness class and adding three new configurations one for each CompanyFactory implementation.

There is one configuration failing when running with the latest JPOX version (I'm using the jars attached to JDO-375): lifecyclecompletenessPMInterface.conf with applicationidentity. Method verifyOids checks that the target class of a SingleFieldIdentity oid is the class passed to the pm.newInstance call (see last paragraph of 12.6.6. on page 116 of the spec). In this configuration pm.newInstance is called with a PC interface, but the target class is the implementing class that was generatd by JPOX.

Thanks for continuing to improve the test.

I think that the new test is valuable but needs a bit more work to make sure the comments are clean and the structure is correct. In particular, the list of allInstances only includes instances that have a managed extent (excludes Address). This inappropriately ties the factory to the usage in the configurations (e.g. another configuration might have the Address require an extent, or might instantiate a Person).

The LifecycleCompleteness has an awkward name and there is only one JUnit-visible test method with several sub-tests. The sub-test pattern is useful but we need to rationalize the naming convention for sub-tests. This was done to avoid setup and teardown of the persistent instances but the ramification is that the individual tests cannot be excluded if there is some issue. Perhaps we can modify the exclude list to include not only class name but test name as well.

So I think we should continue to work on this after the release of JDO 2.

[Erik Bengtson]

if you dont plan to commit it for JDO 2, we, JPOX, will give lower priority to the issue. We can work on it for the maintenance release 1

[Craig Russell]

I won't commit this for the final JDO 2.0 tck release. I'm sure JPOX have their hands full making the official 1.1.0 release.