Added unit test for HostDaemonFacade.

Added unit test for HostDaemonFacade.

BUG=471928
Committed: https://crrev.com/133be790fe112ae62c358ddd733751d4756a03c4
Cr-Commit-Position: refs/heads/master@{#324165}

[John Williams, 2015-04-04 01:57:51]

jrw@chromium.org changed reviewers:
+ garykac@chromium.org, kelvinp@chromium.org

[kelvinp, 2015-04-06 17:25:10]

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/crd/js/host...
File remoting/webapp/crd/js/host_daemon_facade_unittest.js (right):

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/crd/js/host...
remoting/webapp/crd/js/host_daemon_facade_unittest.js:42: function
sendMockHostResponse(message) {
message seems to be unused in this function.  Is it only for JSCompile?

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/crd/js/host...
remoting/webapp/crd/js/host_daemon_facade_unittest.js:143: });
Should probably fail the test if the promise rejects
var done = assert.async();
...
.catch(function(error) {
  assert.ok(false);
  done();
})

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/crd/js/host...
remoting/webapp/crd/js/host_daemon_facade_unittest.js:154: function
resolveMulti(resolve) {
Can this simply be

function() {
  resolve(arguments);
}

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/crd/js/host...
remoting/webapp/crd/js/host_daemon_facade_unittest.js:252: var supported =
Math.random() > 0.5;
Not a big fan of random values in test.  You can imagine if there is a bug in
which the test fails only when supported is set to false.  The test will pass
sometime but fail in other cases.

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/js_proto/ch...
File remoting/webapp/js_proto/chrome_mocks.js (right):

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/js_proto/ch...
remoting/webapp/js_proto/chrome_mocks.js:113: if (port == null) {
use ===

[John Williams, 2015-04-06 19:14:59]

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/crd/js/host...
File remoting/webapp/crd/js/host_daemon_facade_unittest.js (right):

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/crd/js/host...
remoting/webapp/crd/js/host_daemon_facade_unittest.js:42: function
sendMockHostResponse(message) {
On 2015/04/06 17:25:10, kelvinp wrote:
> message seems to be unused in this function.  Is it only for JSCompile?

Removed.

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/crd/js/host...
remoting/webapp/crd/js/host_daemon_facade_unittest.js:143: });
On 2015/04/06 17:25:09, kelvinp wrote:
> Should probably fail the test if the promise rejects
> var done = assert.async();
> ...
> .catch(function(error) {
>   assert.ok(false);
>   done();
> })

QUnit automatically fails the test if the promise is rejected.

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/crd/js/host...
remoting/webapp/crd/js/host_daemon_facade_unittest.js:154: function
resolveMulti(resolve) {
On 2015/04/06 17:25:09, kelvinp wrote:
> Can this simply be
> 
> function() {
>   resolve(arguments);
> }

That doesn't work, but I did find a shorter pattern that does.

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/crd/js/host...
remoting/webapp/crd/js/host_daemon_facade_unittest.js:252: var supported =
Math.random() > 0.5;
On 2015/04/06 17:25:10, kelvinp wrote:
> Not a big fan of random values in test.  You can imagine if there is a bug in
> which the test fails only when supported is set to false.  The test will pass
> sometime but fail in other cases.

I was somewhat inspired by QuickCheck, a Haskell library that uses random values
for unit testing.  I've found it works very well in practice because unit tests
get run over and over, and it's not hard to debug when the failing values are
printed.  In practice it seems to result in better coverage than hardcoded test
values.

With something like a string or a number, I'm OK with a hardcoded value because
it's easy to check that the correct value has been propagated from the input to
the output, but with booleans, each value is essentially a separate test case.

Anyway, I converted the tests to be exhaustive.

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/js_proto/ch...
File remoting/webapp/js_proto/chrome_mocks.js (right):

https://codereview.chromium.org/1060793003/diff/1/remoting/webapp/js_proto/ch...
remoting/webapp/js_proto/chrome_mocks.js:113: if (port == null) {
On 2015/04/06 17:25:10, kelvinp wrote:
> use ===

Done.

[kelvinp, 2015-04-07 20:34:45]

lgtm

[John Williams, 2015-04-07 20:54:31]

The CQ bit was checked by jrw@chromium.org

[John Williams, 2015-04-07 20:54:32]

The patchset sent to the CQ was uploaded after l-g-t-m from kelvinp@chromium.org
Link to the patchset: https://codereview.chromium.org/1060793003/#ps60001
(title: "merge")

[commit-bot: I haz the power, 2015-04-07 20:55:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1060793003/60001

[commit-bot: I haz the power, 2015-04-07 21:08:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-07 21:08:18]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[John Williams, 2015-04-07 23:22:04]

The CQ bit was checked by jrw@chromium.org

[John Williams, 2015-04-07 23:22:05]

The patchset sent to the CQ was uploaded after l-g-t-m from kelvinp@chromium.org
Link to the patchset: https://codereview.chromium.org/1060793003/#ps80001
(title: "license")

[commit-bot: I haz the power, 2015-04-07 23:22:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1060793003/80001

[commit-bot: I haz the power, 2015-04-08 01:12:34]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-04-08 01:13:17]

Patchset 5 (id:??) landed as
https://crrev.com/133be790fe112ae62c358ddd733751d4756a03c4
Cr-Commit-Position: refs/heads/master@{#324165}