Re: RFR: 8193154: methods in java.time's TCKZoneRules OpenJDK test miss @Test annotation

2018-05-27 Thread Lance Andersen
Hi Vivek,

I suspect the submitter of the issue, was not aware of the option of putting 
the annotation on the class.  If you needed additional attributes for the 
annotation, then it makes sense to add them but given there are no attributes 
needed, the annotation on the class should be enough

Best
Lance
> On May 27, 2018, at 3:25 AM, Vivek Theeyarath <vivek.theeyar...@oracle.com> 
> wrote:
> 
> Hi Lance,
>Thanks for the feedback. I did notice the @Test at the class 
> level. The motivation behind the fix was a comment in the bug saying not 
> having the annotation at the method level affects correct processing of 
> TCKZoneRules and inclusion of test methods into master test list where this 
> test has been ported to. So, I am assuming this would still help. But for 
> that, I agree with you that this may not be needed.
>  
> Regards
> Vivek
> From: Lance Andersen 
> Sent: Saturday, May 26, 2018 6:06 PM
> To: Vivek Theeyarath <vivek.theeyar...@oracle.com 
> <mailto:vivek.theeyar...@oracle.com>>
> Cc: core-libs-dev <core-libs-dev@openjdk.java.net 
> <mailto:core-libs-dev@openjdk.java.net>>
> Subject: Re: RFR: 8193154: methods in java.time's TCKZoneRules OpenJDK test 
> miss @Test annotation
>  
> Hi Vivek,
>  
> The original test has the @Test annotation on the class which indicates that 
> all public methods are test methods:
>  
> http://testng.org/doc/documentation-main.html#class-level 
> <http://testng.org/doc/documentation-main.html#class-level>
>  
> So adding @Test to each method is OK but not needed.
>  
> HTH
>  
> Best
> Lance
>  
>  
> On May 26, 2018, at 3:51 AM, Vivek Theeyarath <vivek.theeyar...@oracle.com 
> <mailto:vivek.theeyar...@oracle.com>> wrote:
>  
> Hi All,
> 
>   Please review fix for 
> https://bugs.openjdk.java.net/browse/JDK-8193154 
> <https://bugs.openjdk.java.net/browse/JDK-8193154> . 
> 
> 
> 
> http://cr.openjdk.java.net/~vtheeyarath/8193154/webrev.00/ 
> <http://cr.openjdk.java.net/~vtheeyarath/8193154/webrev.00/> 
> 
> 
> 
> The test ran fine with Mach5 
> http://java.se.oracle.com:10065/mdash/jobs/root-jdk-dev-20180526-0645-24095 
> <http://java.se.oracle.com:10065/mdash/jobs/root-jdk-dev-20180526-0645-24095> 
> 
> 
> 
> Regards
> 
> Vivek
>  
>  <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>
>  
> 
> 
>  

 <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>





RE: RFR: 8193154: methods in java.time's TCKZoneRules OpenJDK test miss @Test annotation

2018-05-27 Thread Vivek Theeyarath
Hi Lance,

   Thanks for the feedback. I did notice the @Test at the class 
level. The motivation behind the fix was a comment in the bug saying not having 
the annotation at the method level affects correct processing of TCKZoneRules 
and inclusion of test methods into master test list where this test has been 
ported to. So, I am assuming this would still help. But for that, I agree with 
you that this may not be needed.

 

Regards

Vivek

From: Lance Andersen 
Sent: Saturday, May 26, 2018 6:06 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8193154: methods in java.time's TCKZoneRules OpenJDK test 
miss @Test annotation

 

Hi Vivek,

 

The original test has the @Test annotation on the class which indicates that 
all public methods are test methods:

 

http://testng.org/doc/documentation-main.html#class-level

 

So adding @Test to each method is OK but not needed.

 

HTH

 

Best

Lance

 

 

On May 26, 2018, at 3:51 AM, Vivek Theeyarath mailto:vivek.theeyar...@oracle.com"vivek.theeyar...@oracle.com> wrote:

 

Hi All,

  Please review fix for 
https://bugs.openjdk.java.net/browse/JDK-8193154 . 



http://cr.openjdk.java.net/~vtheeyarath/8193154/webrev.00/ 



The test ran fine with Mach5 
http://java.se.oracle.com:10065/mdash/jobs/root-jdk-dev-20180526-0645-24095 



Regards

Vivek

 

http://oracle.com/us/design/oracle-email-sig-198324.gif

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

 





 


Re: RFR: 8193154: methods in java.time's TCKZoneRules OpenJDK test miss @Test annotation

2018-05-26 Thread Lance Andersen
Hi Vivek,

The original test has the @Test annotation on the class which indicates that 
all public methods are test methods:

http://testng.org/doc/documentation-main.html#class-level 


So adding @Test to each method is OK but not needed.

HTH

Best
Lance


> On May 26, 2018, at 3:51 AM, Vivek Theeyarath  
> wrote:
> 
> Hi All,
> 
>   Please review fix for 
> https://bugs.openjdk.java.net/browse/JDK-8193154 . 
> 
> 
> 
> http://cr.openjdk.java.net/~vtheeyarath/8193154/webrev.00/ 
> 
> 
> 
> The test ran fine with Mach5 
> http://java.se.oracle.com:10065/mdash/jobs/root-jdk-dev-20180526-0645-24095 
> 
> 
> 
> Regards
> 
> Vivek

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 





RFR: 8193154: methods in java.time's TCKZoneRules OpenJDK test miss @Test annotation

2018-05-26 Thread Vivek Theeyarath
Hi All,

   Please review fix for 
https://bugs.openjdk.java.net/browse/JDK-8193154 . 

 

http://cr.openjdk.java.net/~vtheeyarath/8193154/webrev.00/ 

 

The test ran fine with Mach5 
http://java.se.oracle.com:10065/mdash/jobs/root-jdk-dev-20180526-0645-24095 

 

Regards

Vivek