eclipse-sirius / sirius-desktop

Sirius Desktop: desktop-based graphical modelers for dedicated DSLs
https://eclipse.dev/sirius/
Eclipse Public License 2.0
15 stars 12 forks source link

[test] Set PREF_SAVE_WHEN_NO_EDITOR to false by default for JUnit tests #469

Closed lredor closed 2 months ago

lredor commented 2 months ago

Some tests fail since the commit dc87bccb "[test] improves the restoration of preferences in the test " [1]. Its goals was to allow better restoration of the initial preference values when using "changeXXXPreference" methods of SiriusTestCase or AbstractSiriusSwtBotGefTestCase superclasses. The side effect is that some tests were OK before because they relied on "wrong" default values ​​for some preferences.

Previous commits try to fix tests one by one. But some of them are unreliable. It seems that the SaveSessionJob, launched randomly, can have several effects. So a drastic solution is to set the preference SiriusUIPreferencesKeys.PREF_SAVE_WHEN_NO_EDITOR to false by default for all Junit tests. The preference PREF_RELOAD_ON_LAST_EDITOR_CLOSE is also set to false (as explained in the javadoc of the preferences).

An example of random failure is the test "TreeLabelProviderTest.testCrossTableLabelProvider". It fails with this stack: 09:04:33 org.eclipse.sirius.tests.unit.table.tests.provider.TreeLabelProviderTest.testCrossTableLabelProvider -- Time elapsed: 0.352 s <<< FAILURE! 09:04:33 java.lang.AssertionError: 09:04:33 Error(s) raised during test : org.eclipse.sirius.tests.unit.table.tests.provider.TreeLabelProviderTest 09:04:33 . Log Plugin : org.eclipse.core.runtime 09:04:33 . Error from plugin:org.eclipse.core.commands, message: Problems occurred when invoking code from plug-in: "org.eclipse.core.commands"., info: java.util.ConcurrentModificationException 09:04:33 . Stack trace: java.util.ConcurrentModificationException 09:04:33 at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013) 09:04:33 at java.base/java.util.ArrayList$Itr.next(ArrayList.java:967) 09:04:33 at org.eclipse.core.commands.operations.AbstractOperation.hasContext(AbstractOperation.java:110) ... 09:04:33 at org.eclipse.emf.workspace.impl.WorkspaceCommandStackImpl.doExecute(WorkspaceCommandStackImpl.java:203) 09:04:33 at org.eclipse.emf.transaction.impl.AbstractTransactionalCommandStack.execute(AbstractTransactionalCommandStack.java:160) 09:04:33 at org.eclipse.emf.transaction.impl.AbstractTransactionalCommandStack.execute(AbstractTransactionalCommandStack.java:214) 09:04:33 at org.eclipse.sirius.table.ui.tools.internal.editor.AbstractDTableEditor.launchRefresh(AbstractDTableEditor.java:330) 09:04:33 at org.eclipse.sirius.table.ui.tools.internal.editor.AbstractDTableEditor.init(AbstractDTableEditor.java:212) ... 09:04:33 at org.eclipse.sirius.common.ui.tools.api.util.EclipseUIUtil.displaySyncExec(EclipseUIUtil.java:337) 09:04:33 at org.eclipse.sirius.table.ui.business.internal.dialect.TableDialectUIServices.openEditor(TableDialectUIServices.java:136) 09:04:33 at org.eclipse.sirius.ui.business.internal.dialect.DialectUIManagerImpl.openEditor(DialectUIManagerImpl.java:94) 09:04:33 at org.eclipse.sirius.tests.unit.table.tests.provider.TreeLabelProviderTest.testCrossTableLabelProvider(TreeLabelProviderTest.java:99)

And an analysis reveals that the SaveSessionJob can impact the contexts list with: Thread [Worker-4: Session saving] (Suspended (breakpoint at line 109 in AbstractOperation)) owns: Object (id=412) owns: AtomicBoolean (id=470) EMFCommandOperation(AbstractOperation).hasContext(IUndoContext) line: 109 DefaultOperationHistory.filter(List, IUndoContext) line: 543 DefaultOperationHistory.getUndoHistory(IUndoContext) line: 792 WorkspaceCommandStackImpl.saveIsDone() line: 628 DAnalysisSessionImpl.doSave(Map<?,?>, IProgressMonitor, boolean) line: 881 Saver.wrappedSave(Map<?,?>, IProgressMonitor, boolean) line: 144 Saver$1.run(IProgressMonitor) line: 121 Workspace.run(ICoreRunnable, ISchedulingRule, int, IProgressMonitor) line: 2380 Workspace.run(IWorkspaceRunnable, IProgressMonitor) line: 2400 Saver.saveNow(Map<?,?>, IProgressMonitor, boolean) line: 118 Saver.save(Map<?,?>, IProgressMonitor) line: 95 DAnalysisSessionImpl.save(Map<?,?>, IProgressMonitor) line: 835 DAnalysisSessionImpl.save(IProgressMonitor) line: 830 SaveSessionJob.performSave(IProgressMonitor) line: 91 SaveSessionJob.run(IProgressMonitor) line: 56 Worker.run() line: 63

and then:

undoableOperations[i].removeContext(getSavedContext());

[1] https://github.com/eclipse-sirius/sirius-desktop/commit/dc87bccb24c6637c0b7a842a4baa99a4a89253b5