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

[Semyon Sadetsky]

On 6/9/2016 12:37 PM, Sergey Bylokhov wrote:

> On 08.06.16 19:53, Semyon Sadetsky wrote:
>> Yes, but the Object#equlas() does not prohibit different class instances
>> to be equal. The purpose of the test is to prove that existing
>> component's FTP instances remain untouchable during the default FTP
>> change regardless of the specific FTP implementation. This may be tested
>> only by == operator. Or please provide a scenario when they are allowed
>> be replaced by some equivalent FTP's.
>
> For our policies the different instances will be different because   
> our classes do not override equals. If we override at some point the 
> equal for our public classes then we will consider the different 
> instances as the same policy. Do you have some other comments?
These class instances may be replaced later. Since the fix pretends to 
improve the test quality, it is desirable that the limitations, like 
this, have been corrected in the fix.  equals() is not needed and should 
be replaced by ==.
>>> Why? It is useful to know what policy is used when the test passed.
>> Because the main purpose of the test is to automatically compare FTPs,
>> so this verbose output is not necessary and this extra printout
>> duplicates printout in case the test is failed.
>
> The author of the test and the author of this fix, consider this 
> output useful. Do you have some other comments?
But you could not answer: "how is it useful?".
Any test is a small product which is used for execution. This debug 
output might be useful for the test development, but for the test 
execution it is parasitic, because it consumes tester's attention when 
the test passes, which means that the FTPs are equal and there are no 
need to compare them using this output.
If the test fails the similar output is printed, so there will be the 
same information printed twice, and this will consumes more time to sort 
out the output.
To improve the test quality, please consider to remove this output or to 
make it optional.
>
>> Please answer the above question to avoid incoherence in the discussion.
>
> I already asked your question, "This is an emulation of the common 
> dialog used by the application which have some components inside which 
> can be focused". Do you have some other comments?
Since, you could not show how those dummy component are involved in the 
default FTP change, it is absolutely unclear what benefits may be 
achieved by adding those components, and hence, they may be discarded 
without any loss in the test coverage, but with a real improvement in 
test clearness.
>
>
>>>>
>>>> --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
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>