Issue Details (XML | Word | Printable)

Key: JDO-432
Type: Task Task
Status: Resolved Resolved
Resolution: Fixed
Priority: Minor Minor
Assignee: Michelle Caisse
Reporter: Andy Jefferson
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
JDO

Test for behaviour of null passed to detachCopy(), makePersistent(), etc

Created: 04/Oct/06 06:34 PM   Updated: 18/Oct/06 08:17 AM
Component/s: tck2
Affects Version/s: JDO 2 final
Fix Version/s: JDO 2 maintenance release 1

Time Tracking:
Not Specified

File Attachments:
  Size
Text File Licensed for inclusion in ASF works jdo-432-2.patch 2006-10-16 07:34 PM Michelle Caisse 61 kB
Text File Licensed for inclusion in ASF works jdo-432.patch 2006-10-09 05:39 PM Michelle Caisse 25 kB

Resolution Date: 18/Oct/06 03:58 AM


 Description  « Hide
The JDO2 spec adequately defines (section 12.6.0 "Null Management") behaviour when null values are passed in to the PersistenceManager methods makePersistent, detachCopy, etc but there doesnt seem to be a test for these situations currently. JPOX 1.1.2 on calling detachCopy(null) actually throws an NPE, which is outside of spec ... now fixed.

 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Michelle Caisse added a comment - 09/Oct/06 05:39 PM
The attached patch supplies code for the first of a set of tests. This one tests makePersistent() and makePersistentAll(). It creates a new package org.apache.jdo.tck.api.persistencemanager.nullargs, a new superclass for null argument tests PersistenceManagerNullsTest.java and a new test class MakePersistentNullArgs.java. Please review and comment.

The RI fails on three of the five test methods by throwing an NPE rather than doing nothing with a null pc instance or a null element of an array or Collection of pc instances.

Michelle Caisse added a comment - 16/Oct/06 07:34 PM
This patch is complete for this issue. It contains a new superclass and a test class for each PersistanceManager API family under test, in a new package. I updated pm.conf to invoke these tests. Please review.

There are three failures in the 50 new test cases:

    [java] 1) testDetachCopyCollectionNullElement(org.apache.jdo.tck.api.persistencemanager.nullargs.DetachCopyNullArgs)junit.framework.AssertionFailedError: Assertion A12.6-5 failed:
    [java] detachCopyAll(Collection) on a null Collection element should do nothing. Instead we get: org.jpox.exceptions.ClassNotDetachableException: The class "org.apache.jdo.tck.pc.mylib.PCPoint" is
 not Detachable. This means that the MetaData for the class did not have the "detachable" attribute set to true.
    [java] at org.apache.jdo.tck.JDO_Test.fail(JDO_Test.java:638)
...
    [java] 2) testDetachCopyArrayNullElement(org.apache.jdo.tck.api.persistencemanager.nullargs.DetachCopyNullArgs)junit.framework.AssertionFailedError: Assertion A12.6-5 failed:
    [java] detachCopyAll(Array) on a null array element should do nothing. Instead we get: org.jpox.exceptions.ClassNotDetachableException: The class "org.apache.jdo.tck.pc.mylib.PCPoint" is not Detac
hable. This means that the MetaData for the class did not have the "detachable" attribute set to true.
...
    [java] 3) testMakeNontransactionalNullObject(org.apache.jdo.tck.api.persistencemanager.nullargs.MakeNontransactionalNullArgs)junit.framework.AssertionFailedError: Assertion A12.6-3 failed:
    [java] makeNontransactional(null)on a null object should do nothing. Instead we get: java.lang.NullPointerException
    [java] at org.apache.jdo.tck.JDO_Test.fail(JDO_Test.java:638)
    [java] at org.apache.jdo.tck.api.persistencemanager.nullargs.PersistenceManagerNullsTest.executeNullObjectParameter(PersistenceManagerNullsTest.java:222)
    [java] at org.apache.jdo.tck.api.persistencemanager.nullargs.MakeNontransactionalNullArgs.testMakeNontransactionalNullObject(MakeNontransactionalNullArgs.java:69)
    [java] at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    [java] at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    [java] at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    [java] at org.apache.jdo.tck.JDO_Test.runBare(JDO_Test.java:253)
    [java] at org.apache.jdo.tck.util.BatchTestRunner.doRun(BatchTestRunner.java:107)
    [java] at org.apache.jdo.tck.util.BatchTestRunner.start(BatchTestRunner.java:147)
    [java] at org.apache.jdo.tck.util.BatchTestRunner.main(BatchTestRunner.java:122)

Craig Russell added a comment - 16/Oct/06 09:34 PM
Looks good; just a few comments.

1. The Assertion Description section of javadoc exceeds 80 characters in most of the tests. Perhaps the description could be abstracted to the superclass and reduced to 80 character lines, and then the description in the subclass would reference the superclass and name the method, e.g. "Test null arguments for makePersistent."

2. Javadoc for checkReturn methods (both Collection and Object[ ]) is incorrect. It only checks that corresponding parameters are either null or not null.

+ /**
+ * Checks if expected and actual arguments are equal
+ * @return true if arguments are equal
+ * @param expected Collection
+ * @param actual Collection
+ */
+ protected boolean checkReturn(Collection expected, Collection actual) {
...
+ if ( (eElem == null && aElem != null)
+ || (aElem == null && eElem != null) )
+ return false;

3. The method makeTestInstancesPersistent should only make the instance persistent, and not use makePersistentAll methods, since these methods are supposed to be tested with these test cases.

4. Consider putting getPM() into the makeTestInstancesPersistent method instead of in localSetUp. Consider getting rid of makeTestInstancesPersistent entirely and putting all the logic into localSetUp, its only use.


Michelle Caisse added a comment - 17/Oct/06 11:23 PM
Re preceding comments:

1. The Assertion Description in all the TCK tests is an exact copy of the assertion text and generally exceeds 80 characters.

2., 3., 4. Done.

Thanks!

Michelle Caisse added a comment - 18/Oct/06 03:58 AM
Completed: At revision: 465134

Andy Jefferson added a comment - 18/Oct/06 08:17 AM
The 2 remaining issues are due to JPOX implementing part of an earlier version of the JDO2 spec. Some time ago it used to say that calling detachCopyXXX() on an object that was of a non-Detachable class would throw JDOUserException. JDO2-final says that it should return a transient and previously there was seemingly no TCK test for that condition until this test for nulls that actually also tests this too.

I'll leave it to you to decide if you want to just pass in a single "null" element so the test just tests the null part of the spec, and add a new test for the return of a transient when passing in a non-Detachable object.

Anyway, its monitorable via http://www.jpox.org/servlet/jira/browse/CORE-3037