+1. Thanks, Jay
> On 22-Aug-2019, at 2:10 PM, Tejpal Rebari <[email protected]> wrote: > > Looking for the second “+1”. > > Thanks > Tejpal > >> On 07-Aug-2019, at 2:20 PM, Ambarish Rapte <[email protected] >> <mailto:[email protected]>> wrote: >> >> Looks good to me, +1. >> >> Regards, >> Ambarish >> >> From: Tejpal Rebari >> Sent: Tuesday, August 06, 2019 12:36 PM >> To: Ambarish Rapte <[email protected] >> <mailto:[email protected]>> >> Cc: Prasanta Sadhukhan <[email protected] >> <mailto:[email protected]>>; [email protected] >> <mailto:[email protected]> >> Subject: Re: <Swing Dev> [13] RFR 8194944:Regression automated test >> 'open/test/jdk/javax/swing/JInternalFrame/8145896/TestJInternalFrameMaximize.java' >> fails >> >> Hi All, >> >> >> On 01-Aug-2019, at 2:20 PM, Ambarish Rapte <[email protected] >> <mailto:[email protected]>> wrote: >> >> Hi Tejpal, >> Here are few comments, >> 1. Update copyright year to include 2019 >> 2. Add bug id 8145896 to @bug >> 3. Add a new line before imports begin, line #33 >> 4. point should be volatile, line #58 [Won’t be needed, if changes are >> made to address comment #7] >> 5. Add space after try, line #66 >> 6. The empty lines #75, #151 can be removed. >> 7. In blockTillDisplayed(): If frame.getLocationOnScreen() throws an >> exception then it won't get caught. >> So if frame.getLocationOnScreen() throws an exception, the test will fail, >> but instead the test should sleep and try again. >> The blockTillDisplayed() method defined in test bug6427244.java looks more >> correct, you can use that method here. >> >> Regards, >> Ambarish >> >> I have modified the test according to the above comments and here is the >> latest changes. >> webrev : http://cr.openjdk.java.net/~trebari/swing/8194944/webrev3/ >> <http://cr.openjdk.java.net/~trebari/swing/8194944/webrev3/> >> >> Thanks and regards >> Tejpal >> >> >> >> >> From: Prasanta Sadhukhan >> Sent: Thursday, July 25, 2019 3:43 PM >> To: Tejpal Rebari <[email protected] >> <mailto:[email protected]>> >> Cc: [email protected] <mailto:[email protected]> >> Subject: Re: <Swing Dev> [13] RFR 8194944:Regression automated test >> 'open/test/jdk/javax/swing/JInternalFrame/8145896/TestJInternalFrameMaximize.java' >> fails >> >> +1 >> Regards >> Prasanta >> On 25-Jul-19 3:40 PM, Tejpal Rebari wrote: >> Hi All, >> I have verified that the test fails without the changes of the fix 8145896. >> >> Regards >> Tejpal >> On 25-Jul-2019, at 12:40 PM, Prasanta Sadhukhan >> <[email protected] <mailto:[email protected]>> wrote: >> >> OK. Last thing is, please confirm if the modified test still fails if we >> remove 8145896 fix >> [http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/ef8fd276efe1 >> <http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/ef8fd276efe1>]. >> Regards >> Prasanta >> On 25-Jul-19 12:33 PM, Tejpal Rebari wrote: >> Hi Prasanta, >> Agreed that the two points should be different, so retained the point >> variable for executeTest(). >> >> Here is the latest changes. >> webrev : http://cr.openjdk.java.net/~trebari/swing/8194944/webrev2/ >> <http://cr.openjdk.java.net/~trebari/swing/8194944/webrev2/> >> >> Thanks and regards >> Tejpal >> >> >> >> On 22-Jul-2019, at 4:08 PM, Prasanta Sadhukhan >> <[email protected] <mailto:[email protected]>> wrote: >> >> Looks better but I guess frame's locationOnScreen point would be different >> from Menu's centrepoint so we should not use same "point" instance variable >> for both. Guess local "point" variable for executeTest() should be retained. >> Regards >> Prasanta >> On 19-Jul-19 1:11 PM, Tejpal Rebari wrote: >> Hi All, >> Added the blockTillDisplayed() method instead of 1000ms delay. >> >> here is the latest changes. >> webrev : http://cr.openjdk.java.net/~pkbalakr/Tejpal/8194944/webrev1 >> <http://cr.openjdk.java.net/~pkbalakr/Tejpal/8194944/webrev1> >> >> Thanks and regards >> Tejpal Rebari >> >> >> >> >> On 08-Jul-2019, at 1:43 PM, Prasanta Sadhukhan >> <[email protected] <mailto:[email protected]>> wrote: >> >> Adding delay is indeterministic. I guess it will be better if you use >> blockTillDisplayed() method which you can find in many of the testcase(s) >> like javax/swing/JComboBox/8182031/ComboPopupTest.java, >> javax/swing/JEditorPane/6917744/bug6917744.java >> Regards >> Prasanta >> On 08-Jul-19 1:02 PM, Tejpal Rebari wrote: >> Hi All, >> Please review a test fix for the issue where the test fails on Mac and >> Linux intermittently. >> >> The test fails because of “component must be showing on the screen to >> determine its location” error. >> Which is because the test tries to get the location of the component and the >> component is not displayed on the screen yet. >> >> The test tries to get the location of menu in the executeTest method so By >> adding some delay before the executeTest call solves the problem. >> >> Tested in our internal build and test system and test passes on all the >> three Platforms. >> >> Bug : https://bugs.openjdk.java.net/browse/JDK-8194944 >> <https://bugs.openjdk.java.net/browse/JDK-8194944> >> webrev: http://cr.openjdk.java.net/~pkbalakr/Tejpal/8194944/webrev0 >> <http://cr.openjdk.java.net/~pkbalakr/Tejpal/8194944/webrev0> >> >> Thanks and regards >> Tejpal >> >> >> >> >
