[
https://issues.apache.org/jira/browse/DERBY-2293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12470489
]
Kristian Waagan commented on DERBY-2293:
----------------------------------------
Wow, this test was big! Good work on converting it :)
Regarding your worry about the try/catch and no exception, maybe you can use
fail("Some informative message")?
This way you don't have to set a flag and check it later.
Personally, I think using 'verifyBatchUpdateCounts' looks better. Maybe you
could even rename it to something like 'assertBatchUpdateCounts' to clearly
signal the intention. If needed, it could easily be moved into a super/utility
class.
Comments/questions to BatchUpdateTest.java:
a) Missing header/licence.
b) testStatementBatchUpdateNegative: the two calls to getConnection will return
the same connection. Is this intended?
I have a few nits as well:
1) There are a few tabs in the diff.
2) Typo in the class JavaDoc: fo -> of
3) Lines longer than 80 characters.
4) resulti -> result in verifyBatchUpdateCounts
5) Out of curiosity (first catch in runStatementWithResultSetBatch):
if (updateCount != null) {
assertEquals(
"Select is the first statement in the batch, so there
should not be any update count",
0, updateCount.length);
}
Can we assert null/not null here instead, or does this vary with the
exception?
If this test had been written from scratch, I would have split it up into more
independent fixtures/test methods. Since it has been converted, and because of
its size, I think the current approach is okay.
The point of splitting it up, is to better isolate the thing that is failing
(and to see that other things are not failing).
The class is big, and I had to stop reviewing somewhere in the middle. There is
more to be reviewed for the willing ;)
With 'DERBY-2293_20070205.diff ' I get this result:
Tests run: 10, Failures: 4, Errors: 1
> 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
>
>
> 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.
