chore: refactor worker test

[pihme]

Description

• remove Thread.sleep()
• wait until process instance gets completed

Related issues

closes #244

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

• The changes are backwards compatibility with previous versions
• If it fixes a bug then PRs are created to backport the fix

Testing:

• There are unit/integration tests that verify all acceptance criterias of the issue
• New tests are written to ensure backwards compatibility with further versions
• The behavior is tested manually

Documentation:

• Javadoc has been written
• The documentation is updated

[pihme]

@remcowesterhoud Have a look and tell me what you think. I think it's definitely a step forward and hopefully removes flakiness.

This line is a bit awkward and not intuitive: BpmnAssert.initRecordStream(recordStream);

What could also help would be something like engine.waitForJobCompletion(jobType);

[github-actions]

Unit Test Results

128 files  128 suites   6m 28s ⏱️
325 tests 325 ✔️ 0 💤 0 ❌
788 runs  788 ✔️ 0 💤 0 ❌

Results for commit 6453fb1.

♻️ This comment has been updated with latest results.

[pihme]

We could also do a startProcessInstanceWithResult to make this example less non-sensical.

Btw, in the testbench test this will make sense read as a test, because it will be something like:

// when
startParentProcess();
await().until( parenProcessHasCompleted);
engine.waitForIdleState();

//then
assertThat(childProcess).isStarted().hasVariable(...)

[remcowesterhoud]

I like using withResult() better than the await where you need to initialise the RecordStream. I don't want users to have to worry about this. Though, I'm not sure what happens when the process doesn't complete but runs in an incident.

I liked the idea of adding a waitForJobCompletion but I think we should make it more generic. We could make a waitFor(ValueType, Intent). That way we can wait for everything with a single method.

[pihme]

I like using withResult() better than the await where you need to initialise the RecordStream. I don't want users to have to worry about this. Though, I'm not sure what happens when the process doesn't complete but runs in an incident.

This will produce a request timeout exception.

I liked the idea of adding a waitForJobCompletion but I think we should make it more generic. We could make a waitFor(ValueType, Intent). That way we can wait for everything with a single method.

Maybe a little too technical, but a good idea.

[pihme]

I now have implemented only the refactoring for the test. The additional methods on Engine could be a follow up issue, independently prioritized.

[remcowesterhoud]

Thanks! I like the changes so much that I'd like to see them a 2nd time in the testcontainer version of this test 😉 (this duplication is already annoying me so this will be one of the first things I want to get rid of next).

I've created a separate issue for the waitFor method: #252

[remcowesterhoud]

LGTM!