Bug 818251

here is the promised reproducer:

https://github.com/droolsjbpm/jbpm/pull/99

Tests fails due to inspecting disconnected snapshot of the process instance since it uses persistence after completion the process instance does not reflect the latest status. Instead of verifying of the disconnected status you should use helper assertion methods that come with JbpmBpmn2TestCase, for instance to check if instance is completed:
assertProcessInstanceCompleted(pi.getId(), ksession);

instead of 
assertEquals(ProcessInstance.STATE_COMPLETED, pi.getState());

more details about test support can be found in documentation: https://hudson.qa.jboss.com/hudson/view/Drools%20jBPM/job/jbpm-5.2.x/lastSuccessfulBuild/artifact/jbpm-distribution/target/jbpm-5.2.1-SNAPSHOT-docs-build/jbpm-docs/html_single/index.html#d0e7010

Hi Maciej,

actually I don't use these assertions, because ProcessInstance class has defined 5 different states and there are only 3 assert methods: completed, aborted, active. Moreover, assertProcessInstanceCompleted and assertProcessInstanceAborted both check if process instance is null => you can't distinguish between completed and aborted process instance. AssertProcessInstanceActive checks if the process instance is not null. Again, no state checking.

If you take a look on JBPM unit tests - especially BPMN2 module, you can see there are used both ways - assertProcessInstanceCompleted and checking ProcessInstance.STATE_COMPLETED.

But as far as I understand it, none of them can be used safely.

Regards,

Radek

Radek, I got your point and in fact I agree with it that we could think about improved version of tests assertion to not make user confused with the results and I believe that was the intention of having this helper methods to shield users from checking it differently if it uses persistence or not. Session will have only processes that are active and thus you get null for completed processes. Additionally you could verify completed nodes with another helper method to secure they were visited as expected (assertNodeTriggered).

I believe the issue reported by this bz could be considered as work as design since the process is completed as expected. Whyt?

The reported issue is not a bug, as everything is working as expected.  I agree that we can improve the assertions, and depending on what features you have enabled (persistence, audit log, etc.) we can then make the assertions more intelligent.

We will take these recommendations into account.  Adding dev_ack- for now, as there's no time to do these improvements for this release unfortunately.

assertions has been improved as can be seen here: https://github.com/droolsjbpm/jbpm/blob/master/jbpm-bpmn2/src/test/java/org/jbpm/bpmn2/JbpmBpmn2TestCase.java#L633

Shall this bz be moved to version 6 somehow? If so I would mark it as verified.