Looking for the second “+1”. Thanks Tejpal
> On 07-Aug-2019, at 2:20 PM, Ambarish Rapte <[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 > > > >
