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
