Avoid reading org.eclipse.swt.* properties too early #2591
+60
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I think some of the tests that were supposed to have been guarded by isX11 may have bitrotten, for example test_activateEventSend fails on my machine now that I have fixed the isX11 test. |
It also fails on the build machine. A fix for the unstable test is in #2592 |
jonahgraham
added a commit
to jonahgraham/eclipse.platform.swt
that referenced
this pull request
Oct 4, 2025
test_activateEventSend hasn't run in a while due to a test condition error (see eclipse-platform#2591). Therefore we are going to disable this test that is a feature only on GTK3/x11
|
I'm deferring resolving this until #2593 is complete as this change makes the tests look unstable, but it really is just exposing some long non-running tests. |
jonahgraham
added a commit
to jonahgraham/eclipse.platform.swt
that referenced
this pull request
Oct 4, 2025
test_activateEventSend hasn't run in a while due to a test condition error (see eclipse-platform#2591). Therefore we are going to disable this test that is a feature only on GTK3/x11
jonahgraham
added a commit
to jonahgraham/eclipse.platform.swt
that referenced
this pull request
Oct 7, 2025
It may take some event cycles before the parent shell that is getting focus/activation back gets the notification. Therefore wait until the Activate listener is called This test had bitrotten due to order of operation issues, see eclipse-platform#2591 for details on that. This test was a regression test for https://bugs.eclipse.org/436841, but only tested 1 of the 4 events. I added to the test all the events. The test was also written before GTK4 (???) and was disabled on non-x11. The test is now enabled on x11.
jonahgraham
added a commit
to jonahgraham/eclipse.platform.swt
that referenced
this pull request
Oct 7, 2025
It may take some event cycles before the parent shell that is getting focus/activation back gets the notification. Therefore wait until the Activate listener is called This test had bitrotten due to order of operation issues, see eclipse-platform#2591 for details on that. This test was a regression test for https://bugs.eclipse.org/436841, but only tested 1 of the 4 events. I added to the test all the events. The test was also written before GTK4 (???) and was disabled on non-x11. The test is now enabled on x11.
jonahgraham
added a commit
that referenced
this pull request
Oct 7, 2025
It may take some event cycles before the parent shell that is getting focus/activation back gets the notification. Therefore wait until the Activate listener is called This test had bitrotten due to order of operation issues, see #2591 for details on that. This test was a regression test for https://bugs.eclipse.org/436841, but only tested 1 of the 4 events. I added to the test all the events. The test was also written before GTK4 (???) and was disabled on non-x11. The test is now enabled on x11.
jonahgraham
force-pushed
the
x11
branch
2 times, most recently
from
149d92c to
f9b8b8c
Compare
jonahgraham
changed the title
jonahgraham
commented
Oct 7, 2025
There was a problem hiding this comment.
I updated OP comment to reflect slight increase of scope #2591 (comment)
bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java
Outdated
Show resolved
Hide resolved
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 eclipse-platform#2592 for a test that was silently being skipped and had bitrotten as a result.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.