2012/4/21 Pavel Porvatov <pavel.porva...@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.
> 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.
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 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!)
Cheers,
Mario
--
pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF
Fingerprint: BA39 9666 94EC 8B73 27FA FC7C 4086 63E3 80F2 40CF
IcedRobot: www.icedrobot.org
Proud GNU Classpath developer: http://www.classpath.org/
Read About us at: http://planet.classpath.org
OpenJDK: http://openjdk.java.net/projects/caciocavallo/
Please, support open standards:
http://endsoftpatents.org/
