+1
On 9/21/17 01:29, Pankaj Bansal wrote:
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.
--
Best regards, Sergey.
