Hi Lance,
Thanks for the explanation. That's an interesting/smart use of the
annotation (enabled). I was guessing there might be issues in the
implementation :-)
It's good to get a baseline in and revisit later, similarly as you've
seen in the newly-migrated jaxp tests. I'm just glad I have a good set
of working tests that I can use now, and can worry about refining them
later. This set of tests of yours is in a much better shape than those,
I think you can push them as they are.
Best,
Joe
On 1/9/2015 5:49 PM, Lance Andersen wrote:
Hi Joe,
On Jan 9, 2015, at 8:12 PM, huizhe wang <huizhe.w...@oracle.com
<mailto:huizhe.w...@oracle.com>> wrote:
Hi Lance,
Looks good to me.
Thank you
Are classes CachedRowSetTests and WebRowSetTests used? The Common*
tests seem to me all extends CommonCachedRowSetTests.
Yes, they are, Each type of RowSet (Cached/Web/Join/Filter) have a
XXXRowSetTest class which extend some form of CommonXXXRowSet:
CommonCachedRowSetTests extends CommonRowSetTests.
CommonWebRowSetTests extends CommonCachedRowSetTests
BaseRowSetTests extends CommonRowSetTests
WebRowSetTests extends CommonWebRowSetTests
CachedRowSetTests extends CommonCachedRowSetTests
FilteredRowSetTests extends CommonWebRowSetTests
JoinRowSetTests extends CommonWebRowSetTests
The above is similar to how the XXXRowSet interfaces are desgined
Many of the tests are applicable to other RowSets but some are only
applicable to a subset and reduces potential duplication of common tests
A minor point: would it make sense to add a rowSetType data provider
that includes listener(s)?
I can possibly look at this for some of the tests in
CommonCachedRowSet but in some cases like BaseRowSet, it would not be
applicable based on how the API was originally designed.
Some of the tests in CommonCachedRowSetTests are disabled, did they
not work? The unsetMatchColumn - SQLException tests that follow them
imply that the setMatchColumn method works.
Yes there are some tests which I have left in but disabled as I found
implementation bugs.
setMatchColumn is a good example as you should be able to specify the
index or columnLabel interchangeably but the implementation does not
account for this
You will notice this and some of the other RowSet tests where the
tests are overridden because of implementation bugs and are basically
a no-op. This allows me to just enable or remove the overridden tests
but not lose the coverage where it is actually not buggy.
Let me know if the above is clear(as mud) otherwise I will try and
clarify further…. :-)
There probably some additional refactoring but wanted to get a
baseline in and will revisit as I add additional tests as I did for
the BaseRowSet tests
Have a nice weekend!
Best,
Lance
Best,
Joe
On 1/9/2015 7:35 AM, Lance Andersen wrote:
Hi all,
Please find the webrev for adding an initial set of tests for
RowSets. The webrev is at
http://cr.openjdk.java.net/~lancea/8068732/webrev.00/
<http://cr.openjdk.java.net/%7Elancea/8068732/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>
