moquette-io / moquette

Java MQTT lightweight broker
http://moquette-io.github.io/moquette/
Apache License 2.0
2.27k stars 814 forks source link

Fix a flaky test #781

Closed dserfe closed 8 months ago

dserfe commented 8 months ago

This PR is to fix a flaky test o.moquette.broker.MemoryRetainedRepositoryTest#testRetainedOnTopicReturnsWildcardTopicMatch in module broker.

Test failures

In line 84 of the test file, retainedMessages = repository.retainedOnTopic("foo/#"); calls retainedOnTopic from broker/src/main/java/io/moquette/broker/MemoryRetainedRepository.java. In line 33 of the main class file provided, storage is defined as a ConcurrentHashMap, its keys are not in sorted order. Consequently, in line 58, when the retainedOnTopic method calls storage.entrySet(), the returned elements' order is non-deterministic. This leads to inconsistencies in the order of the elements in matchingMessages. When it is returned to the test, the assertion in lines 87-88 (in the test file) assumes the orders are consistent, causing the test to fail.

Fix

To fix the flakiness, we can either change the test code or change the main code: 1) One fix is to sort the elements of retainedMessages before making assertions of them in the test, which is the code change in this PR. 2) If you prefer to resolve the flakiness from the main code, the fix is to use ConcurrentSkipListMap instead of ConcurrentMap. ConcurrentSkipListMap is sorted and thread-safe. If you prefer the code changes of the following PR: https://github.com/dserfe/moquette/pull/2/files, please let me know!