[
https://issues.apache.org/jira/browse/DERBY-2293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12470986
]
Daniel John Debrunner commented on DERBY-2293:
----------------------------------------------
Looks good in general, some potential cleanup, I think these could all be
applied once this patch is committed.
TestConfiguration.defaultSuite already adds CleanDatabaseTestSetup so there's
no need to add another one.
For the methods that take a connection, such as
runStatementNonBatchStuffInBatch and only ever use the default connection it
might be better to not have them pass a connection. They can use the utility
method commit() or getConnection() if they need access to the connection. The
code then becomes like the general style of other tests and it becomes obvious
to the reader that the standard default connection is being used. Then of
course the run methods start to look a lot like individual test fixtures, which
they really are, so it might make sense to convert them them all the way to
testXXX methods.
There is no need to have a assertBatchExecuteError() method that takes a
PreparedStatement. Since a PreparedStatement is a Statement and the
executeBatch() method is defined on Statement, the code can use the
assertBatchExecuteError that takes a Statement.
In cases where you need to have a try/catch block like
runTransactionErrorBatch() I think it is clearer and better if the try catch
block only encloses the statement you expect to fail. So I think in this case
it would be just the statement
updateCount = stmt2.executeBatch();
With the code as it is, if an earlier statement in the block incorrectly fails
with a timeout then there is a chance the test will pass even though it should
fail.
As an alternative to:
} catch (SQLException sqle) {
/* Ensure the exception is time out while getting lock */
assertSQLState("40XL1",sqle);
assertTrue("we should get a BatchUpdateException",
(sqle instanceof BatchUpdateException));
one could just catch BatchUpdateException
} catch (BatchUpdateException sqle) {
/* Ensure the exception is time out while getting lock */
assertSQLState("40XL1",sqle);
Any non-batch exception will continue to fail the test.
> convert batchUpdate.java to junit
> ---------------------------------
>
> Key: DERBY-2293
> URL: https://issues.apache.org/jira/browse/DERBY-2293
> Project: Derby
> Issue Type: Improvement
> Components: Test
> Reporter: Myrna van Lunteren
> Assigned To: Myrna van Lunteren
> Priority: Minor
> Attachments: DERBY-2293_20070205.diff, DERBY-2293_20070205.stat,
> DERBY-2293_20070206.diff, DERBY-2293_20070206.stat, DERBY-2293_20070206_2.diff
>
>
> Convert the test jdbcapi.batchUpdate.java to junit framework
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
