-
Notifications
You must be signed in to change notification settings - Fork 15
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
Realistic example test #276
Conversation
@korthout Would you mind reviewing this? It may seem big but most code is processes that have been added 😄 I've split it into small commits. I believe this will give you a good view into how you write tests with ZPT! |
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.
Nice! I like it! 👍
I have some small suggestions, but nothing major. Please have a look, apply the hints you agree with, and merge the PR.
❓ There are still a lot of elements unused in this test case. Do we plan on expanding this test case; or plan to add additional test cases with more elements?
engine/src/main/java/io/camunda/zeebe/process/test/engine/EngineStateMonitor.java
Outdated
Show resolved
Hide resolved
assertions/src/main/java/io/camunda/zeebe/process/test/assertions/ProcessInstanceAssert.java
Outdated
Show resolved
Hide resolved
assertions/src/main/java/io/camunda/zeebe/process/test/assertions/ProcessInstanceAssert.java
Outdated
Show resolved
Hide resolved
assertions/src/main/java/io/camunda/zeebe/process/test/assertions/ProcessInstanceAssert.java
Outdated
Show resolved
Hide resolved
final String messageName, | ||
final String correlationKey, | ||
final Duration timeToLive) | ||
final Map<String, Object> variables) |
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.
🔧 Please reduce the number of parameters by combining the messageName
, correlationKey
, timeToLive
and variables
into a parameter object.
qa/src/test/java/io/camunda/zeebe/process/test/qa/abstracts/examples/AbstractPrCreatedTest.java
Outdated
Show resolved
Hide resolved
qa/src/test/java/io/camunda/zeebe/process/test/qa/abstracts/examples/AbstractPrCreatedTest.java
Show resolved
Hide resolved
qa/src/test/java/io/camunda/zeebe/process/test/qa/abstracts/examples/AbstractPrCreatedTest.java
Show resolved
Hide resolved
@korthout I'm interested to know what you think is missing. We don't have all the specific elements but I believe we did hit all the concepts that we can assert over.
The only thing missing here might be an incident. |
We are still finetuning this to a sensible value. I had flaky tests when using 30ms. Therefore it is increased to 50ms to solve the flaky tests.
When a process contains a call activity it will call another process instance. If we want to assert this process we need to create a new ProcessInstanceAssert object, with the process instance key of the called process instance. Thus far we had no way to do this, except for using inspections. This was a bit of a hassle. With this new method we can now assert that a process instance has called a different process and using extractingLatestCalledProcess we can obtain the assertions from this process. This fits the other extracting... methods we have for asserting over e.g. incidents.
All our tests contain very minimalistic processes in order to be able to test something very specific. These new tests contain a bigger process with lots of different types of elements. This would be more in line with a process our users will be testing. These tests can now serve as an example for users, as well as verifying our assertions will remain functional when testing more complex processes.
In our testing Utilities class we had some helper methods to send variables. These methods did not allow for sending message with variables. Now they do. Note: our testing Utilities are not exposed to users. There are no backwards compatibility concerns here.
Our utility method completeTask would look for any jobs with a specific elementId that were created. It would not filter out jobs that were already completed. Because of this, if a process passed an element multiple times, we were only able to complete one job. This fix filters out the jobs that were already completed. This way we can keep completing the task until there are no more jobs available.
7e9614b
to
3f10e8b
Compare
Successfully created backport PR #278 for |
@remcowesterhoud We're missing some of the special combinations like timer start event, message throw event, etc. There are many different combinations possible, see https://docs.camunda.io/docs/components/modeler/bpmn/bpmn-coverage/ |
Thanks @korthout , I believe these specific cases are tested by our smaller QA tests. The examples you mentioned for example have their own processes (https://github.com/camunda/zeebe-process-test/blob/main/qa/src/test/java/io/camunda/zeebe/process/test/qa/util/Utilities.java#L252-L256) which are being used throughout the tests. I believe these smaller tests are sufficient to verify that those elements are working as expected. I wasn't planning on writing more realistic example tests at this time. If we find out in the future that what we have currently is not sufficient we can always add these later. They would mainly serve as an example for users on how they could write their tests. Functionality wise we should have it all covered. |
Description
We currently only have very minimal processes in our tests. I've created a realistic process and created tests for this. This can be used as an example for users, as well as verify that more complex processes are testable without breaking.
During the creation of this test I've found a few things that have been added/fixed:
extractingLatestCalledProcess
method toProcessInstanceAssert
. This allows users to conveniently access the assertions of a process that is called by a call activity. With this we also have a newhasCalledProcess
assertion.Related issues
closes #273
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
Testing:
Documentation: