<AWT Dev> [9] Review Request: 8004693 TEST_BUG: java/awt/KeyboardFocusmanager/DefaultPolicyChange/DefaultPolicyChange_Swing.java fails

[Semyon Sadetsky]

On 6/7/2016 3:31 PM, Sergey Bylokhov wrote:

> On 07.06.16 12:31, Semyon Sadetsky wrote:
>> I cannot agree with this assumption. Since the policy should not be
>> touched for the existing components, the test should prove that the
>> policy object instances are the same, other behavior is an error.
>
> The policy will be the same if two policies will return true in 
> equals, by default our policy do not implement the equal, so == is 
> used. But if some of them will implemented it and return true will 
> mean that the same policy is used, but the different instance.
Since method equals() is not specified in the FTP class, you cannot make 
any assumption how it is implemented, it may return true for different 
classes instances. So in the test the policy untouchability should be 
checked by ==, without any additional assumptions about the equals() 
method implementation.
>
>> And how is it useful? The toString() method is not implemented for the
>> printed objects, so the printouts are not readable. I found this print
>> out as excessive. If it invites to compare the memory addresses, that
>> has no sense, because it is automatically executed at the end of the
>> test and the same printout is produced in case of error.
>
> It print the class of the policy, which is different for awt and swing.
I know. But this is not needed because it does not produce any valuable 
information for a normal test execution (when it is passed).
>
>> In this case, please, provide a link to the test which covers the
>> KFM#setDefaultFocusTraversalPolicy() method.
>
> It is tested in FocusPolicyTest.java
>
>>> They are used as dummy content for JFrame jf.
>> That is what I asked. For what purpose this dummy content is added? Can
>> you clarify that?
>
> By the same reason why JDialog, JWindow and JFrame are added, it 
> increase the coverage if any side affects exists.
JDialog, JWindow and JFrame are focus traversal cycle roots by default, 
but those "dummy" components are not, i.e. their FTPs are not used and 
are nulls by default.
So could you point to the code related to the handling of the default 
FTP change, that is additionally covered by the test due to those 
"dummy" components?

--Semyon
>
>>>> On 5/30/2016 7:39 PM, Sergey Bylokhov wrote:
>>>>> Hello.
>>>>> Please review the fix for jdk9.
>>>>>
>>>>> The test DefaultPolicyChange_Swing.java has two issues:
>>>>>  - It uses invokeLater(), so the test usually pass before the code is
>>>>> executed on the EDT, because the main thread completes before.
>>>>>  - The test fetches the FocusTraversalPolicy from the current
>>>>> KeyboardFocusManager. But default FocusTraversalPolicy can be changed
>>>>> during the Swing initialization(JDK-7125044). The test should save 
>>>>> the
>>>>> state before setDefaultFocusTraversalPolicy() but after the Swing
>>>>> initialization, and validate that the FocusTraversalPolicy was not
>>>>> changed for windows which were already shown.
>>>>>
>>>>> The fix proposed in the CR is applied + small cleanup(regtesthelpers
>>>>> removed and InvokeAndWait is used instead of InvokeLater+realSync)
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8004693
>>>>> Webrev can be found at:
>>>>> http://cr.openjdk.java.net/~serb/8004693/webrev.01
>>>>>
>>>>
>>>
>>>
>>
>
>