Hi Sergey/Semyon,

I have updated the webrev according to your comments. Please review.
Webrev:   http://cr.openjdk.java.net/~psadhukhan/pankaj/webrev/

Regards,
Pankaj Bansal

-----Original Message-----
From: Semyon Sadetsky 
Sent: Tuesday, September 19, 2017 8:40 PM
To: Sergey Bylokhov; Pankaj Bansal; awt-dev@openjdk.java.net
Subject: Re: <AWT Dev> [10] Review Request: JDK-8081311 : [TEST_BUG] 
java/awt/ComponentOrientation/WindowTest.java

On 09/18/2017 11:04 AM, Sergey Bylokhov wrote:

> On 9/18/17 10:14, Semyon Sadetsky wrote:
>>> It is fine because it tests the ComponentOrientation class which is 
>>> loaded from the resource bundle and applied to the window.
>> I would agree with you if the resource bundle at least contained a 
>> value for the component orientation but it sets some specific locale 
>> codes. And java's locale functionality is not a subject of the 
>> client-libs, it is an another module. Also, as it was already pointed 
>> out, the method was deprecated long time ago.
>
> Where did you check these bundles? Because resource bundles for this 
> test contains the all variations of orientation 
> property(UNKNOWN/LEFT_TO_RIGHT/RIGHT_TO_LEFT/none).
OK. I missed those bundle classes with the orientation properties. I think the 
test may either be left as it is, or moved to the i18n suite with adding a test 
to ComponentOrientation suite that tests
applyComponentOrientation() directly.
>
>>>
>>>>
>>>>
>>>> On 9/15/17 4:53 PM, Sergey Bylokhov wrote:
>>>>> I guess you missed something, the possibility to configure the 
>>>>> components via ResourceBudnle or locale is something different 
>>>>> that the hardcore the orientation in the application. And the fact 
>>>>> that this test found the bug in jdk9 just show that the test is 
>>>>> valid.
>>>>>
>>>>> On 9/15/17 16:29, Semyon Sadetsky wrote:
>>>>>> The component orientation test suite should use the right API, 
>>>>>> not the deprecated one. This was missed when the new API is created.
>>>>>> So the change is valid from this point with the corresponding 
>>>>>> comment in JIRA.
>>>>>>
>>>>>> --Semyon
>>>>>>
>>>>>> On 9/15/2017 3:09 PM, Sergey Bylokhov wrote:
>>>>>>>
>>>>>>> On 9/14/17 23:15, Pankaj Bansal wrote:
>>>>>>>> This test is not failing with JDK 9_181 and JDK10 as methods 
>>>>>>>> are deprecated not removed. It is giving lot of warning for 
>>>>>>>> using deprecated things, so I made these changes. I think the 
>>>>>>>> bug description should be changed to notify that.
>>>>>>>
>>>>>>> I am not sure that it is correct to replace one API to another 
>>>>>>> one. This test was written as part of i18n support and it checks 
>>>>>>> that our API can be configured by the ResourceBundles. The 
>>>>>>> updated test doing the different things - it checks that 
>>>>>>> getComponentOrientation will return the value which were set by 
>>>>>>> applyComponentOrientation().
>>>>>>>
>>>>>>> Since it was reported that the test failed because of some bug, 
>>>>>>> but looks like now it work as expected, this CR can be closed as 
>>>>>>> not reproducible. Or you can clean the test but without changing 
>>>>>>> its logic.
>>>>>>>
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Pankaj Bansal
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Sergey Bylokhov
>>>>>>>> Sent: Thursday, September 14, 2017 11:30 PM
>>>>>>>> To: Pankaj Bansal; awt-dev@openjdk.java.net
>>>>>>>> Subject: Re: <AWT Dev> [10] Review Request: JDK-8081311 : 
>>>>>>>> [TEST_BUG] java/awt/ComponentOrientation/WindowTest.java
>>>>>>>>
>>>>>>>> Hi, Pankaj.
>>>>>>>> Can you please clarify why this test fails on jdk9? This method 
>>>>>>>> is deprecated but it was not removed, so it should work.
>>>>>>>> Probably at some point of jdk9 development it was broken?
>>>>>>>>
>>>>>>>> On 9/14/17 03:25, Pankaj Bansal wrote:
>>>>>>>>> Hi All,
>>>>>>>>>
>>>>>>>>> Please review the fix for test program 
>>>>>>>>> test/java/awt/ComponentOrientation/WindowTest.java for JDK 10.
>>>>>>>>>
>>>>>>>>> Bug:
>>>>>>>>>
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8081311
>>>>>>>>>
>>>>>>>>> Webrev:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~aghaisas/pankaj/8081311/webrev.0/
>>>>>>>>>
>>>>>>>>> Issue:
>>>>>>>>>
>>>>>>>>> The test java/awt/ComponentOrientation/WindowTest.java uses 
>>>>>>>>> depreciated function  applyResourceBundle. So the test should 
>>>>>>>>> be updated to use applyComponentOrientation function.
>>>>>>>>>
>>>>>>>>> Fix:
>>>>>>>>>
>>>>>>>>> Made changes in java/awt/ComponentOrientation/WindowTest.java
>>>>>>>>> to use
>>>>>>>>> applyComponentOrientation function and did corresponding changes.
>>>>>>>>> Along with that, the test is also using java.applet.Applet 
>>>>>>>>> which is also deprecated, so changed the test to use Frame 
>>>>>>>>> instead.
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>>
>>>>>>>>> Pankaj Bansal
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards, Sergey.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>

Reply via email to