[ http://issues.apache.org/jira/browse/JDO-432?page=comments#action_12442711 ] Craig Russell commented on JDO-432: -----------------------------------
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. > 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 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. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira