Honor map key comparison semantics with containsOnly assertions - #2167

@filiphr feel free to review and contribute to this PR if you have time, this one supersedes #2160.

The build fails due to too many lib to shade, not a blocker right now for us to play with objenesis

I am not sure that using Objenesis or even creating a copy of the map should be the way forward. At least not for all maps.

Have a look at my additional test cases in filiphr@96360d7. Using Collections.unmodifiableMap works, using Guava immutable map works as well. Using SingletonMap doesn't work.

A potential (not 100% cases solution) would be to only create a copy if the map is a case insensitive map. e.g. like in filiphr@d7a77b9.

thanks for fixing the pom @scordio.

I try to summarize the topic to see if I got it correctly.

The containsOnly / containsExactly assertions consist of two steps:

1. Checking if the expected elements are present
2. Finding the unexpected ones

Elements can be either keys or entries depending on the specific assertion.

Checking the expected elements

The check should always be performed on actual to honor the original semantic.

This is mostly relevant with special implementations like java.util.IdentityHashMap, org.apache.commons.collections4.map.CaseInsensitiveMap or org.springframework.util.LinkedCaseInsensitiveMap.

Finding of unexpected elements

In the ideal case, we want to have a copy of the map and remove the expected elements, so what is left are the unexpected elements.

Being able to do this strongly depends on the map implementation.

1. Cloneable implementation

   A new instance can be obtained via clone(), then we try to remove() the expected elements

   • remove succeeds → unexpected elements are identified
   • remove fails with UnsupportedOperationException, (i.e., unmodifiable) → new LinkedHashMap initialized with the elements of actual, remove() is repeated again → unexpected elements are identified but with the potential loss of the original semantic.

2. Non Cloneable implementation, but a new instance can be obtained

   A new instance can be obtained via reflection (e.g., plain, Objenesis), then we try to putAll() the elements of actual

   • putAll() succeeds → we try to remove() the expected elements as in the Cloneable case.
   • putAll() fails with UnsupportedOperationException, (i.e., unmodifiable) → new LinkedHashMap is initialized with the elements of actual, remove() is repeated again → unexpected elements are identified but with the potential loss of the original semantic.

3. No new instance can be obtained

   New LinkedHashMap is initialized with the elements of actual, the remove() of the expected elements is performed → unexpected elements are identified but with the potential loss of the original semantic.

@joel-costigliola @filiphr what do you think about this approach? Do you see any other cases to be handled?

Sounds good to me, a few comments:

• containsExactly also checks the order of keys/entries
• IdentityHashMap is another map implementation that does not rely on equals but being part of the JDK we could simply instantiate it directly.
• We would need to state in the javadoc that in some cases the key comparison semantics won't honored

I think that your approach @scordio makes sense.

IdentityHashMap is another map implementation that does not rely on equals but being part of the JDK we could simply instantiate it directly.

@joel-costigliola IdentityHashMap implements Cloneable. Therefore I think that it will be part of the first example from @scordio. I think that we should try and do as little key equal checks, which I think is being only done for containsExactly.

@joel-costigliola @filiphr assertContainsOnly and assertContainsOnlyKeys are in a reviewable shape, happy to get any feedback about the new implementation.

Most likely this PR will also include assertContainsExactly.

Having the same pattern for assertContainsExactly will increase the complexity of this PR as the tests are more complicated due to the ordering requirement.

I will focus on containsOnly related assertions and update assertContainsExactly in a separate PR.

Thanks @filiphr and @scordio, this is now integrated.