Added partial unit tests for host_controller.js. More to come.

remoting/webapp/crd/js/host_controller_unittest.js

Index: remoting/webapp/crd/js/host_controller_unittest.js
diff --git a/remoting/webapp/crd/js/host_controller_unittest.js b/remoting/webapp/crd/js/host_controller_unittest.js
new file mode 100644
index 0000000000000000000000000000000000000000..8987a339f033bc487d6270f5f2794867316b7817
--- /dev/null
+++ b/remoting/webapp/crd/js/host_controller_unittest.js
@@ -0,0 +1,384 @@
+/**
+ * @fileoverview
+ * Unit tests for host_controller.js.
+ */
+
+(function() {
+
+'use strict';
+
+/** @type {remoting.HostController} */
+var controller = null;
+
+/** @type {sinon.Mock} */
+var hostListMock = null;
+
+/** @type {sinon.TestStub} */
+var generateUuidStub;
+
+/** @type {remoting.MockHostDaemonFacade} */
+var mockHostDaemonFacade;
+
+/** @type {sinon.TestStub} */
+var hostDaemonFacadeCtorStub;
+
+var FAKE_HOST_PIN = '<FAKE_HOST_PIN>';
+var FAKE_USER_EMAIL = '<FAKE_USER_EMAIL>';
+var FAKE_USER_NAME = '<FAKE_USER_NAME>';
+var FAKE_UUID = '0bad0bad-0bad-0bad-0bad-0bad0bad0bad';
+var FAKE_DAEMON_VERSION = '1.2.3.4';
+var FAKE_HOST_NAME = '<FAKE_HOST_NAME>';
+var FAKE_PUBLIC_KEY = '<FAKE_PUBLIC_KEY>';
+var FAKE_PRIVATE_KEY = '<FAKE_PRIVATE_KEY>';
+var FAKE_AUTH_CODE = '<FAKE_AUTH_CODE>';
+var FAKE_REFRESH_TOKEN = '<FAKE_REFRESH_TOKEN>';
+var FAKE_PIN_HASH = '<FAKE_PIN_HASH>';
+
+/** @type {sinon.Spy|Function} */
+var getCredentialsFromAuthCodeSpy;
+
+/** @type {sinon.Spy|Function} */
+var getPinHashSpy;
+
+/** @type {sinon.Spy|Function} */
+var startDaemonSpy;
+
+/** @type {sinon.Spy} */
+var unregisterHostByIdSpy;
+
+/** @type {sinon.Spy} */
+var onLocalHostStartedSpy;
+
+QUnit.module('host_controller', {
+  beforeEach: function(/** QUnit.Assert */ assert) {
+    chromeMocks.activate(['identity', 'runtime']);
+    chromeMocks.identity.mock$setToken('my_token');
+    remoting.identity = new remoting.Identity();
+    remoting.MockXhr.activate();
+    base.debug.assert(remoting.oauth2 === null);
+    remoting.oauth2 = new remoting.OAuth2();
+    base.debug.assert(remoting.hostList === null);
+    remoting.hostList = /** @type {remoting.HostList} */
+        (Object.create(remoting.HostList.prototype));
+
+    // When the HostList's unregisterHostById method is called, make
+    // sure the argument is correct.
+    unregisterHostByIdSpy =
+        sinon.stub(remoting.hostList, 'unregisterHostById', function(
+            /** string */ hostId) {
+          assert.equal(hostId, FAKE_UUID);
+        });
+
+    // When the HostList's onLocalHostStarted method is called, make
+    // sure the arguments are correct.
+    onLocalHostStartedSpy =
+        sinon.stub(
+            remoting.hostList, 'onLocalHostStarted', function(
+                /** string */ hostName,
+                /** string */ newHostId,
+                /** string */ publicKey) {
+              assert.equal(hostName, FAKE_HOST_NAME);
+              assert.equal(newHostId, FAKE_UUID);
+              assert.equal(publicKey, FAKE_PUBLIC_KEY);
+            });
+
+    hostDaemonFacadeCtorStub = sinon.stub(remoting, 'HostDaemonFacade');
+    mockHostDaemonFacade = new remoting.MockHostDaemonFacade();
+    hostDaemonFacadeCtorStub.returns(mockHostDaemonFacade);
+    generateUuidStub = sinon.stub(base, 'generateUuid');
+    generateUuidStub.returns(FAKE_UUID);
+    getCredentialsFromAuthCodeSpy = sinon.spy(
+        mockHostDaemonFacade, 'getCredentialsFromAuthCode');
+    getPinHashSpy = sinon.spy(mockHostDaemonFacade, 'getPinHash');
+    startDaemonSpy = sinon.spy(mockHostDaemonFacade, 'startDaemon');
+  },
+  afterEach: function() {

[Jamie, 2015/04/07 01:13:12]

Blank line before afterEach for readability.

[John Williams, 2015/04/07 20:10:26]

Done.

+    controller = null;
+    getCredentialsFromAuthCodeSpy.restore();
+    generateUuidStub.restore();
+    hostDaemonFacadeCtorStub.restore();
+    remoting.hostList = null;
+    remoting.oauth2 = null;
+    remoting.MockXhr.restore();
+    chromeMocks.restore();
+    remoting.identity = null;
+  }
+});
+
+// Check that hasFeature returns false by default.
+QUnit.test('hasFeature returns false', function(assert) {
+  controller = new remoting.HostController();

[Jamie, 2015/04/07 01:13:12]

Can this be moved into beforeEach (for symmetry with it being nulled in
afterEach)? Alternatively, can this be a local variable within in each test (ie,
remove the global and the nulling in afterEach)?

[John Williams, 2015/04/07 20:10:25]

Made local.

+  return new Promise(function(resolve, reject) {
+    controller.hasFeature(
+        remoting.HostController.Feature.PAIRING_REGISTRY,
+        resolve);
+  }).then(function(/** boolean */ result) {
+    assert.equal(result, false);
+  });
+});
+
+// Check what happens when the HostDaemonFacade's getHostName method
+// fails.
+QUnit.test('start with getHostName failure', function(assert) {
+  controller = new remoting.HostController();
+  return new Promise(function(resolve, reject) {
+    controller.start(FAKE_HOST_PIN, true, function() {
+      reject(null);
+    }, function(/** remoting.Error */ e) {
+      assert.equal(e.getDetail(), 'getHostName');
+      resolve(null);
+    });
+  });
+});
+
+// Prepare the to execute HostController.start up to the point where

[Jamie, 2015/04/07 01:13:12]

Prepare the what? (here and below)

[John Williams, 2015/04/07 20:10:25]

The word "the" wasn't supposed to be there.

+// the hostname is retreived.
+function initThroughHostName() {

[Jamie, 2015/04/07 01:13:12]

Using "through" to mean "until" is idiomatic to US English and not generally
understood elsewhere. I suggest initFieldsUpToHostName as an alternative.

However, can you just initialize everything (maybe in the ctor) and set to
undefined the field where you want to simulate failure? That has the added
advantage of being robust to the order in which the various properties are
requested.

[John Williams, 2015/04/07 20:10:26]

News to me. I'll keep that in mind.
The idea was to do the minimal initialization required for each test so that,
for instance, getting the daemon version can't depend creating a PIN hash.

Anyway, I rewrote it to initialize everything in beforeEach.  I don't want to do
it in the MockHostDaemonFacade constructor because I want all the specific
values used in the test to be in one place.  Since HostDaemonFacade is only used
by HostController, this won't result in any duplication of code.

+  mockHostDaemonFacade.daemonVersion = FAKE_DAEMON_VERSION;
+  mockHostDaemonFacade.hostName = FAKE_HOST_NAME;
+};
+
+// Check what happens when the HostDaemonFacade's generateKeyPair
+// method fails.
+QUnit.test('start with generateKeyPair failure', function(assert) {
+  initThroughHostName();
+  controller = new remoting.HostController();
+  return new Promise(function(resolve, reject) {
+    controller.start(FAKE_HOST_PIN, true, function() {
+      reject(null);
+    }, function(/** remoting.Error */ e) {
+      assert.equal(e.getDetail(), 'generateKeyPair');

[Jamie, 2015/04/07 01:13:12]

Check the tag as well?

[John Williams, 2015/04/07 20:10:26]

Seems redundant since it's always UNEXPECTED and the detail field uniquely
identifies which throw statement the exception came from.

[Jamie, 2015/04/08 00:16:14]

It's only UNEXPECTED if the code is bug-free :) If this code were to start
returning a different error code, it would result in confusing UI being
displayed to the user, so I think it's worth checking.

[John Williams, 2015/04/08 20:20:09]

Done.

+      resolve(null);
+    });
+  });
+});
+
+// Prepare the to execute HostController.start up to the point where
+// a key pair is generated.
+function initThroughPublicPrivateKey() {
+  initThroughHostName();
+  mockHostDaemonFacade.privateKey = FAKE_PRIVATE_KEY;
+  mockHostDaemonFacade.publicKey = FAKE_PUBLIC_KEY;
+}
+
+// Check what happens when the HostDaemonFacade's getHostClientId
+// method fails.
+QUnit.test('start with getHostClientId failure', function(assert) {
+  initThroughPublicPrivateKey();
+  mockHostDaemonFacade.features = [
+    remoting.HostController.Feature.OAUTH_CLIENT

[Jamie, 2015/04/07 01:13:12]

Is there a reason why you don't want this Feature to be supported for the other
tests?

[John Williams, 2015/04/07 20:10:25]

Minimalism again. I want the tests to demonstrate that everything works without
this feature.

I changed things to include that feature my default, since it seems to be the
default case in real life.

+  ];
+  controller = new remoting.HostController();
+  return new Promise(function(resolve, reject) {
+    controller.start(FAKE_HOST_PIN, true, function() {
+      reject(null);
+    }, function(/** remoting.Error */ e) {
+      assert.equal(e.getDetail(), 'getHostClientId');
+      resolve(null);
+    });
+  });
+});
+
+// Check what happens when the registry returns an HTTP when we try to
+// register a host.
+QUnit.test('start with host registration failure', function(assert) {
+  initThroughPublicPrivateKey();
+  controller = new remoting.HostController();
+  remoting.MockXhr.setEmptyResponseFor(
+      'POST', 'DIRECTORY_API_BASE_URL/@me/hosts', 500);
+  return new Promise(function(resolve, reject) {
+    controller.start(FAKE_HOST_PIN, true, function() {
+      reject(null);
+    }, function(/** remoting.Error */ e) {
+      assert.equal(e.getTag(), remoting.Error.Tag.REGISTRATION_FAILED);
+      resolve(null);
+    });
+  });
+});
+
+// Check what happens when the HostDaemonFacade's
+// getCredentialsFromAuthCode method fails.
+QUnit.test('start with getCredentialsFromAuthCode failure', function(assert) {
+  initThroughPublicPrivateKey();
+  remoting.MockXhr.setTextResponseFor(
+      'POST', 'DIRECTORY_API_BASE_URL/@me/hosts', JSON.stringify({
+        data: {
+          authorizationCode: FAKE_AUTH_CODE
+        }
+      }));
+  controller = new remoting.HostController();
+  return new Promise(function(resolve, reject) {
+    controller.start(FAKE_HOST_PIN, true, function() {
+      reject(null);
+    }, function(/** remoting.Error */ e) {
+      assert.equal(e.getDetail(), 'getCredentialsFromAuthCode');
+      resolve(null);
+    });
+  });
+});
+
+// Prepare the to execute HostController.start up to the point where
+// a refresh token is generated.
+function initThroughRefreshToken() {
+  initThroughPublicPrivateKey();
+  sinon.stub(remoting.identity, 'getEmail').returns(
+      Promise.resolve(FAKE_USER_EMAIL));
+  sinon.stub(remoting.oauth2, 'getRefreshToken').returns(
+      FAKE_REFRESH_TOKEN);
+  mockHostDaemonFacade.userEmail = FAKE_USER_EMAIL;
+  mockHostDaemonFacade.refreshToken = FAKE_REFRESH_TOKEN;
+}
+
+// Check what happens when the HostDaemonFacade's getPinHash method
+// fails, and verify that getPinHash is called when the registry
+// does't return an auth code.
+QUnit.test('start with getRefreshToken+getPinHash failure', function(assert) {
+  initThroughRefreshToken();
+  remoting.MockXhr.setTextResponseFor(
+      'POST', 'DIRECTORY_API_BASE_URL/@me/hosts', '{}');
+  controller = new remoting.HostController();
+  return new Promise(function(resolve, reject) {
+    controller.start(FAKE_HOST_PIN, true, function() {
+      reject(null);
+    }, function(/** remoting.Error */ e) {
+      assert.equal(unregisterHostByIdSpy.callCount, 0);
+      assert.equal(e.getDetail(), 'getPinHash');
+      resolve(null);
+    });
+  });
+});
+
+// Check what happens when the HostController's getClientBaseJid_
+// method fails.
+QUnit.test('start with getClientBaseJid_ failure', function(assert) {
+  initThroughRefreshToken();
+  remoting.MockXhr.setTextResponseFor(
+      'POST', 'DIRECTORY_API_BASE_URL/@me/hosts', JSON.stringify({
+        data: {
+          authorizationCode: FAKE_AUTH_CODE
+        }
+      }));
+  controller = new remoting.HostController();
+  sinon.stub(controller, 'getClientBaseJid_').
+      callsArgWith(1, remoting.Error.unexpected('getClientBaseJid_'));
+  return new Promise(function(resolve, reject) {
+    controller.start(FAKE_HOST_PIN, true, function() {
+      reject(null);
+    }, function(/** remoting.Error */ e) {
+      assert.equal(unregisterHostByIdSpy.callCount, 1);
+      assert.equal(e.getDetail(), 'getClientBaseJid_');
+      resolve(null);
+    });
+  });
+});
+
+// Prepare the to execute HostController.start up to the point where a
+// PIN hash is generated.  This function also installs a check to
+// ensure that the registry is updated correctly.
+function initThroughPinHash(/** QUnit.Assert */ assert) {
+  initThroughRefreshToken();
+  remoting.MockXhr.setResponseFor(
+      'POST', 'DIRECTORY_API_BASE_URL/@me/hosts',
+      function(/** remoting.MockXhr */ xhr) {
+        assert.deepEqual(
+            xhr.params.jsonContent,
+            { data: {
+              hostId: FAKE_UUID,
+              hostName: FAKE_HOST_NAME,
+              publicKey: FAKE_PUBLIC_KEY
+            } });
+        xhr.setTextResponse(200, JSON.stringify({
+          data: {
+            authorizationCode: FAKE_AUTH_CODE
+          }
+        }));
+      });
+  mockHostDaemonFacade.pinHash = FAKE_PIN_HASH;
+};
+
+// Check what happens when the HostDaemonFacade's startDaemon method
+// fails and calls its onError argument.
+QUnit.test('start with startDaemon failure', function(assert) {
+  initThroughPinHash(assert);
+  controller = new remoting.HostController();
+  sinon.stub(controller, 'getClientBaseJid_').callsArgWith(0, FAKE_USER_EMAIL);
+  return new Promise(function(resolve, reject) {
+    controller.start(FAKE_HOST_PIN, true, function() {
+      reject(null);
+    }, function(/** remoting.Error */ e) {
+      assert.equal(unregisterHostByIdSpy.callCount, 1);
+      assert.equal(e.getDetail(), 'startDaemon');
+      resolve(null);
+    });
+  });
+});
+
+// Check what happens when the HostDaemonFacade's startDaemon method
+// calls is onDone method with an async error code.
+QUnit.test('start with startDaemon cancelled', function(assert) {
+  initThroughPinHash(assert);
+  mockHostDaemonFacade.startDaemonResult =
+      remoting.HostController.AsyncResult.CANCELLED;
+  controller = new remoting.HostController();
+  sinon.stub(controller, 'getClientBaseJid_').callsArgWith(0, FAKE_USER_EMAIL);
+  return new Promise(function(resolve, reject) {
+    controller.start(FAKE_HOST_PIN, true, function() {
+      reject(null);
+    }, function(/** remoting.Error */ e) {
+      assert.equal(unregisterHostByIdSpy.callCount, 1);
+      assert.equal(e.getTag(), remoting.Error.Tag.CANCELLED);
+      resolve(null);
+    });
+  });
+});
+
+// Check what happens when the entire host registration process
+// succeeds.
+[false, true].forEach(function(/** boolean */ consent) {
+  QUnit.test('start succeeds with consent=' + consent, function(assert) {
+    initThroughPinHash(assert);
+    mockHostDaemonFacade.startDaemonResult =
+        remoting.HostController.AsyncResult.OK;
+    controller = new remoting.HostController();
+    sinon.stub(controller, 'getClientBaseJid_').
+        callsArgWith(0, FAKE_USER_EMAIL);
+    return new Promise(function(resolve, reject) {
+      controller.start(FAKE_HOST_PIN, consent, function() {
+        assert.equal(getCredentialsFromAuthCodeSpy.callCount, 1);
+        assert.deepEqual(
+            getCredentialsFromAuthCodeSpy.args[0][0],
+            FAKE_AUTH_CODE);
+        assert.equal(getPinHashSpy.callCount, 1);
+        assert.deepEqual(
+            getPinHashSpy.args[0].slice(0, 2),
+            [FAKE_UUID, FAKE_HOST_PIN]);
+
+        assert.equal(unregisterHostByIdSpy.callCount, 0);
+        assert.equal(onLocalHostStartedSpy.callCount, 1);
+        assert.equal(startDaemonSpy.callCount, 1);
+        assert.deepEqual(
+            startDaemonSpy.args[0].slice(0, 2),
+            [{
+              xmpp_login: FAKE_USER_EMAIL,
+              oauth_refresh_token: FAKE_REFRESH_TOKEN,
+              host_id: FAKE_UUID,
+              host_name: FAKE_HOST_NAME,
+              host_secret_hash: FAKE_PIN_HASH,
+              private_key: FAKE_PRIVATE_KEY
+            }, consent]);
+        resolve(null);
+      }, reject);
+    });
+  });
+});
+
+// TODO(jrw): Add tests for |stop| method.
+// TODO(jrw): Add tests for |updatePin| method.
+// TODO(jrw): Add tests for |getLocalHostState| method.
+// TODO(jrw): Add tests for |getLocalHostId| method.
+// TODO(jrw): Add tests for |getPairedClients| method.
+// TODO(jrw): Add tests for |deletePairedClient| method.
+// TODO(jrw): Add tests for |clearPairedClients| method.
+// TODO(jrw): Make sure getClientBaseJid_ method is tested.
+
+})();