StackRendererTest should remove the used window before each test #668

vogella · 2023-03-29T14:33:01Z

During debugging of this test I realized that we never cleanup the model elements after a test, add this to the test method.

github-actions · 2023-03-29T16:43:34Z

Test Results

1 815 files ±0 1 815 suites ±0 1h 29m 42s ⏱️ -50s
7 700 tests ±0 7 459 ✅ - 13 228 💤 ±0 2 ❌ +2 11 🔥 +11
24 261 runs ±0 23 501 ✅ - 13 747 💤 ±0 2 ❌ +2 11 🔥 +11

For more details on these failures and errors, see this check.

Results for commit 323ea9c. ± Comparison against base commit 8a8979f.

♻️ This comment has been updated with latest results.

marcushoepfner · 2023-04-03T14:36:51Z

After having written tests in StackRendererTest in #689, I'm able to understand and review this PR.
LGTM

vogella · 2023-04-04T07:17:28Z

Thanks @marcushoepfner IIRC is noticed this while looking for potential places for unit tests for the onboarding stuff. Version update is done in 692

marcushoepfner · 2023-04-18T08:05:28Z

Can this be merged?

iloveeclipse · 2023-04-18T08:11:00Z

Can this be merged?

This PR shows 10 commits and 60 changed files. I guess something is wrong here. Can we have one commit rebased on top of master?

vogella · 2023-04-18T08:11:59Z

Yes, not sure that happened here. Will update or abondon.

BeckerWdf · 2023-05-05T08:07:29Z

Yes, not sure that happened here. Will update or abondon.

@vogella: Any updates here?

jukzi · 2023-05-08T12:04:27Z

...org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererTest.java

+	@After
+	public void cleanUp() throws Exception {
+		ems.deleteModelElement(partStack);
+		ems.deleteModelElement(window);


@laeubi why does github think this is JRE 11 source? (line 173)

BeckerWdf · 2023-08-09T13:38:42Z

@vogella: What's the state of this PR?

BeckerWdf · 2023-12-01T13:42:57Z

@vogella: What's the state of this PR?

@vogella: Do you wanna finish this one?

StackRendererTest should remove the used window before each test Before this change the the model elements were not removed after a test.

marcushoepfner · 2024-09-30T10:00:43Z

I have rebased. Let's wait for the results and merge this afterwards.
Ok from your side @vogella ?

vogella · 2024-09-30T10:06:03Z

Thanks @marcushoepfner and @hannesN for finishing this one. The change is (of course) fine with me.

HeikoKlare · 2024-09-30T20:47:02Z

Was it intended that this PR is merged despite test failures on MacOS? Subsequent PRs show test failures in StackRendererTest and TabStateHandlerTest (which also uses a StackRendererTest internally) in the cleanUp() method, which has been changed by this PR. See, e.g. https://github.com/eclipse-platform/eclipse.platform.ui/pull/2337/checks?check_run_id=30870517513

The final GitHub Actions run for MacOS of this PR before merging shows the same test failures in the logs (https://github.com/eclipse-platform/eclipse.platform.ui/actions/runs/11103802200/job/30846537413), so I guess they were introduced by this PR.
This is one of multiple according stack traces in the logs:

org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4922)
	at org.eclipse.swt.SWT.error(SWT.java:4837)
	at org.eclipse.swt.SWT.error(SWT.java:4808)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:853)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:632)
	at org.eclipse.swt.widgets.Control.getParent(Control.java:1846)
	at org.eclipse.swt.widgets.Shell.getShells(Shell.java:1190)
	at org.eclipse.swt.widgets.Shell.updateParent(Shell.java:2269)
	at org.eclipse.swt.widgets.Shell.setWindowVisible(Shell.java:2135)
	at org.eclipse.swt.widgets.Shell.open(Shell.java:1386)
	at org.eclipse.jface.window.Window.open(Window.java:795)
	at org.eclipse.jface.dialogs.ErrorDialog.open(ErrorDialog.java:347)
	at org.eclipse.e4.ui.internal.workbench.swt.WorkbenchStatusReporter.openDialog(WorkbenchStatusReporter.java:124)
	at org.eclipse.e4.ui.internal.workbench.swt.WorkbenchStatusReporter.report(WorkbenchStatusReporter.java:62)
	at org.eclipse.e4.core.services.statusreporter.StatusReporter.show(StatusReporter.java:133)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5$1.eventLoopException(PartRenderingEngine.java:1128)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.handle(PartRenderingEngine.java:1171)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.lambda$1(PartRenderingEngine.java:1146)
	at org.eclipse.swt.internal.ExceptionStash.stash(ExceptionStash.java:63)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1432)
	at org.eclipse.swt.widgets.Control.release(Control.java:2938)
	at org.eclipse.swt.widgets.Widget.dispose(Widget.java:730)
	at org.eclipse.e4.ui.workbench.renderers.swt.SWTPartRenderer.disposeWidget(SWTPartRenderer.java:186)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.safeRemoveGui(PartRenderingEngine.java:934)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run(PartRenderingEngine.java:857)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:47)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.removeGui(PartRenderingEngine.java:841)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.subscribeTopicToBeRendered(PartRenderingEngine.java:184)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:56)
	at org.eclipse.e4.core.di.internal.extensions.EventObjectSupplier$DIEventHandler.handleEvent(EventObjectSupplier.java:92)
	at org.eclipse.equinox.internal.event.EventHandlerWrapper.handleEvent(EventHandlerWrapper.java:206)
	at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:201)
	at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:1)
	at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:230)
	at org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(ListenerQueue.java:151)
	at org.eclipse.equinox.internal.event.EventAdminImpl.dispatchEvent(EventAdminImpl.java:131)
	at org.eclipse.equinox.internal.event.EventAdminImpl.sendEvent(EventAdminImpl.java:73)
	at org.eclipse.equinox.internal.event.EventComponent.sendEvent(EventComponent.java:44)
	at org.eclipse.e4.ui.services.internal.events.EventBroker.send(EventBroker.java:55)
	at org.eclipse.e4.ui.internal.workbench.UIEventPublisher.notifyChanged(UIEventPublisher.java:60)
	at org.eclipse.emf.common.notify.impl.BasicNotifierImpl.eNotify(BasicNotifierImpl.java:424)
	at org.eclipse.e4.ui.model.application.ui.impl.UIElementImpl.setToBeRendered(UIElementImpl.java:316)
	at org.eclipse.e4.ui.internal.workbench.ModelServiceImpl.deleteModelElement(ModelServiceImpl.java:164)
	at org.eclipse.e4.ui.workbench.renderers.swt.StackRendererTest.cleanUp(StackRendererTest.java:99)

