[
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
