Flaky test fixes #234
Merged
Flaky test fixes #234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
remcowesterhoud
force-pushed
the
202-flaky-test
branch
6 times, most recently
from
f6e7650
to
9892291
Compare
pihme
approved these changes
Mar 4, 2022
...gent/src/main/java/io/camunda/zeebe/process/test/engine/agent/RecordStreamSourceWrapper.java
Show resolved
Hide resolved
This dependency had been changed to slf4j-api to prevent bothering users with an implementation of slf4j. However, the engine-agent is not supposed to be included as a dependency by users. It's sole purpose is running the engine in a testcontainer. Doing this requires an implementation of slf4j.
This test was flaky because we immediately increased the engine time after doing the deployment. This could result in the engine time being increased, before the timer is scheduled. That means if the timer is expected to be scheduled in 1 day. It would now be scheduled in 1 day + the increase in engine time. This caused the timer to not be triggered at all.
This makes sure the tests are started on ubuntu-latest first. This is the fastest runner, so if any tests fail we will notice it sooner than if we started with macos or windows.
This test was only flaky when running it using testcontainers. The problem lies in the RecordStreamSourceWrapper. In this class we would get all the records from the engine (RecordStreamSource). These records would be mapped to json and put into a list. The problem for this test was that we just override all records everytime we request them from the engine. With this change we keep track of the latest event we've mapped. When we get a new request we will fetch all the records of the engine. We will filter those so we only have the records that we haven't stored in our list yet. These will be mapped and added. When the mapping is done we return a copy of the list. We need to make sure no more records are added, because the calling method will iterate over this list.
remcowesterhoud
force-pushed
the
202-flaky-test
branch
from
9892291
to
6a554b2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
testLastProcessInstance
This test was flaky because we immediately increased the engine time after doing the deployment. This could result in the engine time being increased, before the timer is scheduled.
That means if the timer is expected to be scheduled in 1 day. It would now be scheduled in 1 day + the increase in engine time. This caused the timer to not be triggered at all.
MultiThreadTest
I also encountered a different flaky test which is also fixed by this pr. This is the
MutltiThreadTest.This test was only flaky when running it using testcontainers. The problem lies in the
RecordStreamSourceWrapper. In this class we would get all the records from the engine (RecordStreamSource). These records would be mapped to json and put into a list. The problem for this test was that we just override all records everytime we request them from the engine.With this change we keep track of the latest event we've mapped. When we get a new request we will fetch all the records of the engine. We will filter those so we only have the records that we haven't stored in our list yet. These will be mapped and added. When the mapping is done we return a copy of the list. We need to make sure no more records are added, because the calling method will iterate over this list.
Related issues
closes #202
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
Testing:
Documentation: