Migrate RCPTestWorkbenchAdvisor fields to non-static #1517

Converted static fields in RCPTestWorkbenchAdvisor to instance fields
to avoid shared state issues between test runs. This ensures each
test execution uses a fresh advisor instance with its own state.
Updated PlatformUITest to access these fields via the advisor instance.

Author: vogella

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

@@ -36,25 +36,23 @@
  */
 public class RCPTestWorkbenchAdvisor extends WorkbenchAdvisor {
 
-	public static Boolean asyncDuringStartup = null;
+	public Boolean asyncDuringStartup = null;
 
 	// the following fields are set by the threads that attempt sync/asyncs
 	// during startup.
-	public static volatile Boolean syncWithDisplayAccess = null;
-	public static volatile Boolean asyncWithDisplayAccess = null;
-	public static volatile Boolean syncWithoutDisplayAccess = null;
-	public static volatile Boolean asyncWithoutDisplayAccess = null;
+	public volatile Boolean syncWithDisplayAccess = null;
+	public volatile Boolean asyncWithDisplayAccess = null;
+	public volatile Boolean syncWithoutDisplayAccess = null;
+	public volatile Boolean asyncWithoutDisplayAccess = null;
 
-	private static boolean started = false;
+	private boolean started = false;
 
 	// CountDownLatch to wait for async/sync operations with DisplayAccess to complete
 	// We need to wait for 2 operations: asyncWithDisplayAccess and syncWithDisplayAccess
-	private static CountDownLatch displayAccessLatch = null;
+	private CountDownLatch displayAccessLatch = null;
 
-	public static boolean isSTARTED() {
-		synchronized (RCPTestWorkbenchAdvisor.class) {
-			return started;
-		}
+	public synchronized boolean isStarted() {
+		return started;
 	}
 
 	/** Default value of -1 causes the option to be ignored. */
@@ -66,7 +64,7 @@ public static boolean isSTARTED() {
 	 * Traps whether or not calls to displayAccess in the UI thread resulted in
 	 * an exception. Should be false.
 	 */
-	public static boolean displayAccessInUIThreadAllowed;
+	public boolean displayAccessInUIThreadAllowed;
 
 	public RCPTestWorkbenchAdvisor() {
 		// default value means the advisor will not trigger the workbench to
@@ -127,15 +125,6 @@ public void eventLoopIdle(final Display display) {
 
 	@Override
 	public void preStartup() {
-		synchronized (RCPTestWorkbenchAdvisor.class) {
-			started = false;
-		}
-		asyncDuringStartup = null;
-		syncWithDisplayAccess = null;
-		asyncWithDisplayAccess = null;
-		syncWithoutDisplayAccess = null;
-		asyncWithoutDisplayAccess = null;
-
 		super.preStartup();
 		final Display display = Display.getCurrent();
 
@@ -144,14 +133,14 @@ public void preStartup() {
 
 		if (display != null) {
 			display.asyncExec(() -> {
-				asyncDuringStartup = !isSTARTED();
+				asyncDuringStartup = !isStarted();
 			});
 		}
 
 		// start a bunch of threads that are going to do a/sync execs. For some
 		// of them, call DisplayAccess.accessDisplayDuringStartup. For others,
 		// dont. Those that call this method should have their runnables invoked
-		// prior to the method isSTARTED returning true.
+		// prior to the method isStarted returning true.
 
 		setupAsyncDisplayThread(true, display);
 		setupSyncDisplayThread(true, display);
@@ -176,13 +165,13 @@ public void run() {
 				try {
 					display.syncExec(() -> {
 						if (callDisplayAccess) {
-							syncWithDisplayAccess = !isSTARTED();
+							syncWithDisplayAccess = !isStarted();
 							// Count down after the runnable executes
 							if (displayAccessLatch != null) {
 								displayAccessLatch.countDown();
 							}
 						} else {
-							syncWithoutDisplayAccess = !isSTARTED();
+							syncWithoutDisplayAccess = !isStarted();
 						}
 					});
 				} catch (SWTException e) {
@@ -207,13 +196,13 @@ public void run() {
 					DisplayAccess.accessDisplayDuringStartup();
 				display.asyncExec(() -> {
 					if (callDisplayAccess) {
-						asyncWithDisplayAccess = !isSTARTED();
+						asyncWithDisplayAccess = !isStarted();
 						// Count down after the runnable executes
 						if (displayAccessLatch != null) {
 							displayAccessLatch.countDown();
 						}
 					} else {
-						asyncWithoutDisplayAccess = !isSTARTED();
+						asyncWithoutDisplayAccess = !isStarted();
 					}
 				});
 			}
@@ -243,8 +232,8 @@ public void postStartup() {
 
 		// Now mark as started - operations with DisplayAccess should have completed
 		// Operations without DisplayAccess should still be pending (deferred)
-		synchronized (RCPTestWorkbenchAdvisor.class) {
+		synchronized (this) {
 			started = true;
 		}
 	}
-}
+}

tests/org.eclipse.ui.tests.rcp/Eclipse RCP Tests/org/eclipse/ui/tests/rcp/PlatformUITest.java

@@ -61,20 +61,20 @@ public void testCreateAndRunWorkbench() {
 		assertTrue(display.isDisposed());
 
 		assertAll(//
-				() -> assertFalse(RCPTestWorkbenchAdvisor.asyncDuringStartup,
+				() -> assertFalse(wa.asyncDuringStartup,
 						"Async run during startup.  See RCPTestWorkbenchAdvisor.preStartup()"),
 				// the following four asserts test the various combinations of Thread +
 				// DisplayAccess + a/sync exec. Anything without a call to DisplayAccess
 				// should be deferred until after startup.
-				() -> assertTrue(RCPTestWorkbenchAdvisor.syncWithDisplayAccess,
+				() -> assertTrue(wa.syncWithDisplayAccess,
 						"Sync from qualified thread did not run during startup.  See RCPTestWorkbenchAdvisor.preStartup()"),
-				() -> assertTrue(RCPTestWorkbenchAdvisor.asyncWithDisplayAccess,
+				() -> assertTrue(wa.asyncWithDisplayAccess,
 						"Async from qualified thread did not run during startup.  See RCPTestWorkbenchAdvisor.preStartup()"),
-				() -> assertFalse(RCPTestWorkbenchAdvisor.syncWithoutDisplayAccess,
+				() -> assertFalse(wa.syncWithoutDisplayAccess,
 						"Sync from un-qualified thread ran during startup.  See RCPTestWorkbenchAdvisor.preStartup()"),
-				() -> assertFalse(RCPTestWorkbenchAdvisor.asyncWithoutDisplayAccess,
+				() -> assertFalse(wa.asyncWithoutDisplayAccess,
 						"Async from un-qualified thread ran during startup.  See RCPTestWorkbenchAdvisor.preStartup()"),
-				() -> assertFalse(RCPTestWorkbenchAdvisor.displayAccessInUIThreadAllowed,
+				() -> assertFalse(wa.displayAccessInUIThreadAllowed,
 						"DisplayAccess.accessDisplayDuringStartup() in UI thread did not result in exception."));
 	}