Skip to content

Fix flaky AsyncContentAssistTest.testCompleteActivationChar #3779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vogella wants to merge 2 commits into
base: master
Choose a base branch
from
+11 −12
Open

Fix flaky AsyncContentAssistTest.testCompleteActivationChar #3779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a4f171
Fix flaky AsyncContentAssistTest.testCompleteActivationChar
vogella Mar 16, 2026
c4003c2
Version bump(s) for 4.40 stream
eclipse-platform-bot Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/org.eclipse.jface.text.tests/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %Plugin.name
Bundle-SymbolicName: org.eclipse.jface.text.tests
Bundle-Version: 3.14.0.qualifier
Bundle-Version: 3.14.100.qualifier
Bundle-Vendor: %Plugin.providerName
Bundle-Localization: plugin
Export-Package:
Expand Down
2 changes: 1 addition & 1 deletion ....text.tests/src/org/eclipse/jface/text/tests/contentassist/AbstractContentAssistTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ protected static List<Shell> findNewShells(Collection<Shell> beforeShells) {

protected static Shell findNewShell(Collection<Shell> beforeShells) {
List<Shell> afterShells= findNewShells(beforeShells);
for (int attempt= 0; afterShells.size() != 1 && attempt < 10; attempt++) {
for (int attempt= 0; afterShells.size() != 1 && attempt < 50; attempt++) {
DisplayHelper.sleep(getDisplay(), 100);
afterShells= findNewShells(beforeShells);
}
Expand Down
19 changes: 9 additions & 10 deletions ...ace.text.tests/src/org/eclipse/jface/text/tests/contentassist/AsyncContentAssistTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeFalse;

import java.util.Arrays;
import java.util.Collection;
Expand All @@ -41,8 +40,6 @@
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Platform;

import org.eclipse.jface.util.Util;

import org.eclipse.jface.text.Document;
import org.eclipse.jface.text.IDocument;
import org.eclipse.jface.text.contentassist.ContentAssistant;
Expand Down Expand Up @@ -134,7 +131,6 @@ protected boolean condition() {

@Test
public void testCompleteActivationChar() {
assumeFalse(Util.isWindows(), "test fails on Windows, see https://github.com/eclipse-platform/eclipse.platform.ui/issues/890");
shell.setLayout(new FillLayout());
shell.setSize(500, 300);
SourceViewer viewer= new SourceViewer(shell, null, SWT.NONE);
Expand All @@ -151,17 +147,20 @@ public void testCompleteActivationChar() {
contentAssistant.install(viewer);
shell.open();
Display display= shell.getDisplay();
Event keyEvent= new Event();
DisplayHelper.runEventLoop(display, 0);
Control control= viewer.getTextWidget();
control.forceFocus();
DisplayHelper.runEventLoop(display, 0);
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
Copy link
Contributor

@trancexpress trancexpress Mar 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@vogella vogella Mar 17, 2026

Choose a reason for hiding this comment

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

Display.post() injects events into the native OS event queue, which requires a focused window to receive them. In headless CI environments (Xvfb/virtual framebuffer), there's no window manager (reliably managing focus). Other tests using Display.post() may succeed because they don't depend on content assist popup detection within a tight timing window.

Event keyEvent= new Event();
keyEvent.widget= control;
keyEvent.type= SWT.KeyDown;
keyEvent.character= 'b';
keyEvent.keyCode= 'b';
control.getShell().forceActive();
control.getDisplay().post(keyEvent);
keyEvent.type= SWT.KeyUp;
control.getDisplay().post(keyEvent);
final Collection<Shell> beforeShells= AbstractContentAssistTest.getCurrentShells();
control.notifyListeners(SWT.KeyDown, keyEvent);
Copy link
Contributor

@trancexpress trancexpress Mar 16, 2026

Choose a reason for hiding this comment

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

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).

Copy link
Contributor Author

@vogella vogella Mar 17, 2026

Choose a reason for hiding this comment

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

That's a fair point. You could add a diagnostic version first that adds a KeyDown listener on the widget, records whether the event arrives, and logs/asserts on that before the timeout. This would confirm the hypothesis. However, note that notifyListeners() still triggers the same SWT listener chain, ContentAssistant installs a VerifyKeyListener on the StyledText widget, and notifyListeners(SWT.KeyDown, ...) triggers that same path. The test still validates that typing an activation character triggers content assist and that the popup appears with the correct proposals. The only difference is the event delivery mechanism, direct widget notification vs. native OS queue.

AbstractContentAssistTest.processEvents();
Shell newShell= AbstractContentAssistTest.findNewShell(beforeShells);
assertTrue(new DisplayHelper() {
Expand Down
Loading