vogella · 2024-10-01T11:35:13Z

Was it intended that this PR is merged despite test failures on MacOS?

IIRC everything was green before the merge. Now it looks like this memory was is. We should obviously not merge PR with new test errors. I will revert

vogella · 2024-10-01T11:38:55Z

#2348

vogella · 2024-10-01T12:14:55Z

Ah, these days Mac test results are out-of-sight. Sorry did not notice that.

jukzi · 2024-10-01T12:24:31Z

Depends on where you look. Somehow it's not consistent. Especially after rebuilding something i have seen test result was not updated appropriately sometimes.

vogella force-pushed the stackrenderer-test-reset-after-each-test branch from 7a17dae to fa75317 Compare March 30, 2023 10:21

marcushoepfner approved these changes Apr 3, 2023

View reviewed changes

vogella force-pushed the stackrenderer-test-reset-after-each-test branch from d2d198f to 3f1ef79 Compare April 18, 2023 08:19

jukzi reviewed May 8, 2023

View reviewed changes

vogella force-pushed the stackrenderer-test-reset-after-each-test branch from 3f1ef79 to 747ad3f Compare May 9, 2023 07:24

akurtakov force-pushed the stackrenderer-test-reset-after-each-test branch from 747ad3f to 00e131f Compare July 1, 2023 06:15

marcushoepfner force-pushed the stackrenderer-test-reset-after-each-test branch from 00e131f to c7e1056 Compare July 26, 2023 06:21

marcushoepfner force-pushed the stackrenderer-test-reset-after-each-test branch 2 times, most recently from d2728ce to 39a8a86 Compare December 13, 2023 08:15

HannesWell force-pushed the stackrenderer-test-reset-after-each-test branch 2 times, most recently from 39a8a86 to 323ea9c Compare September 29, 2024 22:42

HannesWell mentioned this pull request Sep 29, 2024

Too many unrelated branches pushed to main repository #1409

Closed

StackRendererTest should restet the model after each test

973f1ad

StackRendererTest should remove the used window before each test Before this change the the model elements were not removed after a test.

marcushoepfner force-pushed the stackrenderer-test-reset-after-each-test branch from 323ea9c to 973f1ad Compare September 30, 2024 09:59

vogella merged commit f9b743f into master Sep 30, 2024
9 of 12 checks passed

vogella deleted the stackrenderer-test-reset-after-each-test branch September 30, 2024 10:38

BeckerWdf added this to the 4.34 M2 milestone Sep 30, 2024

HeikoKlare mentioned this pull request Oct 1, 2024

Multiple tests of StackRendererTest / TabStateHandlerTest / CompositePartClosingTest failing on MacOS #2346

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

StackRendererTest should remove the used window before each test #668

StackRendererTest should remove the used window before each test #668

vogella commented Mar 29, 2023

github-actions bot commented Mar 29, 2023 •

edited

Loading

marcushoepfner commented Apr 3, 2023

vogella commented Apr 4, 2023

marcushoepfner commented Apr 18, 2023

iloveeclipse commented Apr 18, 2023

vogella commented Apr 18, 2023

BeckerWdf commented May 5, 2023

jukzi May 8, 2023

BeckerWdf commented Aug 9, 2023

BeckerWdf commented Dec 1, 2023

marcushoepfner commented Sep 30, 2024

vogella commented Sep 30, 2024

HeikoKlare commented Sep 30, 2024

vogella commented Oct 1, 2024 •

edited

Loading

vogella commented Oct 1, 2024

vogella commented Oct 1, 2024

jukzi commented Oct 1, 2024

StackRendererTest should remove the used window before each test #668

StackRendererTest should remove the used window before each test #668

Conversation

vogella commented Mar 29, 2023

github-actions bot commented Mar 29, 2023 • edited Loading

Test Results

marcushoepfner commented Apr 3, 2023

vogella commented Apr 4, 2023

marcushoepfner commented Apr 18, 2023

iloveeclipse commented Apr 18, 2023

vogella commented Apr 18, 2023

BeckerWdf commented May 5, 2023

jukzi May 8, 2023

Choose a reason for hiding this comment

BeckerWdf commented Aug 9, 2023

BeckerWdf commented Dec 1, 2023

marcushoepfner commented Sep 30, 2024

vogella commented Sep 30, 2024

HeikoKlare commented Sep 30, 2024

vogella commented Oct 1, 2024 • edited Loading

vogella commented Oct 1, 2024

vogella commented Oct 1, 2024

jukzi commented Oct 1, 2024

github-actions bot commented Mar 29, 2023 •

edited

Loading

vogella commented Oct 1, 2024 •

edited

Loading