Improve HostOptions unit tests

remoting/webapp/base/js/host_options_unittest.js

Index: remoting/webapp/base/js/host_options_unittest.js
diff --git a/remoting/webapp/base/js/host_options_unittest.js b/remoting/webapp/base/js/host_options_unittest.js
index a6b1d461d01f22b80f1d152f59471f6242276224..7e4a948adef2dc63ba8e3cdb306f4156e4e6be32 100644
--- a/remoting/webapp/base/js/host_options_unittest.js
+++ b/remoting/webapp/base/js/host_options_unittest.js
@@ -56,6 +56,7 @@ QUnit.test('Loading a non-existent host yields default values',
 QUnit.test('Saving and loading a host preserves the saved values',
   function(assert) {
     var options = new remoting.HostOptions('host-id');
+    var optionsNew = new remoting.HostOptions('host-id');

[Jamie, 2015/09/26 00:40:41]

Loading into the same object as we saved would trivially pass if load() was a
no-op, which is not what we want.

[kelvinp, 2015/09/28 18:36:40]

Nit: optionsLoaded may be a more descriptive name. 

[Jamie, 2015/09/28 21:33:48]

Done.

     options.setShrinkToFit(false);
     options.setResizeToClient(false);
     options.setDesktopScale(2);
@@ -67,18 +68,20 @@ QUnit.test('Saving and loading a host preserves the saved values',
     var optionsCopy = base.deepCopy(options);
 
     return options.save().then(function() {
-      return options.load();
+      return optionsNew.load();
     }).then(function() {
-      assert.deepEqual(optionsCopy, base.deepCopy(options));
+      assert.deepEqual(optionsCopy, base.deepCopy(optionsNew));
     });
 });
 
 QUnit.test('Saving a host ignores unset values',
   function(assert) {
     var options = new remoting.HostOptions('host-id');
+    options.setShrinkToFit(false);
     var optionsCopy = base.deepCopy(options);
+    var optionsEmpty = new remoting.HostOptions('host-id');

[Jamie, 2015/09/26 00:40:41]

This test wasn't really testing anything previously. Now we assert that
non-default values are not reset when loading an object that didn't have an
override set when it was saved.

 
-    return options.save().then(function() {
+    return optionsEmpty.save().then(function() {
       return options.load();
     }).then(function() {
       assert.deepEqual(optionsCopy, base.deepCopy(options));
@@ -104,3 +107,17 @@ QUnit.test('Old-style (string-formatted) key remappings are parsed correctly',
           assert.deepEqual(options1, options2);
         });
 });
+
+QUnit.test('New options are loaded and saved without updating the code',
+  function(assert) {
+    var options = new remoting.HostOptions('host-id');
+    var optionsNew = new remoting.HostOptions('host-id');
+    options['undefined-option'] = 42;
+    var optionsCopy = base.deepCopy(options);
+
+    return options.save().then(function() {
+      return optionsNew.load();
+    }).then(function() {
+      assert.deepEqual(optionsCopy, base.deepCopy(optionsNew));

[kelvinp, 2015/09/28 18:36:40]

Nit: maybe good to assert options['undefined-options] == 42 to more explicitly
call out what the expected behavior is.

[Jamie, 2015/09/28 21:33:48]

Done.

+    });
+});