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.ri...@oracle.com
<mailto: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
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.ander...@oracle.com <mailto:lance.ander...@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.ander...@oracle.com <mailto:lance.ander...@oracle.com>
