Community
Participate
Working Groups
The test case org.eclipse.e4.ui.tests.workbench.PartOnTopManagerTest.test_PlaceholderOnTopStackSwitch fails sometimes. It was observed on two unrelated changes recently, and gone away after retriggering. Failed in https://ci.eclipse.org/platform/job/eclipse.platform.ui-Gerrit/20318/ , change https://git.eclipse.org/r/150491 https://ci.eclipse.org/platform/job/eclipse.platform.ui-Gerrit/20305/ , change https://git.eclipse.org/r/#/c/150300/ We should investigate this.
Jens, you recently stabilized several tests, can you have a look? This one fails frequently in the last days, e.g., https://ci.eclipse.org/platform/job/eclipse.platform.ui-Gerrit/20783/testReport/
(In reply to Lars Vogel from comment #1) > Jens, you recently stabilized several tests, can you have a look? Okay, I'll have a look in a few days.
The test doesn't fail in any of the currently available integration tests (I20190909-1520 to I20191112-1800) so it's hard to figure out what's going wrong. Next time anyone sees it, please post a stack trace to this ticket!
New Gerrit change created: https://git.eclipse.org/r/152620
java.lang.AssertionError: CleanupAddon should ensure that partStack is not rendered anymore, as all childs have been removed at org.eclipse.e4.ui.tests.workbench.PartRenderingEngineTests.ensureCleanUpAddonCleansUp(PartRenderingEngineTests.java:2479)
(In reply to Lars Vogel from comment #5) > java.lang.AssertionError: CleanupAddon should ensure that partStack is not > rendered anymore, as all childs have been removed > at > org.eclipse.e4.ui.tests.workbench.PartRenderingEngineTests. > ensureCleanUpAddonCleansUp(PartRenderingEngineTests.java:2479) See https://ci.eclipse.org/platform/job/eclipse.platform.ui-Gerrit/20856/
The easy fix I tried to stabilize this test didn't work. I'll look at it a little more when I have time, but I don't know if I can do anything about it. Since the fix didn't work that means that it took the code more than 10 s to fulfil the properties that were being tested. That suggests that either the test environment is really shaky, or that there is some real problem with the functionallity that is being tested.
I managed to catch the failure at the first 'assertTrue(isPartOnTop(part))' in the test. It shows that the IWorkbench.ON_TOP key is not set in the context, so the part cannot be on top. The key is set either when the widget is added to a part (bootstrapping part of the PartOnTopManager), or when the selection changes of a container. The part for which the test fails is added with a PlaceHolder in the perspective. So the question is why the ON_TOP key is not set, it should have been set when the widget was added. This could only happen if the widget was added before a context was added or the widget added event was not submitted/processed yet.
Guys, how do you run this test locally from IDE? I can't, it explodes with hundreds of opened shells and errors like bug 511697 comment 2.
(In reply to Andrey Loskutov from comment #9) > Guys, how do you run this test locally from IDE? I can't, it explodes with > hundreds of opened shells and errors like bug 511697 comment 2. Same with PartRenderingEngineTests. Do I have to close or open some "special" projects to be able to run this test successfully? I have a lot of SDK in the workspace.
(In reply to Andrey Loskutov from comment #9) > Guys, how do you run this test locally from IDE? I can't, it explodes with > hundreds of opened shells and errors like bug 511697 comment 2. There is a launch target pre-defined that works for the E4 tests: org.eclipse.e4.ui.tests\UIAllTests.launch. I think that you have to run in 'Headless Mode' (i.e. Run an application: No Application)
(In reply to Rolf Theunissen from comment #11) > I think that you have to run in 'Headless Mode' (i.e. Run an application: No > Application) Thanks, this was it. The test runs now fine on my Linux box, even with stress-ng eating all CPU :-( So I assume our test instance is disturbed by other means.
New Gerrit change created: https://git.eclipse.org/r/152964
*** Bug 511697 has been marked as a duplicate of this bug. ***
*** Bug 511027 has been marked as a duplicate of this bug. ***
(In reply to Andrey Loskutov from comment #12) > (In reply to Rolf Theunissen from comment #11) > > I think that you have to run in 'Headless Mode' (i.e. Run an application: No > > Application) > > Thanks, this was it. The test runs now fine on my Linux box, even with > stress-ng eating all CPU :-( > > So I assume our test instance is disturbed by other means. I was able to trigger the failures only by running the whole testsuite (on Windows), so it might also be a bad interaction with a previous testcase. As mentioned in comment 8, it also depends on the order in which selection-changed-event is handled by the PartOnTopManager and the Renderers. I also wonder if the widget-event is propagated for a shared part. Or that this event is propagated when the context is not available yet. Given that the key is not in the context yet, all PartOnTopManger event handlers run before the context is available.
Been searching some more, somehow the event subscriptions on PartOnTopManager are not always triggered, even when the SWT eventloop is spinned. Sometimes they initially work, and fail after the second 'setSelectedElement' in the test. What can suppress or delay these events?
New Gerrit change created: https://git.eclipse.org/r/153016
(In reply to Rolf Theunissen from comment #17) > Been searching some more, somehow the event subscriptions on > PartOnTopManager are not always triggered, even when the SWT eventloop is > spinned. Sometimes they initially work, and fail after the second > 'setSelectedElement' in the test. > > What can suppress or delay these events? (In reply to Eclipse Genie from comment #18) > New Gerrit change created: https://git.eclipse.org/r/153016 So I've moved the test to be the first one in the test suite, to check if any other tests may influence the result - still failing. java.lang.AssertionError at org.eclipse.e4.ui.tests.workbench.PartOnTopManagerTest.test_PlaceholderOnTopStackSwitch(PartOnTopManagerTest.java:175) Corresponding test code: assertTrue(isPartOnTop(part)); assertFalse(isPartOnTop(secondPart)); partStack.setSelectedElement(secondPart); assertFalse(isPartOnTop(part)); <-------- assert here assertTrue(isPartOnTop(secondPart)); I can imagine that we see the effect of "random" event dispatching order in org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(int, E) // We can't guarantee any delivery order for synchronous events. // Attempts to do so result in deadly embraces. In debugger there are 5 elements in the queue.
Finally getting somewhere: In UIEventObjectSupplier#UIEventHandler#handleEvent sometimes 'unsubscribe' is called on the events registered for PartOnTopManager. After unsubscribe is called, the test fails (which makes sense). The question now is why unsubscribe is called. This happens when the requestor becomes inValid, i.e., when a weak-reference looses its reference. So, is there a strong reference to PartOnTopManager? E4Workbench provides the answer (line 135), and it is *no*.
New Gerrit change created: https://git.eclipse.org/r/153067
I think the last patch from Rolf would be good candidate for RC1. WDYT?
(In reply to Andrey Loskutov from comment #22) > I think the last patch from Rolf would be good candidate for RC1. WDYT? IIRC for test and example code no approval is required
(In reply to Lars Vogel from comment #23) > (In reply to Andrey Loskutov from comment #22) > > I think the last patch from Rolf would be good candidate for RC1. WDYT? > > IIRC for test and example code no approval is required The change is in production code.
(In reply to Andrey Loskutov from comment #24) > (In reply to Lars Vogel from comment #23) > > (In reply to Andrey Loskutov from comment #22) > > > I think the last patch from Rolf would be good candidate for RC1. WDYT? > > > > IIRC for test and example code no approval is required > > The change is in production code. Ah, didn't notice. Please find another lead to approve, I'm currently really busy with other work.
Gerrit change https://git.eclipse.org/r/153067 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=dcd6b902e242ce6988067cdb779dcd1690e77792
Many thanks for Rolf for fixing that and to all which worked on this. This was a major annoyance in your Gerrit stream.
