[ 
https://issues.apache.org/jira/browse/DERBY-3587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12586859#action_12586859
 ] 

Kathey Marsden commented on DERBY-3587:
---------------------------------------

Thanks for the patch. Below are some comments:

- remove the old test from the suite and remove associated test and master 
files.
- In RelativeTest, keep connections  local to the fixtures  instead of making  
a field.
- Instead of creating your table in setup, use CleanDatabaseSetup and 
decorateSQL, (see BatchUpdateTest for an example).  This way it will get 
automatically cleaned up.
- Instead of con.prepareStatement() and con.createStatement()  you can use just 
prepareStatement  createStatement() and statements  will get closed 
automatically.

In the following code, only the call that is expected to fail should be in the 
try block
        try {
                        rs.relative(10);
                        /*
                         * Attempting to move beyond the first/last row in the 
                         * result set positions the cursor before/after the the 
                         * first/last row. Therefore, attempting to get value 
now
                         * will throw an exception.
                         */
                        rs.getString("name");
                } catch (SQLException sqle) {
                        assertSQLState("Unexpected SQL state.", 
NO_CURRENT_ROW_SQL_STATE, sqle);
                }
-  I wonder if using 
        assertSQLState("Unexpected SQL state.", NO_CURRENT_ROW_SQL_STATE, sqle);

instead of 
        assertSQLState(NO_CURRENT_ROW_SQL_STATE, sqle);

will actually give us less information on failure, because I think with the 
latter format we will see what the actual SQLException is and with the former 
we will not.

I am not sure, but I think with some JVM's the test order may vary, so 
initializing NO_CURRENT_ROW_SQL_STATE = "24000"; and then changing it on setUp 
if we are running with derbyclient might be a problem if the network server 
tests go first. It might be clearer just to put the condition in the catch 
block or make an assertNoCurrrentRow() method that has the condition.

Otherwise looks good.

Thanks

Kathey


> Convert jdbcapi/testRelative.java to JUnit
> ------------------------------------------
>
>                 Key: DERBY-3587
>                 URL: https://issues.apache.org/jira/browse/DERBY-3587
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>            Reporter: Suran Jayathilaka
>            Assignee: Suran Jayathilaka
>            Priority: Minor
>         Attachments: jdbcapi-testRelative-converted.diff
>
>
> Convert jdbcapi/testRelative.java from the old testHarness to JUnit.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to