Skip to content

Commit d500209

Browse files
committed
Fix flaky MMenuItemTest by retaining processing add-on references
The handler in testElementHierarchyInContext_HandledItem was activated by a HandlerProcessingAddon created via ContextInjectionFactory.make, whose dependency-injection event subscription holds the add-on only weakly. Because the test discarded the make() result, the add-on could be garbage collected before the window context was set, leaving the handler inactive so the command was not executable and the menu item selection never ran it. Hold strong references to the created add-ons and remove the ineffective sleep loops that never addressed this.
1 parent b7e7e0a commit d500209

1 file changed

Lines changed: 19 additions & 26 deletions

File tree

tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/MMenuItemTest.java

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323

2424
import jakarta.inject.Inject;
2525
import jakarta.inject.Named;
26+
import java.util.ArrayList;
27+
import java.util.List;
2628
import org.eclipse.e4.core.commands.CommandServiceAddon;
2729
import org.eclipse.e4.core.contexts.ContextInjectionFactory;
2830
import org.eclipse.e4.core.contexts.IEclipseContext;
@@ -78,6 +80,14 @@ public class MMenuItemTest {
7880
@Inject
7981
private MApplication application;
8082

83+
/**
84+
* Strong references to processing add-ons created during a test. The
85+
* dependency injection event subscriptions that activate handlers reference
86+
* these add-ons only weakly, so without a strong reference they can be
87+
* garbage collected before the handler is activated.
88+
*/
89+
private final List<Object> processingAddons = new ArrayList<>();
90+
8191
@BeforeEach
8292
public void setUp() {
8393
ContextInjectionFactory.make(CommandServiceAddon.class, appContext);
@@ -763,15 +773,8 @@ public void execute(MUIElement uiElement, MMenuItem menuItem,
763773
activePart.getContext().set("key", "active");
764774
inactivePart.getContext().set("key", "inactive");
765775

766-
// Ensure all pending UI events are processed before triggering the menu item
767-
for (int i = 0; i < 10; i++) {
768-
contextRule.spinEventLoop();
769-
try {
770-
Thread.sleep(5);
771-
} catch (InterruptedException e) {
772-
// ignore
773-
}
774-
}
776+
// Process pending UI events before triggering the menu item.
777+
contextRule.spinEventLoop();
775778

776779
assertFalse(executed[0]);
777780

@@ -830,10 +833,11 @@ public void execute(MUIElement uiElement, MMenuItem menuItem,
830833

831834
application.getCommands().add(command);
832835
application.getChildren().add(window);
833-
// The handler processing addon cannot run until the context
834-
// contains the MApplication
835-
ContextInjectionFactory.make(CommandProcessingAddon.class, appContext);
836-
ContextInjectionFactory.make(HandlerProcessingAddon.class, appContext);
836+
// The handler processing addon cannot run until the context contains the
837+
// MApplication. Keep strong references so the add-ons are not garbage
838+
// collected before they activate the handler (see processingAddons).
839+
processingAddons.add(ContextInjectionFactory.make(CommandProcessingAddon.class, appContext));
840+
processingAddons.add(ContextInjectionFactory.make(HandlerProcessingAddon.class, appContext));
837841

838842
contextRule.createAndRunWorkbench(window);
839843

@@ -846,19 +850,8 @@ public void execute(MUIElement uiElement, MMenuItem menuItem,
846850
activePart.getContext().set("key", "active");
847851
inactivePart.getContext().set("key", "inactive");
848852

849-
// Ensure all pending UI events are processed before triggering the menu item
850-
// This prevents race conditions where the handler may not be fully registered yet
851-
for (int i = 0; i < 50; i++) {
852-
contextRule.spinEventLoop();
853-
if (((MenuItem) menuItem.getWidget()).getEnabled()) {
854-
break;
855-
}
856-
try {
857-
Thread.sleep(5);
858-
} catch (InterruptedException e) {
859-
// ignore
860-
}
861-
}
853+
// Process pending UI events before triggering the menu item.
854+
contextRule.spinEventLoop();
862855

863856
assertFalse(executed[0]);
864857
assertEquals(activePart, window.getContext().get(EPartService.class)

0 commit comments

Comments
 (0)