Fix flaky tests in LogMessageTest #1500
Merged
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.
Change Assert condition to check on both map orders in a flaky test in LogMessageTest to make it non-flaky.
Flaky test
hippo4j/infra/common/src/test/java/cn/hippo4j/common/toolkit/logtracing/LogMessageTest.java
Line 73 in 26212e4
Problem
Test
testKvShouldPutAllKeyAndValuePairs
inLogMessageTest
is detected as flaky with the NonDex tool. The test failed with the following error:Root cause
In this test, 2 key-value pairs are set to the
kvs
field (ConcurrentHashMap) oflogMessage
object. Then,logMessage.toString()
is called and the response is compared with hard-coded strings. logMessage.toString() converts the given map to string form along with some other processing. But, ConcurrentHashMap/HashMap may not maintain the order of elements. Therefore, when NonDex tests are run, the order of elements are shuffled and the output is reversed and incorrect thus making the test flaky.Key-values are inserted this way:
hippo4j/infra/common/src/test/java/cn/hippo4j/common/toolkit/logtracing/LogMessageTest.java
Lines 74 to 75 in 26212e4
Output is compared this way:
hippo4j/infra/common/src/test/java/cn/hippo4j/common/toolkit/logtracing/LogMessageTest.java
Line 76 in 26212e4
Fix
Usually, switching from HashMap to LinkedHashMap will make tests non-flaky because LinkedHashMap will preserve order of elements. But, we cannot change a ConcurrentHashMap to LinkedHashMap because LinkedHashMap is not thread-safe like ConcurrentHashMap. Therefore, I updated the assert statement to check the output for both the possible orders of elements.
This fix will not affect the code since the change is only made in tests.
How this has been tested?
Java: openjdk version "11.0.20.1"
Maven: Apache Maven 3.6.3
Command used -
Command used -
Command used -
NonDex tests passed after the fix.