Skip to main content

Again: spurious test failures ...

12 replies [Last post]
Anonymous

.. with the ominous ConcurrentModificationExc. Recently it's always the
JXTaskPaneTest.testSingleExpanded, comes and goes at will (nothing
changed near it) at the server only, locally everything's okay.

Any idea how we could cope with it? Nothing really wrong with the
code/tests, but a unstable build looks bad.

Jeanette

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
Kleopatra

jdnc-interest@javadesktop.org schrieb:
> Anyway that was a long and rambling way to say, let's try invokeAndWait first.
>
>

harrrr .. too lazy to play with invokeAndWait - just committed the
synchronized versions (the old are tagged with jw_beforesynchronized..
so we can revert). Feel free to cleanup

Thanks for the explanation!
Jeanette

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net

i30817
Offline
Joined: 2006-05-02

There is another reason for that exception, you just need to be iterating with an iterator, and interleaved using get(int index) or something like that in the collection. Normally this can be in another thread, but not always.
(i remember a game where object collision made me get the other object - in a lop iterating over the object list).

Kleopatra

jdnc-interest@javadesktop.org schrieb:
> There is another reason for that exception, you just need to be iterating with an iterator, and interleaved using get(int index) or something like that in the collection. Normally this can be in another thread, but not always.
>

