-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
#3835 fluent assertions #3837
base: main
Are you sure you want to change the base?
#3835 fluent assertions #3837
Conversation
...cess-assertions/src/main/java/org/flowable/assertions/process/FlowableProcessAssertions.java
Outdated
Show resolved
Hide resolved
...-assertions/src/main/java/org/flowable/assertions/process/HistoricProcessInstanceAssert.java
Outdated
Show resolved
Hide resolved
...-assertions/src/main/java/org/flowable/assertions/process/HistoricProcessInstanceAssert.java
Outdated
Show resolved
Hide resolved
...-assertions/src/main/java/org/flowable/assertions/process/HistoricProcessInstanceAssert.java
Outdated
Show resolved
Hide resolved
...-assertions/src/main/java/org/flowable/assertions/process/HistoricProcessInstanceAssert.java
Outdated
Show resolved
Hide resolved
...-process-assertions/src/main/java/org/flowable/assertions/process/ProcessInstanceAssert.java
Outdated
Show resolved
Hide resolved
…in/java/org/flowable/assertions/process/HistoricProcessInstanceAssert.java Co-authored-by: David B Malkovsky <[email protected]>
…in/java/org/flowable/assertions/process/HistoricProcessInstanceAssert.java Co-authored-by: David B Malkovsky <[email protected]>
…in/java/org/flowable/assertions/process/ProcessInstanceAssert.java Co-authored-by: David B Malkovsky <[email protected]>
…o #3835_fluent_assertions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an interesting idea @martin-grofcik. It allows for some good testing of processes / cases. I have some questions / ideas.
- In my opinion I think that a single
flowable-assertj
module where we would add optional support for different modules is more correct. Users don't need to care which module for asserting they want to bring. They can just addflowable-assertj
and use whatever they want from there. We would need to have different entry points, but that is fine. - I see that in the asserts we are doing DB calls all the time. What is the purpose of that? e.g. for
hasVariableWithValue
we first dohasVariable
which will do a DB call and then we do another DB call. I think that it would be better if we treat the instance as is and don't do additional DB calls if it isn't needed.
Hi @filiphr. re: single re: DB calls. Lines 35 to 55 in ee24b3c
One |
@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 |
assertThatProcessInstance(processInstance.getId()).isRunning() is too verbose and non fluent. I would prefer assertThat(processInstance).isRunning()... from the code reading perspective. 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? |
Any update? |
Check List: