Skip to content

Map hasEntry combined with collection matcher seems to fail incorrrectly #284

Open

Description

@jamesmudd

Here is some example code showing what I think is a bug

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.not;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

class Scratch {

	public static void main(String[] args) {
		Map<String, Object> map = new HashMap<>();
		List<String> value = List.of("value1", "value2");
		map.put("key", value);
		assertThat(map, hasEntry("key", not(empty())));

	}
}

The assert fails with the output

Exception in thread "main" java.lang.AssertionError: 
Expected: map containing ["key"-><not an empty collection>]
     but: map was [<key=[value1, value2]>]
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)

But key is mapped to a "not empty collection"? So I think it should pass.

Activity

  1. jamesmudd commented on Mar 4, 2020

    @jamesmudd
    ContributorAuthor

    Note on Java 11 with Hamcrest 2.1

  2. nhojpatrick commented on Mar 4, 2020

    @nhojpatrick
    Member

    Can you test with hamcrest v2.2

  3. nhojpatrick commented on Mar 4, 2020

    @nhojpatrick
    Member

    Your saying Java 11, are you aware if it works on another Java version?

  4. jamesmudd commented on Mar 4, 2020

    @jamesmudd
    ContributorAuthor

    Its exactly the same issue on Java 11 with Hamcrest v2.2 and on Java 8 with Hamcrest v2.2

  5. nhojpatrick commented on Mar 4, 2020

    @nhojpatrick
    Member

    Can you try the following and see if it works for you hasEntry(equalTo("key"), not(empty())).

    The method hasEntry(K, V) assumes your giving literal Key and Value parameters. So it takes your not(empty()) and assumes it's a value, and instantly wraps it as equalTo(not(empty())).

    The other method hasEntry(Matcher<K>, Matcher<V>) works for me as it takes matchers for both Key and Value. We don't have a method that takes the combination for hasEntry(K, Matcher<V>) currently which we would need for your original test.

  6. jamesmudd commented on Mar 4, 2020

    @jamesmudd
    ContributorAuthor

    Yes that works. Ok so what would be needed for this is 2 extra signatures for hasEntry

    • hasEntry(K key, Matcher<? super V> valueMatcher)
    • hasEntry(Matcher<? super K> keyMatcher, V value)

    If you think that would be an acceptable solution I can look at making a PR? I think it would be a nice feature.

  7. nhojpatrick commented on Mar 4, 2020

    @nhojpatrick
    Member

    Yes that works. Ok so what would be needed for this is 2 extra signatures for hasEntry

    • hasEntry(K key, Matcher<? super V> valueMatcher)
    • hasEntry(Matcher<? super K> keyMatcher, V value)

    If you think that would be an acceptable solution I can look at making a PR? I think it would be a nice feature.

    It would make sense to overload org.hamcrest.collection.IsMapContaining.hasEntry to have those signatures you have listed. I'm sure additional methods would get approved.

  8. sf105 commented on Mar 4, 2020

    @sf105
    Member

    Please be careful with this. Things get hard to predict with this level of overloading. It's easy to end up trying to do equals() on a matcher object. Perhaps a better option would be both matchers?

  9. added a commit that references this issue on Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions