Skip to content

Commit 43d06c5

Browse files
committed
Avoid reading org.eclipse.swt.* properties too early
The value of org.eclipse.swt.internal.gdk.backend is not set until a display is created at least once since this value is assigned in the Display constructor. To be able to detect if Display has been created, set "org.eclipse.swt.internal.gdk.backend" on wayland too. A similar problem exists for org.eclipse.swt.internal.gtk.version in relation to OS. This is a fix for a regression caused by moving the setting of org.eclipse.swt.internal.gdk.backend from OS to Display in 5d67ce6. That commit fixed an unrelated bug, but sometimes bad initialization of isX11 is an unintended side effect causing tests to be skipped or run in unexpected ways since isX11 was not correct. See for example #2592 for a test that was silently being skipped and had bitrotten as a result. Also removes comment on related code that didn't say anything beyond what the code on the next line did.
1 parent 6fa4ee7 commit 43d06c5

File tree

5 files changed

+58
-14
lines changed

5 files changed

+58
-14
lines changed

bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1209,8 +1209,13 @@ void createDisplay (DeviceData data) {
12091209
themeDark = checkAndSetThemeDetails(themeName);
12101210
if (OS.isX11()) {
12111211
xDisplay = GTK.GTK4 ? 0 : GDK.gdk_x11_get_default_xdisplay();
1212-
// set GDK backend if we are on X11
12131212
System.setProperty("org.eclipse.swt.internal.gdk.backend", "x11");
1213+
} else if (OS.isWayland()) {
1214+
System.setProperty("org.eclipse.swt.internal.gdk.backend", "wayland");
1215+
} else {
1216+
// If other items are added in the future this should be changed
1217+
// from unknown to real value.
1218+
System.setProperty("org.eclipse.swt.internal.gdk.backend", "unknown");
12141219
}
12151220
if (OS.SWT_DEBUG) Device.DEBUG = true;
12161221
if (!GTK.GTK4) {

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,49 @@ public class SwtTestUtil {
100100
/** Useful if you want some tests not to run on Jenkins with user "genie.platform" */
101101
public final static boolean isRunningOnContinousIntegration = isGTK && ("genie.platform".equalsIgnoreCase(System.getProperty("user.name")));
102102

103-
public final static boolean isX11 = isGTK
104-
&& "x11".equals(System.getProperty("org.eclipse.swt.internal.gdk.backend"));
105-
public final static boolean isGTK4 = isGTK
106-
&& System.getProperty("org.eclipse.swt.internal.gtk.version", "").startsWith("4");
103+
/**
104+
* Return whether running on x11. This is dynamically set at runtime and cannot
105+
* be accessed before the corresponding property is initialized in Display.
106+
*
107+
* <strong>Note:</strong> this method still must be called after the first
108+
* Display is created to be valid
109+
*/
110+
public final static boolean isX11() {
111+
if (!isGTK) {
112+
return false;
113+
}
114+
String backend = System.getProperty("org.eclipse.swt.internal.gdk.backend");
115+
116+
if ("x11".equals(backend)) {
117+
return true;
118+
} else if ("wayland".equals(backend)) {
119+
return false;
120+
}
121+
fail("org.eclipse.swt.internal.gdk.backend System property is not set yet. Create a new Display before calling isX11");
122+
throw new IllegalStateException("unreachable");
123+
}
124+
125+
/**
126+
* Return whether running on GTK4. This is dynamically set at runtime and cannot
127+
* be accessed before the corresponding property is initialized in OS.
128+
*
129+
* <strong>Note:</strong> this method still must be called after the static
130+
* block of OS is run.
131+
*/
132+
public final static boolean isGTK4() {
133+
if (!isGTK) {
134+
return false;
135+
}
136+
137+
String version = System.getProperty("org.eclipse.swt.internal.gtk.version", "");
138+
if (version.startsWith("4")) {
139+
return true;
140+
} else if (!version.isBlank()) {
141+
return false;
142+
}
143+
fail("org.eclipse.swt.internal.gtk.version System property is not set yet. Create a new Display (or otherwise access OS) before calling isGTK4");
144+
throw new IllegalStateException("unreachable");
145+
}
107146

108147
/**
109148
* The palette used by images. See {@link #getAllPixels(Image)} and {@link #createImage}

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_dnd_Clipboard.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public void setUp() {
8080
}
8181

8282
private void sleep() throws InterruptedException {
83-
if (SwtTestUtil.isGTK4) {
83+
if (SwtTestUtil.isGTK4()) {
8484
/**
8585
* TODO remove all uses of sleep and change them to processEvents with the
8686
* suitable conditional, or entirely remove them
@@ -351,7 +351,7 @@ public void test_setContents() throws Exception {
351351
String result = runOperationInThread(remote::getStringContents);
352352
assertEquals(helloWorld, result);
353353
} catch (Exception | AssertionError e) {
354-
if (SwtTestUtil.isGTK4 && !SwtTestUtil.isX11) {
354+
if (SwtTestUtil.isGTK4() && !SwtTestUtil.isX11()) {
355355
// TODO make the code + test stable
356356
throw new RuntimeException(
357357
"This test is really unstable on wayland backend, at least with Ubuntu 25.04", e);

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_Display.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,7 +1247,7 @@ public void test_setCursorLocationII(TestInfo info) {
12471247
display.setCursorLocation(location.x, location.y); // don't put cursor into a corner, since that could trigger special platform events
12481248
drainEventQueue(display, 150); // workaround for https://bugs.eclipse.org/492569
12491249
Point actual = display.getCursorLocation();
1250-
if (!BUG_492569 && SwtTestUtil.isX11) {
1250+
if (!BUG_492569 && SwtTestUtil.isX11()) {
12511251
if (!location.equals(actual)) {
12521252
Screenshots.takeScreenshot(getClass(), info.getDisplayName()); // Bug 528968 This call causes crash on Wayland.
12531253
fail("\nExpected:"+location.toString()+" Actual:"+actual.toString());
@@ -1279,7 +1279,7 @@ public void test_setCursorLocationLorg_eclipse_swt_graphics_Point(TestInfo info)
12791279
}
12801280
drainEventQueue(display, 150); // workaround for https://bugs.eclipse.org/492569
12811281
Point actual = display.getCursorLocation();
1282-
if (!BUG_492569 && SwtTestUtil.isX11) {
1282+
if (!BUG_492569 && SwtTestUtil.isX11()) {
12831283
if (!location.equals(actual)) {
12841284
Screenshots.takeScreenshot(getClass(), info.getDisplayName()); // Bug 528968 This call causes crash on Wayland.
12851285
fail("\nExpected:"+location.toString()+" Actual:"+actual.toString());

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_Shell.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ public void test_getImeInputMode() {
460460
@Test
461461
public void test_getLocation() {
462462
//Setting location for Windows is not supported in GTK4
463-
if (SwtTestUtil.isGTK4) {
463+
if (SwtTestUtil.isGTK4()) {
464464
return;
465465
}
466466
shell.setLocation(10,15);
@@ -656,7 +656,7 @@ public void test_setBoundsLorg_eclipse_swt_graphics_Rectangle() {
656656
*/
657657
@Test
658658
public void test_activateEventSend() throws InterruptedException {
659-
assumeTrue((SwtTestUtil.isGTK && SwtTestUtil.isX11) || SwtTestUtil.isGTK4,
659+
assumeTrue((SwtTestUtil.isGTK && SwtTestUtil.isX11()) || SwtTestUtil.isGTK4(),
660660
"Feature only works on GTK3 in x11 or GTK4 - https://bugs.eclipse.org/436841");
661661

662662
AtomicBoolean activateCalled = new AtomicBoolean();
@@ -708,7 +708,7 @@ public void test_activateEventSend() throws InterruptedException {
708708
*/
709709
@Test
710710
public void test_setBounds() throws Exception {
711-
if (SwtTestUtil.isX11) {
711+
if (SwtTestUtil.isX11()) {
712712
Rectangle bounds = new Rectangle(100, 200, 200, 200);
713713
Rectangle bounds2 = new Rectangle(150, 250, 250, 250);
714714

@@ -1037,7 +1037,7 @@ public void test_Issue450_NoShellActivateOnSetFocus() {
10371037
@Override
10381038
public void test_setLocationLorg_eclipse_swt_graphics_Point() {
10391039
//Setting location for Windows is not supported in GTK4
1040-
if (SwtTestUtil.isGTK4) {
1040+
if (SwtTestUtil.isGTK4()) {
10411041
return;
10421042
}
10431043
super.test_setLocationLorg_eclipse_swt_graphics_Point();
@@ -1047,7 +1047,7 @@ public void test_setLocationLorg_eclipse_swt_graphics_Point() {
10471047
@Override
10481048
public void test_setLocationII() {
10491049
//Setting location for Windows is not supported in GTK4
1050-
if (SwtTestUtil.isGTK4) {
1050+
if (SwtTestUtil.isGTK4()) {
10511051
return;
10521052
}
10531053
super.test_setLocationII();

0 commit comments

Comments
 (0)