On 07.06.16 10:23, Semyon Sadetsky wrote:
Sergey,

- You need to compare with the original policy by reference. Only by
that you may prove that the original policies were not affected.

I do not see the reason why the equal cannot be used, if some policy will override equal and return true then two policies will be treated as equal/unchanged.


- Please remove the printouts of policies objects before and after the
default policy change. It looks too verbose.

It was there before, and I found it quite useful during the fix.


- It would be nice to have a check if the default policy has been really
changed to the new one.

I did not get it, the bug which was bound to this test is about the incorrect changing the policy by setDefaultFocusTraversalPolicy(). the tests which cover other behavior are bound to other bugs like JDK-7125044


- What is purpose of jb1, jt1, jp1? They are created but never used.

They are used as dummy content for JFrame jf.

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




--
Best regards, Sergey.

Reply via email to