Unflake tests with published messages

[korthout]

Description

ZPT test cases that manipulate time should not buffer messages for a shorter period than they want to travel through time because the message expires.

Most often, the message expiry is not the event that your test wants to travel through time for, but rather you're doing so to wait for a timer event to trigger. However, time travel in ZPT is only deterministic in the sense that it awaits busy and idle state after time manipulation. That includes message expiry.

I've changed all tests to default to publishing messages without a TTL, so they aren't buffered. This means they cannot expire later during time travel. Instead, messages published without a TTL expire immediately when published.

I've also updated the example test case that showcases both message publication and time travel in the same test case.

This resolves a flaky test.

Related issues

closes #984

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

[korthout]

I've run the related flaky test #984 locally, noticing that it failed about every third run. After the fix, I ran it successfully dozens of times. I've also run the entire test suite several times to verify that I hadn't created any new flaky tests.

[github-actions]

Test Results

 50 files  ±0   50 suites  ±0   1m 42s ⏱️ -32s
134 tests ±0  134 ✅ ±0  0 💤 ±0  0 ❌ ±0 
426 runs  ±0  426 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 5e5221e. ± Comparison against base commit c3c1ec1.

♻️ This comment has been updated with latest results.

[tmetzke]

@korthout, have you run ZPT test cases by any chance, using your changes?
If I get your changes right, this test case in ZPT needs to be adjusted because now the message expires directly and the test case fails: https://github.com/camunda/zeebe-process-test/blob/main/qa/abstracts/src/main/java/io/camunda/zeebe/process/test/qa/abstracts/assertions/AbstractMessageAssertTest.java#L269.

Off-topic: another good reason to move ZPT into the repo 🙂

[korthout]

@korthout, have you run ZPT test cases by any chance, using your changes? If I get your changes right, this test case in ZPT needs to be adjusted because now the message expires directly and the test case fails: https://github.com/camunda/zeebe-process-test/blob/main/qa/abstracts/src/main/java/io/camunda/zeebe/process/test/qa/abstracts/assertions/AbstractMessageAssertTest.java#L269.

The testHasExpiredFailure test case expects the message to not expire after publishing it. It depended on the message being published with a TTL of 1 minute (in Utilities.sendMessage). However, as I'm changing the behavior of this utility method to publish messages without a TTL, the message immediately expires on publication. That's expected behavior. The test must be adjusted because it explicitly needs to publish a message with some TTL.

I have run all ZPT tests multiple times:

I've also run the entire test suite several times to verify that I hadn't created any new flaky tests.

Does that make sense @tmetzke ?

[tmetzke]

@korthout, have you run ZPT test cases by any chance, using your changes? If I get your changes right, this test case in ZPT needs to be adjusted because now the message expires directly and the test case fails: https://github.com/camunda/zeebe-process-test/blob/main/qa/abstracts/src/main/java/io/camunda/zeebe/process/test/qa/abstracts/assertions/AbstractMessageAssertTest.java#L269.

The testHasExpiredFailure test case expects the message to not expire after publishing it. It depended on the message being published with a TTL of 1 minute (in Utilities.sendMessage). However, as I'm changing the behavior of this utility method to publish messages without a TTL, the message immediately expires on publication. That's expected behavior. The test must be adjusted because it explicitly needs to publish a message with some TTL.

I have run all ZPT tests multiple times:

I've also run the entire test suite several times to verify that I hadn't created any new flaky tests.

Does that make sense @tmetzke ?

That makes a ton of sense, exactly what I expected. Thanks, @korthout 👍
I had mistaken this for a change in Zeebe itself first, that's why I was wondering how it'll affect ZPT. Since it's in ZPT itself, you took care of everything I mentioned already.

[tmetzke]

👍 A really cool change, @korthout, I like the simplicity of it and the effect it has, making things more straightforward and robust here at the same time.
💭 I added a thought that shouldn't block you from merging here so I'll approve this already from my end.

[tmetzke]

👍 Perfect, @korthout, that's nice ⭐

[korthout]

Still using bors here - another reason to move ZPT into the Zeebe repo 🌟

bors merge

[ghost]

Build succeeded:

• Test summary

[backport-action]

Successfully created backport PR for stable/8.1:

• [Backport stable/8.1] Unflake tests with published messages #1041

[backport-action]

Successfully created backport PR for stable/8.2:

• [Backport stable/8.2] Unflake tests with published messages #1042

[backport-action]

Successfully created backport PR for stable/8.3:

• [Backport stable/8.3] Unflake tests with published messages #1043

[backport-action]

Successfully created backport PR for stable/8.4:

• [Backport stable/8.4] Unflake tests with published messages #1044