Hmm ... could well be another reason: happened again after using the
synchronized versions of the collections (BTW: why did Hudson started
build #1046 - can't see anything which could have triggered it?) But I'm
completely clueless, can't see how different threads might be involved.
So here's a summary of the different parts, maybe somebody more
experienced can shed some light onto it

The stacktrace:

[code]

java.util.ConcurrentModificationException
at java.util.LinkedList$ListItr.checkForComodification(LinkedList.java:761)
at java.util.LinkedList$ListItr.next(LinkedList.java:696)
at org.jdesktop.test.PropertyChangeReport.getEventNames(PropertyChangeReport.java:153)
at org.jdesktop.test.TestUtils.assertPropertyChangeEvent(TestUtils.java:78)
at org.jdesktop.swingx.JXTaskPaneTest.testSingleExpanded(JXTaskPaneTest.java:230)

[/code]

The test which triggers it:

[code]
public void testSingleExpanded() {
JXTaskPane pane = new JXTaskPane();
PropertyChangeReport report = new PropertyChangeReport();
pane.addPropertyChangeListener(report);
pane.setExpanded(!pane.isExpanded());
TestUtils.assertPropertyChangeEvent(report, "expanded",
!pane.isExpanded(), pane.isExpanded(), false);

[/code]

for comparision a very similar test which runs _without_ problems:

[code]
public void testExpandedNotification() {
JXTaskPane group = new JXTaskPane();
boolean initial = group.isCollapsed();
PropertyChangeReport report = new PropertyChangeReport();
group.addPropertyChangeListener(report);
group.setCollapsed(!initial);
TestUtils.assertPropertyChangeEvent(report, "expanded", !initial,
initial, false);
}

[/code]

the convenience method in testUtils, the logging line accesses the
method which ultimately causes the error:

[code]
public static void assertPropertyChangeEvent(PropertyChangeReport
report,
String property, Object oldValue, Object newValue, boolean
single) {
if (report.getEventCount() > 1) {
LOG.info("events: " + report.getEventNames());
}
if (single) {
....
} else {
assertEquals("one event of property " + property, 1,
report.getEventCount(property));
assertEquals("old property", oldValue,
report.getLastOldValue(property));
assertEquals("new property", newValue,
report.getLastNewValue(property));
}
}

[/code]

and the collection ( = events) access

[code]
protected List events =
Collections.synchronizedList(new LinkedList());
...
public String getEventNames() {
StringBuffer buffer = new StringBuffer();
for (PropertyChangeEvent event : events) {
buffer.append(event.getPropertyName());
buffer.append(" :: ");
}
return buffer.toString();
}
[/code]

The test itself if insignificant (tests deprecated api and will be
removed soon anyway, but maybe it's failure is an indicator to doing
something basically wrong?

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net

kschaefe
Offline
Joined: 2006-06-08

I can't get the test to fail locally, at all. Not enough background processes to alter the scheduler, I guess. The server is more susceptible to that, I'm sure.

Looking at the method the execution of the JXTaskPane code occurs on the main thread and not on the EDT.

We should rewrite it (I think) to be the following:
[code]final PropertyChangeReport report = new PropertyChangeReport();
SwingUtilities.invokeAndWait(new Runnable() {
public void run() {
JXTaskPane group = new JXTaskPane();
group.addPropertyChangeListener(report);
group.setCollapsed(group.isCollapsed());
}
};
TestUtils.assertPropertyChangeEvent(report, "expanded", false,
true, false);[/code]

It improves the test in the following ways:
1. Runs the Swing code on the EDT.
2. Assert that the initial state of the JXTaskPane is not collapsed. Currently, we don't do ensure that it begins in a specific state.
3. It ensures that everything will run in a specified order. Walking the code, I don't believe that it is possible to achieve concurrent modification.

Karl

Kleopatra

jdnc-interest@javadesktop.org schrieb:
> We should rewrite it (I think) to be the following:
>

yeah, probably - but too lazy: the expanded is deprecated so the test
will be removed anyway soon (hint, hint )

For now, I lost nerves and commented it ...

Cheers
Jeanette

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net

kschaefe
Offline
Joined: 2006-06-08

Well, I might try to rewrite it. I want to know, or at least have some confidence, that what I suggested will work. I can't imagine that this is the only place this could happen.

Karl

Kleopatra

jdnc-interest@javadesktop.org schrieb:
> Well, I might try to rewrite it. I want to know, or at least have some confidence, that what I suggested will work. I can't imagine that this is the only place this could happen.
>
>

Yeah, I'm pretty sure that your suggestion will work. Just

- maybe we want all/most (component related) tests on the EDT (like
Laird suggested)?
- don't see much use in putting effort into a test that is deprecated
and will be removed the deprecated code in the tested comp will be
removed (nudging ... :-)
- the taskPane is faily singular with its animated expand/collapse.
Which is something I overlooked when writing that test. So I don't
expect a lot of places to happen. Crossing fingers

Cheers
Jeanette

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net

kschaefe
Offline
Joined: 2006-06-08

Change the LinkedList to a Vector or use Collections.synchronizedList to wrap the LinkedList. We're getting changes made to the collection on more than one thread.

Karl

Kleopatra

jdnc-interest@javadesktop.org schrieb:
> Change the LinkedList to a Vector or use Collections.synchronizedList to wrap the LinkedList. We're getting changes made to the collection on more than one thread.
>
>

Hmm .. aren't we always on the EDT in the test runner? Or not when in
headless context? It's really fun that everything's find in local
context ;-)

Anyway, good idea. Let's hope we don't have race or other harmful
conditions somewhere

Thanks
Jeanette

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net

kschaefe
Offline
Joined: 2006-06-08

> Hmm .. aren't we always on the EDT in the test
> runner? Or not when in
> headless context?
Nope. The test is rarely if ever on the EDT. It doesn't start there, it starts on the main thread (i don't think JUnit uses it's own thread). Our work may be occuring on the EDT, depending on the test.

We should probably fix some of these tests with invokeAndWait to ensure that the main thread continues execution only after the EDT is finished with the assigned task. invokeAndWait semantics may avoid the need to adding synchronization to the report classes and in fact may be the better choice now that I think on it. The invokeAndWait semantics are predictable, whereas synchronization just adds protection not predictability. The lack of synchronization can generate the CME that we can use the properly add invokeAndWait semantics.

Anyway that was a long and rambling way to say, let's try invokeAndWait first.

Karl

ljnelson
Offline
Joined: 2003-08-04

In case it helps, I've found this article (http://stuffthathappens.com/blog/2007/09/14/junit-3x-and-the-event-dispa...) to be very helpful.

Best,
Laird

Kleopatra

Laird
> In case it helps, I've found this article (http://stuffthathappens.com/blog/2007/09/14/junit-3x-and-the-event-dispa...) to be very helpful.
>
>

thanks for the link - I faintly remember having read it earlier and
shrugged ;-). In case we want to go that approach (read: Jan would be
the one to setup the infrastructure :-) I filed an issue #865-swingx and
added a link to this thread, so we don't forget a possible solution.

Cheers
Jeanette

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net