Code review comment for lp://staging/~gary/juju-gui/setconfigfix

Revision history for this message
Gary Poster (gary) wrote :

Reviewers: mp+158380_code.launchpad.net,

Message:
Please take a look.

Description:
Fix sandbox setconfig and associated test

The sandbox setconfig test was taking over a second--a pretty clear
indication that it was waiting on the fake delta stream rather than a
direct response. Investigation revealed that the sandbox method was
incorrectly using the ASYNCOP helper rather than the OP helper. This
small branch fixes the setconfig problem and adjusts the tests. The
test is very fast now (not yellow or red in the mocha output).

https://code.launchpad.net/~gary/juju-gui/setconfigfix/+merge/158380

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8663043/

Affected files:
   A [revision details]
   M app/store/env/sandbox.js
   M test/test_sandbox.js

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: test/test_sandbox.js
=== modified file 'test/test_sandbox.js'
--- test/test_sandbox.js 2013-04-10 16:38:13 +0000
+++ test/test_sandbox.js 2013-04-11 13:56:45 +0000
@@ -808,10 +808,12 @@
            request_id: 99
          };
          client.onmessage = function(received) {
+ var parsed = Y.JSON.parse(received.data);
+ assert.deepEqual(parsed.result, {'blog-title': 'Inimical'});
            var service = state.db.services.getById('wordpress');
            assert.equal(service.get('config')['blog-title'], 'Inimical');
            // Error should be undefined.
- done(received.error);
+ done(parsed.error);
          };
          client.send(Y.JSON.stringify(op));
        });

Index: app/store/env/sandbox.js
=== modified file 'app/store/env/sandbox.js'
--- app/store/env/sandbox.js 2013-04-10 16:38:13 +0000
+++ app/store/env/sandbox.js 2013-04-11 13:52:22 +0000
@@ -529,7 +529,7 @@
                        of key/value pairs.
      */
      performOp_set_config: function(data) {
- ASYNC_OP(this, 'setConfig', ['service_name', 'config'])(data);
+ OP(this, 'setConfig', ['service_name', 'config'], data);
      },

      /**

« Back to merge proposal