Refactor sinon helper to make it easier to use.

Previously unittests using sinon were required to use sinon.$setupStub
in order to typecheck correctly. It was usable only for the case when
the test wants to stub a single method, but not when a whole object is
stubbed. This CL gets rid of $setupStub and adds $testStub() function
that simply casts parameter to the TestStub type.

Review URL: https://codereview.chromium.org/1002553002

Cr-Commit-Position: refs/heads/master@{#320154}

10 files changed, 47 insertions, 103 deletions

diff --git a/remoting/remoting_webapp_files.gypi b/remoting/remoting_webapp_files.gypi
index 447714b..52e4dab 100644
--- a/remoting/remoting_webapp_files.gypi
+++ b/remoting/remoting_webapp_files.gypi
@@ -84,7 +84,6 @@
       'webapp/unittests/chrome_mocks.js',
       'webapp/unittests/mock_signal_strategy.js',
       'webapp/unittests/sinon_helpers.js',
-      'webapp/unittests/test_start.js',
     ],
     # Prototypes for objects that are not mocked.
     'remoting_webapp_unittest_js_proto_files': [
@@ -93,7 +92,6 @@
       'webapp/js_proto/remoting_proto.js',
       'webapp/js_proto/qunit_proto.js',
       'webapp/js_proto/sinon_proto.js',
-      'webapp/js_proto/sinon_stub_proto.js',
     ],
     'remoting_webapp_unittest_all_js_files': [
       '<@(remoting_webapp_unittest_js_files)',
diff --git a/remoting/webapp/crd/html/template_unittest.html b/remoting/webapp/crd/html/template_unittest.html
index cce27c7..eb99c5c 100644
--- a/remoting/webapp/crd/html/template_unittest.html
+++ b/remoting/webapp/crd/html/template_unittest.html
@@ -22,7 +22,6 @@ found in the LICENSE file.
     <script src="blanketjs/qunit_adapter.js"></script>
     <script src="sinonjs/sinon.js"></script>
     <script src="sinonjs/sinon-qunit.js"></script>
-    <script src="test_start.js"></script>
 
     <!-- product files and unit test files-->
     <meta-include type="javascript"/>
diff --git a/remoting/webapp/js_proto/sinon_stub_proto.js b/remoting/webapp/js_proto/sinon_stub_proto.js
deleted file mode 100644
index 9abaa56..0000000
--- a/remoting/webapp/js_proto/sinon_stub_proto.js
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright 2015 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.
-
-// How to create sinon.stubs that work with jscompile.
-//
-// To create the stub:
-//   sinon.$setupStub(<object>, <function-name>)
-//
-// To access the stub in unittests:
-//   <object>.<function-name>.$testStub.<sinon-test>
-//
-// For example:
-//    sinon.$setupStub(chrome.socket, 'create');
-//    chrome.socket.create.$testStub.restore();
-//
-// For jscompile to analyze these corectly, you'll also need to add an entry
-// in this file for any object you stub out this way. For example:
-//    chrome.socket.create.$testStub = new sinon.TestStub();
-
-base.debug.assert.$testStub = new sinon.TestStub();
-base.isAppsV2.$testStub = new sinon.TestStub();
-
-chrome.i18n.getMessage.$testStub = new sinon.TestStub();
-
-chrome.socket.connect.$testStub = new sinon.TestStub();
-chrome.socket.create.$testStub = new sinon.TestStub();
-chrome.socket.destroy.$testStub = new sinon.TestStub();
-chrome.socket.read.$testStub = new sinon.TestStub();
-chrome.socket.secure.$testStub = new sinon.TestStub();
-chrome.socket.write.$testStub = new sinon.TestStub();
-
-remoting.setMode.$testStub = new sinon.TestStub();
-remoting.Host.needsUpdate.$testStub = new sinon.TestStub();
diff --git a/remoting/webapp/unittests/apps_v2_migration_unittest.js b/remoting/webapp/unittests/apps_v2_migration_unittest.js
index 805995a..097a80f 100644
--- a/remoting/webapp/unittests/apps_v2_migration_unittest.js
+++ b/remoting/webapp/unittests/apps_v2_migration_unittest.js
@@ -59,7 +59,7 @@ function setMigrationData_(v1UserName, v1UserEmail, v1HasHosts) {
 module('AppsV2Migration', {
   setup: function() {
     chromeMocks.activate(['storage']);
-    mockIsAppsV2 = sinon.$setupStub(base, 'isAppsV2');
+    mockIsAppsV2 = sinon.stub(base, 'isAppsV2');
     remoting.identity = new remoting.Identity();
   },
   teardown: function() {
diff --git a/remoting/webapp/unittests/base_unittest.js b/remoting/webapp/unittests/base_unittest.js
index 288ff91..a5a6a6f 100644
--- a/remoting/webapp/unittests/base_unittest.js
+++ b/remoting/webapp/unittests/base_unittest.js
@@ -22,14 +22,14 @@ test('mix(dest, src) should assert if properties are overwritten',
     var src = { a: 'a', b: 'b'};
     var dest = { a: 'a'};
 
-    sinon.$setupStub(base.debug, 'assert');
+    sinon.stub(base.debug, 'assert');
 
     try {
       base.mix(dest, src);
     } catch (e) {
     } finally {
       sinon.assert.called(base.debug.assert);
-      base.debug.assert.$testStub.restore();
+      $testStub(base.debug.assert).restore();
     }
 });
 
@@ -244,13 +244,13 @@ test('raiseEvent() should not invoke listeners of a different event',
 
 test('raiseEvent() should assert when undeclared events are raised',
   function() {
-    sinon.$setupStub(base.debug, 'assert');
+    sinon.stub(base.debug, 'assert');
     try {
       source.raiseEvent('undefined');
     } catch (e) {
     } finally {
       sinon.assert.called(base.debug.assert);
-      base.debug.assert.$testStub.restore();
+      $testStub(base.debug.assert).restore();
     }
 });
 
diff --git a/remoting/webapp/unittests/host_table_entry_unittest.js b/remoting/webapp/unittests/host_table_entry_unittest.js
index 7718688..9f36d78 100644
--- a/remoting/webapp/unittests/host_table_entry_unittest.js
+++ b/remoting/webapp/unittests/host_table_entry_unittest.js
@@ -28,14 +28,14 @@ module('HostTableEntry', {
                         '<div id="cancel-host-delete"></div>';
     setHost('LocalHost', 'ONLINE');
     fixture.appendChild(hostTableEntry_.element());
-    sinon.$setupStub(chrome.i18n, 'getMessage', function(/** string */ tag){
+    sinon.stub(chrome.i18n, 'getMessage', function(/** string */ tag){
       return tag;
     });
   },
   teardown: function() {
     hostTableEntry_.dispose();
     hostTableEntry_ = null;
-    chrome.i18n.getMessage.$testStub.restore();
+    $testStub(chrome.i18n.getMessage).restore();
   }
 });
 
@@ -77,7 +77,7 @@ function verifyVisible(
 test('Clicking on the confirm button in the confirm dialog deletes the host',
   function() {
   // Setup.
-  sinon.$setupStub(remoting, 'setMode', function(/** remoting.AppMode */ mode) {
+  sinon.stub(remoting, 'setMode', function(/** remoting.AppMode */ mode) {
     if (mode === remoting.AppMode.CONFIRM_HOST_DELETE) {
       document.getElementById('confirm-host-delete').click();
     }
@@ -90,14 +90,14 @@ test('Clicking on the confirm button in the confirm dialog deletes the host',
   sinon.assert.calledWith(onDelete_, hostTableEntry_);
 
   // Cleanup.
-  remoting.setMode.$testStub.restore();
+  $testStub(remoting.setMode).restore();
 });
 
 test(
   'Clicking on the cancel button in the confirm dialog cancels host deletion',
   function() {
   // Setup.
-  sinon.$setupStub(remoting, 'setMode', function(/** remoting.AppMode */ mode) {
+  sinon.stub(remoting, 'setMode', function(/** remoting.AppMode */ mode) {
     if (mode === remoting.AppMode.CONFIRM_HOST_DELETE) {
       document.getElementById('cancel-host-delete').click();
     }
@@ -110,7 +110,7 @@ test(
   sinon.assert.notCalled(onDelete_);
 
   // Cleanup.
-  remoting.setMode.$testStub.restore();
+  $testStub(remoting.setMode).restore();
 });
 
 test('Clicking on the rename button shows the input field.', function() {
@@ -168,7 +168,7 @@ test('HostTableEntry renders an offline host correctly.', function() {
 });
 
 test('HostTableEntry renders an out-of-date host correctly', function() {
-  sinon.$setupStub(remoting.Host, 'needsUpdate').returns(true);
+  sinon.stub(remoting.Host, 'needsUpdate').returns(true);
   setHost('LocalHost', 'ONLINE');
   var warningOverlay =
       hostTableEntry_.element().querySelector('.warning-overlay');
diff --git a/remoting/webapp/unittests/l10n_unittest.js b/remoting/webapp/unittests/l10n_unittest.js
index 043aaab..78ea48e 100644
--- a/remoting/webapp/unittests/l10n_unittest.js
+++ b/remoting/webapp/unittests/l10n_unittest.js
@@ -8,10 +8,10 @@
 
 module('l10n', {
   setup: function() {
-    sinon.$setupStub(chrome.i18n, 'getMessage');
+    sinon.stub(chrome.i18n, 'getMessage');
   },
   teardown: function() {
-    chrome.i18n.getMessage.$testStub.restore();
+    $testStub(chrome.i18n.getMessage).restore();
   }
 });
 
@@ -23,7 +23,7 @@ test('getTranslationOrError(tag) should return tag on error', function() {
 test('localizeElementFromTag() should replace innerText by default',
   function() {
     var element = document.createElement('div');
-    chrome.i18n.getMessage.$testStub.withArgs('tag')
+    $testStub(chrome.i18n.getMessage).withArgs('tag')
           .returns('<b>Hello World</b>');
 
     l10n.localizeElementFromTag(element, 'tag');
@@ -34,7 +34,7 @@ test('localizeElementFromTag() should replace innerText by default',
 test('localizeElementFromTag() should replace innerHTML if flag is set',
   function() {
     var element = document.createElement('div');
-    chrome.i18n.getMessage.$testStub.withArgs('tag')
+    $testStub(chrome.i18n.getMessage).withArgs('tag')
           .returns('<b>Hello World</b>');
 
     l10n.localizeElementFromTag(element, 'tag', null, true);
@@ -48,7 +48,7 @@ test(
   function() {
     var element = document.createElement('div');
     element.setAttribute('i18n-content', 'tag');
-    chrome.i18n.getMessage.$testStub.withArgs('tag')
+    $testStub(chrome.i18n.getMessage).withArgs('tag')
           .returns('<b>Hello World</b>');
 
     l10n.localizeElement(element);
@@ -62,7 +62,9 @@ test(
   function() {
     var fixture = document.getElementById('qunit-fixture');
     fixture.innerHTML = '<div class="target" i18n-title="tag"></div>';
-    chrome.i18n.getMessage.$testStub.withArgs('tag').returns('localized title');
+    $testStub(chrome.i18n.getMessage)
+        .withArgs('tag')
+        .returns('localized title');
 
     l10n.localize();
 
@@ -79,7 +81,7 @@ test('localize() should support string substitutions', function() {
       'i18n-value-2="param2">' +
   '</div>';
 
-  chrome.i18n.getMessage.$testStub.withArgs('tag', ['param1', 'param2'])
+  $testStub(chrome.i18n.getMessage).withArgs('tag', ['param1', 'param2'])
       .returns('localized');
 
   l10n.localize();
@@ -94,10 +96,11 @@ test('localize() should support tag substitutions', function() {
       '<div class="target" i18n-content="tag"' +
       ' i18n-value-name-1="tag1" i18n-value-name-2="tag2"></div>';
 
-  var getMessage = chrome.i18n.getMessage.$testStub;
-  getMessage.withArgs('tag1').returns('param1');
-  getMessage.withArgs('tag2').returns('param2');
-  getMessage.withArgs('tag', ['param1', 'param2']).returns('localized');
+  $testStub(chrome.i18n.getMessage).withArgs('tag1').returns('param1');
+  $testStub(chrome.i18n.getMessage).withArgs('tag2').returns('param2');
+  $testStub(chrome.i18n.getMessage)
+      .withArgs('tag', ['param1', 'param2'])
+      .returns('localized');
 
   l10n.localize();
 
diff --git a/remoting/webapp/unittests/sinon_helpers.js b/remoting/webapp/unittests/sinon_helpers.js
index 1524b40..ff4c944 100644
--- a/remoting/webapp/unittests/sinon_helpers.js
+++ b/remoting/webapp/unittests/sinon_helpers.js
@@ -2,21 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-var sinonHelpers = {};
-
-sinonHelpers.reset = function() {
-
 /**
- * @param {Object} obj
- * @param {string} method
- * @param {Function=} opt_stubFunction
+ * Casts an |object| to sinon.TestStub verifying that it's really a stub.
  * @return {sinon.TestStub}
- * @suppress {reportUnknownTypes}
  */
-sinon.$setupStub = function(obj, method, opt_stubFunction) {
-  sinon.stub(obj, method, opt_stubFunction);
-  obj[method].$testStub = /** @type {sinon.TestStub} */ (obj[method]);
-  return obj[method].$testStub;
-};
-
+function $testStub(/** Object */ object) {
+  base.debug.assert(object.hasOwnProperty("getCall"));
+  return /** @type {sinon.TestStub} */ (object);
 };
diff --git a/remoting/webapp/unittests/test_start.js b/remoting/webapp/unittests/test_start.js
deleted file mode 100644
index 8159ce7..0000000
--- a/remoting/webapp/unittests/test_start.js
+++ /dev/null
@@ -1,12 +0,0 @@
-// Copyright 2015 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.
-
-// Global test setup.
-// This code is run before each test in every module.
-
-QUnit.testStart(
-    /** @param {{module:string, name:string}} details */
-    function(details) {
-      sinonHelpers.reset();
-    });
diff --git a/remoting/webapp/unittests/xmpp_connection_unittest.js b/remoting/webapp/unittests/xmpp_connection_unittest.js
index f99f94c..fe635c1 100644
--- a/remoting/webapp/unittests/xmpp_connection_unittest.js
+++ b/remoting/webapp/unittests/xmpp_connection_unittest.js
@@ -27,12 +27,12 @@ module('XmppConnection', {
       onStanzaStr(new XMLSerializer().serializeToString(stanza));
     }
 
-    sinon.$setupStub(chrome.socket, 'create');
-    sinon.$setupStub(chrome.socket, 'connect');
-    sinon.$setupStub(chrome.socket, 'write');
-    sinon.$setupStub(chrome.socket, 'read');
-    sinon.$setupStub(chrome.socket, 'destroy');
-    sinon.$setupStub(chrome.socket, 'secure');
+    sinon.stub(chrome.socket, 'create');
+    sinon.stub(chrome.socket, 'connect');
+    sinon.stub(chrome.socket, 'write');
+    sinon.stub(chrome.socket, 'read');
+    sinon.stub(chrome.socket, 'destroy');
+    sinon.stub(chrome.socket, 'secure');
 
     connection = new remoting.XmppConnection();
     connection.setStateChangedCallback(
@@ -42,12 +42,12 @@ module('XmppConnection', {
   },
 
   teardown: function() {
-    chrome.socket.create.$testStub.restore();
-    chrome.socket.connect.$testStub.restore();
-    chrome.socket.write.$testStub.restore();
-    chrome.socket.read.$testStub.restore();
-    chrome.socket.destroy.$testStub.restore();
-    chrome.socket.secure.$testStub.restore();
+    $testStub(chrome.socket.create).restore();
+    $testStub(chrome.socket.connect).restore();
+    $testStub(chrome.socket.write).restore();
+    $testStub(chrome.socket.read).restore();
+    $testStub(chrome.socket.destroy).restore();
+    $testStub(chrome.socket.secure).restore();
   }
 });
 
@@ -57,11 +57,11 @@ test('should go to FAILED state when failed to connect', function() {
   sinon.assert.calledWith(onStateChange,
                           remoting.SignalStrategy.State.CONNECTING);
   sinon.assert.calledWith(chrome.socket.create, "tcp", {});
-  chrome.socket.create.$testStub.getCall(0).args[2]({socketId: socketId});
+  $testStub(chrome.socket.create).getCall(0).args[2]({socketId: socketId});
 
   sinon.assert.calledWith(
       chrome.socket.connect, socketId, "xmpp.example.com", 123);
-  chrome.socket.connect.$testStub.getCall(0).args[3](-1);
+  $testStub(chrome.socket.connect).getCall(0).args[3](-1);
 
   QUnit.equal(connection.getError().tag, remoting.Error.Tag.NETWORK_FAILURE);
 });
@@ -73,11 +73,11 @@ test('should use XmppLoginHandler to complete handshake and read data',
   sinon.assert.calledWith(onStateChange,
                           remoting.SignalStrategy.State.CONNECTING);
   sinon.assert.calledWith(chrome.socket.create, "tcp", {});
-  chrome.socket.create.$testStub.getCall(0).args[2]({socketId: socketId});
+  $testStub(chrome.socket.create).getCall(0).args[2]({socketId: socketId});
 
   sinon.assert.calledWith(
       chrome.socket.connect, socketId, "xmpp.example.com", 123);
-  chrome.socket.connect.$testStub.getCall(0).args[3](0);
+  $testStub(chrome.socket.connect).getCall(0).args[3](0);
 
   sinon.assert.calledWith(onStateChange,
                           remoting.SignalStrategy.State.HANDSHAKE);
@@ -96,7 +96,7 @@ test('should use XmppLoginHandler to complete handshake and read data',
   var data = base.encodeUtf8('<iq id="1">hello</iq>');
   sinon.assert.calledWith(chrome.socket.read, socketId);
   var appendDataCalled = parserMock.expects('appendData').once().withArgs(data);
-  chrome.socket.read.$testStub.getCall(0).args[1]({resultCode: 0, data: data});
+  $testStub(chrome.socket.read).getCall(0).args[1]({resultCode: 0, data: data});
   appendDataCalled.verify();
 });