Skip to content

Commit bf9d370

Browse files
committed
Convert Zest tests into proper SWTBot tests
The current approach of executing the Zest-SWTBot tests is prone to timing issues when interacting with the Draw2D Animation class. To avoid this, the test suite itself should be executed in a separate thread and all UI access should be done using the SWTBot API. Note that SWTBot doesn't provide a Zest API, so a prototype defining the required methods has been created as well.
1 parent c030f5e commit bf9d370

15 files changed

+1110
-569
lines changed

org.eclipse.zest.tests/pom.xml

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,36 @@
4444
<groupId>org.eclipse.tycho</groupId>
4545
<artifactId>tycho-surefire-plugin</artifactId>
4646
<version>${tycho-version}</version>
47+
<executions>
48+
<execution>
49+
<!-- Overwrites default goal! -->
50+
<id>default-test</id>
51+
<goals>
52+
<goal>test</goal>
53+
</goals>
54+
<configuration>
55+
<includes>
56+
<include>**/ZestTestSuite.class</include>
57+
</includes>
58+
<useUIHarness>false</useUIHarness>
59+
<useUIThread>false</useUIThread>
60+
</configuration>
61+
</execution>
62+
<execution>
63+
<id>swtbot-test</id>
64+
<goals>
65+
<goal>test</goal>
66+
</goals>
67+
<configuration>
68+
<includes>
69+
<include>**/SWTBotTestSuite.class</include>
70+
</includes>
71+
<useUIHarness>true</useUIHarness>
72+
<useUIThread>false</useUIThread>
73+
</configuration>
74+
</execution>
75+
</executions>
4776
<configuration>
48-
<includes>
49-
<include>**/ZestTestSuite.class</include>
50-
</includes>
51-
<useUIHarness>false</useUIHarness>
52-
<useUIThread>false</useUIThread>
5377
<argLine>${test.vmargs}</argLine>
5478
</configuration>
5579
</plugin>

org.eclipse.zest.tests/src/org/eclipse/zest/tests/ZestTestSuite.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@
1414

1515
import org.eclipse.zest.tests.cloudio.TagCloudTests;
1616
import org.eclipse.zest.tests.cloudio.TagCloudViewerTests;
17-
import org.eclipse.zest.tests.examples.GraphJFaceTests;
18-
import org.eclipse.zest.tests.examples.GraphSWTTests;
19-
import org.eclipse.zest.tests.examples.GraphUMLTests;
2017

2118
import org.junit.platform.suite.api.SelectClasses;
2219
import org.junit.platform.suite.api.Suite;
@@ -33,9 +30,6 @@
3330
GraphViewerTests.class,
3431
LayoutAlgorithmTest.class,
3532
LayoutAlgorithmTests.class,
36-
GraphJFaceTests.class,
37-
GraphSWTTests.class,
38-
GraphUMLTests.class,
3933
TagCloudTests.class,
4034
TagCloudViewerTests.class
4135
})

org.eclipse.zest.tests/src/org/eclipse/zest/tests/examples/AbstractGraphTest.java

Lines changed: 68 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import static org.junit.jupiter.api.Assertions.assertEquals;
1717
import static org.junit.jupiter.api.Assertions.assertFalse;
1818
import static org.junit.jupiter.api.Assertions.assertTrue;
19+
import static org.junit.jupiter.api.Assertions.fail;
1920

2021
import java.lang.invoke.MethodHandle;
2122
import java.lang.invoke.MethodHandles;
@@ -32,14 +33,18 @@
3233
import org.eclipse.swt.widgets.Shell;
3334

3435
import org.eclipse.core.runtime.Assert;
36+
import org.eclipse.swtbot.swt.finder.SWTBot;
37+
import org.eclipse.swtbot.swt.finder.finders.UIThreadRunnable;
3538
import org.eclipse.zest.core.widgets.Graph;
3639
import org.eclipse.zest.core.widgets.GraphConnection;
3740
import org.eclipse.zest.core.widgets.GraphNode;
38-
import org.eclipse.zest.core.widgets.IContainer;
3941
import org.eclipse.zest.core.widgets.internal.GraphLabel;
4042
import org.eclipse.zest.layouts.algorithms.SpringLayoutAlgorithm;
4143
import org.eclipse.zest.tests.examples.AbstractGraphTest.SWTBotExtension;
42-
import org.eclipse.zest.tests.utils.GraphicalRobot;
44+
import org.eclipse.zest.tests.utils.ISWTBotGraphContainer;
45+
import org.eclipse.zest.tests.utils.SWTBotGraph;
46+
import org.eclipse.zest.tests.utils.SWTBotGraphConnection;
47+
import org.eclipse.zest.tests.utils.SWTBotGraphNode;
4348
import org.eclipse.zest.tests.utils.Snippet;
4449

