Merge remote-tracking branch 'origin/flacky-test-RCP-test-advisor' into fix-flaky-progress-view-test

Author: vogella

tests/org.eclipse.ui.tests.harness/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: Harness Plug-in
 Bundle-SymbolicName: org.eclipse.ui.tests.harness;singleton:=true
-Bundle-Version: 1.10.600.qualifier
+Bundle-Version: 1.10.700.qualifier
 Eclipse-BundleShape: dir
 Require-Bundle: org.eclipse.ui,
  org.eclipse.core.runtime,

tests/org.eclipse.ui.tests.harness/src/org/eclipse/ui/tests/harness/util/RCPTestWorkbenchAdvisor.java

@@ -13,6 +13,10 @@
  *******************************************************************************/
 package org.eclipse.ui.tests.harness.util;
 
+import java.util.concurrent.Phaser;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
 import org.eclipse.jface.preference.IPreferenceStore;
 import org.eclipse.swt.SWTException;
 import org.eclipse.swt.widgets.Display;
@@ -44,6 +48,10 @@ public class RCPTestWorkbenchAdvisor extends WorkbenchAdvisor {
 
 	private static boolean started = false;
 
+	// Use a Phaser to ensure async operations are scheduled before postStartup
+	// Each thread registers itself and arrives when the operation is scheduled
+	private static final Phaser asyncPhaser = new Phaser(1); // 1 for the main thread
+
 	public static boolean isSTARTED() {
 		synchronized (RCPTestWorkbenchAdvisor.class) {
 			return started;
@@ -122,12 +130,10 @@ public void eventLoopIdle(final Display display) {
 	public void preStartup() {
 		super.preStartup();
 		final Display display = Display.getCurrent();
+
 		if (display != null) {
 			display.asyncExec(() -> {
-				if (isSTARTED())
-					asyncDuringStartup = Boolean.FALSE;
-				else
-					asyncDuringStartup = Boolean.TRUE;
+				asyncDuringStartup = !isSTARTED();
 			});
 		}
 
@@ -151,24 +157,28 @@ public void preStartup() {
 	}
 
 	private void setupSyncDisplayThread(final boolean callDisplayAccess, final Display display) {
+		// Register this thread with the phaser
+		asyncPhaser.register();
 		Thread syncThread = new Thread() {
 			@Override
 			public void run() {
 				if (callDisplayAccess)
 					DisplayAccess.accessDisplayDuringStartup();
 				try {
 					display.syncExec(() -> {
-						synchronized (RCPTestWorkbenchAdvisor.class) {
-							if (callDisplayAccess)
-								syncWithDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
-							else
-								syncWithoutDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
+						if (callDisplayAccess) {
+							syncWithDisplayAccess = !isSTARTED();
+						} else {
+							syncWithoutDisplayAccess = !isSTARTED();
 						}
 					});
 				} catch (SWTException e) {
 					// this can happen because we shut down the workbench just
 					// as soon as we're initialized - ie: when we're trying to
 					// run this runnable in the deferred case.
+				} finally {
+					// Signal that this operation has completed
+					asyncPhaser.arriveAndDeregister();
 				}
 			}
 		};
@@ -177,19 +187,26 @@ public void run() {
 	}
 
 	private void setupAsyncDisplayThread(final boolean callDisplayAccess, final Display display) {
+		// Register this thread with the phaser
+		asyncPhaser.register();
 		Thread asyncThread = new Thread() {
 			@Override
 			public void run() {
 				if (callDisplayAccess)
 					DisplayAccess.accessDisplayDuringStartup();
-				display.asyncExec(() -> {
-					synchronized (RCPTestWorkbenchAdvisor.class) {
-						if (callDisplayAccess)
-							asyncWithDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
-						else
-							asyncWithoutDisplayAccess = !isSTARTED() ? Boolean.TRUE : Boolean.FALSE;
-					}
-				});
+				try {
+					display.asyncExec(() -> {
+						if (callDisplayAccess) {
+							asyncWithDisplayAccess = !isSTARTED();
+						} else {
+							asyncWithoutDisplayAccess = !isSTARTED();
+						}
+					});
+				} finally {
+					// Signal that this operation has been scheduled (not necessarily executed yet)
+					// This avoids deadlock since we're not waiting for execution on the UI thread
+					asyncPhaser.arriveAndDeregister();
+				}
 			}
 		};
 		asyncThread.setDaemon(true);
@@ -199,6 +216,29 @@ public void run() {
 	@Override
 	public void postStartup() {
 		super.postStartup();
+
+		// Wait for all async/sync operations to be scheduled (not blocking UI thread)
+		try {
+			// Wait up to 5 seconds for all operations to be scheduled
+			// The main thread arrives and deregisters, waiting for all other registered threads
+			asyncPhaser.awaitAdvanceInterruptibly(asyncPhaser.arrive(), 5, TimeUnit.SECONDS);
+		} catch (TimeoutException e) {
+			// Log warning but don't throw - we need to mark as started to avoid breaking subsequent tests
+			System.err.println("WARNING: Not all async/sync operations were scheduled within timeout");
+			e.printStackTrace();
+		} catch (InterruptedException e) {
+			Thread.currentThread().interrupt();
+			System.err.println("WARNING: Interrupted while waiting for async/sync operations");
+		}
+
+		// Pump the event loop to ensure async runnables execute before marking as started
+		// This prevents the original race condition where async variables might not be set yet
+		// Wait until the variables that should be set during startup are actually set
+		UITestUtil.processEventsUntil(() -> syncWithDisplayAccess != null && asyncWithDisplayAccess != null, 5000);
+		// Process any remaining events to allow variables that should NOT be set during startup
+		// to accidentally execute (to detect regression)
+		UITestUtil.processEvents();
+
 		synchronized (RCPTestWorkbenchAdvisor.class) {
 			started = true;
 		}