Skip to content

Conversation

jonahgraham
Copy link
Contributor

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

@jonahgraham
Copy link
Contributor Author

This + #2591 shows that the test still isn't stable. I am deferring resolving this until #2593 is complete.

@jonahgraham jonahgraham force-pushed the fix-unstable-test branch 2 times, most recently from d27585a to ffba498 Compare October 6, 2025 12:31
@jonahgraham
Copy link
Contributor Author

Test commit with RepeatedTest of 1000 to see if this is stable or not on the build machine with the added processEvents

@eclipse-platform eclipse-platform deleted a comment from github-actions bot Oct 6, 2025
Copy link
Contributor

github-actions bot commented Oct 6, 2025

Test Results

  118 files  ±0    118 suites  ±0   11m 53s ⏱️ ±0s
4 583 tests ±0  4 565 ✅  - 1  18 💤 +1  0 ❌ ±0 
  330 runs  ±0    326 ✅ ±0   4 💤 ±0  0 ❌ ±0 

Results for commit 6777ba1. ± Comparison against base commit 1eabfa3.

This pull request skips 1 test.
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Shell ‑ test_activateEventSend

♻️ This comment has been updated with latest results.

@jonahgraham jonahgraham force-pushed the fix-unstable-test branch 2 times, most recently from 8cc0c1c to 50a12a5 Compare October 7, 2025 13:55
@jonahgraham
Copy link
Contributor Author

The test I am modifying was a regression test for https://bugs.eclipse.org/bugs/show_bug.cgi?id=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.

I'll updated the commit message to include the above on my next push.

@jonahgraham
Copy link
Contributor Author

Test commit with RepeatedTest of 1000 to see if this is stable or not on the build machine with the added processEvents

The tests have successfully run 1000x, so I think this is ready to go.

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 jonahgraham marked this pull request as ready for review October 7, 2025 15:05
@jonahgraham jonahgraham merged commit 1460543 into eclipse-platform:master Oct 7, 2025
10 of 12 checks passed
@jonahgraham jonahgraham deleted the fix-unstable-test branch October 7, 2025 15:06
assertTrue(listenerCalled);
}
public void test_activateEventSend() throws InterruptedException {
assumeTrue((SwtTestUtil.isGTK && SwtTestUtil.isX11) || SwtTestUtil.isGTK4,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that because of #2591 this assumption fails, even on GTK3 + x11.

When I had done the 1000x repeated test I had included the fix in this PR as well to make sure it ran.

Once #2591 is done this test will actually run properly.

In hindsight, I think I made it too complicated by having this as two PRs. Two commits in the one PR would have been sufficient (and simpler for me)

jonahgraham added a commit to jonahgraham/eclipse.platform.swt that referenced this pull request Oct 7, 2025
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.
jonahgraham added a commit to jonahgraham/eclipse.platform.swt that referenced this pull request Oct 7, 2025
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant