From 75e20f1ae37fa49a383c54ab66f5f600b06bb380 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Fri, 2 Jan 2026 17:09:47 +0100 Subject: [PATCH] Make MarkerTest properly handle asynchronous change listener #106 The MarkerTest implementations currently assume that resource change listeners are notifies synchronously about changes due to modified markers. This assumption is not correct and leads to several race conditions in the tests which can make them fail sporadically. This change adapts the MarkerTests to properly handle asynchronous execution of resource change listeners: - Wait until a timeout for the number of expected resource changes being processed by the used resource change listener - Make the data structures inside the resource listeners thread safe Fixes https://github.com/eclipse-platform/eclipse.platform/issues/106 --- .../eclipse/core/tests/resources/MarkerTest.java | 2 ++ .../tests/resources/MarkersChangeListener.java | 16 ++++++++-------- .../MarkersNumberOfDeltasChangeListener.java | 9 +++++---- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkerTest.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkerTest.java index bcf9caaeffb..9b174b9b8de 100644 --- a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkerTest.java +++ b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkerTest.java @@ -345,6 +345,7 @@ public void testThatSettingAttributesTriggerAdditionalResourceChangeEvent() thro IMarker marker = resource.createMarker(TEST_PROBLEM_MARKER); marker.setAttribute(IMarker.MESSAGE, createRandomString()); marker.setAttribute(IMarker.PRIORITY, IMarker.PRIORITY_HIGH); + TestUtil.waitForCondition(() -> listener.numberOfChanges() == 3, 5000); assertThat(listener.numberOfChanges()).isEqualTo(3); } } @@ -362,6 +363,7 @@ public void testThatMarkersWithAttributesOnlyTriggerOnResourceChangeEvent() thro // each setAttributes triggers one resource change event resource.createMarker(TEST_PROBLEM_MARKER, Map.of(IMarker.MESSAGE, createRandomString(), IMarker.PRIORITY, IMarker.PRIORITY_HIGH)); + TestUtil.waitForCondition(() -> listener.numberOfChanges() == 1, 5000); assertThat(listener.numberOfChanges()).isEqualTo(1); } } diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkersChangeListener.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkersChangeListener.java index 56a831bd7db..0118f0e5f63 100644 --- a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkersChangeListener.java +++ b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkersChangeListener.java @@ -16,9 +16,12 @@ import static org.assertj.core.api.Assertions.assertThat; -import java.util.HashMap; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Vector; +import java.util.concurrent.ConcurrentHashMap; import org.eclipse.core.resources.IMarker; import org.eclipse.core.resources.IMarkerDelta; import org.eclipse.core.resources.IResource; @@ -31,7 +34,7 @@ * A support class for the marker tests. */ public class MarkersChangeListener implements IResourceChangeListener { - protected HashMap> changes; + private final Map> changes = new ConcurrentHashMap<>(); public MarkersChangeListener() { reset(); @@ -72,11 +75,12 @@ public void assertChanges(IResource resource, IMarker[] added, IMarker[] removed * changes since last reset. */ public void assertNumberOfAffectedResources(int expectedNumberOfResource) { + TestUtil.waitForCondition(() -> changes.size() == expectedNumberOfResource, 5000); assertThat(changes).hasSize(expectedNumberOfResource); } public void reset() { - changes = new HashMap<>(); + changes.clear(); } /** @@ -96,11 +100,7 @@ protected void resourceChanged(IResourceDelta delta) { } if ((delta.getFlags() & IResourceDelta.MARKERS) != 0) { IPath path = delta.getFullPath(); - List v = changes.get(path); - if (v == null) { - v = new Vector<>(); - changes.put(path, v); - } + List v = changes.computeIfAbsent(path, p -> Collections.synchronizedList(new ArrayList<>())); IMarkerDelta[] markerDeltas = delta.getMarkerDeltas(); for (IMarkerDelta markerDelta : markerDeltas) { v.add(markerDelta); diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkersNumberOfDeltasChangeListener.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkersNumberOfDeltasChangeListener.java index c2f511f7efc..099e2255633 100644 --- a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkersNumberOfDeltasChangeListener.java +++ b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/MarkersNumberOfDeltasChangeListener.java @@ -13,6 +13,7 @@ *******************************************************************************/ package org.eclipse.core.tests.resources; +import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.core.resources.IResourceChangeEvent; import org.eclipse.core.resources.IResourceChangeListener; @@ -21,25 +22,25 @@ */ public class MarkersNumberOfDeltasChangeListener implements IResourceChangeListener { - private int number = 0; + private AtomicInteger number = new AtomicInteger(); /** * Returns the number of resource changed calls. */ public int numberOfChanges() { - return number; + return number.get(); } public void reset() { - number = 0; + number.set(0); } /** * Notification from the workspace. Extract the marker changes. */ @Override public void resourceChanged(IResourceChangeEvent event) { - number++; + number.incrementAndGet(); } }