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)
"Select is the first statement in the batch, so there should not be any update count",
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
DERBY-2293_20070205.diff ' I get this result:
Tests run: 10, Failures: 4, Errors: 1