RFR 8040760: Addition of new java.sql tests

[roger riggs]

Hi Lance,

Looks fine.

Thanks, Roger


On 4/18/2014 4:12 PM, Lance Andersen wrote:
> Hi Roger,
>
> Thanks for the suggestions
> On Apr 17, 2014, at 9:44 AM, Roger Riggs <Roger.Riggs at oracle.com 
> <mailto: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
fyi, There is are checkbox(es) in the Netbeans test generation dialog 
for which methods to generate.
>
>> - 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
Looks good.
>> - 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/ 
> <http://cr.openjdk.java.net/%7Elancea/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/ 
>>> <http://cr.openjdk.java.net/%7Elancea/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 <mailto:Lance.Andersen at oracle.com>
>>>
>>>
>>>
>>
>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>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 <mailto:Lance.Andersen at oracle.com>
>
>
>