Re: RFR: 8195976: Add JNDI test javax/naming/dns/AttributeTests/GetAny.java

[Chris Yin]

Hi, Roger

Many thanks for your review. 

Yes, there are also negative tests planned

Thanks & Regards,
Chris

> On 13 Feb 2018, at 6:36 AM, Roger Riggs  wrote:
> 
> Hi Chris,
> 
> Looks fine.
> 
> These are only positive tests, are there negative tests planned?
> 
> Thanks, Roger
> 
> On 1/23/2018 5:14 AM, Chris Yin wrote:
>> Thank you Alan, I just moved it to com/sun/jndi/dns/ as you suggested and 
>> removed unused "@modules jdk.naming.dns/com.sun.jndi.dns”, updated webrev as 
>> below, thanks
>> 
>> http://cr.openjdk.java.net/~xiaofeya/8195976/webrev.00/ 
>> 
>> 
>> Regards,
>> Chris
>> 
>>> On 23 Jan 2018, at 3:53 PM, Alan Bateman  wrote:
>>> 
>>> 
>>> 
>>> On 23/01/2018 07:01, Chris Yin wrote:
 Please review the added JNDI test 
 javax/naming/dns/AttributeTests/GetAny.java, thanks
 
 
>>> You may want to move it to com/sun/jndi/dns so that it's with the other 
>>> tests for the DNS provider (as there is no javax.naming.dns API). Also I 
>>> suspect you don't need "@modules jdk.naming.dns/com.sun.jndi.dns" as it 
>>> doesn't appear to make direct use of the classes in the implementation.
>>> 
>>> -Alan
> 

Re: RFR: 8195976: Add JNDI test javax/naming/dns/AttributeTests/GetAny.java

[Roger Riggs]

Hi Chris,

Looks fine.

These are only positive tests, are there negative tests planned?

Thanks, Roger

On 1/23/2018 5:14 AM, Chris Yin wrote:

Thank you Alan, I just moved it to com/sun/jndi/dns/ as you suggested and removed 
unused "@modules jdk.naming.dns/com.sun.jndi.dns”, updated webrev as below, 
thanks

http://cr.openjdk.java.net/~xiaofeya/8195976/webrev.00/ 


Regards,
Chris


On 23 Jan 2018, at 3:53 PM, Alan Bateman  wrote:



On 23/01/2018 07:01, Chris Yin wrote:

Please review the added JNDI test javax/naming/dns/AttributeTests/GetAny.java, 
thanks



You may want to move it to com/sun/jndi/dns so that it's with the other tests for the DNS 
provider (as there is no javax.naming.dns API). Also I suspect you don't need 
"@modules jdk.naming.dns/com.sun.jndi.dns" as it doesn't appear to make direct 
use of the classes in the implementation.

-Alan

Re: RFR: 8195976: Add JNDI test javax/naming/dns/AttributeTests/GetAny.java

[Chris Yin]

A reminder that this need a reviewer's approval, many thanks

Regards,
Chris

> On 23 Jan 2018, at 6:14 PM, Chris Yin  wrote:
> 
> Thank you Alan, I just moved it to com/sun/jndi/dns/ as you suggested and 
> removed unused "@modules jdk.naming.dns/com.sun.jndi.dns”, updated webrev as 
> below, thanks
> 
> http://cr.openjdk.java.net/~xiaofeya/8195976/webrev.00/ 
> 
> 
> Regards,
> Chris
> 
>> On 23 Jan 2018, at 3:53 PM, Alan Bateman  wrote:
>> 
>> 
>> 
>> On 23/01/2018 07:01, Chris Yin wrote:
>>> Please review the added JNDI test 
>>> javax/naming/dns/AttributeTests/GetAny.java, thanks
>>> 
>>> 
>> You may want to move it to com/sun/jndi/dns so that it's with the other 
>> tests for the DNS provider (as there is no javax.naming.dns API). Also I 
>> suspect you don't need "@modules jdk.naming.dns/com.sun.jndi.dns" as it 
>> doesn't appear to make direct use of the classes in the implementation.
>> 
>> -Alan
> 

Re: RFR: 8195976: Add JNDI test javax/naming/dns/AttributeTests/GetAny.java

[Chris Yin]

Thank you Alan, I just moved it to com/sun/jndi/dns/ as you suggested and 
removed unused "@modules jdk.naming.dns/com.sun.jndi.dns”, updated webrev as 
below, thanks

http://cr.openjdk.java.net/~xiaofeya/8195976/webrev.00/ 


Regards,
Chris

> On 23 Jan 2018, at 3:53 PM, Alan Bateman  wrote:
> 
> 
> 
> On 23/01/2018 07:01, Chris Yin wrote:
>> Please review the added JNDI test 
>> javax/naming/dns/AttributeTests/GetAny.java, thanks
>> 
>> 
> You may want to move it to com/sun/jndi/dns so that it's with the other tests 
> for the DNS provider (as there is no javax.naming.dns API). Also I suspect 
> you don't need "@modules jdk.naming.dns/com.sun.jndi.dns" as it doesn't 
> appear to make direct use of the classes in the implementation.
> 
> -Alan

Re: RFR: 8195976: Add JNDI test javax/naming/dns/AttributeTests/GetAny.java

[Alan Bateman]

On 23/01/2018 07:01, Chris Yin wrote:

Please review the added JNDI test javax/naming/dns/AttributeTests/GetAny.java, 
thanks


You may want to move it to com/sun/jndi/dns so that it's with the other 
tests for the DNS provider (as there is no javax.naming.dns API). Also I 
suspect you don't need "@modules jdk.naming.dns/com.sun.jndi.dns" as it 
doesn't appear to make direct use of the classes in the implementation.


-Alan