fix(no-loop): return no next elements if no call activity loop #156
Conversation
test/camunda-cloud/no-loop.spec.js
Outdated
| @@ -71,25 +71,28 @@ const valid = [ | |||
| moddleElement: createModdle(createDefinitions(` | |||
| <bpmn:process id="Process_1" isExecutable="true"> | |||
| <bpmn:startEvent id="StartEvent_1"> | |||
| <bpmn:outgoing>SequenceFlow_1</bpmn:outgoing> | |||
| <bpmn:outgoing>SeqeuenceFlow_1</bpmn:outgoing> | |||
There was a problem hiding this comment.
Feedback: I find it hard to assess the changes that we're testing here as I cannot see the BPMN diaram.
Why don't we source the test samples from a diagram that I can open?
Please share the before and after diagrams that you used.
There was a problem hiding this comment.
Since there are so many test cases they each have their own diagram, we'd end up with hundreds of diagrams. I agree that it's difficult without being able to open the diagram. 😞
There was a problem hiding this comment.
Before 🤡
After
There was a problem hiding this comment.
Since there are so many test cases they each have their own diagram, we'd end up with hundreds of diagrams.
As long as we don't dynamically generate the contents, what's wrong with 100 test diagrams? 🙂
There was a problem hiding this comment.
Looking at before and after what did we fix? And why don't we keep before as a valid test case, and add another one (after)? What does adding the X gateway brings to the table?
A general good practice could be to give each of these tests human readable names:
NOT:
- call activity
BUT:
- call activity (calling other process)
- call activity (calling dynamically computed process)
- ...
- call activity (calling itself)
There was a problem hiding this comment.
I do understand that what we're testing here is
call activity (calling other process)
There was a problem hiding this comment.
We could have a separate folder for each of the rules.
philippfromme
force-pushed
the
fix-no-loop
branch
from
e16027f
to
2da2db4
Compare
| @@ -122,6 +122,8 @@ function getNextFlowElements(flowElement) { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| return []; | |||
There was a problem hiding this comment.
From what I understand we either dive into the called process (when invoked via a call activity) or fallback to the default behavior, which is to continue the normal flow. Is my assumption correct?
Then why would we want to not continue the normal flow if referenced start event cannot be found / called reference cannot resolve?
There was a problem hiding this comment.
It seems I got a little bit confused here. There is no bug and the behavior was correct. 🙈 I will close this pull request and the issue and leave a message in the Slack thread.
Test was broken. We need to return no next elements if the call activity does not call the parent process.
Closes #155