Community
Participate
Working Groups
Since N20150731-2000. PartRenderingEngineTests.testBug374326 fails on all platforms expected:<Shell {}> but was:<Shell {PartRenderingEngine's limbo}> junit.framework.AssertionFailedError: expected:<Shell {}> but was:<Shell {PartRenderingEngine's limbo}> at org.eclipse.e4.ui.tests.workbench.PartRenderingEngineTests.testBug374326(PartRenderingEngineTests.java:3303) at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:692) at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:319) at org.eclipse.test.CoreTestApplication.runTests(CoreTestApplication.java:36) at org.eclipse.test.CoreTestApplication.run(CoreTestApplication.java:32) at org.eclipse.equinox.internal.app.EclipseAppContainer.callMethodWithException(EclipseAppContainer.java:587) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:198) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:388) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:243) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:669) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:608) at org.eclipse.equinox.launcher.Main.run(Main.java:1515) at org.eclipse.equinox.launcher.Main.main(Main.java:1488) at org.eclipse.core.launcher.Main.main(Main.java:34)
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=89784095c81872bd7b2e3ea042504b0501461ca9 is causing the test failure. Resetting the code to the commit previous to the one mentioned above causes the test to pass and the test fails when this commit is included.
I'm already working on a fix for this. Since the ToolBar is wrapped with an ImageBasedFrame to be able to be dragged (before it wasn't the case) this hack https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/TrimBarLayout.java#n191 kicks in and sends it to the limbo. I'm working towards a solution but it's hard to navigate as this trim/toolbar rendering story is full of hacks. How does it work in this situation? Do I have some time to salvage this for M1 or shall we revert?
(In reply to Sopot Cela from comment #2) > How does it work in this situation? Do I have some time to salvage this for > M1 or shall we revert? AFAICS the issue will not affect users which have their CSS enabled. So I think we can leave the code in for M1. Of course it would be good if it can be fixed for M1.
(In reply to Lars Vogel from comment #3) > (In reply to Sopot Cela from comment #2) > > How does it work in this situation? Do I have some time to salvage this for > > M1 or shall we revert? > > AFAICS the issue will not affect users which have their CSS enabled. So I > think we can leave the code in for M1. Of course it would be good if it can > be fixed for M1. I'll be talking a look in the next hours and possibly tomorrow and see if I can come up with a fix.
(In reply to Lars Vogel from comment #3) > (In reply to Sopot Cela from comment #2) > > How does it work in this situation? Do I have some time to salvage this for > > M1 or shall we revert? > > AFAICS the issue will not affect users which have their CSS enabled. So I > think we can leave the code in for M1. Of course it would be good if it can > be fixed for M1. Thanks! If we can't find a fix we should revert this. It came in late and I don't think we can be sure it won't affect any of our users.
So this is what is going on. This method https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/TrimBarLayout.java#n176 does the following: - When a layout of the trim is requested it tries to find the empty *draggable* toolbars and set them to invisible - The line "if (kids.length != 1 || !(kids[0] instanceof ToolBar))" differentiates draggable and non-draggable toolbars, because when a toolbar is not draggable the widget is just a ToolBar with no kids but when it is draggable it is an ImageBasedFrame which has a kid and that kid is a toolbar. This is the condition that flipped because of my patch as now all toolbars are draggable by default. - The test case's toolbar was not draggable before (no css engine in the testing env). Now it is and it is being hidden by the before-mentioned code which causes the test to fail. So the test was passing because the toolbar was not draggable (because there was no CSS engine). - The status quo in the wrokbench now is that if you have a draggable toolbar which is empty upon layouting of the trimbar it will be hidden by the above code. The test calls the layouting before adding the ToolControl to the ToolBar. That means the toolbar will be hidden and the test fails. Again, this was not the case before just because it was not draggable (the kids.length != 1 || !(kids[0] instanceof ToolBar) condition). So the test was 'hanging by a thread' and it was surviving only because it was not draggable. I will propose a change to the test to make it not be dependent on the visibility of the ToolBar as it is not what is trying to prove. This test needs to prove that a ToolControl of which visibility is flipped from false to true actually shows up. And that is the case. Now it's failing because its parent (the toolbar) is not visible, and that's why they are in limbo. The alternative is to revert the change which I would not favor. Even though this is late in the milestone context we are relatively early in the release context and I'll be monitoring bugzilla for potential issues related to this. My manual testing of the workbench has not shown yet any issue created by the change. I also think it is a neat feature to have.
New Gerrit change created: https://git.eclipse.org/r/53219
(In reply to Sopot Cela from comment #7) > New Gerrit change created: https://git.eclipse.org/r/53219 This change got merged with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a15de5497a2492462214c8a05bdb3acc8ce7df76 See bug 474349 why this bug was not automatically updated by Gerrit.
Verified in I20150805-2000.
