Assertj-core: MapAssert::containsAllEntriesOf fails for non-empty actual and empty other map

[aksh1618]

Summary

MapAssert::containsAllEntriesOf throws assertion error for non empty actual and empty other map:

java.lang.AssertionError: actual is not empty

    at org.assertj.core.internal.Maps.failIfEmptySinceActualIsNotEmpty(Maps.java:884)
    at org.assertj.core.internal.Maps.assertContains(Maps.java:350)
    at org.assertj.core.api.AbstractMapAssert.containsAllEntriesOf(AbstractMapAssert.java:580)
        // ...

Shouldn't this succeed as it does contain all 0 entries in the expected map? IMHO this would be more consistent with #2013 and non empty check should be explicit.

Example

Map<Ring, TolkienCharacter> ringBearers = new HashMap<>();
ringBearers.put(nenya, galadriel);
ringBearers.put(narya, gandalf);
ringBearers.put(vilya, elrond);
ringBearers.put(oneRing, frodo);

Map<Ring, TolkienCharacter> octopusRingBearers = new HashMap<>();

// java.lang.AssertionError: actual is not empty
assertThat(ringBearers).containsAllEntriesOf(octopusRingBearers);

[joel-costigliola]

Thanks for reporting this, you have a point, we'll discuss this with the team before reaching a decision.

[joel-costigliola]

Team agrees to do it, moreover it's consistent with containsAll iterable assertion.

[joel-costigliola]

@aksh1618 keen to contribute it?

[aksh1618]

Sure @joel-costigliola, happy to :)

[aksh1618]

@joel-costigliola I've made the changes allowing empty values to pass for following methods:

contains(Map.Entry<? extends K, ? extends V>... entries)
containsAllEntriesOf(Map<? extends K, ? extends V> other)
containsAnyOf(Map.Entry<? extends K, ? extends V>... entries)

Am I correct in assuming the other two should also have similar behavior?

[joel-costigliola]

For contains and containsAnyOf, it is most likely a user error who has forgotten to pass value(s).
I mean why would anyone write assertThat(map).contains()? Some might even interpret that as containsNothing.

Better keep the current behavior for these assertions, on the other hand I agree we need to change containsAllEntriesOf since the other map could be empty.

[aksh1618]

@joel-costigliola Might be a bit extreme, but there could be cases where this won't be a user error, for instance:

Map<Ring, TolkienCharacter> ringBearers = new HashMap<>();
ringBearers.put(nenya, galadriel);
ringBearers.put(narya, gandalf);
ringBearers.put(vilya, elrond);
ringBearers.put(oneRing, frodo);

List<String> dwarveBearers = new ArrayList<>();
Map.Entry<Ring, TolkienCharacter>[] dwarvenRingBearers = 
        ringBearers.entrySet().stream().filter(entry -> dwarveBearers.contains(entry.getValue())).toArray(Map.Entry[]::new);

assertThat(ringBearers).contains(dwarvenRingBearers);

If such cases are to be considered too extreme for the scope of the library, should the absence of arguments throw an (IllegalArguments) exception in all cases and be documented as such?

[joel-costigliola]

For contains and containsAnyOf, it is most likely a user error who has forgotten to pass value(s).

I have to contradict myself on this one :facepalm:

Let's go with the same approach we followed for for iterable contains and containsAnyOf assertions, they fail if you pass an empty array and actual was not empty.

[aksh1618]

@joel-costigliola Added a PR