db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kristian Waagan (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-2293) convert batchUpdate.java to junit
Date Tue, 06 Feb 2007 09:06:05 GMT

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


Mime
View raw message