Skip to content

Conversation

jonahgraham
Copy link
Contributor

@jonahgraham jonahgraham commented Sep 25, 2025

This test taking a very long time (10 seconds) on Wayland backend. The test as written was invalid because the loop had a timeout without the exit condition being true.

This commit updates the test to:

  • resolve the false positive by checking condition after timeout loop
  • using common code, SwtTestUtil.processEvents
  • asserts on the display.post so the underlying error is flagged as the error
  • adds SwtTestUtil.processEvents at the beginning of the function to ensure window is focused so that the current windown can be posted to
  • for styled text changes from invokeAction to posting key events, this is to ensure the original intention of the test worked to ensure we don't have a regression in the future on sending BS event - see https://bugs.eclipse.org/565164. test_invokeActionI also has a test for invokeAction(ST.DELETE_PREVIOUS) so nothing is lost here

@jonahgraham

This comment was marked as resolved.

@jonahgraham
Copy link
Contributor Author

I think this test would have been disabled years ago if it hadn't false positive'd at the time that Display.post tests were all disabled in Bug 553754 commit b846e85

@jonahgraham
Copy link
Contributor Author

The tests failed locally for me, but they pass on the build machine for all the targets currently tested.

@akurtakov
Copy link
Member

Do you run on Wayland or X11? Note that XWayland inherits all security changes of Wayland so one has to use plain X11 for real test. Test machines are running on X11 only AFAIK.

@jonahgraham
Copy link
Contributor Author

Yes, I think that is probably the issue. I will set up an x11 only run to see what happens.

This test taking a very long time (10 seconds) on Wayland backend.
The test as written was invalid because the loop had a timeout
without the exit condition being true.

This commit updates the test to:

- resolve the false positive by checking condition after timeout loop
- using common code, SwtTestUtil.processEvents
- asserts on the display.post so the underlying error is flagged as
  the error
- adds SwtTestUtil.processEvents at the beginning of the function
  to ensure window is focused so that the current windown can be posted
  to
- for styled text changes from invokeAction to posting key events, this
  is to ensure the original intention of the test worked to ensure
  we don't have a regression in the future on sending BS
  event - see https://bugs.eclipse.org/565164. `test_invokeActionI`
  also has a test for `invokeAction(ST.DELETE_PREVIOUS)` so nothing
  is lost here
@eclipse-platform eclipse-platform deleted a comment from github-actions bot Sep 30, 2025
@jonahgraham jonahgraham self-assigned this Sep 30, 2025
@jonahgraham
Copy link
Contributor Author

These tests will now show as a failure on gtk4 - which is fine because it was previously showing as a false pass (for me at least).

Thanks @akurtakov - you were right, setting up a real x11 backend shows the expected behaviour.

I have update the top comment with the new commit message as I cleaned up theses two tests, and learned a bunch of new stuff too.

Copy link
Contributor

Test Results

  118 files  ±0    118 suites  ±0   11m 5s ⏱️ - 2m 34s
4 582 tests ±0  4 546 ✅ ±0  36 💤 ±0  0 ❌ ±0 
  330 runs  ±0    307 ✅ ±0  23 💤 ±0  0 ❌ ±0 

Results for commit adbd0ca. ± Comparison against base commit 333b73e.

@jonahgraham jonahgraham marked this pull request as ready for review September 30, 2025 15:23
@jonahgraham
Copy link
Contributor Author

Yes, I think that is probably the issue. I will set up an x11 only run to see what happens.

I documented my setup in https://kichwacoders.com/2025/09/30/testing-and-developing-swt-on-gtk/

@jonahgraham jonahgraham merged commit cc5eb77 into eclipse-platform:master Sep 30, 2025
10 of 12 checks passed
@jonahgraham jonahgraham deleted the fix-backspace-test branch September 30, 2025 15:24
@jonahgraham
Copy link
Contributor Author

The updated tests pass on GitHub actions, all platforms. But failed on Windows I-build https://download.eclipse.org/eclipse/downloads/drops4/I20250930-1800/testResults.php - so more work (and more for me to learn about different platforms)

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.

2 participants