Fix flaky AsyncContentAssistTest.testCompleteActivationChar#3779
Fix flaky AsyncContentAssistTest.testCompleteActivationChar#3779vogella wants to merge 2 commits intoeclipse-platform:masterfrom
Conversation
Replace Display.post() with control.notifyListeners() for reliable event delivery in headless CI environments. Display.post() sends native OS events that may not be processed, causing the content assist popup to never appear. Also fix race condition by capturing beforeShells before posting key events, process events after shell.open(), and increase findNewShell timeout from 1s to 5s for slower CI environments. Fixes eclipse-platform#890
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
|
@trancexpress please review |
@laeubi can you review the change? I'm not sure what the intent is of sending SWT events. Sending the event directly to a widget seems odd to me, but so does sending SWT events. |
| final Collection<Shell> beforeShells= AbstractContentAssistTest.getCurrentShells(); | ||
| // Use notifyListeners instead of Display.post() for reliable event delivery | ||
| // Display.post() sends native OS events that may not be processed reliably | ||
| // in headless CI environments |
There was a problem hiding this comment.
Any idea why that would be? For better or worse we have a lot of tests that sent SWT events and we don't see sporadic fails with them.
Is something buggy in the the test or in the Eclipse CI environment, when it comes to UI event handling?
| keyEvent.type= SWT.KeyUp; | ||
| control.getDisplay().post(keyEvent); | ||
| final Collection<Shell> beforeShells= AbstractContentAssistTest.getCurrentShells(); | ||
| control.notifyListeners(SWT.KeyDown, keyEvent); |
There was a problem hiding this comment.
I would start with adding a listener to the widget and recording if the sent event is received. Then a check can validate that the event was received, prior to the timeout assert fail.
That way we would be certain that the problem really is event handling and not something else. If we do know event processing is buggy, a change like this still seems odd but it would be fine from my POV - if the test still tests what it should (Christoph can comment on this, I cannot).
Summary
Display.post()withcontrol.notifyListeners()for reliable event delivery in headless CI environments —Display.post()sends native OS events that may not be processed, causing the content assist popup to never appearbeforeShellsbefore posting key events, and process events aftershell.open()findNewShelltimeout from 1s to 5s for slower CI environmentsassumeFalseskip — the test now works on all platformsTest plan
AsyncContentAssistTest5 times consecutively — all passesorg.eclipse.jface.text.testssuite — no regressions (only pre-existingCodeMiningTestfailures)Fixes #890