Hi Roger,
Thanks for the suggestions
On Apr 17, 2014, at 9:44 AM, Roger Riggs <roger.ri...@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.ander...@oracle.com
>>
>>
>>
>
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
