Change path-observer to lookup properties aggressively and report errors

pkg/observe/test/path_observer_test.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
 import 'package:observe/observe.dart';
 import 'package:unittest/unittest.dart';
 import 'observe_test_utils.dart';
 
 // This file contains code ported from:
 // https://github.com/rafaelw/ChangeSummary/blob/master/tests/test.js
-
 main() => dirtyCheckZone().run(() {
   group('PathObserver', observePathTests);
 
   group('PropertyPath', () {
     test('toString length', () {
       expectPath(p, str, len) {
         var path = new PropertyPath(p);
         expect(path.toString(), str);
         expect(path.length, len, reason: 'expected path length $len for $path');
       }
@@ skipping 38 matching lines @@
           reason: 'test depends on 2 and 3 having different hashcodes');
 
       var a = new PropertyPath([2]);
       var b = new PropertyPath([3]);
       expect(a, isNot(b), reason: 'different paths');
       expect(a.hashCode, isNot(b.hashCode), reason: 'different hashCodes');
     });
   });
 });
 
-
 observePathTests() {
   test('Degenerate Values', () {
     expect(new PathObserver(null, '').value, null);
     expect(new PathObserver(123, '').value, 123);
-    expect(new PathObserver(123, 'foo.bar.baz').value, null);
+    new Future(() {}).then((_) {

[Jennifer Messerly, 2014/03/25 22:32:44]

return the Future?

also does it work to start with:

return new Future(() {
  ...
}).then(...

[Siggi Cherem (dart-lang), 2014/03/26 00:32:07]

(obsolete)

+      // should throw:
+      return _asyncError('foo',
+          () => expect(new PathObserver(123, 'foo.bar.baz').value, null));

[Jennifer Messerly, 2014/03/25 22:32:44]

as noted earlier, I'd expect this error to be sync.

[Siggi Cherem (dart-lang), 2014/03/26 00:32:07]

Done.

+    }).then((_) {
+      // shouldn't throw:
+      new PathObserver(123, '')..open((_) {})..close();
+      new PropertyPath('').setValueFrom(null, null);
+      new PropertyPath('').setValueFrom(123, 42);
 
-    // shouldn't throw:
-    new PathObserver(123, '')..open((_) {})..close();
-    new PropertyPath('').setValueFrom(null, null);
-    new PropertyPath('').setValueFrom(123, 42);
-    new PropertyPath('foo.bar.baz').setValueFrom(123, 42);
+      // should throw:
+      return _asyncError('foo',

[Jennifer Messerly, 2014/03/25 22:32:44]

when i'm changing behavior w.r.t. JS I usually put in a note like:

Dart note: getting invalid properties is an error, unlike in JS where it returns
undefined.

maybe just one note at the top since it's common to several tests

[Siggi Cherem (dart-lang), 2014/03/26 00:32:07]

Done (made the note on the source code too)

+        () => new PropertyPath('foo.bar.baz').setValueFrom(123, 42));
+    }).then((_) {
+      var foo = {};
+      expect(new PathObserver(foo, '').value, foo);
 
-    var foo = {};
-    expect(new PathObserver(foo, '').value, foo);
+      foo = new Object();
+      expect(new PathObserver(foo, '').value, foo);
 
-    foo = new Object();
-    expect(new PathObserver(foo, '').value, foo);
-
-    expect(new PathObserver(foo, 'a/3!').value, null);
+      expect(new PathObserver(foo, 'a/3!').value, null);
+    });
   });
 
   test('get value at path ObservableBox', () {
     var obj = new ObservableBox(new ObservableBox(new ObservableBox(1)));
 
     expect(new PathObserver(obj, '').value, obj);
     expect(new PathObserver(obj, 'value').value, obj.value);
     expect(new PathObserver(obj, 'value.value').value, obj.value.value);
     expect(new PathObserver(obj, 'value.value.value').value, 1);
 
     obj.value.value.value = 2;
     expect(new PathObserver(obj, 'value.value.value').value, 2);
 
     obj.value.value = new ObservableBox(3);
     expect(new PathObserver(obj, 'value.value.value').value, 3);
 
     obj.value = new ObservableBox(4);
-    expect(new PathObserver(obj, 'value.value.value').value, null);
     expect(new PathObserver(obj, 'value.value').value, 4);
+    return _asyncError('value',
+        () => expect(new PathObserver(obj, 'value.value.value').value, null));
   });
 
 
   test('get value at path ObservableMap', () {
     var obj = toObservable({'a': {'b': {'c': 1}}});
 
     expect(new PathObserver(obj, '').value, obj);
     expect(new PathObserver(obj, 'a').value, obj['a']);
     expect(new PathObserver(obj, 'a.b').value, obj['a']['b']);
     expect(new PathObserver(obj, 'a.b.c').value, 1);
 
     obj['a']['b']['c'] = 2;
     expect(new PathObserver(obj, 'a.b.c').value, 2);
 
     obj['a']['b'] = toObservable({'c': 3});
     expect(new PathObserver(obj, 'a.b.c').value, 3);
 
     obj['a'] = toObservable({'b': 4});
-    expect(new PathObserver(obj, 'a.b.c').value, null);
     expect(new PathObserver(obj, 'a.b').value, 4);
+    return _asyncError('c',
+        () => expect(new PathObserver(obj, 'a.b.c').value, null));
   });
 
   test('set value at path', () {
     var obj = toObservable({});
     new PropertyPath('foo').setValueFrom(obj, 3);
     expect(obj['foo'], 3);
 
     var bar = toObservable({ 'baz': 3 });
     new PropertyPath('bar').setValueFrom(obj, bar);
     expect(obj['bar'], bar);
 
-    new PropertyPath('bar.baz.bat').setValueFrom(obj, 'not here');
-    expect(new PathObserver(obj, 'bar.baz.bat').value, null);
+    new Future(() {}).then((_) {
+      return _asyncError('bat=',
+          () => new PropertyPath('bar.baz.bat').setValueFrom(obj, 'not here'));
+    }).then((_) {
+      return _asyncError('bat',
+          () => expect(new PathObserver(obj, 'bar.baz.bat').value, null));
+    });
   });
 
   test('set value back to same', () {
     var obj = toObservable({});
     var path = new PathObserver(obj, 'foo');
     var values = [];
     path.open((x) {
       expect(x, path.value, reason: 'callback should get current value');
       values.add(x);
     });
@@ skipping 79 matching lines @@
     });
   });
 
   for (var createModel in [() => new TestModel(), () => new WatcherModel()]) {
     test('Path Observation - ${createModel().runtimeType}', () {
       var model = createModel()..a =
           (createModel()..b = (createModel()..c = 'hello, world'));
 
       var path = new PathObserver(model, 'a.b.c');
       var lastValue = null;
-      path.open((x) { lastValue = x; });
+      var errorSeen = false;
+      var future = _asyncError('c', () => path.open((x) { lastValue = x; }))
+          .then((_) { errorSeen = true; });
 
       model.a.b.c = 'hello, mom';
 
       expect(lastValue, null);
       return new Future(() {
         expect(lastValue, 'hello, mom');
 
         model.a.b = createModel()..c = 'hello, dad';
       }).then(newMicrotask).then((_) {
         expect(lastValue, 'hello, dad');
 
         model.a = createModel()..b =
             (createModel()..c = 'hello, you');
       }).then(newMicrotask).then((_) {
         expect(lastValue, 'hello, you');
 
         model.a.b = 1;
-      }).then(newMicrotask).then((_) {
+        expect(errorSeen, false); // update will be seen on next micro task
+      }).then((_) => future).then((_) {
+        expect(errorSeen, true);
         expect(lastValue, null);
 
         // Stop observing
         path.close();
 
         model.a.b = createModel()..c = 'hello, back again -- but not observing';
       }).then(newMicrotask).then((_) {
         expect(lastValue, null);
 
         // Resume observing
@@ skipping 22 matching lines @@
       model['a'] = 3;
     }).then(newMicrotask).then((_) {
       expect(values, [1, 2]);
     });
   });
 
   test('errors thrown from getter/setter', () {
     var model = new ObjectWithErrors();
     var observer = new PathObserver(model, 'foo');
 
-    expect(() => observer.value, throws);
-    expect(model.getFooCalled, 1);
-
-    expect(() { observer.value = 123; }, throws);
-    expect(model.setFooCalled, [123]);
+    new Future(() {}).then((_) {
+      return _asyncError('bar', () => observer.value);
+    }).then((_) {
+      expect(model.getFooCalled, 1);
+      return _asyncError('bar=', () { observer.value = 123; });
+    }).then((_) {
+      expect(model.setFooCalled, [123]);
+    });
   });
 
   test('object with noSuchMethod', () {
     var model = new NoSuchMethodModel();
     var observer = new PathObserver(model, 'foo');
 
     expect(observer.value, 42);
     observer.value = 'hi';
     expect(model._foo, 'hi');
     expect(observer.value, 'hi');
@@ skipping 27 matching lines @@
     expect(observer.value, null, reason: 'path not found');
     expect(model.log, ['[] bar']);
     model.log.clear();
 
     observer.value = 42;
     expect(model.log, ['[]= bar 42']);
     model.log.clear();
   });
 }
 
