db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kristian Waagan (JIRA)" <derby-...@db.apache.org>
Subject [jira] Commented: (DERBY-940) Add JDBC 4 Wrapper support
Date Fri, 31 Mar 2006 16:21:36 GMT
    [ http://issues.apache.org/jira/browse/DERBY-940?page=comments#action_12372694 ] 

Kristian Waagan commented on DERBY-940:
---------------------------------------

I only had a look at the test code for CallableStatementTest.java.

I have the following comments (also see the separate comment at the end):
1) Tests should be small, and you should consider splitting your single test into two. This
should be easy, as you have already marked parts of the test code "Case1" and "Case2".
2) Use descriptive test method names. Long method names are not frowned upon in JUnit :)
3) Remove double spaces in comments (nit-pick).
4) Consider moving the try-catch block for cStmt.unwrap (Case2) into the else to more clearly
indicate were the exception is expected to be thrown. 
5) Using SQLState is not encouraged, as it is not part of the public API. As I understand,
you can either hardcore the SQL states to be checked for, create you own local constants,
or use the newly created file 'functionTests/util/SQLStateConstants.java'. Also, instead of
throwing an exception, you can use assertSQLState (from BaseJDBCTestCase).

My last separate comment might need some feedback from others, so I guess you can ignore it
for now. It is about the "skip if running under DerbyNetClient" issue. What is done with the
current approach, is that the test passes even though nothing is tested. This is in general
not good. Another approach would be to only add this test to the suite when running under
embedded. Since we don't keep count of the number of tests run in JUnit, it doesn't matter
too much now, but by putting logic for skipping the test in the test method itself it can
be easily forgotten.

Here is a suggestion on how this could have been done in the suite-method, which requires
that the tests that need special handling are renamed:
TestSuite suite = new TestSuite(CallableStatementTest.class, "suitename");
if (!usingDerbyNetClient()) {
    suite.addTest("specialTestWrapperSupport1"); // Name must not start with "test"
    suite.addTest("specialTestWrapperSupport2");
}
return suite;

Any opinions on this?

> Add JDBC 4 Wrapper support
> --------------------------
>
>          Key: DERBY-940
>          URL: http://issues.apache.org/jira/browse/DERBY-940
>      Project: Derby
>         Type: New Feature
>   Components: JDBC
>     Reporter: Rick Hillegas
>     Assignee: V.Narayanan
>      Fix For: 10.2.0.0
>  Attachments: DERBY940_embedded.html, wrapper_ver1_embedded.diff, wrapper_ver1_embedded.stat,
wrapper_ver2_embedded.diff, wrapper_ver2_embedded.stat, wrapper_ver3_embedded.diff, wrapper_ver3_embedded.stat
>
> As described in the JDBC 4 spec, sections 21 and 3.1.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Mime
View raw message