RFR 8040760: Addition of new java.sql tests

Lance Andersen lance.andersen at oracle.com
Fri Apr 18 20:12:57 UTC 2014


Hi Roger,

Thanks for the suggestions
On Apr 17, 2014, at 9:44 AM, Roger Riggs <Roger.Riggs at oracle.com> wrote:

> Hi Lance,
> 
> - The empty method for @BeforeClass, @AfterClass, @BeforeMethod, @AfterMethod
>   could be deleted (and related imports)
> - Ditto the empty public constructors

Agree.  They are generated automagically via netbeans and I figured seeing they cause no harm, I will leave them as is as I might end up using some of them later with additional tests

> - The javadoc first sentences should end with a "."  (Though we rarely generate javadoc for tests).

Agree, I will look at a future date to go through all of the tests to update date these.  I just tried to add some comments to give an idea as to what the tests do
> - Binary files (BatchUpdateException_JDBC42.ser) are rarely a good idea.
>  Typically the bytes  are dumped to a source byte array and embedded in the test.

There is only 1 test which required a checked in  Serialized file.  Per your suggestion, I created a byte array instead
> - Some tests (like BatchUpdateException) would be easier to read if there was method for 'equals(ex1, ex2)'.

I can look at this at some point in the future to see if there might be some additional value.  Each Constructor had different defaults for various parameters so not sure it is worth more time now but made note of it.  Have a lot of other tests to go through
and port
> - Only BatchUpdateException has a serialization test for compatibility with previous versions.

Correct and the reason for that is in Java SE 8,  another field was  added to BatchUpdateException and I wanted to make sure that we were backwards compatible.

The other exceptions did not change and there was no existing test.  I can add this as a todo for the future, but it was not as important at this time given I was converting my own existing unit tests

>  (the round trip write/read tests only test the current version).
> - StubDriverDA logs an error to the Logger instead of causing a test failure - is that correct?
This is harmless as the stub will new throw the Exception that DriverManager requires (nor will DriverManager), so I just left the code that Netbeans generated
> 
> - BatchUpdateExceptionTests + 320 check indentation
fixed
> - DriverManagerTests  + 291 "to to" -> "to"
fixed
> - StubConnection   +44 add a space before "{"
fixed

Revised webrev at http://cr.openjdk.java.net/~lancea/8040760/webrev.01/

Best
Lance
> 
> Roger
> 
> 
> On 04/16/2014 05:41 PM, Lance Andersen wrote:
>> Hi,
>> 
>> Looking for a reviewer for some new java.sql tests.
>> 
>> The webrev can be found at http://cr.openjdk.java.net/~lancea/8040760/webrev.00/
>> 
>> Best,
>> Lance
>> 
>> 
>> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering
>> 1 Network Drive
>> Burlington, MA 01803
>> Lance.Andersen at oracle.com
>> 
>> 
>> 
> 



Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com






More information about the core-libs-dev mailing list