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

remoting/webapp/crd/js/mock_xhr.js

 /**
  * @fileoverview A mock version of remoting.Xhr.  Compared to
  * sinon.useMockXhr, this allows unit tests to be written at a higher
  * level, and it eliminates a fair amount of boilerplate involved in
  * making the sinon mocks work with asynchronous calls
  * (cf. gcd_client_unittest.js vs. gcd_client_with_mock_xhr.js for an
  * example).
  */
 
 (function() {
@@ skipping 162 matching lines @@
     method, urlPattern, callback, opt_reuse) {
   handlers.push({
     method: method,
     urlPattern: urlPattern,
     callback: callback,
     reuse: !!opt_reuse
   });
 };
 
 /**
  * Calls |setResponseFor| to add a 204 (no content) response.

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

204 is now just the default. However, it's not clear from this CL why you've
added this parameter. Is it for future tests? If so, then it would be better to
save it until the CL where you need it.

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

I added this so I could use this method for a 500 response in
host_controller_unittest.js (line 188 in the first patch).

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

Okay, but please update the comment, since it doesn't necessarily use 204. You
can add that detail as documentation for the opt_status parameter.

  *
  * @param {?string} method
  * @param {?string|!RegExp} urlPattern
+ * @param {number=} opt_status
  * @param {boolean=} opt_reuse
  */
-remoting.MockXhr.setEmptyResponseFor = function(method, urlPattern, opt_reuse) {
+remoting.MockXhr.setEmptyResponseFor = function(
+    method, urlPattern, opt_status, opt_reuse) {
   remoting.MockXhr.setResponseFor(
       method, urlPattern, function(/** remoting.MockXhr */ xhr) {
-        xhr.setEmptyResponse(204);
+        xhr.setEmptyResponse(opt_status == null ? 204 : opt_status);

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

For the most common invocation, opt_status will be undefined, not null. This
will work, but I think it would be clearer if you compare to undefined. == is
still probably preferable to === though.

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

In google3 I'd use goog.isDef, but here I've been using "== null" to test for
undefined in cases where null is either not allowed or it should be treated the
same as undefined.  For future reference, would you prefer "=== undefined" in
those cases?

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

I guess the question here is whether or not we want to explicitly allow:

  setEmptyResponseFor(method, pattern, null)

in place of:

  setEmptyResponseFor(method, pattern)

I can't think of any reason why a caller would want to write that (maybe if they
wanted to set opt_reuse but not opt_status, but even then undefined would be
more "correct") so I lean towards using '=== undefined' and having the code fail
is the caller passes null. If you want to be more lenient, then '== undefined'
is acceptable, but I feel that '== null' is misleading as to the expected value
of omitted parameters.

       }, opt_reuse);
 };
 
 /**
  * Calls |setResponseFor| to add a 200 response with text content.
  *
  * @param {?string} method
  * @param {?string|!RegExp} urlPattern
  * @param {string} content
  * @param {boolean=} opt_reuse
@@ skipping 84 matching lines @@
 // Can't put put typedefs inside a function :-(
 /**
  * @typedef {{
  *   method:?string,
  *   urlPattern:(?string|RegExp),
  *   callback:function(!remoting.MockXhr):void,
  *   reuse:boolean
  * }}
  */
 remoting.MockXhr.UrlHandler;