Fix bug, tidy up. (#28)

* Fix bug, tidy up.

* Address feedback

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.20.3
+
+* Bug fix: Avoid emitting an empty list via `ObservableList.listChanges`
+
 ## 0.20.2
 
 * Bug fix: Avoid emitting a no-op `MapChangeRecord`

lib/src/collections/observable_list.dart

@@ -216,9 +216,9 @@ class _ObservableDelegatingList<E> extends DelegatingList<E>
   bool get hasListObservers => _listChanges.hasObservers;
 
   @override
-  Stream<List<ListChangeRecord<E>>> get listChanges {
-    return _listChanges.changes.map((r) => projectListSplices(this, r));
-  }
+  Stream<List<ListChangeRecord<E>>> get listChanges => _listChanges.changes
+      .map((r) => projectListSplices(this, r))
+      .where((c) => c.isNotEmpty);
 
   @override
   void notifyListChange(

lib/src/observable.dart

@@ -53,8 +53,6 @@ abstract class Observable<C extends ChangeRecord> {
   ///
   /// Returns `true` if changes were emitted.
   @Deprecated('Use ChangeNotifier instead to have this method available')
-  // REMOVE IGNORE when https://github.com/dart-lang/observable/issues/10
-  // ignore: invalid_use_of_protected_member
   bool deliverChanges() => _delegate.deliverChanges();
 
   /// Notify that the [field] name of this object has been changed.

test/collections/observable_list_test.dart

@@ -310,6 +310,61 @@ _runObservableListTests() {
 
 // These are tests we will remove after deprecations occur.
 _runDeprecatedTests() {
+  group('listChanges stream', () {
+    Completer<List<ListChangeRecord>> completer;
+    ObservableList<String> list;
+    List<String> previousState;
+    StreamSubscription sub;
+
+    Future next() {
+      completer = new Completer<List<ListChangeRecord>>.sync();
+      return completer.future;
+    }
+
+    Future<Null> expectChanges(List<ListChangeRecord> changes) {
+      // Applying these change records in order should make the new list.
+      for (final change in changes) {
+        change.apply(previousState);
+      }
+      expect(list, previousState);
+
+      // If these fail, it might be safe to update if optimized/changed.
+      return next().then((actualChanges) {
+        expect(actualChanges, changes);
+      });
+    }
+
+    setUp(() {
+      previousState = ['a', 'b', 'c'];
+      list = new ObservableList<String>.from(previousState);
+      sub = list.listChanges.listen((c) {
+        if (completer?.isCompleted == false) {
+          completer.complete(c.toList());
+        }
+        previousState = list.toList();
+      });
+    });
+
+    tearDown(() => sub.cancel());
+
+    test('updates when an index changes', () async {
+      list[0] = 'd';
+      await expectChanges([
+        new ListChangeRecord.replace(list, 0, ['a']),
+      ]);
+    });
+
+    test('should not emit an empty list', () async {
+      // This is optimized into a no-op.
+      list[0] = 'd';
+      list[0] = 'a';
+      next().then((_) {
+        fail('Should not emit change records');
+      });
+      await new Future.value();
+    });
+  });
+
   group('length changes', () {
     Completer<List<ListChangeRecord>> completer;
     ObservableList<String> list;

tool/presubmit.sh

@@ -26,13 +26,18 @@ echo "PASSED"
 # Fail on anything that fails going forward.
 set -e
 
+# Allow running this script locally, otherwise it fails outside of travis.
+if [[ ${TEST_PLATFORM} == "" ]]
+then
+  TEST_PLATFORM="vm"
+fi
+
 THE_COMMAND="pub run test -p $TEST_PLATFORM"
-if [ $TEST_PLATFORM == 'firefox' ]; then
+if [[ ${TEST_PLATFORM} == "firefox" ]]
+then
   export DISPLAY=:99.0
   sh -e /etc/init.d/xvfb start
   t=0; until (xdpyinfo -display :99 &> /dev/null || test $t -gt 10); do sleep 1; let t=$t+1; done
 fi
 echo $THE_COMMAND
 exec $THE_COMMAND
-
-pub run test