Thanks @qz0610 for finding this flaky test and submitting a fix. I have one concern with the solution which is that the array returned by parameters.getKeyValues() isn't entirely unordered. It is built up by iterating through a HashMap which isn't ordered, but each pair of elements in the resulting array is an ordered key-value pair. I think it's important that the test still asserts that key-value matching does not get scrambled.

For example new Object[] {"a", null, "b", "bat", "c", "cat"} and new Object[] {"b", "bat", "c", "cat", "a", null} should both be valid results, but new Object[] {"a", "bat" , "b", "cat", "c", null} is not valid.

Perhaps we could reconstruct the returned parameters back into a HashMap and assert equality on that to ensure the key-value pairing remains correct.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.47%. Comparing base (2d32517) to head (7672782).
Report is 349 commits behind head on master.

@@             Coverage Diff              @@
##             master    #2820      +/-   ##
============================================
+ Coverage     76.16%   76.47%   +0.31%     
- Complexity    13170    13934     +764     
============================================
  Files          1085     1076       -9     
  Lines         65189    63324    -1865     
  Branches       7289     7457     +168     
============================================
- Hits          49651    48428    -1223     
+ Misses        12830    12367     -463     
+ Partials       2708     2529     -179     

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

Thank you @Cole-Greer for pointing out my overlook of the reservation of key-value pairs, and the suggestion of reconstructing a HashMap. I updated the fix by comparing the expected HashMap with the one reconstructed from params.

I'm curious, wouldn't this same problem affect other tests in that class like shouldGetKeyValues() and shouldGetKeyValuesIgnoringSomeKeys()

Hi @kenhuuu, thank you for the insight. You are absolutely correct. This same problem also affects shouldGetKeyValues() and shouldGetKeyValuesIgnoringSomeKeys() in ParametersTest. I updated the fix to include fixing these two tests. After the fix, all tests in ParametersTest have no more failures with nondex configuration.

I think it would have been a little nicer if a short helper method was used to create those Maps but it's OK since this is just a test file.

VOTE +1.

Thanks for all of the updates to the tests. Normally we ask that a PR such as this would be targeted to the 3.6-dev branch, our standard process is to target the oldest release branch which a change should be included in, and then whoever merges the PR will ensure the changes are merged up to all future branches. In this case I will cherry-pick the fix back to 3.6-dev and 3.7-dev branches.

VOTE +1 (merging as a CTR)