RFR: 8068732, Adding Initial RowSet tests

[huizhe wang]

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.wang at oracle.com 
> <mailto:huizhe.wang at 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.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>
>
>
>