flowable-engine icon indicating copy to clipboard operation
flowable-engine copied to clipboard

#3835 fluent assertions

Open martin-grofcik opened this issue 1 year ago • 4 comments

Check List:

  • Unit tests: YES
  • Documentation: NA yet

martin-grofcik avatar Feb 02 '24 19:02 martin-grofcik

Hi @filiphr.

re: single flowable-assertj I will change the structure.

re: DB calls. What I would like to achieve is:

https://github.com/flowable/flowable-engine/blob/ee24b3c87b3432d86672fd8d6c8e9e239d0e7e27/modules/flowable-assertions/flowable-process-assertions/src/test/java/org/flowable/assertions/process/LongRunningProcessInstanceAssertTest.java#L35-L55

One asserThatProcessInstance evolves in the time, so there is no need to create a new instance. If we "cache" the instance status, it is not possible to achieve it.

martin-grofcik avatar Feb 15 '24 14:02 martin-grofcik

@martin-grofcik my question is why would you like to cache the instance? Why not do something like

 asserThatProcessInstance(processInstance.getId()).isRunning() 
         .activities().extracting(ActivityInstance::getActivityId).containsExactlyInAnyOrder( 
             "theStart", "theStart-theTask", "theTask1" 
         ); 
  
 taskService.complete(geTaskIdForProcessInstance(twoTasksProcess.getId(), taskService)); 
  
asserThatProcessInstance(processInstance.getId()).isRunning().activities().extracting(ActivityInstance::getActivityId).containsExactlyInAnyOrder( 
         "theStart", "theStart-theTask", "theTask1", "theTask1-theTask2", "theTask2" 
 ); 

 taskService.complete(geTaskIdForProcessInstance(twoTasksProcess.getId(), taskService)); 
  
asserThatProcessInstance(processInstance.getId()).doesNotExist().inHistory() 
         .activities().extracting(HistoricActivityInstance::getActivityId).containsExactlyInAnyOrder( 
             "theStart", "theStart-theTask", "theTask1", "theTask1-theTask2", "theTask2", "theTask-theEnd", 
                 "theEnd" 
         ); 

Perhaps we can even have a ProcessAssert that you can instantiate and then use it via processAssert.assertThatXXX

filiphr avatar Feb 15 '24 14:02 filiphr

assertThatProcessInstance(processInstance.getId()).isRunning()

is too verbose and non fluent. I would prefer

assertThat(processInstance).isRunning()...

from the code reading perspective. assertThat(processInstance) still allows us to keep query result in the instance.

I do not know whether following adds the value:

   var assertThatOneTaskProcess = assertThat(processInstance).isRunning();

  taskService.complete(oneTaskProcessTask.getId());

  assertThatOneTaskProcess.doesNotExist();

My initial impl was storing process instance state in the assertion instance. So I do not mind.

Conclusion: What do you prefer?

martin-grofcik avatar Feb 15 '24 14:02 martin-grofcik

Any update?

martin-grofcik avatar May 13 '24 07:05 martin-grofcik