dart-lang / test

A library for writing unit tests in Dart.
https://pub.dev/packages/test
495 stars 213 forks source link

Support for MapEntry #2377

Open davidmorgan opened 6 years ago

davidmorgan commented 6 years ago

Per https://github.com/dart-lang/sdk/issues/32559 MapEntry does not implement operator== or hashCode; it would be useful to support it in package:matcher, e.g. default deep equals.

natebosch commented 6 years ago

I think it's worth adding. Some decisions to make:

  1. Should we understand MapEntry directly the same way to we do with collections and handle it similarly to other "deep equality" stuff, or should we add a utility entryWhere(...). I think I'd lean towards the entryWhere(...).
  2. Assuming we go with entryWhere, should it be entryWhere(key: ..., value: ...)? If so, what would it mean to omit one of those?
jakemac53 commented 6 years ago

Looking at the current support in matcher I see containsPair, and equals. It looks like contains already has a meaning for maps (just checks the key, which makes sense).

A base level matcher such as entryWhere has value (although I don't like the name because it isn't consistent with other matchers), we could possibly even add support directly to equals (although we might have to do a breaking change for that, not sure).

Once you have a lower level entry matcher for MapEntry that lets you do nice things with the map.entries iterable also, and it should generally compose well with other existing matchers.

marcglasberg commented 4 years ago

Too bad it's been 2 years and this simple functionality wasn't implemented. I vote for understanding MapEntry directly the same way we do with collections. This would be great:

expect(entry, MapEntry("a", 1));
jakemac53 commented 4 years ago

So in that sense we would also allow matchers for the key/value of the entry right?

For instance:

expect(entry, MapEntry(startsWith("a"), greaterThan(1)));
jakemac53 commented 4 years ago

Note that I don't think there is a much disagreement on supporting this, but there isn't a lot of motivation (not many people asking for it).

This is the type of change that might be a good candidate for a pull request if it is something that you feel you would get a lot of benefit from (want to make sure we reach consensus on the api here first though).

natebosch commented 4 years ago

So in that sense we would also allow matchers for the key/value of the entry right?

For instance:

expect(entry, MapEntry(startsWith("a"), greaterThan(1)));

This would require an ugly hack to work and wouldn't work the same way as things like collections.

jakemac53 commented 4 years ago

Wouldn't it just be using equals to check if the keys/values are equal instead of ==?

natebosch commented 4 years ago

EDIT: I think I'm wrong about this, we likely could get it to work...

It would mean that MapEntry needs to be added as a special case for wrapMatcher.

https://github.com/dart-lang/matcher/blob/225cc1399476b66f3c80d415c2dd959236d293ff/lib/src/util.dart#L37-L51

else if (x is MapEntry) {
  return entryWhere(x.key, x.value);
}

It would be analogous to supporting:

expect(['a', 'b'], [startsWith('a'), startsWith('b')]);

which we don't support today. which we do support today, so this might be able to work too.

marcglasberg commented 4 years ago

@jakemac53

but there isn't a lot of motivation (not many people asking for it).

It's my experience that a lot of people suffer with a lot of problems they don't complain about. They just use some workaround or get used to it. Complaining is difficult, specially for Dart related matters (Flutter is easier). You have to find where to complain, and then usually they see issues like this, opened for years, and just realize nobody will do anything about it, so they don't bother.

marcglasberg commented 3 years ago

While implementing fast_immutable_collections I need to compare an Iterable of MapEntry:

    expect(
        IMap({"a": 1, "b": 2, "c": 3}).addMap({"a": 1, "b": 8, "d": 10}).entries,
        [MapEntry("c", 3), MapEntry("a", 1), MapEntry("b", 8), MapEntry("d", 10)];

Is there a matcher for that? I believe this is part of supporting MapEntry, since the entries getter returns an Iterable of MapEntry.

zmoshansky commented 2 years ago

In case it helps someone else...

For testing, I ended up creating a custom matcher as follows... Note, the value is a subtype of List and thus the listEquals in the matches fn, but I'm sure there's a much better/more generic way to compare the values.

class MapEntryMatcher<K> extends Matcher {
  final MapEntry<K, List> _value;
  MapEntryMatcher(this._value);

  @override
  Description describe(Description description) =>
      description.add(_value.toString());

  @override
  bool matches(item, Map matchState) {
    if (item is MapEntry) {
      return item.key == _value.key && listEquals(item.value, _value.value);
    } else {
      return false;
    }
  }
}