Unit tests for utility.js

chrome/browser/resources/google_now/common_test_util.js

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Common test utilities.
+
+function emptyMock() {}

[skare_, 2013/08/29 00:03:37]

the extension is assembled with sourcefile concatenation right? Need to guard
against naming collisions in the global namespace? if so, self-execing
anonymouse function?

[vadimt, 2013/08/29 00:28:22]

This is not a part of the extension. There are 3 independent gtestjs files.
Each of them is assembled independently, like:
  extraLibraries: [
    'common_test_util.js',
    'utility_test_util.js',
    'utility.js'
  ]
and we don't have collisions.

+
+// Container for event handlers added by mocked 'addListener' functions.
+var mockEventHandlers = {};
+
+/**
+ * Mocks 'addListener' function of an API event. The mocked function will add
+ * handlers to arrays stored inside mockEventHandlers.

[skare_, 2013/08/29 00:03:37]

hide implementation details? e.g. "mocked function will keep track of handlers?"
(or not)

[vadimt, 2013/08/29 00:28:22]

Done.

+ * @param {Object} topLevelContainer Top-level container of the original
+ *     function. Can be either chrome or instrumented.

[skare_, 2013/08/29 00:03:37]

nit: maybe 
'chrome' or 'instrumented'.

for consistency with 'addListener' and 'runtime.onSuspend' elsewhere in comment.

[vadimt, 2013/08/29 00:28:22]

Done.

+ * @param {string} eventIdentifier Event identifier, such as
+ *     'runtime.onSuspend'.
+ */
+function mockChromeEvent(topLevelContainer, eventIdentifier) {
+  var eventIdentifierParts = eventIdentifier.split('.');
+  var eventName = eventIdentifierParts.pop();

[skare_, 2013/08/29 00:03:37]

maybe check that (eventName != undefined) or (eventIdentifierParts.length > 0)

[vadimt, 2013/08/29 00:28:22]

Then it will crash. This is just a test anyways.

+  var originalMethodContainer = topLevelContainer;
+  var mockEventContainer = mockEventHandlers;
+  eventIdentifierParts.forEach(function(fragment) {

[skare_, 2013/08/29 00:03:37]

js styleguide suggests using normal iteration over for-in with arrays; not sure
if that extends to array.foreach. Seems to avoid the underlying problem
(expandos aren't included with forEach) though, so probably no action required.

[vadimt, 2013/08/29 00:28:22]

This is because for-in can lead to errors, not because of stylistic preferences.
forEach works correctly, therefore, there are no reasons to not use it.
arv@, one of style guide authors, suggested using it (as a better alternative to
map()).

+    originalMethodContainer =
+        originalMethodContainer[fragment] =

[skare_, 2013/08/29 00:03:37]

quick check on this and 29-31 -- intended to be (x = x[prop] = x[prop] || {})?
can chat quickly in person

[vadimt, 2013/08/29 00:28:22]

This is exactly what happens:
originalMethodContainer = originalMethodContainer[fragment] =
originalMethodContainer[fragment] || {};

+        originalMethodContainer[fragment] || {};
+    mockEventContainer =
+        mockEventContainer[fragment] =
+        mockEventContainer[fragment] || {};
+  });
+
+  mockEventContainer[eventName] = [];
+  originalMethodContainer[eventName] = {
+    addListener: function(callback) {
+      mockEventContainer[eventName].push(callback);
+    }
+  };
+}
+
+/**
+ * Gets the array of event handlers added by a mocked 'addListener' function.
+ * @param {string} eventIdentifier Event identifier, such as
+ *     'runtime.onSuspend'.
+ * @return {Array.<Function>} Array of handlers.
+ */
+function getMockHandlerContainer(eventIdentifier) {
+  var eventIdentifierParts = eventIdentifier.split('.');
+  var mockEventContainer = mockEventHandlers;
+  eventIdentifierParts.forEach(function(fragment) {
+    mockEventContainer = mockEventContainer[fragment];
+  });
+
+  return mockEventContainer;
+}