4550
import org.eclipse.draw2d.EventDispatcher;
@@ -53,6 +58,7 @@
5358
import org.eclipse.draw2d.geometry.Point;
5459
import org.eclipse.draw2d.geometry.Rectangle;
5560

61+
import org.junit.jupiter.api.AfterEach;
5662
import org.junit.jupiter.api.extension.ExtendWith;
5763
import org.junit.jupiter.api.extension.ExtensionContext;
5864
import org.junit.jupiter.api.extension.InvocationInterceptor;
@@ -64,8 +70,10 @@
6470
*/
6571
@ExtendWith(SWTBotExtension.class)
6672
public abstract class AbstractGraphTest {
67-
protected Graph graph;
68-
protected GraphicalRobot robot;
73+
private Graph graph;
74+
private Shell shell;
75+
protected SWTBot robot;
76+
protected SWTBotGraph graphRobot;
6977

7078
public static class SWTBotExtension implements InvocationInterceptor {
7179
@Override
@@ -81,6 +89,13 @@ public void interceptTestMethod(Invocation<Void> invocation,
8189
}
8290
}
8391

92+
@AfterEach
93+
public void tearDown() {
94+
if (shell != null) {
95+
UIThreadRunnable.syncExec(shell::dispose);
96+
}
97+
}
98+
8499
/**
85100
* Wrapper method to handle the instantiation of the example class and the
86101
* execution of the unit test. Each example must satisfy the following
@@ -95,6 +110,13 @@ public void interceptTestMethod(Invocation<Void> invocation,
95110
* @throws Throwable If the example could not be instantiated.
96111
*/
97112
private void doTest(Snippet annotation, Invocation<Void> statement) throws Throwable {
113+
if (Display.getCurrent() != null) {
114+
fail("""
115+
SWTBot test needs to run in a non-UI thread.
116+
Make sure that "Run in UI thread" is unchecked in your launch configuration or that useUIThread is set to false in the pom.xml
117+
"""); //$NON-NLS-1$
118+
}
119+
98120
Class<?> clazz = annotation.type();
99121

100122
Semaphore lock = new Semaphore(0);
@@ -107,43 +129,48 @@ private void doTest(Snippet annotation, Invocation<Void> statement) throws Throw
107129
// Fail early, otherwise the example might block indefinitely
108130
Assert.isTrue(hasGraph(lookup, annotation), "Graph object not found for " + clazz); //$NON-NLS-1$
109131

110-
// The actual test has to be executed asynchronously, so that it is run as part
111-
// of the readAndDispatch() call. Otherwise we end up in a deadlock, as both
112-
// snippet and test run in the UI thread.
113-
Display.getCurrent().asyncExec(() -> {
114-
Shell shell = null;
115-
try {
116-
graph = getGraph(lookup, annotation);
117-
118-
// Make sure the layout is reproducible
119-
if (graph.getLayoutAlgorithm() instanceof SpringLayoutAlgorithm springLayout) {
120-
springLayout.setRandom(annotation.random());
132+
// Create snippet
133+
Display.getDefault().asyncExec(() -> {
134+
// Non-Blocking! Executed after the widget has been created
135+
Display.getCurrent().asyncExec(() -> {
136+
try {
137+
graph = getGraph(lookup, annotation);
138+
139+
// Make sure the layout is reproducible
140+
if (graph.getLayoutAlgorithm() instanceof SpringLayoutAlgorithm springLayout) {
141+
springLayout.setRandom(annotation.random());
142+
}
143+
144+
graphRobot = new SWTBotGraph(graph);
145+
shell = graph.getShell();
146+
robot = new SWTBot(shell);
147+
// Wait for layout to be applied
148+
waitEventLoop(10);
149+
} catch (Throwable e) {
150+
throwable.set(e);
151+
} finally {
152+
lock.release();
121153
}
122-
123-
robot = new GraphicalRobot(graph);
124-
shell = graph.getShell();
125-
// Wait for layout to be applied
126-
waitEventLoop(10);
127-
// Run the actual test
128-
statement.proceed();
154+
});
155+
// Blocking! Creates the snippet
156+
try {
157+
methodHandle.invoke(null);
129158
} catch (Throwable e) {
130159
throwable.set(e);
131160
} finally {
132-
// Close the snippet
133-
if (shell != null) {
134-
shell.dispose();
135-
}
136161
lock.release();
137162
}
138163
});
139164

140-
methodHandle.invoke(null);
141-
// Wait for asynchronous test execution
165+
// Wait for shell to be created
142166
lock.acquire();
143167
// Propagate any errors
144168
if (throwable.get() != null) {
145169
throw throwable.get();
146170
}
171+
172+
// Run the actual test
173+
statement.proceed();
147174
}
148175

149176
/**
@@ -174,7 +201,7 @@ private void doTest(Snippet annotation, Invocation<Void> statement) throws Throw
174201
*
175202
* @return The Euclidean length of the given {@code connection}.
176203
*/
177-
protected static double getLength(GraphConnection connection) {
204+
protected static double getLength(SWTBotGraphConnection connection) {
178205
Point c1 = getCenter(connection.getSource());
179206
Point c2 = getCenter(connection.getDestination());
180207
int x = c1.x - c2.x;
@@ -197,7 +224,7 @@ protected static double getLength(Point vec) {
197224
*
198225
* @return The center of the given {@code node}.
199226
*/
200-
protected static Point getCenter(GraphNode node) {
227+
protected static Point getCenter(SWTBotGraphNode node) {
201228
Point location = node.getLocation();
202229
Dimension size = node.getSize();
203230
return new Rectangle(location, size).getCenter();
@@ -264,28 +291,29 @@ protected static void assertConnection(GraphConnection connection, String source
264291
* Asserts that the given {@code connection} uses a {@link PolylineConnection}
265292
* with given {@code curveDepth}.
266293
*
267-
* @param connection The graph connection to validate.
294+
* @param bot The graph connection to validate.
268295
* @param curveDepth The expected curveDepth of the connection
269296
*/
270-
protected static void assertCurve(GraphConnection connection, int curveDepth) throws ReflectiveOperationException {
297+
protected static void assertCurve(SWTBotGraphConnection bot, int curveDepth) throws ReflectiveOperationException {
298+
GraphConnection connection = bot.widget;
271299
MethodHandles.Lookup lookup = MethodHandles.privateLookupIn(connection.getClass(), MethodHandles.lookup());
272300
VarHandle field = lookup.findVarHandle(connection.getClass(), "curveDepth", int.class); //$NON-NLS-1$
273301
assertEquals(curveDepth, field.get(connection), "Unexpected connection curve"); //$NON-NLS-1$
274302
}
275303

276304
/**
277-
* Asserts that no graph nodes in the given {@link IContainer} intersect with
278-
* one another. This method doesn't check nested containers.
305+
* Asserts that no graph nodes in the given {@link ISWTBotGraphContainer}
306+
* intersect with one another. This method doesn't check nested containers.
279307
*
280-
* @param container The {@link IContainer} to validate.
308+
* @param container The {@link ISWTBotGraphContainer} to validate.
281309
*/
282-
protected static void assertNoOverlap(IContainer container) {
283-
List<? extends GraphNode> nodes = container.getNodes();
310+
protected static void assertNoOverlap(ISWTBotGraphContainer container) {
311+
List<SWTBotGraphNode> nodes = container.getNodes();
284312
for (int i = 0; i < nodes.size(); ++i) {
285313
for (int j = i + 1; j < nodes.size(); ++j) {
286-
GraphNode node1 = nodes.get(i);
314+
SWTBotGraphNode node1 = nodes.get(i);
287315
Rectangle bounds1 = new Rectangle(node1.getLocation(), node1.getSize());
288-
GraphNode node2 = nodes.get(j);
316+
SWTBotGraphNode node2 = nodes.get(j);
289317
Rectangle bounds2 = new Rectangle(node2.getLocation(), node2.getSize());
290318
assertFalse(bounds1.intersects(bounds2));
291319
}
@@ -307,7 +335,7 @@ protected static void assertInstanceOf(Object object, Class<?> clazz) {
307335
* Pumps the event loop for the given number of milliseconds. At least one
308336
* events loop will be executed.
309337
*/
310-
protected static void waitEventLoop(int time) {
338+
private static void waitEventLoop(int time) {
311339
long start = System.currentTimeMillis();
312340
do {
313341
while (Display.getCurrent().readAndDispatch()) {

0 commit comments

Comments
 (0)