+_asyncError(String missingPropertyName, Function code) {
+  var completer = new Completer();
+  runZoned(() {
+    code();
+    return new Future(() {});
+  }, onError: (e) {
+    completer.complete(true);
+    expect(e, predicate((e) => e is NoSuchMethodError));
+
+    // Dart2js and VM error messages are a bit different, but they are both
+    // contain the missingPropertyName.
+    expect('$e', predicate((message) =>
+      message.contains("'$missingPropertyName'") || // VM error
+      message.contains('\'Symbol("$missingPropertyName")\''))); // dart2js error
+  });
+  return completer.future;
+}
+
 class ObjectWithErrors {
   int getFooCalled = 0;
   List setFooCalled = [];
   @reflectable get foo {
     getFooCalled++;
     (this as dynamic).bar;
   }
   @reflectable set foo(value) {
     setFooCalled.add(value);
     (this as dynamic).bar = value;
@@ skipping 11 matching lines @@
     log.add(name);
     if (name == #foo && invocation.isGetter) return _foo;
     if (name == const Symbol('foo=')) {
       _foo = invocation.positionalArguments[0];
       return null;
     }
     return super.noSuchMethod(invocation);
   }
 }
 
-class IndexerModel {
+class IndexerModel implements StringIndexer {
   var _foo = 42;
   List log = [];
 
   operator [](index) {
     log.add('[] $index');
     if (index == 'foo') return _foo;
   }
 
   operator []=(index, value) {
     log.add('[]= $index $value');
@@ skipping 28 matching lines @@
 
 class WatcherModel extends Observable {
   // TODO(jmesserly): dart2js does not let these be on the same line:
   // @observable var a, b, c;
   @observable var a;
   @observable var b;
   @observable var c;
 
   WatcherModel();
 }