bpmn-io / bpmn-js-token-simulation

A BPMN 2.0 specification compliant token simulator.
https://bpmn-io.github.io/bpmn-js-token-simulation/modeler.html?e=1&pp=1
MIT License
243 stars 69 forks source link

More precise simulation log icons. #168

Closed timKraeuter closed 3 months ago

timKraeuter commented 4 months ago

More precise simulation log icons (Closes #167).

I didn't add any tests since there were none before, but I could do so if needed. I used the following models to test the improved logging manually: intermediate_events.txt end_events.txt start_events.txt tasks.txt

I added two TODOS for other broken/missing functionality:

  1. There are no traces for terminate end events. Probably since the process is aborted.
  2. There is code that tries to log which outgoing sequence flow is chosen for an exclusive gateway, but this does not work when I tried to test it.
nikku commented 3 months ago

@timKraeuter Do you think it makes sense to create a test case for this? We could run through all supported elements and see if respective LOG entries are in the trace.

nikku commented 3 months ago

Logged what you reported as a bug (termination) here.

timKraeuter commented 3 months ago

Tests would be nice and we can use the four models I provided earlier to write one test case each for tasks, start events, intermediate events and end events. We could also merge it into one test if you prefer that.

Is it easy to trigger events/receive tasks programmatically? Maybe you can give me a hint so I do not have to search too much for it.

timKraeuter commented 3 months ago

Logged what you reported as a bug (termination) here.

What do you think about my 2nd point? Is it worth it to change the trace event to include the chosen sf for an exclusive gateway and fix that part?

nikku commented 3 months ago

A test looks like this, asserting the path. You trigger tests through the triggerElement utility.

I suggest that we replace the Log#log component to capture actual log statements, and assert what was logged:


class CustomLog extends Log {
  constructor(...) {
    super(...);

    this._logged = [];
    this.log = (args) => {
      this._logged.push(args);
    };

    ...
  }
}

beforeEach(bootstrapBpmnModeler({
  additionalModules: [ {
    log: [ 'type', CustomLog ]
  } ]
});
nikku commented 3 months ago

Logged what you reported as a bug (termination) https://github.com/bpmn-io/bpmn-js-token-simulation/issues/169.

What do you think about my 2nd point? Is it worth it to change the trace event to include the chosen sf for an exclusive gateway and fix that part?

We could consider to log sequence flows (or message flows) taken. Ultimately this depends on whether it is information overkill or not. At some point a backtrace (understand where a token came from) may be a better visualization.

timKraeuter commented 3 months ago

Just added the tests. Let me know what you think.

timKraeuter commented 3 months ago

Logged what you reported as a bug (termination) #169.

What do you think about my 2nd point? Is it worth it to change the trace event to include the chosen sf for an exclusive gateway and fix that part?

We could consider to log sequence flows (or message flows) taken. Ultimately this depends on whether it is information overkill or not. At some point a backtrace (understand where a token came from) may be a better visualization.

Alright, I will remove the dead code for now. I also saw we are not logging parallel gateways. Should we do that to stay consistent with exclusive gateways or is it just information overload?

nikku commented 3 months ago

I think we should consistently log all flow nodes and may consider to log sequence and message flows in the future.

timKraeuter commented 3 months ago

I will change that and add one more test for gateways. Inclusive gateways also need to be included.

timKraeuter commented 3 months ago

The changes should be ready for review now.

nikku commented 3 months ago

Thanks for the follow-up. I pushed a bunch of style changes in top. Great contribution + great test coverage added. :rocket: