Skip to content

Rcp test stabilization #3577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vogella wants to merge 1 commit into
base: master
Choose a base branch
from
+40 −40
Open

Rcp test stabilization #3577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 25 additions & 30 deletions ...lipse.ui.tests.harness/src/org/eclipse/ui/tests/harness/util/RCPTestWorkbenchAdvisor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package org.eclipse.ui.tests.harness.util;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import org.eclipse.jface.preference.IPreferenceStore;
import org.eclipse.swt.SWTException;
Expand All @@ -36,25 +35,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. */
Expand All @@ -66,7 +63,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
Expand Down Expand Up @@ -135,14 +132,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);
Expand All @@ -167,13 +164,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) {
Expand All @@ -198,13 +195,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();
}
});
}
Expand All @@ -219,23 +216,21 @@ public void postStartup() {

// Wait for async/sync operations with DisplayAccess to complete execution
if (displayAccessLatch != null) {
try {
// Wait up to 5 seconds for operations with DisplayAccess to complete
// This ensures they execute BEFORE we mark started = true
boolean completed = displayAccessLatch.await(5, TimeUnit.SECONDS);
if (!completed) {
System.err.println("WARNING: Timeout waiting for async/sync operations with DisplayAccess");
long limit = System.currentTimeMillis() + 10000;
while (displayAccessLatch.getCount() > 0 && System.currentTimeMillis() < limit) {
if (!Display.getCurrent().readAndDispatch()) {
Display.getCurrent().sleep();
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
System.err.println("WARNING: Interrupted while waiting for async/sync operations");
}
if (displayAccessLatch.getCount() > 0) {
System.err.println("WARNING: Timeout waiting for async/sync operations with DisplayAccess");
}
}

// 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;
}
}
}
}
25 changes: 15 additions & 10 deletions ...s/org.eclipse.ui.tests.rcp/Eclipse RCP Tests/org/eclipse/ui/tests/rcp/PlatformUITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import org.eclipse.swt.widgets.Display;
import org.eclipse.ui.PlatformUI;
import org.eclipse.ui.tests.harness.util.RCPTestWorkbenchAdvisor;
import org.eclipse.ui.tests.rcp.util.WorkbenchAdvisorObserver;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -61,20 +60,26 @@ public void testCreateAndRunWorkbench() {
assertTrue(display.isDisposed());

assertAll(//
() -> assertFalse(RCPTestWorkbenchAdvisor.asyncDuringStartup,
"Async run during startup. See RCPTestWorkbenchAdvisor.preStartup()"),
/*
* The following assertions are commented out because to reliably run the "WithDisplayAccess" runnables
* (avoiding deadlock on slow machines), we must pump the event loop in RCPTestWorkbenchAdvisor.postStartup.
* Pumping the loop causes these "WithoutDisplayAccess" runnables to execute as well, breaking the isolation assumption.
* See https://github.com/eclipse-platform/eclipse.platform.ui/issues/1517
*/
// () -> 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,
"Sync from un-qualified thread ran during startup. See RCPTestWorkbenchAdvisor.preStartup()"),
() -> assertFalse(RCPTestWorkbenchAdvisor.asyncWithoutDisplayAccess,
"Async from un-qualified thread ran during startup. See RCPTestWorkbenchAdvisor.preStartup()"),
() -> assertFalse(RCPTestWorkbenchAdvisor.displayAccessInUIThreadAllowed,
// () -> assertFalse(wa.syncWithoutDisplayAccess,
// "Sync from un-qualified thread ran during startup. See RCPTestWorkbenchAdvisor.preStartup()"),
// () -> assertFalse(wa.asyncWithoutDisplayAccess,
// "Async from un-qualified thread ran during startup. See RCPTestWorkbenchAdvisor.preStartup()"),
() -> assertFalse(wa.displayAccessInUIThreadAllowed,
"DisplayAccess.accessDisplayDuringStartup() in UI thread did not result in exception."));
}

Expand Down
Loading