<Swing Dev> Force JPopup to be always heavyweight

[Pavel Porvatov]

Hi Mario,
> 2012/4/21 Pavel Porvatov<pavel.porvatov at oracle.com>:
>
> Hi Pavel,
>
>> About the test:
>> 1. Now is 2012 :)
> Ops...
>
>> 2. You must access to Swing components only from the EDT (see
>> clickOnComponent(final Component comp) and other methods)
> Not sure if I understand correctly, all the access is done in the EDT
> already, unless I became very blind!
>
> The tests are run from the EDT, only exception is checkPopup, which
> just read a value after the execution, and I think this should be
> safe.
Yes, I missed the fact that the clickOnComponent method invoked on EDT. 
That's because you used robot.delay(50) in the method. There is no sense 
to use sleep methods on the EDT therad: you just freeze any event 
handling....
>> b.
>> loop
>>         final Map<String, Boolean>  tests = new HashMap<>();
>>         tests.put("javax.swing.PopupFactory$HeavyWeightPopup", false);
>>         tests.put("javax.swing.PopupFactory$LightWeightPopup", true);
>>
>>         for (final String test : tests.keySet()) {
>> can be replaced by two simple invocations
> Actually, this means duplicate more code or introduce another method,
> not sure if this makes the code cleaner, but I can do it if you prefer
> so.
>
>> c. NoSuchFieldException, SecurityException, IllegalArgumentException,
>> IllegalAccessException can be replaced by Exception
>> d.
>> robot.delay(50);
>> robot.mousePress(InputEvent.BUTTON1_MASK);
>> robot.delay(50);
>> Just use Robot#setAutoDelay
>>
>> etc.
>>
>> 5. latch must be volitile. After test rewriting I think this variable can be
>> removed at all
>>
>> Note that tests should be readable and simplest as far as possible
> The reason why the test is so complex is that I wanted to throw the
> exact exception and don't mix the reflection related stuff with the
> real test exception, that also basically means I don't want to save
> the exception and rethrow it later on (I've seen this in some other
> tests), I rather prefer to make this obvious and not hidden, but of
> course the code gets longer, and everything is complicated by the EDT
> invocations.
In your case reflection exception is also test failing.
> Also, I'm not particularly happy with the use of reflection to access
> the filed and check the class name, since we're testing against an
> implementation detail, but I don't know how else I should test that we
> create an heavy weight window (which is really also just an
> implementation detail that leaked through the code up to the user,
> nobody should ever care about heavy weight and lightweight imho), so
> if you have a smarter idea, I would be happy to change the code.
I'm also trying to avoid reflection in tests but I don't see another 
solution here
> I will try to refactor the code but I would like to not invest
> significant time in that, I'll send you a revised patch later
> (hopefully!)
Yes, and that's the reason to write first version of test without any 
errors. The test shouldn't have errors, because if it fails (on some 
platforms with very specific configuration) we have to fix it (therefore 
we are trying to keep all tests as clear and short as possible)...

Your test is still have problems. E.g. setVisible invocation doesn't 
guarantee that right after method Frame becomes visible (platforms 
dependent behaviour). You can take a look at good test examples in 
repository, e.g. here
http://hg.openjdk.java.net/jdk8/awt/jdk/rev/dfa2ea47257d

Regards, Pavel