From 01535844c506557be9af899ed64be840e85e7279 Mon Sep 17 00:00:00 2001 From: Terence Lei Date: Mon, 10 Dec 2018 13:36:43 -0800 Subject: [PATCH] Fix ChangeNotifier generic typing issue (#76) Fix ChangeNotifier generic typing issue - ChangeRecord.ANY and ChangeRecord.None is not typesafe for any subclasses of ChangeNotifier that subclass the generic. Proposed solution is to output a `ChangeRecords extends List` with additional metadata to indicate the change is ANY or NONE. Advantage of this change is that it is backwards compatible with existing code while fixing type exceptions for future code. --- CHANGELOG.md | 7 ++ lib/observable.dart | 9 ++- lib/src/change_notifier.dart | 14 ++-- lib/src/records.dart | 65 ++++++++++++++++++- test/change_notifier_test.dart | 100 +++++++++++++++++++++++++++++ test/list_change_test.dart | 22 ++++--- test/observable_list_test.dart | 86 +++++++++++++------------ test/observable_map_test.dart | 110 ++++++++++++++++++-------------- test/observable_test.dart | 20 +++--- test/observable_test_utils.dart | 53 +++++++++++++-- 10 files changed, 361 insertions(+), 125 deletions(-) create mode 100644 test/change_notifier_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index a0aac1a..50607cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 0.22.1+5 + +Fix generic type error that occurs when using ChangeNotifier with a subclass of ChangeRecord. +Previously, calling `notifyChanges()` on `class Foo with ChangeNotifier {}` +would throw a type error. Now, the `changes` stream emits a custom `ChangeRecords` class that +implements the `List` interface. This change is backwards compatible. + ## 0.22.1+4 * Support Dart 2 stable. diff --git a/lib/observable.dart b/lib/observable.dart index 516ff70..ec037dc 100644 --- a/lib/observable.dart +++ b/lib/observable.dart @@ -6,9 +6,14 @@ library observable; export 'src/change_notifier.dart' show ChangeNotifier, PropertyChangeNotifier; export 'src/differs.dart' show Differ, EqualityDiffer, ListDiffer, MapDiffer; -export 'src/records.dart' - show ChangeRecord, ListChangeRecord, MapChangeRecord, PropertyChangeRecord; export 'src/observable.dart'; export 'src/observable_list.dart'; export 'src/observable_map.dart'; +export 'src/records.dart' + show + ChangeRecord, + ChangeRecords, + ListChangeRecord, + MapChangeRecord, + PropertyChangeRecord; export 'src/to_observable.dart'; diff --git a/lib/src/change_notifier.dart b/lib/src/change_notifier.dart index 9464740..2aeaf63 100644 --- a/lib/src/change_notifier.dart +++ b/lib/src/change_notifier.dart @@ -51,18 +51,16 @@ class ChangeNotifier implements Observable { @override @mustCallSuper bool deliverChanges() { - List changes; if (_scheduled && hasObservers) { - if (_queue != null) { - changes = freezeInDevMode(_queue); - _queue = null; - } else { - changes = ChangeRecord.ANY; - } + final changes = _queue == null + ? ChangeRecords.any() + : ChangeRecords.wrap(freezeInDevMode(_queue)); + _queue = null; _scheduled = false; _changes.add(changes); + return true; } - return changes != null; + return false; } /// Whether [changes] has at least one active listener. diff --git a/lib/src/records.dart b/lib/src/records.dart index 87d7d4f..0ef770d 100644 --- a/lib/src/records.dart +++ b/lib/src/records.dart @@ -19,10 +19,71 @@ class ChangeRecord { /// /// May be used to produce lower-GC-pressure records where more verbose change /// records will not be used directly. - static const List ANY = const [const ChangeRecord()]; + static const ANY = ChangeRecords.any(); /// Signifies no changes occurred. - static const List NONE = const []; + static const NONE = ChangeRecords.none(); const ChangeRecord(); } + +/// Represents a list of change records. +/// +/// The motivation for implementing the list interface is to fix a typing +/// issue with ChangeRecord.ANY while maintaining backwards compatibility with +/// existing code. +class ChangeRecords + extends DelegatingList { + // This is a covariant unfortunately because generics cannot be used in a + // const constructor. Should be sound however since the equality check does + // not do any mutations. + static const _listEquals = ListEquality(); + + final bool _isAny; + + final List _delegate; + + /// Represents any change where the list of changes is irrelevant. + const ChangeRecords.any() : this._(const [], true); + + /// Represents a null change where nothing happened. + const ChangeRecords.none() : this._(const [], false); + + /// Wraps around a list of records. + /// + /// Note: this wraps around a shallow copy of [list]. If [list] is modified, + /// then it is modified within this change record as well. This is provide a + /// const constructor for [ChangeRecords]. + const ChangeRecords.wrap(List list) : this._(list, false); + + /// Creates a change record list from a deep copy of [it]. + ChangeRecords.fromIterable(Iterable it) + : this._(List.unmodifiable(it), false); + + const ChangeRecords._(this._delegate, this._isAny) : super(_delegate); + + @override + int get hashCode => hash2(_delegate, _isAny); + + /// Equal if this and [other] have the same generic type and either both are + /// any records or both are not any records and have the same list of entries. + /// + /// E.g. + /// ChangeRecords.any() == ChangeRecords.any() + /// ChangeRecords.any() != ChangeRecords.any() + /// + /// List of records checked with deep comparison. + @override + bool operator ==(Object other) => + identical(this, other) || + other is ChangeRecords && + runtimeType == other.runtimeType && + ((_isAny && other._isAny) || + (!_isAny && + !other._isAny && + _listEquals.equals(_delegate, other._delegate))); + + @override + String toString() => + _isAny ? 'ChangeRecords.any' : 'ChangeRecords($_delegate)'; +} diff --git a/test/change_notifier_test.dart b/test/change_notifier_test.dart new file mode 100644 index 0000000..c2a3698 --- /dev/null +++ b/test/change_notifier_test.dart @@ -0,0 +1,100 @@ +import 'dart:async'; + +import 'package:observable/observable.dart'; +import 'package:test/test.dart'; + +import 'observable_test_utils.dart'; + +void main() { + group(ChangeRecords, () { + test('any changes', () { + expectChanges(const ChangeRecords.any(), const ChangeRecords.any()); + expectChanges(ChangeRecords.any(), ChangeRecords.any()); + expectNotChanges(ChangeRecords.any(), ChangeRecords.wrap([])); + expectNotChanges(ChangeRecords.any(), ChangeRecords.any()); + expectNotChanges(ChangeRecords.any(), ChangeRecords.any()); + }); + + test('some changes', () { + expectChanges(ChangeRecords.fromIterable([A()]), + ChangeRecords.fromIterable([A()])); + expectChanges(ChangeRecords.fromIterable([B(1), B(2)]), + ChangeRecords.fromIterable([B(1), B(2)])); + expectNotChanges(ChangeRecords.fromIterable([A()]), + ChangeRecords.fromIterable([A(), A()])); + expectNotChanges(ChangeRecords.fromIterable([B(1)]), + ChangeRecords.fromIterable([B(2)])); + expectNotChanges(ChangeRecords.fromIterable([B(1)]), + ChangeRecords.fromIterable([C()])); + }); + }); + + group(ChangeNotifier, () { + Future runTest( + FutureOr runFn(ChangeNotifier cn), + FutureOr testFn(ChangeRecords cr)) async { + final cn = ChangeNotifier(); + + cn.changes.listen((value) { + expect(value, TypeMatcher>()); + testFn(value); + }); + + await runFn(cn); + + return Future(() {}); + } + + test( + 'delivers any record when no change notified', + () => runTest((cn) { + cn.notifyChange(); + }, (cr) { + expectChanges(cr, ChangeRecords.any()); + })); + + test( + 'delivers expectChangesed changes', + () => runTest((cn) { + cn..notifyChange(B(1))..notifyChange(B(2))..notifyChange(B(3)); + }, (cr) { + expectChanges(cr, ChangeRecords.wrap([B(1), B(2), B(3)])); + })); + }); +} + +class A extends ChangeRecord { + @override + bool operator ==(Object other) => + identical(this, other) || other is A && runtimeType == other.runtimeType; + + @override + int get hashCode => 0; +} + +class B extends A { + final int value; + + B(this.value); + + @override + bool operator ==(Object other) => + identical(this, other) || + super == other && + other is B && + runtimeType == other.runtimeType && + this.value == other.value; + + @override + int get hashCode => value.hashCode; +} + +class C extends A { + @override + bool operator ==(Object other) => + identical(this, other) || + super == other && other is C && runtimeType == other.runtimeType; + + @override + int get hashCode => 2; +} diff --git a/test/list_change_test.dart b/test/list_change_test.dart index 858b27b..4d47a72 100644 --- a/test/list_change_test.dart +++ b/test/list_change_test.dart @@ -15,7 +15,7 @@ import 'observable_test_utils.dart'; main() => listChangeTests(); // TODO(jmesserly): port or write array fuzzer tests -listChangeTests() { +void listChangeTests() { StreamSubscription sub; var model; @@ -24,13 +24,15 @@ listChangeTests() { model = null; }); - _delta(i, r, a) => new ListChangeRecord(model, i, removed: r, addedCount: a); + ListChangeRecord _delta(int i, List r, int a, + {ObservableList typedModel}) => + ListChangeRecord(typedModel ?? model, i, removed: r, addedCount: a); test('sequential adds', () { - model = toObservable([]); + final model = ObservableList(); model.add(0); - var summary; + List summary; sub = model.listChanges.listen((r) => summary = r); model.add(1); @@ -38,29 +40,29 @@ listChangeTests() { expect(summary, null); return new Future(() { - expectChanges(summary, [_delta(1, [], 2)]); + expect(summary, [_delta(1, [], 2, typedModel: model)]); expect(summary[0].added, [1, 2]); expect(summary[0].removed, []); }); }); test('List Splice Truncate And Expand With Length', () { - model = toObservable(['a', 'b', 'c', 'd', 'e']); + final model = ObservableList.from(['a', 'b', 'c', 'd', 'e']); - var summary; + List> summary; sub = model.listChanges.listen((r) => summary = r); model.length = 2; return new Future(() { - expectChanges(summary, [ - _delta(2, ['c', 'd', 'e'], 0) + expect(summary, [ + _delta(2, ['c', 'd', 'e'], 0, typedModel: model) ]); expect(summary[0].added, []); expect(summary[0].removed, ['c', 'd', 'e']); summary = null; model.length = 5; }).then(newMicrotask).then((_) { - expectChanges(summary, [_delta(2, [], 3)]); + expect(summary, [_delta(2, [], 3, typedModel: model)]); expect(summary[0].added, [null, null, null]); expect(summary[0].removed, []); }); diff --git a/test/observable_list_test.dart b/test/observable_list_test.dart index b999cfd..634d9e8 100644 --- a/test/observable_list_test.dart +++ b/test/observable_list_test.dart @@ -43,7 +43,7 @@ _runTests() { list.add(4); expect(list, [1, 2, 3, 4]); return new Future(() { - expectChanges(changes, [_lengthChange(3, 4)]); + expect(changes, changeMatchers([_lengthChange(3, 4)])); }); }); @@ -52,7 +52,7 @@ _runTests() { expect(list, orderedEquals([1, 3])); return new Future(() { - expectChanges(changes, [_lengthChange(3, 2)]); + expect(changes, changeMatchers([_lengthChange(3, 2)])); }); }); @@ -61,7 +61,8 @@ _runTests() { list.removeRange(1, 3); expect(list, [1, 4]); return new Future(() { - expectChanges(changes, [_lengthChange(3, 4), _lengthChange(4, 2)]); + expect(changes, + changeMatchers([_lengthChange(3, 4), _lengthChange(4, 2)])); }); }); @@ -70,7 +71,8 @@ _runTests() { list.removeWhere((e) => e == 2); expect(list, [1, 3]); return new Future(() { - expectChanges(changes, [_lengthChange(3, 4), _lengthChange(4, 2)]); + expect(changes, + changeMatchers([_lengthChange(3, 4), _lengthChange(4, 2)])); }); }); @@ -78,7 +80,7 @@ _runTests() { list.length = 5; expect(list, [1, 2, 3, null, null]); return new Future(() { - expectChanges(changes, [_lengthChange(3, 5)]); + expect(changes, changeMatchers([_lengthChange(3, 5)])); }); }); @@ -86,7 +88,7 @@ _runTests() { list[2] = 9000; expect(list, [1, 2, 9000]); return new Future(() { - expectChanges(changes, null); + expect(changes, null); }); }); @@ -94,7 +96,7 @@ _runTests() { list.clear(); expect(list, []); return new Future(() { - expectChanges(changes, [_lengthChange(3, 0)]); + expect(changes, changeMatchers([_lengthChange(3, 0)])); }); }); }); @@ -116,7 +118,7 @@ _runTests() { list.add(4); expect(list, [1, 2, 3, 4]); return new Future(() { - expectChanges(changes, []); + expect(changes, []); }); }); @@ -124,7 +126,7 @@ _runTests() { list[1] = 777; expect(list, [1, 777, 3]); return new Future(() { - expectChanges(changes, [ + expect(changes, [ _change(1, addedCount: 1, removed: [2]) ]); }); @@ -134,7 +136,7 @@ _runTests() { list[2] = 9000; expect(list, [1, 2, 9000]); return new Future(() { - expectChanges(changes, []); + expect(changes, []); }); }); @@ -143,7 +145,7 @@ _runTests() { list[1] = 42; expect(list, [1, 42, 3]); return new Future(() { - expectChanges(changes, [ + expect(changes, [ _change(1, addedCount: 1, removed: [2]), ]); }); @@ -153,7 +155,7 @@ _runTests() { list.length = 2; expect(list, [1, 2]); return new Future(() { - expectChanges(changes, []); + expect(changes, []); }); }); @@ -161,7 +163,7 @@ _runTests() { list.length = 1; expect(list, [1]); return new Future(() { - expectChanges(changes, [ + expect(changes, [ _change(1, removed: [2, 3]) ]); }); @@ -172,7 +174,7 @@ _runTests() { list.add(42); expect(list, [1, 42]); return new Future(() { - expectChanges(changes, [ + expect(changes, [ _change(1, removed: [2, 3], addedCount: 1) ]); }); @@ -183,7 +185,7 @@ _runTests() { list.add(2); expect(list, [1, 2]); return new Future(() { - expectChanges(changes, []); + expect(changes, []); }); }); }); @@ -220,8 +222,8 @@ _runTests() { expect(copy, orderedEquals([1, 2, 3, 1, 3, 4])); return new Future(() { // no change from read-only operators - expectChanges(propRecords, null); - expectChanges(listRecords, null); + expect(propRecords, null); + expect(listRecords, null); }); }); @@ -231,11 +233,13 @@ _runTests() { expect(list, orderedEquals([1, 2, 3, 1, 3, 4, 5, 6])); return new Future(() { - expectChanges(propRecords, [ - _lengthChange(6, 7), - _lengthChange(7, 8), - ]); - expectChanges(listRecords, [_change(6, addedCount: 2)]); + expect( + propRecords, + changeMatchers([ + _lengthChange(6, 7), + _lengthChange(7, 8), + ])); + expect(listRecords, [_change(6, addedCount: 2)]); }); }); @@ -244,8 +248,8 @@ _runTests() { expect(list, orderedEquals([1, 4, 3, 1, 3, 4])); return new Future(() { - expectChanges(propRecords, null); - expectChanges(listRecords, [ + expect(propRecords, null); + expect(listRecords, [ _change(1, addedCount: 1, removed: [2]) ]); }); @@ -256,8 +260,8 @@ _runTests() { expect(list, orderedEquals([1, 2, 3, 1, 3])); return new Future(() { - expectChanges(propRecords, [_lengthChange(6, 5)]); - expectChanges(listRecords, [ + expect(propRecords, changeMatchers([_lengthChange(6, 5)])); + expect(listRecords, [ _change(5, removed: [4]) ]); }); @@ -268,8 +272,8 @@ _runTests() { expect(list, orderedEquals([1, 3, 4])); return new Future(() { - expectChanges(propRecords, [_lengthChange(6, 3)]); - expectChanges(listRecords, [ + expect(propRecords, changeMatchers([_lengthChange(6, 3)])); + expect(listRecords, [ _change(1, removed: [2, 3, 1]) ]); }); @@ -280,8 +284,8 @@ _runTests() { expect(list, orderedEquals([1, 2, 1, 4])); return new Future(() { - expectChanges(propRecords, [_lengthChange(6, 4)]); - expectChanges(listRecords, [ + expect(propRecords, changeMatchers([_lengthChange(6, 4)])); + expect(listRecords, [ _change(2, removed: [3]), _change(3, removed: [3]) ]); @@ -293,8 +297,8 @@ _runTests() { expect(list, orderedEquals([1, 1, 2, 3, 3, 4])); return new Future(() { - expectChanges(propRecords, null); - expectChanges(listRecords, [ + expect(propRecords, null); + expect(listRecords, [ _change(1, addedCount: 1), _change(4, removed: [1]) ]); @@ -320,12 +324,14 @@ _runTests() { expect(list, []); return new Future(() { - expectChanges(propRecords, [ - _lengthChange(6, 0), - new PropertyChangeRecord(list, #isEmpty, false, true), - new PropertyChangeRecord(list, #isNotEmpty, true, false), - ]); - expectChanges(listRecords, [ + expect( + propRecords, + changeMatchers([ + _lengthChange(6, 0), + PropertyChangeRecord(list, #isEmpty, false, true), + PropertyChangeRecord(list, #isNotEmpty, true, false), + ])); + expect(listRecords, [ _change(0, removed: [1, 2, 3, 1, 3, 4]) ]); }); @@ -335,8 +341,8 @@ _runTests() { ObservableList list; -PropertyChangeRecord _lengthChange(int oldValue, int newValue) => - new PropertyChangeRecord(list, #length, oldValue, newValue); +PropertyChangeRecord _lengthChange(int oldValue, int newValue) => + new PropertyChangeRecord(list, #length, oldValue, newValue); _change(int index, {List removed: const [], int addedCount: 0}) => new ListChangeRecord(list, index, removed: removed, addedCount: addedCount); diff --git a/test/observable_map_test.dart b/test/observable_map_test.dart index 04aeccc..1bc0062 100644 --- a/test/observable_map_test.dart +++ b/test/observable_map_test.dart @@ -41,7 +41,7 @@ _runTests() { map['d'] = 4; expect(map, {'a': 1, 'b': 2, 'c': 3, 'd': 4}); return new Future(() { - expectChanges(changes, [_lengthChange(map, 3, 4)]); + expect(changes, changeMatchers([_lengthChange(map, 3, 4)])); }); }); @@ -49,7 +49,7 @@ _runTests() { map.putIfAbsent('d', () => 4); expect(map, {'a': 1, 'b': 2, 'c': 3, 'd': 4}); return new Future(() { - expectChanges(changes, [_lengthChange(map, 3, 4)]); + expect(changes, changeMatchers([_lengthChange(map, 3, 4)])); }); }); @@ -58,10 +58,12 @@ _runTests() { map.remove('a'); expect(map, {'b': 2}); return new Future(() { - expectChanges(changes, [ - _lengthChange(map, 3, 2), - _lengthChange(map, 2, 1), - ]); + expect( + changes, + changeMatchers([ + _lengthChange(map, 3, 2), + _lengthChange(map, 2, 1), + ])); }); }); @@ -69,7 +71,7 @@ _runTests() { map.remove('d'); expect(map, {'a': 1, 'b': 2, 'c': 3}); return new Future(() { - expectChanges(changes, null); + expect(changes, null); }); }); @@ -77,7 +79,7 @@ _runTests() { map['c'] = 9000; expect(map, {'a': 1, 'b': 2, 'c': 9000}); return new Future(() { - expectChanges(changes, []); + expect(changes, []); }); }); @@ -85,7 +87,7 @@ _runTests() { map.clear(); expect(map, {}); return new Future(() { - expectChanges(changes, [_lengthChange(map, 3, 0)]); + expect(changes, changeMatchers([_lengthChange(map, 3, 0)])); }); }); }); @@ -109,7 +111,7 @@ _runTests() { map.putIfAbsent('d', () => 4); expect(map, {'a': 1, 'b': 2, 'c': 3, 'd': 4}); return new Future(() { - expectChanges(changes, []); + expect(changes, []); }); }); @@ -117,7 +119,7 @@ _runTests() { map['b'] = null; expect(map, {'a': 1, 'b': null, 'c': 3}); return new Future(() { - expectChanges(changes, [_changeKey('b', 2, null)]); + expect(changes, [_changeKey('b', 2, null)]); }); }); @@ -125,7 +127,7 @@ _runTests() { map['b'] = 777; expect(map, {'a': 1, 'b': 777, 'c': 3}); return new Future(() { - expectChanges(changes, [_changeKey('b', 2, 777)]); + expect(changes, [_changeKey('b', 2, 777)]); }); }); @@ -133,7 +135,7 @@ _runTests() { map.putIfAbsent('b', () => 1234); expect(map, {'a': 1, 'b': 2, 'c': 3}); return new Future(() { - expectChanges(changes, null); + expect(changes, null); }); }); @@ -141,7 +143,7 @@ _runTests() { map['c'] = 9000; expect(map, {'a': 1, 'b': 2, 'c': 9000}); return new Future(() { - expectChanges(changes, []); + expect(changes, []); }); }); @@ -150,7 +152,7 @@ _runTests() { map['b'] = 42; expect(map, {'a': 1, 'b': 42, 'c': 3}); return new Future(() { - expectChanges(changes, [ + expect(changes, [ _changeKey('b', 2, 9001), _changeKey('b', 9001, 42), ]); @@ -161,7 +163,7 @@ _runTests() { map.remove('a'); expect(map, {'b': 2, 'c': 3}); return new Future(() { - expectChanges(changes, []); + expect(changes, []); }); }); @@ -169,7 +171,7 @@ _runTests() { map.remove('b'); expect(map, {'a': 1, 'c': 3}); return new Future(() { - expectChanges(changes, [_removeKey('b', 2)]); + expect(changes, [_removeKey('b', 2)]); }); }); @@ -178,7 +180,7 @@ _runTests() { map['b'] = 2; expect(map, {'a': 1, 'b': 2, 'c': 3}); return new Future(() { - expectChanges(changes, [ + expect(changes, [ _removeKey('b', 2), _insertKey('b', 2), ]); @@ -305,12 +307,14 @@ _runTests() { expect(map, {'a': 1, 'b': 2, 'c': 3}); return new Future(() { - expectChanges(records, [ - _lengthChange(map, 2, 3), - _insertKey('c', 3), - _propChange(map, #keys), - _propChange(map, #values), - ]); + expect( + records, + changeMatchers([ + _lengthChange(map, 2, 3), + _insertKey('c', 3), + _propChange(map, #keys), + _propChange(map, #values), + ])); }); }); @@ -322,14 +326,16 @@ _runTests() { expect(map, {'a': 42, 'b': 2, 'c': 3}); return new Future(() { - expectChanges(records, [ - _changeKey('a', 1, 42), - _propChange(map, #values), - _lengthChange(map, 2, 3), - _insertKey('c', 3), - _propChange(map, #keys), - _propChange(map, #values), - ]); + expect( + records, + changeMatchers([ + _changeKey('a', 1, 42), + _propChange(map, #values), + _lengthChange(map, 2, 3), + _insertKey('c', 3), + _propChange(map, #keys), + _propChange(map, #values), + ])); }); }); @@ -338,12 +344,14 @@ _runTests() { expect(map, {'a': 1}); return new Future(() { - expectChanges(records, [ - _removeKey('b', 2), - _lengthChange(map, 2, 1), - _propChange(map, #keys), - _propChange(map, #values), - ]); + expect( + records, + changeMatchers([ + _removeKey('b', 2), + _lengthChange(map, 2, 1), + _propChange(map, #keys), + _propChange(map, #values), + ])); }); }); @@ -352,13 +360,15 @@ _runTests() { expect(map, {}); return new Future(() { - expectChanges(records, [ - _removeKey('a', 1), - _removeKey('b', 2), - _lengthChange(map, 2, 0), - _propChange(map, #keys), - _propChange(map, #values), - ]); + expect( + records, + changeMatchers([ + _removeKey('a', 1), + _removeKey('b', 2), + _lengthChange(map, 2, 0), + _propChange(map, #keys), + _propChange(map, #values), + ])); }); }); }); @@ -379,13 +389,15 @@ _runTests() { }); } -_lengthChange(map, int oldValue, int newValue) => - new PropertyChangeRecord(map, #length, oldValue, newValue); +PropertyChangeRecord _lengthChange(map, int oldValue, int newValue) => + PropertyChangeRecord(map, #length, oldValue, newValue); -_changeKey(key, old, newValue) => new MapChangeRecord(key, old, newValue); +MapChangeRecord _changeKey(key, old, newValue) => + MapChangeRecord(key, old, newValue); _insertKey(key, newValue) => new MapChangeRecord.insert(key, newValue); _removeKey(key, oldValue) => new MapChangeRecord.remove(key, oldValue); -_propChange(map, prop) => new PropertyChangeRecord(map, prop, null, null); +PropertyChangeRecord _propChange(map, prop) => + new PropertyChangeRecord(map, prop, null, null); diff --git a/test/observable_test.dart b/test/observable_test.dart index e88495d..094c718 100644 --- a/test/observable_test.dart +++ b/test/observable_test.dart @@ -24,7 +24,7 @@ void observableTests() { }); test('handle future result', () { - var callback = expectAsync0(() {}); + var callback = expectAsync(() {}); return new Future(callback); }); @@ -48,7 +48,7 @@ void observableTests() { var t = createModel(123); int called = 0; - subs.add(t.changes.listen(expectAsync1((records) { + subs.add(t.changes.listen(expectAsync((records) { called++; expectPropertyChanges(records, 2); }))); @@ -62,7 +62,7 @@ void observableTests() { var t = createModel(123); int called = 0; - subs.add(t.changes.listen(expectAsync1((records) { + subs.add(t.changes.listen(expectAsync((records) { called++; expectPropertyChanges(records, 1); if (called == 1) { @@ -81,8 +81,8 @@ void observableTests() { expectPropertyChanges(records, 2); } - subs.add(t.changes.listen(expectAsync1(verifyRecords))); - subs.add(t.changes.listen(expectAsync1(verifyRecords))); + subs.add(t.changes.listen(expectAsync(verifyRecords))); + subs.add(t.changes.listen(expectAsync(verifyRecords))); t.value = 41; t.value = 42; @@ -96,14 +96,14 @@ void observableTests() { })); t.value = 41; t.value = 42; - expectChanges(records, [], reason: 'changes delived async'); + expect(records, [], reason: 'changes delived async'); return new Future(() { expectPropertyChanges(records, 2); records.clear(); t.value = 777; - expectChanges(records, [], reason: 'changes delived async'); + expect(records, [], reason: 'changes delived async'); }).then(newMicrotask).then((_) { expectPropertyChanges(records, 1); }); @@ -112,7 +112,7 @@ void observableTests() { test('cancel listening', () { var t = createModel(123); var sub; - sub = t.changes.listen(expectAsync1((records) { + sub = t.changes.listen(expectAsync((records) { expectPropertyChanges(records, 1); sub.cancel(); t.value = 777; @@ -123,12 +123,12 @@ void observableTests() { test('cancel and reobserve', () { var t = createModel(123); var sub; - sub = t.changes.listen(expectAsync1((records) { + sub = t.changes.listen(expectAsync((records) { expectPropertyChanges(records, 1); sub.cancel(); scheduleMicrotask(() { - subs.add(t.changes.listen(expectAsync1((records) { + subs.add(t.changes.listen(expectAsync((records) { expectPropertyChanges(records, 1); }))); t.value = 777; diff --git a/test/observable_test_utils.dart b/test/observable_test_utils.dart index 990cc86..bbb5b70 100644 --- a/test/observable_test_utils.dart +++ b/test/observable_test_utils.dart @@ -15,10 +15,15 @@ import 'package:test/test.dart'; /// future.then(newMicrotask).then(...) newMicrotask(_) => new Future.value(); -// TODO(jmesserly): use matchers when we have a way to compare ChangeRecords. -// For now just use the toString. -void expectChanges(actual, expected, {String reason}) => - expect('$actual', '$expected', reason: reason); +void expectChanges(List actual, List expected, + {String reason}) { + expect(actual, _EqualsMatcher(expected), reason: reason); +} + +void expectNotChanges(List actual, ChangeRecords expectedNot, + {String reason}) { + expect(actual, isNot(_EqualsMatcher(expectedNot)), reason: reason); +} List getListChangeRecords( List changes, int index) => @@ -28,3 +33,43 @@ List getPropertyChangeRecords( List changes, Symbol property) => new List.from(changes.where( (ChangeRecord c) => c is PropertyChangeRecord && c.name == property)); + +List changeMatchers(List changes) => changes + .map((r) => + r is PropertyChangeRecord ? _PropertyChangeMatcher(r) : equals(r)) + .toList(); + +// Custom equality matcher is required, otherwise expect() infers ChangeRecords +// to be an iterable and does a deep comparison rather than use the == operator. +class _EqualsMatcher extends Matcher { + final ValueType _expected; + + _EqualsMatcher(this._expected); + + @override + Description describe(Description description) => + description.addDescriptionOf(_expected); + + @override + bool matches(dynamic item, Map matchState) => + item is ChangeRecords && _expected == item; +} + +class _PropertyChangeMatcher extends Matcher { + final PropertyChangeRecord _expected; + + _PropertyChangeMatcher(this._expected); + + @override + Description describe(Description description) => + description.addDescriptionOf(_expected); + + @override + bool matches(dynamic other, Map matchState) => + identical(_expected, other) || + other is PropertyChangeRecord && + _expected.runtimeType == other.runtimeType && + _expected.name == other.name && + _expected.oldValue == other.oldValue && + _expected.newValue == other.newValue; +}