Merge pull request #26 from dart-lang/devoncarew_lots_o_tests

improve test coverage

lib/src/usage_impl.dart

@@ -16,11 +16,12 @@ final int _MAX_EXCEPTION_LENGTH = 100;
 // Matches file:/, non-ws, /, non-ws, .dart
 final RegExp _pathRegex = new RegExp(r'file:/\S+/(\S+\.dart)');
 
-String postEncode(Map<String, String> map) {
+String postEncode(Map<String, dynamic> map) {
   // &foo=bar
-  return map.keys
-      .map((key) => "${key}=${Uri.encodeComponent(map[key])}")
-      .join('&');
+  return map.keys.map((key) {
+    String value = '${map[key]}';
+    return "${key}=${Uri.encodeComponent(value)}";
+  }).join('&');
 }
 
 /**
@@ -125,7 +126,7 @@ abstract class AnalyticsImpl implements Analytics {
 
     Map args = {'ec': category, 'ea': action};
     if (label != null) args['el'] = label;
-    if (value != null) args['ev'] = '${value}';
+    if (value != null) args['ev'] = value;
     return _sendPayload('event', args);
   }
 
@@ -140,7 +141,7 @@ abstract class AnalyticsImpl implements Analytics {
         String label}) {
     if (!optIn) return new Future.value();
 
-    Map args = {'utv': variableName, 'utt': '${time}'};
+    Map args = {'utv': variableName, 'utt': time};
     if (label != null) args['utl'] = label;
     if (category != null) args['utc'] = category;
     return _sendPayload('timing', args);
@@ -169,7 +170,7 @@ abstract class AnalyticsImpl implements Analytics {
     if (value == null) {
       _variableMap.remove(param);
     } else {
-      _variableMap[param] = '${value}';
+      _variableMap[param] = value;
     }
   }
 

lib/src/usage_impl_io.dart

@@ -14,9 +14,10 @@ import 'usage_impl.dart';
 
 String _createUserAgent() {
   // Mozilla/5.0 (iPhone; U; CPU iPhone OS 5_1_1 like Mac OS X; en)
+  // Dart/1.8.0-edge.41170 (macos; macos; macos; null)
   String os = Platform.operatingSystem;
   String locale = Platform.environment['LANG'];
-  return "Dart/${Platform.version} (${os}; ${os}; ${os}; ${locale})";
+  return "Dart/${_dartVersion()} (${os}; ${os}; ${os}; ${locale})";
 }
 
 String _userHomeDir() {
@@ -25,19 +26,27 @@ String _userHomeDir() {
   return value == null ? '.' : value;
 }
 
+String _dartVersion() {
+  String ver = Platform.version;
+  int index = ver.indexOf(' ');
+  if (index != -1) ver = ver.substring(0, index);
+  return ver;
+}
+
 class IOPostHandler extends PostHandler {
   final String _userAgent;
+  final HttpClient mockClient;
 
-  IOPostHandler() : _userAgent = _createUserAgent();
+  IOPostHandler({HttpClient this.mockClient}) : _userAgent = _createUserAgent();
 
   Future sendPost(String url, Map<String, String> parameters) {
     // Add custom parameters for OS and the Dart version.
     parameters['cd1'] = Platform.operatingSystem;
-    parameters['cd2'] = 'dart ${Platform.version}';
+    parameters['cd2'] = 'dart ${_dartVersion()}';
 
     String data = postEncode(parameters);
 
-    HttpClient client = new HttpClient();
+    HttpClient client = mockClient != null ? mockClient : new HttpClient();
     client.userAgent = _userAgent;
     return client.postUrl(Uri.parse(url)).then((HttpClientRequest req) {
       req.write(data);

lib/usage.dart

@@ -93,13 +93,21 @@ abstract class Analytics {
   void setSessionValue(String param, dynamic value);
 }
 
+/**
+ * A no-op implementation of the [Analytics] class. This can be used as a
+ * stand-in for that will never ping the GA server, or as a mock in test code.
+ */
 class AnalyticsMock extends Analytics {
   String get trackingId => 'UA-0';
   final bool logCalls;
 
   bool optIn = false;
   bool hasSetOptIn = true;
 
+  /**
+   * Create a new [AnalyticsMock]. If [logCalls] is true, all calls will be
+   * logged to stdout.
+   */
   AnalyticsMock([this.logCalls = false]);
 
   Future sendScreenView(String viewName) {

test/all.dart

@@ -5,9 +5,13 @@
 library usage.all_test;
 
 import 'hit_types_test.dart' as hit_types_test;
+import 'usage_test.dart' as usage_test;
 import 'usage_impl_test.dart' as usage_impl_test;
+import 'usage_impl_io_test.dart' as usage_impl_io_test;
 
 void main() {
   hit_types_test.defineTests();
+  usage_test.defineTests();
   usage_impl_test.defineTests();
+  usage_impl_io_test.defineTests();
 }

test/hit_types_test.dart

@@ -9,16 +9,6 @@ import 'package:unittest/unittest.dart';
 import 'src/common.dart';
 
 void defineTests() {
-  group('hit types', () {
-    test('respects disabled', () {
-      AnalyticsImplMock mock = createMock();
-      mock.optIn = false;
-      mock.sendException('FooBar exception');
-      expect(mock.optIn, false);
-      expect(mock.mockPostHandler.sentValues, isEmpty);
-    });
-  });
-
   group('screenView', () {
     test('simple', () {
       AnalyticsImplMock mock = createMock();
@@ -78,8 +68,3 @@ void defineTests() {
     });
   });
 }
-
-AnalyticsImplMock createMock() => new AnalyticsImplMock('UA-0');
-
-void was(Map m, String type) => expect(m['t'], type);
-void has(Map m, String key) => expect(m[key], isNotNull);

test/src/common.dart

@@ -6,15 +6,23 @@ library usage.common_test;
 
 import 'dart:async';
 
+import 'package:unittest/unittest.dart';
 import 'package:usage/src/usage_impl.dart';
 
+AnalyticsImplMock createMock({bool setOptIn: true}) => new AnalyticsImplMock(
+    'UA-0', setOptIn: setOptIn);
+
+void was(Map m, String type) => expect(m['t'], type);
+void has(Map m, String key) => expect(m[key], isNotNull);
+void hasnt(Map m, String key) => expect(m[key], isNull);
+
 class AnalyticsImplMock extends AnalyticsImpl {
   MockProperties get mockProperties => properties;
   MockPostHandler get mockPostHandler => postHandler;
 
-  AnalyticsImplMock(String trackingId) :
+  AnalyticsImplMock(String trackingId, {bool setOptIn: true}) :
     super(trackingId, new MockProperties(), new MockPostHandler()) {
-    optIn = true;
+    if (setOptIn) optIn = true;
   }
 
   Map<String, dynamic> get last => mockPostHandler.last;

test/usage_impl_io_test.dart

@@ -0,0 +1,74 @@
+// Copyright (c) 2014, 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.
+
+library usage.usage_impl_io_test;
+
+import 'dart:async';
+import 'dart:io';
+
+import 'package:unittest/unittest.dart';
+import 'package:usage/src/usage_impl_io.dart';
+
+void defineTests() {
+  group('IOPostHandler', () {
+    test('sendPost', () {
+      var httpClient = new MockHttpClient();
+      IOPostHandler postHandler = new IOPostHandler(mockClient: httpClient);
+      Map args = {'utv': 'varName', 'utt': 123};
+      return postHandler.sendPost('http://www.google.com', args).then((_) {
+        expect(httpClient.sendCount, 1);
+      });
+    });
+  });
+
+  group('IOPersistentProperties', () {
+    test('add', () {
+      IOPersistentProperties props = new IOPersistentProperties('foo_props');
+      props['foo'] = 'bar';
+      expect(props['foo'], 'bar');
+    });
+
+    test('remove', () {
+      IOPersistentProperties props = new IOPersistentProperties('foo_props');
+      props['foo'] = 'bar';
+      expect(props['foo'], 'bar');
+      props['foo'] = null;
+      expect(props['foo'], null);
+    });
+  });
+}
+
+class MockHttpClient implements HttpClient {
+  String userAgent;
+  int sendCount = 0;
+  int writeCount = 0;
+  bool closed = false;
+  Future<HttpClientRequest> postUrl(Uri url) {
+    return new Future.value(new MockHttpClientRequest(this));
+  }
+  noSuchMethod(Invocation invocation) { }
+}
+
+class MockHttpClientRequest implements HttpClientRequest {
+  final MockHttpClient client;
+  MockHttpClientRequest(this.client);
+  void write(Object obj) {
+    client.writeCount++;
+  }
+  Future<HttpClientResponse> close() {
+    client.closed = true;
+    return new Future.value(new MockHttpClientResponse(client));
+  }
+  noSuchMethod(Invocation invocation) { }
+}
+
+class MockHttpClientResponse implements HttpClientResponse {
+  final MockHttpClient client;
+  MockHttpClientResponse(this.client);
+  Future drain([var futureValue]) {
+    client.sendCount++;
+    return new Future.value();
+  }
+  noSuchMethod(Invocation invocation) { }
+}

test/usage_impl_test.dart

@@ -6,7 +6,8 @@ library usage.impl_test;
 
 import 'package:unittest/unittest.dart';
 import 'package:usage/src/usage_impl.dart';
-import 'package:usage/src/usage_impl_io.dart';
+
+import 'src/common.dart';
 
 void defineTests() {
   group('ThrottlingBucket', () {
@@ -24,6 +25,35 @@ void defineTests() {
     });
   });
 
+  group('AnalyticsImpl', () {
+    test('respects disabled', () {
+      AnalyticsImplMock mock = createMock();
+      mock.optIn = false;
+      mock.sendException('FooBar exception');
+      expect(mock.optIn, false);
+      expect(mock.mockPostHandler.sentValues, isEmpty);
+    });
+
+    test('hasSetOptIn', () {
+      AnalyticsImplMock mock = createMock(setOptIn: false);
+      expect(mock.hasSetOptIn, false);
+      mock.optIn = false;
+      expect(mock.hasSetOptIn, true);
+    });
+
+    test('setSessionValue', () {
+      AnalyticsImplMock mock = createMock();
+      mock.sendScreenView('foo');
+      hasnt(mock.last, 'val');
+      mock.setSessionValue('val', 'ue');
+      mock.sendScreenView('bar');
+      has(mock.last, 'val');
+      mock.setSessionValue('val', null);
+      mock.sendScreenView('baz');
+      hasnt(mock.last, 'val');
+    });
+  });
+
   group('sanitizeFilePaths', () {
     test('replace file', () {
       expect(sanitizeFilePaths(
@@ -39,19 +69,10 @@ void defineTests() {
     });
   });
 
-  group('IOPersistentProperties', () {
-    test('add', () {
-      IOPersistentProperties props = new IOPersistentProperties('foo_props');
-      props['foo'] = 'bar';
-      expect(props['foo'], 'bar');
-    });
-
-    test('remove', () {
-      IOPersistentProperties props = new IOPersistentProperties('foo_props');
-      props['foo'] = 'bar';
-      expect(props['foo'], 'bar');
-      props['foo'] = null;
-      expect(props['foo'], null);
+  group('postEncode', () {
+    test('simple', () {
+      Map map = {'foo': 'bar', 'baz': 'qux norf'};
+      expect(postEncode(map), 'foo=bar&baz=qux%20norf');
     });
   });
 }

test/usage_test.dart

@@ -0,0 +1,22 @@
+// Copyright (c) 2014, 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.
+
+library usage.usage_test;
+
+import 'package:unittest/unittest.dart';
+import 'package:usage/usage.dart';
+
+void defineTests() {
+  group('AnalyticsMock', () {
+    test('simple', () {
+      AnalyticsMock mock = new AnalyticsMock();
+      mock.sendScreenView('main');
+      mock.sendEvent('files', 'save');
+      mock.sendSocial('g+', 'plus', 'userid');
+      mock.sendTiming('compile', 123);
+      mock.sendException('FooException');
+      mock.setSessionValue('val', 'ue');
+    });
+  });
+}

tool/ga.dart

@@ -19,6 +19,8 @@ void main() {
     return ga.sendScreenView('files');
   }).then((_) {
     return ga.sendException('foo exception, line 123:56');
+  }).then((_) {
+    return ga.sendTiming('writeDuration', 123);
   }).then((_) {
     return ga.sendEvent('create', 'consoleapp', label: 'Console App');
   }).then((_) {