Hi Joe, On Jan 9, 2015, at 8:12 PM, huizhe wang <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/ >> >> 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
