eclipse / amalgam

Eclipse Public License 1.0
1 stars 4 forks source link

[Activity Explorer] Model change triggers session load #101

Open eclipse-amalgam-bot opened 3 years ago

eclipse-amalgam-bot commented 3 years ago

I use capella as an example, it's the only project that I now of...

To reproduce:

(Now session b is open, and Session a is closed, and there is one 'active' ActivityExplorer for b, and one 'inactive' ActivityExplorer for a)

=> This opens a session for project a.

πŸ†” ECLIPSE-486811 πŸ‘· felix.dorner πŸ“… 2016-01-29

eclipse-amalgam-bot commented 3 years ago

felix.dorner commented on 2016-01-29

Bumping severity since this affects performance

ci-bot commented on 2016-01-29

New Gerrit change created: https://git.eclipse.org/r/65460

matthieu.helleboid commented on 2016-02-03

As stated on the gerrit change, the proposed patch doesn't seem to solve the scenario.

felix.dorner commented on 2016-02-03

Hmm, it seemed to fix it when I tested, but that was before you rebased it. I have another look.

felix.dorner commented on 2016-02-03

Matthieu, I have just tested again with the rebased patch and for me it still solves the problem. I am testing this against Capella 1.0.0.

felix.dorner commented on 2016-02-03

I don't know how to write an automated test for this since it involves restarting capella.

In any case, since I found about [ECLIPSE-486814 I think there is a design issue with ActivityExplorerEditorInput. See my comments on the other bug. If we can fix that one, this one here should be fixed automatically.](https://github.com/search?q=ECLIPSE-486814 I think there is a design issue with ActivityExplorerEditorInput. See my comments on the other bug. If we can fix that one, this one here should be fixed automatically.&type=Issues)

matthieu.helleboid commented on 2016-02-04

Hello Felix,

(In reply to felix.dorner from comment #5)

Matthieu, I have just tested again with the rebased patch and for me it still solves the problem. I am testing this against Capella 1.0.0.

I tested both changeset (the first one and the rebased one) with a soon to be released capella (https://hudson.polarsys.org/capella/job/capella-v1.0.x/64/) and it doesn't seem to work in my case. The stack is

Thread [main] (Suspended (breakpoint at line 79 in OpenSessionAction))
OpenSessionAction.doOpenSessions() line: 79 OpenSessionAction$1.run(IProgressMonitor) line: 141 OpenSessionAction.run() line: 152
ActivityExplorerEditorInput.loadState(IMemento) line: 254
ActivityExplorerEditorInput.(IMemento) line: 70
ActivityExplorerEditorInput.create(IMemento) line: 311
ActivityExplorerEditorInputFactory.createElement(IMemento) line: 32 EditorReference.getRestoredInput() line: 402
EditorReference.getEditorInput() line: 365
EditingSession.reorderEditorsIfNeeded(DialectEditor) line: 163
EditingSession.attachEditor(DialectEditor) line: 141
DDiagramEditorImpl.init(IEditorSite, IEditorInput) line: 451

(In reply to felix.dorner from comment #6)

I don't know how to write an automated test for this since it involves restarting capella.

Very good question. Maybe with rcptt and a workbench context with both editors, but I think I remember that rcptt workbench context doesn't work well with the activity explorer (and yes we should investigate this and open a bug here)

In any case, since I found about [ECLIPSE-486814 I think there is a](https://github.com/search?q=ECLIPSE-486814 I think there is a&type=Issues) design issue with ActivityExplorerEditorInput. See my comments on the other bug. If we can fix that one, this one here should be fixed automatically.

Yes, I think this is a good road to follow.

Thanks

felix.dorner commented on 2016-02-04

(In reply to matthieu.helleboid from comment #7)

Hello Felix,

(In reply to felix.dorner from comment #5)

Matthieu, I have just tested again with the rebased patch and for me it still solves the problem. I am testing this against Capella 1.0.0.

I tested both changeset (the first one and the rebased one) with a soon to be released capella (https://hudson.polarsys.org/capella/job/capella-v1.0.x/64/) and it doesn't seem to work in my case. The stack is

It would be good to fix this in 1.0.x...

Thread [main] (Suspended (breakpoint at line 79 in OpenSessionAction)) OpenSessionAction.doOpenSessions() line: 79 OpenSessionAction$1.run(IProgressMonitor) line: 141 OpenSessionAction.run() line: 152
ActivityExplorerEditorInput.loadState(IMemento) line: 254
ActivityExplorerEditorInput.(IMemento) line: 70
ActivityExplorerEditorInput.create(IMemento) line: 311
ActivityExplorerEditorInputFactory.createElement(IMemento) line: 32 EditorReference.getRestoredInput() line: 402
EditorReference.getEditorInput() line: 365
EditingSession.reorderEditorsIfNeeded(DialectEditor) line: 163
EditingSession.attachEditor(DialectEditor) line: 141
DDiagramEditorImpl.init(IEditorSite, IEditorInput) line: 451

I see you have another diagram open somewhere. In my test, I only have the two activity explorers. But still a useful stacktrace, as it shows another case where a session is opened too early. When the input is created from a memento, this shouldn't open a session.

aurelien.didier commented on 2016-02-08

Hello Felix and Matthieu,

We already have some RCPTT tests on Capella (capella\tests\plugins\org.polarsys.capella.test.rcptt\ActivityExplorer) concerning the Activity Explorer. However, it only tests the activity explorer version used in capella but it would be nice if you could add your tests here.

Thanks,

AurΓ©lien