10-17-06 12:11 AM
[ [url]http://issues.apache.org/jira/browse/JDO-432?page=comments#action_12442711[/
url] ]
Craig Russell commented on JDO-432:
-----------------------------------
Looks good; just a few comments.
1. The Assertion Description section of javadoc exceeds 80 characters in mos
t of the tests. Perhaps the description could be abstracted to the superclas
s and reduced to 80 character lines, and then the description in the subclas
s would reference the super
class and name the method, e.g. "Test null arguments for makePersistent."
2. Javadoc for checkReturn methods (both Collection and Object[ ]) is in
correct. 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) &
#123;
...
+ if ( (eElem == null && aElem != null)
+ || (aElem == null && eElem != null) )
+ return false;
3. The method makeTestInstancesPersistent should only make the instance pers
istent, and not use makePersistentAll methods, since these methods are suppo
sed to be tested with these test cases.
4. Consider putting getPM() into the makeTestInstancesPersistent method inst
ead of in localSetUp. Consider getting rid of makeTestInstancesPersistent en
tirely and putting all the logic into localSetUp, its only use.
> Test for behaviour of null passed to detachCopy(), makePersistent(), etc
> ------------------------------------------------------------------------
>
> Key: JDO-432
> URL: http://issues.apache.org/jira/browse/JDO-432
> Project: JDO
> Issue Type: Task
> Components: tck20
> Affects Versions: JDO 2 final
> Reporter: Andy Jefferson
> Assigned To: Michelle Caisse
> Priority: Minor
> Fix For: JDO 2 maintenance release 1
>
> Attachments: jdo-432-2.patch, jdo-432.patch
>
>
> The JDO2 spec adequately defines (section 12.6.0 "Null Management") behaviour whe
n null values are passed in to the PersistenceManager methods makePersistent, detach
Copy, 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.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://iss
ues.apache.org/ji...nistrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[ Post a follow-up to this message ]
|