[ http://issues.apache.org/jira/browse/DERBY-2025?page=comments#action_12453601 ] Knut Anders Hatlen commented on DERBY-2025: -------------------------------------------
The patch looks good. Some small comments: - I believe testResultSetPositionedBeforeNextAfterDeleteRow() is missing fail() after rs.getString(1) - testUpdateXXXNotForUpdateColumns() is missing fail() after rs.updateInt() - in testInsertRowReadOnlyRS(), rs.updateInt(2, 5000) is unreachable because fail() is called on the line before - testDeleteRowWithSetCursorName(), testUpdateRowWithSetCursorName(), runTestUpdateXXXWithAllDatatypes(), runTestUpdateObjectWithAllDatatypes() and verifyData() have asserts or calls to fail() which cause errors when an unexpected exception is called. Wouldn't it be better to re-throw the exceptions in order to preserve the stack traces? - testUpdateXXXWithAllDatatypes() and testUpdateObjectWithAllDatatypes() have finally clauses which clean up the database. If an error happens in the test case, an error in the finally clause might hide the actual error. - tearDown() could be removed since it doesn't do anything super.tearDown() wouldn't do - Would it be better if the suite were constructed from a baseSuite() method which returned all test cases with no decorator? suite() could return a test suite with baseSuite() + clientServerDecorator(baseSuite()). Then we wouldn't have to start/stop the network server more than once. - typo in class javadoc: resutlset - typo in comment in suite(): XtestInsertRowAftrerCommit > convert lang/updatableResultSet.java to Junit > --------------------------------------------- > > Key: DERBY-2025 > URL: http://issues.apache.org/jira/browse/DERBY-2025 > Project: Derby > Issue Type: Sub-task > Reporter: Fernanda Pizzorno > Assigned To: Fernanda Pizzorno > Attachments: derby-2025v1.diff, derby-2025v1.stat > > -- 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
