Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for semantic ordering after creation on top of lifeline #1

Closed

Conversation

planger
Copy link

@planger planger commented Sep 12, 2018

Adds tests for issues eclipsesource#343 and eclipsesource#347. To enable more efficient adding of
testing, this change also refactors the tests a bit and extracts common
methods into dedicated model fixture.

This change addresses comment #343#issuecomment-418037298 (third
comment) of #343. When inserting an execution specification as first
element (execParams.isInsertBefore) of a lifeline, the isInsertBefore
flag has not been respected when invoking the nudge command.

Signed-off-by: Philip Langer <planger@eclipsesource.com>
Signed-off-by: Philip Langer <planger@eclipsesource.com>
If there are no elements above the top deleted element, we need to fall
back to the bottom of the lowest related lifeline's header. This change
addresses comment #343#issuecomment-418035483 (second comment) of #343.

Signed-off-by: Philip Langer <planger@eclipsesource.com>
Adds tests for issues #343 and #347. To enable more efficient adding of
testing, this change also refactors the tests a bit and extracts common
methods into dedicated model fixture.

Signed-off-by: Philip Langer <planger@eclipsesource.com>
Signed-off-by: Philip Langer <planger@eclipsesource.com>
@planger
Copy link
Author

planger commented Sep 12, 2018

Please note that three of the tests currently still fail which should be inline with the issues documented in eclipsesource#331 (comment).

Signed-off-by: Philip Langer <planger@eclipsesource.com>
@planger
Copy link
Author

planger commented Sep 12, 2018

Please also note that there is no need to review the commits before the merge in
5420798. Those are already in https://github.com/eclipsesource/papyrus-seqd/tree/master.

Copy link
Owner

@cdamus cdamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tests, @planger! I'll address my own comments when I merge this and then can get to work on fixing stuff to make the failing tests pass.

public Optional<MExecution> getMExecution(ExecutionSpecification execution) {
Optional<MElement<? extends ExecutionSpecification>> element = getMInteraction()
.getElement(execution);
if (element.isPresent() && element.get() instanceof MExecution) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usual way to do this is

return element.filter(MExecution.class::isInstance).map(MExecution.class::cast);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(y)

@SuppressWarnings({"boxing" })
public Object isSemanticallyBefore(MOccurrence<? extends Element> one,
MOccurrence<? extends Element> other) {
EList<InteractionFragment> fragments = getMInteraction().getElement().getFragments();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory, we should be able to rely on the Graph API for this, specifically the Vertex::precedes(Vertex) query. Does that present any problem? I can check.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think it would present a problem. I just purposefully bypassed the graph to be sure the assertion would test directly on the UML model, as it kind of is the goal of the test. However, since the vertex is essentially "only" a facade over the UML model, we might as well use it.

}

@Test
@SuppressWarnings({"boxing" })
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should configure the test projects to ignore boxing. The annotations are so much noise ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(y)

import org.eclipse.uml2.uml.Message;

@SuppressWarnings("restriction")
public class ModelEditFixture extends Edit {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! We've needed this for a while. 👍

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, glad you like it!

@planger
Copy link
Author

planger commented Sep 12, 2018

Thanks for the feedback and for addressing your comments!

@cdamus cdamus closed this Sep 12, 2018
@cdamus
Copy link
Owner

cdamus commented Sep 12, 2018

These changes are squashed and merged to my branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants