From e328cc2844b76d9df7faec0c9f892929eead5f19 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Thu, 18 Dec 2014 13:10:45 -0800 Subject: [PATCH 1/2] improve test coverage --- lib/src/usage_impl.dart | 15 ++++---- lib/src/usage_impl_io.dart | 17 +++++++-- lib/usage.dart | 8 ++++ test/all.dart | 4 ++ test/hit_types_test.dart | 15 -------- test/src/common.dart | 12 +++++- test/usage_impl_io_test.dart | 74 ++++++++++++++++++++++++++++++++++++ test/usage_impl_test.dart | 46 +++++++++++++++------- test/usage_test.dart | 22 +++++++++++ tool/ga.dart | 2 + 10 files changed, 173 insertions(+), 42 deletions(-) create mode 100644 test/usage_impl_io_test.dart create mode 100644 test/usage_test.dart diff --git a/lib/src/usage_impl.dart b/lib/src/usage_impl.dart index e000843..7a24819 100644 --- a/lib/src/usage_impl.dart +++ b/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 map) { +String postEncode(Map 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; } } diff --git a/lib/src/usage_impl_io.dart b/lib/src/usage_impl_io.dart index cd336cb..8f72e7a 100644 --- a/lib/src/usage_impl_io.dart +++ b/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 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); diff --git a/lib/usage.dart b/lib/usage.dart index 63c39dc..9ddcb9e 100644 --- a/lib/usage.dart +++ b/lib/usage.dart @@ -93,6 +93,10 @@ 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; @@ -100,6 +104,10 @@ class AnalyticsMock extends Analytics { 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) { diff --git a/test/all.dart b/test/all.dart index 4622d02..9d4dd3c 100644 --- a/test/all.dart +++ b/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(); } diff --git a/test/hit_types_test.dart b/test/hit_types_test.dart index 57fb144..4a4a775 100644 --- a/test/hit_types_test.dart +++ b/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); diff --git a/test/src/common.dart b/test/src/common.dart index 6833981..f5cd0b6 100644 --- a/test/src/common.dart +++ b/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 get last => mockPostHandler.last; diff --git a/test/usage_impl_io_test.dart b/test/usage_impl_io_test.dart new file mode 100644 index 0000000..a0eec18 --- /dev/null +++ b/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 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 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) { } +} diff --git a/test/usage_impl_test.dart b/test/usage_impl_test.dart index a407731..c71cccd 100644 --- a/test/usage_impl_test.dart +++ b/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,32 @@ 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'); + }); + }); + group('sanitizeFilePaths', () { test('replace file', () { expect(sanitizeFilePaths( @@ -39,19 +66,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'); }); }); } diff --git a/test/usage_test.dart b/test/usage_test.dart new file mode 100644 index 0000000..5edfa45 --- /dev/null +++ b/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'); + }); + }); +} diff --git a/tool/ga.dart b/tool/ga.dart index 1732448..0b57cd6 100644 --- a/tool/ga.dart +++ b/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((_) { From 9243d31452727befb47f564384ab891cd3660c49 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Thu, 18 Dec 2014 13:20:20 -0800 Subject: [PATCH 2/2] cover a branch --- test/usage_impl_test.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/usage_impl_test.dart b/test/usage_impl_test.dart index c71cccd..80b8690 100644 --- a/test/usage_impl_test.dart +++ b/test/usage_impl_test.dart @@ -48,6 +48,9 @@ void defineTests() { mock.setSessionValue('val', 'ue'); mock.sendScreenView('bar'); has(mock.last, 'val'); + mock.setSessionValue('val', null); + mock.sendScreenView('baz'); + hasnt(mock.last, 'val'); }); });