From 289b4f73354f459c1c2d09883b68ebb4e1ade9ae Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sat, 8 Nov 2025 07:59:35 +0100 Subject: [PATCH 1/2] Fix flapping tests by replacing fixed delays with condition-based waiting --- .../application/ESelectionServiceTest.java | 38 ++++++++++++- .../workbench/PartRenderingEngineTests.java | 31 +++++++++-- .../tests/decorators/DecoratorTableTest.java | 2 - .../tests/decorators/DecoratorTestPart.java | 54 +++++++++++++++---- .../tests/decorators/DecoratorTreeTest.java | 2 - .../ui/tests/internal/Bug78470Test.java | 17 +++++- 6 files changed, 123 insertions(+), 21 deletions(-) diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/ESelectionServiceTest.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/ESelectionServiceTest.java index aa4ff2f9794..6038de4aee7 100644 --- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/ESelectionServiceTest.java +++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/ESelectionServiceTest.java @@ -1011,12 +1011,12 @@ public void testBug343984() throws Exception { selectionService.addSelectionListener(listener); selectionService.setSelection(new Object()); - Thread.sleep(1000); + waitForCondition(() -> listener.success, 5000, "Listener should be called after setSelection"); assertTrue(listener.success); listener.reset(); selectionService.setSelection(new Object()); - Thread.sleep(1000); + waitForCondition(() -> listener.success, 5000, "Listener should be called after second setSelection"); assertTrue(listener.success); } @@ -1064,6 +1064,40 @@ private void initialize() { applicationContext.set(UIEventPublisher.class, ep); } + /** + * Wait for a condition to become true, processing UI events while waiting. + * + * @param condition the condition to wait for + * @param timeoutMillis maximum time to wait in milliseconds + * @param message error message if timeout occurs + */ + private void waitForCondition(java.util.function.BooleanSupplier condition, long timeoutMillis, String message) { + Display display = Display.getCurrent(); + if (display == null) { + display = Display.getDefault(); + } + + long startTime = System.currentTimeMillis(); + while (!condition.getAsBoolean()) { + if (System.currentTimeMillis() - startTime > timeoutMillis) { + throw new AssertionError(message + " (timeout after " + timeoutMillis + "ms)"); + } + + // Process pending UI events + if (display.readAndDispatch()) { + continue; + } + + // Small sleep to avoid busy waiting + try { + Thread.sleep(10); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError(message + " (interrupted)", e); + } + } + } + static class SelectionListener implements ISelectionListener { private MPart part; diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/PartRenderingEngineTests.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/PartRenderingEngineTests.java index d597622a8ee..8f991c38799 100644 --- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/PartRenderingEngineTests.java +++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/PartRenderingEngineTests.java @@ -113,12 +113,33 @@ public void tearDown() throws Exception { } private void checkLog() { - try { - // sleep a bit because notifications are done on another thread - Thread.sleep(100); - } catch (Exception e) { - // ignored + // Wait a bit because notifications are done on another thread + // Use a short timeout to allow log events to be processed + long startTime = System.currentTimeMillis(); + long timeout = 1000; // 1 second max wait + + while (System.currentTimeMillis() - startTime < timeout) { + // Process any pending display events if we're on the UI thread + Display display = Display.getCurrent(); + if (display != null) { + while (display.readAndDispatch()) { + // Keep processing + } + } + + // If already logged, no need to wait longer + if (logged) { + break; + } + + try { + Thread.sleep(10); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; + } } + assertFalse(logged); } diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTableTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTableTest.java index 33187ae65e2..ae8e53d6b44 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTableTest.java +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTableTest.java @@ -18,12 +18,10 @@ import org.eclipse.ui.IViewPart; import org.eclipse.ui.IWorkbenchPage; import org.eclipse.ui.PartInitException; -import org.junit.Ignore; /** * The DecoratorTableTest is the test for decorating tables. */ -@Ignore("Disabled due to timing issues") public class DecoratorTableTest extends DecoratorViewerTest { @Override diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTestPart.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTestPart.java index 076fbd1e143..01bca9894cc 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTestPart.java +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTestPart.java @@ -26,11 +26,14 @@ */ public abstract class DecoratorTestPart extends ViewPart { - private static final int DELAY_TIME = 2000;// Wait 2 seconds + private static final int INITIAL_DELAY_TIME = 500;// Initial wait time in milliseconds + private static final int MAX_WAIT_TIME = 10000;// Maximum wait time (10 seconds) + private static final int IDLE_TIME = 200;// Time to wait after last update public boolean waitingForDecoration = true; - private long endTime; + private volatile long lastUpdateTime; + private long startTime; private ILabelProviderListener listener; @@ -54,22 +57,55 @@ protected DecoratingLabelProvider getLabelProvider() { * Get the listener for the suite. */ private ILabelProviderListener getDecoratorManagerListener() { - // Reset the end time each time we get an update - listener = event -> endTime = System.currentTimeMillis() + DELAY_TIME; + // Record the time each time we get an update + listener = event -> lastUpdateTime = System.currentTimeMillis(); return listener; } + /** + * Process events until decorations are applied. This waits for updates to settle + * by ensuring no new updates occur for IDLE_TIME milliseconds, with a maximum + * wait of MAX_WAIT_TIME. + */ public void readAndDispatchForUpdates() { - while (System.currentTimeMillis() < endTime) { - Display.getCurrent().readAndDispatch(); + Display display = Display.getCurrent(); + long elapsed = System.currentTimeMillis() - startTime; + + // Process events and wait for updates to settle + while (elapsed < MAX_WAIT_TIME) { + // Process any pending UI events + while (display.readAndDispatch()) { + // Keep processing + } + + long timeSinceLastUpdate = System.currentTimeMillis() - lastUpdateTime; + + // If we haven't received an update in IDLE_TIME, we're done + if (timeSinceLastUpdate >= IDLE_TIME && elapsed >= INITIAL_DELAY_TIME) { + break; + } + + // Small sleep to avoid busy waiting + try { + Thread.sleep(50); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; + } + + elapsed = System.currentTimeMillis() - startTime; + } + + // Final event processing pass + while (display.readAndDispatch()) { + // Keep processing } - } public void setUpForDecorators() { - endTime = System.currentTimeMillis() + DELAY_TIME; - + startTime = System.currentTimeMillis(); + lastUpdateTime = System.currentTimeMillis(); } @Override diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTreeTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTreeTest.java index 7510e48c790..ae67e74bcd0 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTreeTest.java +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTreeTest.java @@ -19,13 +19,11 @@ import org.eclipse.ui.IViewPart; import org.eclipse.ui.IWorkbenchPage; import org.eclipse.ui.PartInitException; -import org.junit.Ignore; /** * The DecoratorTreeTest tests the font and color support on * tree viewers. */ -@Ignore("Disabled due to timing issues") public class DecoratorTreeTest extends DecoratorViewerTest { @Override diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/Bug78470Test.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/Bug78470Test.java index 8a3bbaf9251..545975707fc 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/Bug78470Test.java +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/Bug78470Test.java @@ -114,7 +114,22 @@ public void partInputChanged(IWorkbenchPartReference partRef) { }); workbench.showPerspective(MyPerspective.ID, activeWorkbenchWindow); processEvents(); - Thread.sleep(2000); + + // Wait for the part to become visible, processing events + long startTime = System.currentTimeMillis(); + long timeout = 5000; // 5 second max wait + while (!partVisibleExecuted && (System.currentTimeMillis() - startTime < timeout)) { + processEvents(); + if (!partVisibleExecuted) { + try { + Thread.sleep(50); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; + } + } + } + assertTrue("view was not made visible", partVisibleExecuted); assertNotNull(activePage.findView(MyViewPart.ID2)); assertNotNull(activePage.findView(MyViewPart.ID3)); From b07b2a8e732d978c4e244fa74ca78829ade46e98 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sat, 8 Nov 2025 12:59:19 +0100 Subject: [PATCH 2/2] Fix DecoratorTestPart timing bug causing premature test exit --- .../tests/decorators/DecoratorTestPart.java | 46 +++++-------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTestPart.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTestPart.java index 01bca9894cc..49077300b52 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTestPart.java +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/decorators/DecoratorTestPart.java @@ -26,15 +26,8 @@ */ public abstract class DecoratorTestPart extends ViewPart { - private static final int INITIAL_DELAY_TIME = 500;// Initial wait time in milliseconds - private static final int MAX_WAIT_TIME = 10000;// Maximum wait time (10 seconds) - private static final int IDLE_TIME = 200;// Time to wait after last update - public boolean waitingForDecoration = true; - private volatile long lastUpdateTime; - private long startTime; - private ILabelProviderListener listener; public DecoratorTestPart() { @@ -57,55 +50,40 @@ protected DecoratingLabelProvider getLabelProvider() { * Get the listener for the suite. */ private ILabelProviderListener getDecoratorManagerListener() { - // Record the time each time we get an update - listener = event -> lastUpdateTime = System.currentTimeMillis(); + // Listener for decorator manager events + listener = event -> { + // Decorator update occurred + }; return listener; } /** - * Process events until decorations are applied. This waits for updates to settle - * by ensuring no new updates occur for IDLE_TIME milliseconds, with a maximum - * wait of MAX_WAIT_TIME. + * Process events until decorations are applied. Uses a fixed delay + * to allow time for decorator updates to be processed. */ public void readAndDispatchForUpdates() { Display display = Display.getCurrent(); - long elapsed = System.currentTimeMillis() - startTime; - - // Process events and wait for updates to settle - while (elapsed < MAX_WAIT_TIME) { - // Process any pending UI events + // Process events for 1 second with regular intervals + for (int i = 0; i < 20; i++) { while (display.readAndDispatch()) { - // Keep processing - } - - long timeSinceLastUpdate = System.currentTimeMillis() - lastUpdateTime; - - // If we haven't received an update in IDLE_TIME, we're done - if (timeSinceLastUpdate >= IDLE_TIME && elapsed >= INITIAL_DELAY_TIME) { - break; + // Process all pending events } - - // Small sleep to avoid busy waiting try { Thread.sleep(50); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - break; + return; } - - elapsed = System.currentTimeMillis() - startTime; } - - // Final event processing pass + // Final pass to process any remaining events while (display.readAndDispatch()) { // Keep processing } } public void setUpForDecorators() { - startTime = System.currentTimeMillis(); - lastUpdateTime = System.currentTimeMillis(); + // No initialization needed for simple fixed delay approach } @Override