From c8e1aba67f7e174f4bedc25a013b754cb887d1a0 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Mon, 10 Nov 2025 09:26:04 +0100 Subject: [PATCH 1/2] Fix flaky HoverTest.testEnabledWhenHover() race condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The testEnabledWhenHover test was failing randomly when trying to retrieve the hover shell for the second part of the test. Root cause: - The test has two parts: first with EnabledPropertyTester.setEnabled(true), then with setEnabled(false) - After the first hover is shown and checked, cleanFileAndEditor() is called - However, the hover shell from the first part may not be fully disposed when the second editor is opened and the second hover is triggered - This causes getHoverShell() to timeout waiting for the new hover shell Fix: - Capture a reference to the first hover shell before calling cleanFileAndEditor() - After cleanFileAndEditor(), explicitly wait for the first shell to be disposed using DisplayHelper.waitForCondition() with a 3000ms timeout - This ensures the hover state is fully reset before the second part begins This approach follows the pattern used in other recent flaky test fixes in this repository (e.g., ProgressContantsTest, ProgressViewTests) which use condition-based waiting to ensure proper cleanup between test phases. Fixes https://github.com/eclipse-platform/eclipse.platform.ui/issues/926 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../src/org/eclipse/ui/genericeditor/tests/HoverTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/HoverTest.java b/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/HoverTest.java index 8bf2aa2fa41..a52be637056 100644 --- a/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/HoverTest.java +++ b/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/HoverTest.java @@ -33,6 +33,7 @@ import org.eclipse.swt.custom.StyledText; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Event; import org.eclipse.swt.widgets.Label; import org.eclipse.swt.widgets.Link; @@ -80,7 +81,13 @@ public void testEnabledWhenHover(TestInfo info) throws Exception { assertNotNull(findControl(shell, StyledText.class, AlrightyHoverProvider.LABEL)); assertNull(findControl(shell, StyledText.class, WorldHoverProvider.LABEL)); + // Capture the first shell and its display to wait for disposal + Shell firstShell = shell; + Display display = firstShell.getDisplay(); cleanFileAndEditor(); + // Wait for the hover shell to be disposed after editor cleanup + DisplayHelper.waitForCondition(display, 3000, () -> firstShell.isDisposed()); + EnabledPropertyTester.setEnabled(false); createAndOpenFile("enabledWhen.txt", "bar 'bar'"); shell= getHoverShell(info, triggerCompletionAndRetrieveInformationControlManager(), true); From 4f377ac91a313664843238e47f539f8664996295 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Tue, 11 Nov 2025 07:34:10 +0100 Subject: [PATCH 2/2] Fix flaky DynamicTest.testDynamicRegistry race condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The testDynamicRegistry test was failing randomly with "Activity Listener not called" or "Category Listener not called" errors. Root cause: - The test uses a plain boolean array to track listener invocations - Extension registry listeners may be invoked on different threads - Without proper synchronization (volatile/atomic), changes to the array made by listener threads may not be visible to the test thread - This causes DisplayHelper.waitForCondition() to timeout even when listeners were actually called Fix: - Replace boolean[] with AtomicBoolean instances for thread-safe visibility - Use activityChanged.set(true) and categoryChanged.set(true) in listeners - Use activityChanged.get() and categoryChanged.get() in assertions - AtomicBoolean provides proper memory barriers ensuring cross-thread visibility This approach ensures proper synchronization between the listener callback threads and the test thread, eliminating the race condition that caused random test failures. Fixes https://github.com/eclipse-platform/eclipse.platform.ui/issues/2458 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../eclipse/ui/tests/activities/DynamicTest.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/activities/DynamicTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/activities/DynamicTest.java index e39e3314405..632e3bab03a 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/activities/DynamicTest.java +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/activities/DynamicTest.java @@ -23,6 +23,7 @@ import java.nio.charset.StandardCharsets; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import org.eclipse.core.internal.registry.ExtensionRegistry; import org.eclipse.core.runtime.ContributorFactoryOSGi; @@ -412,14 +413,15 @@ public void testDynamicRegistry() throws NotDefinedException { assertFalse(category.isDefined()); // set to true when the activity/category in question have had an event // fired - final boolean[] registryChanged = new boolean[] { false, false }; + final AtomicBoolean activityChanged = new AtomicBoolean(false); + final AtomicBoolean categoryChanged = new AtomicBoolean(false); activity.addActivityListener(activityEvent -> { System.err.println("activityChanged"); - registryChanged[0] = true; + activityChanged.set(true); }); category.addCategoryListener(categoryEvent -> { System.err.println("categoryChanged"); - registryChanged[1] = true; + categoryChanged.set(true); }); @@ -442,10 +444,10 @@ public void testDynamicRegistry() throws NotDefinedException { // spin the event loop and ensure that the changes come down the pipe. // 20 seconds should be more than enough DisplayHelper.waitForCondition(PlatformUI.getWorkbench().getDisplay(), 20000, - () -> registryChanged[0] && registryChanged[1]); + () -> activityChanged.get() && categoryChanged.get()); - assertTrue("Activity Listener not called", registryChanged[0]); - assertTrue("Category Listener not called", registryChanged[1]); + assertTrue("Activity Listener not called", activityChanged.get()); + assertTrue("Category Listener not called", categoryChanged.get()); assertTrue(activity.isDefined()); Set patternBindings = activity.getActivityPatternBindings();