[jira] Commented: (DERBY-3587) Convert jdbcapi/testRelative.java to JUnit

["Kathey Marsden (JIRA)" <ji...@apache.org>]

    [ 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.