Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread David Holmes
Hi Amit, Just a couple of style nits: ! iterations++; ! ! if (10 == iterations) { => if (++iterations == 10) { -- !iterations=0; => iterations = 0; Thanks, David On 23/02/2017 4:58 PM, Amit Sapre wrote: Hello,

RE: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread Amit Sapre
Hello, http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.02/ has the updated changes. I ran this test on my VM and roughly takes 250-300 ms to get all the listener count. Thanks, Amit > -Original Message- > From: Amit Sapre > Sent: Thursday, February 23, 2017 11:55 AM

Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread David Holmes
Hi Amit, On 23/02/2017 4:18 PM, Amit Sapre wrote: Hello, Thanks David & Harsha for your inputs. Here is the new webrev : http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.01/ You didn't leave L116 - L121 as-is, you deleted them. Isn't checking they are all 0 a useful sanity

RE: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread Amit Sapre
Hello, Thanks David & Harsha for your inputs. Here is the new webrev : http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.01/ Thanks, Amit > -Original Message- > From: David Holmes > Sent: Thursday, February 23, 2017 9:24 AM > To: serviceability-dev@openjdk.java.net;

Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread David Holmes
Hi Amit, On 23/02/2017 12:18 AM, Harsha Wardhana B wrote: Hi Amit, There is no need to wait in a loop to check we have not received any notifications. Without starting the monitors, the listener count will be zero. The first part of diff L116-L121 could be left as is. Agreed. By relying

Re: RFR: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris

2017-02-22 Thread Harsha Wardhana B
Thanks for the review Daniel. -Harsha On Wednesday 22 February 2017 08:21 PM, Daniel Fuchs wrote: Hi Harsha, OK - makes sense to me. Maybe just wait for tomorrow to see if you get any other comments form other reviewers... best regards, -- daniel On 22/02/17 14:38, Harsha Wardhana B

Re: RFR: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris

2017-02-22 Thread Daniel Fuchs
Hi Harsha, OK - makes sense to me. Maybe just wait for tomorrow to see if you get any other comments form other reviewers... best regards, -- daniel On 22/02/17 14:38, Harsha Wardhana B wrote: Hi Daniel, ("java.lang.RuntimePermission" "accessClassInPackage.sun.security.krb5") was missing.

Re: RFR: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris

2017-02-22 Thread Harsha Wardhana B
Hi Daniel, ("java.lang.RuntimePermission" "accessClassInPackage.sun.security.krb5") was missing. I gave all the RuntimePermissions since we don't want test cases to fail in future. Granting java.lang.RuntimePermission "*" does not affect the test cases in anyway as test cases rely on

Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread Harsha Wardhana B
Hi Amit, There is no need to wait in a loop to check we have not received any notifications. Without starting the monitors, the listener count will be zero. The first part of diff L116-L121 could be left as is. By relying on Jtreg timeout for receiving notifications, we will be unable to

Re: RFR: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris

2017-02-22 Thread Daniel Fuchs
Hi Harsha, Looks good - but do you really need java.lang.RuntimePermission "*" ? What permission was missing? best regards, -- daniel On 22/02/17 13:35, Harsha Wardhana B wrote: Hi All, Please review the below test fix for Issue: JDK-8173130 -

RFR: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris

2017-02-22 Thread Harsha Wardhana B
Hi All, Please review the below test fix for Issue: JDK-8173130 - SubjectDelegation2Test.java and SubjectDelegation3Test.java failing on solaris webrev : http://cr.openjdk.java.net/~hb/8173130/webrev.00/ The fix does the following. 1. Removes explicit platform check

Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread Erik Gahlin
Looks good. Erik Hello, Please review this test bug fix which eliminates test case’s own timeout mechanism to default jtreg timeout. Bug ID : https://bugs.openjdk.java.net/browse/JDK-7132577 Webrev : http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.00/

RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

2017-02-22 Thread Amit Sapre
Hello, Please review this test bug fix which eliminates test case's own timeout mechanism to default jtreg timeout. Bug ID : https://bugs.openjdk.java.net/browse/JDK-7132577 Webrev : http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.00/ Thanks, Amit

Re: RFR: JDK-8162504: [SA testbug] TestInstanceKlassSize.java and TestInstanceKlassSizeForInterface.java fail on Mac OS

2017-02-22 Thread Jini George
Thank you, Serguei. - jini. On 2/22/2017 12:58 PM, serguei.spit...@oracle.com wrote: Hi Jini, It looks good. Thanks, Serguei On 2/21/17 07:48, Jini George wrote: Requesting reviews for the following SA testbug: https://bugs.openjdk.java.net/browse/JDK-8162504 Webrev: