Fix flaky test in SimpleCrane4jGlobalConfigurationTest

[bbelide2]

Change Assert condition to compare two sets instead of lists to make the test non-flaky.

Flaky test

cn.crane4j.core.support.SimpleCrane4jGlobalConfigurationTest#operatePropertyMappingStrategy

crane4j/crane4j-core/src/test/java/cn/crane4j/core/support/SimpleCrane4jGlobalConfigurationTest.java

Line 34 in b65f732

public void operatePropertyMappingStrategy() {

Problem

Test operatePropertyMappingStrategy in SimpleCrane4jGlobalConfigurationTest is detected as flaky with the NonDex tool. The test failed with the following error:

Running cn.crane4j.core.support.SimpleCrane4jGlobalConfigurationTest
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.257 sec <<< FAILURE!
operatePropertyMappingStrategy(cn.crane4j.core.support.SimpleCrane4jGlobalConfigurationTest)  Time elapsed: 0.003 sec  <<< FAILURE!
java.lang.AssertionError: expected:<[cn.crane4j.core.parser.handler.strategy.ReferenceMappingStrategy@3e694b3f, cn.crane4j.core.parser.handler.strategy.OverwriteNotNullMappingStrategy@1bb5a082, cn.crane4j.core.parser.handler.strategy.OverwriteMappingStrategy@78691363]> but was:<[cn.crane4j.core.parser.handler.strategy.OverwriteMappingStrategy@78691363, cn.crane4j.core.parser.handler.strategy.OverwriteNotNullMappingStrategy@1bb5a082, cn.crane4j.core.parser.handler.strategy.ReferenceMappingStrategy@3e694b3f]>
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:120)
        at org.junit.Assert.assertEquals(Assert.java:146)
        at cn.crane4j.core.support.SimpleCrane4jGlobalConfigurationTest.operatePropertyMappingStrategy(SimpleCrane4jGlobalConfigurationTest.java:41)

Root cause

Multiple objects of class PropertyMappingStrategy are added to the configuration object of class SimpleCrane4jGlobalConfiguration using the addPropertyMappingStrategy function. These strategy objects are stored as a HashMap in SimplePropertyMappingStrategyManager. To test whether they are added correctly or not, all the strategies are fetched using function getAllPropertyMappingStrategies() which is converted to a ArrayList for assertion. But, a HashMap may not necessarily maintain the order of elements as they are inserted. Therefore, when fetched we may get a different order of elements and thus this test is detected as flaky using Nondex tool.

Strategies are added here:

crane4j/crane4j-core/src/test/java/cn/crane4j/core/support/SimpleCrane4jGlobalConfigurationTest.java

Line 40 in b65f732

strategies.forEach(configuration::addPropertyMappingStrategy);

HashMap is used here:

crane4j/crane4j-core/src/main/java/cn/crane4j/core/parser/handler/strategy/SimplePropertyMappingStrategyManager.java

Line 22 in b65f732

private final Map<String, PropertyMappingStrategy> propertyMappingStrategies = new HashMap<>();

Assertion is made here:

crane4j/crane4j-core/src/test/java/cn/crane4j/core/support/SimpleCrane4jGlobalConfigurationTest.java

Line 41 in b65f732

Assert.assertEquals(strategies, new ArrayList<>(configuration.getAllPropertyMappingStrategies()));

Fix

We can change the HashMap to a LinkedHashMap to keep the order deterministic. But this requires a change in the actual code and LinkedHashMap is slightly more computationally expensive than HashMap due to the overhead of maintaining the linked list. But since the order is not required in the code, I only updated the test to compare the result as a set instead of a List. This removes the flakiness in the test without touching the actual code.

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

1. Module build - Successful
   Command used -

mvn install -pl crane4j-core -am -DskipTests

2. Regular test - Successful
   Command used -

mvn -pl crane4j-core test -Dtest=cn.crane4j.core.support.SimpleCrane4jGlobalConfigurationTest#operatePropertyMappingStrategy

3. NonDex test - Failed
   Command used -

mvn -pl crane4j-core edu.illinois:nondex-maven-plugin:2.1.1:nondex -DnondexRuns=10 -Dtest=cn.crane4j.core.support.SimpleCrane4jGlobalConfigurationTest#operatePropertyMappingStrategy

NonDex tests passed after the fix.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b65f732) 90.38% compared to head (7380de6) 90.35%.

Additional details and impacted files

@@             Coverage Diff              @@
##                dev     #180      +/-   ##
============================================
- Coverage     90.38%   90.35%   -0.03%     
+ Complexity     1417     1416       -1     
============================================
  Files           141      141              
  Lines          3412     3412              
  Branches        317      317              
============================================
- Hits           3084     3083       -1     
  Misses          186      186              
- Partials        142      143       +1     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.