Merge lp:~gary/juju-gui/fixGoSandboxWatcher2 into lp:juju-gui/experimental

[Gary Poster]

Fix Go sandbox AllWatcher initialization

The Go sandbox AllWatcher was trying to send deltas immediately, which does not match the juju core implementation. This fixes the behavior, with a test.

It also fixes a user-visible bug. To qa, turn off the simulator in the config file, and then deploy a service in the default go sandbox. Before my branch, the unit would never appear, because the delta for this change was lost. After my branch, the unit appears correctly.

https://codereview.appspot.com/14548050/

[Gary Poster (gary)]

Reviewers: mp+190685_code.launchpad.net,

Message:
Please take a look.

Description:
Fix Go sandbox AllWatcher initialization

The Go sandbox AllWatcher was trying to send deltas immediately, which
does not match the juju core implementation. This fixes the behavior,
with a test.

It also fixes a user-visible bug. To qa, turn off the simulator in the
config file, and then deploy a service in the default go sandbox.
Before my branch, the unit would never appear, because the delta for
this change was lost. After my branch, the unit appears correctly.

https://code.launchpad.net/~gary/juju-gui/fixGoSandboxWatcher2/+merge/190685

(do not edit description out of merge proposal)

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

Affected files (+8, -5 lines):
   A [revision details]
   M app/store/env/sandbox.js
   M test/test_sandbox_go.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_go.js
=== modified file 'test/test_sandbox_go.js'
--- test/test_sandbox_go.js 2013-10-01 18:30:10 +0000
+++ test/test_sandbox_go.js 2013-10-11 13:47:43 +0000
@@ -164,7 +164,8 @@
        client.onmessage = function(received) {
          var receivedData = Y.JSON.parse(received.data);
          assert.equal(receivedData.Response.AllWatcherId, 42);
- assert.equal(client.get('juju').get('nextRequestId'), 1066);
+ assert.equal(receivedData.RequestId, 1066);
+ assert.isUndefined(client.get('juju').get('nextRequestId'));
          done();
        };
        client.open();

Index: app/store/env/sandbox.js
=== modified file 'app/store/env/sandbox.js'
--- app/store/env/sandbox.js 2013-10-10 17:00:09 +0000
+++ app/store/env/sandbox.js 2013-10-11 13:47:43 +0000
@@ -1066,12 +1066,12 @@
      @return {undefined} Side effects only.
      */
      handleClientWatchAll: function(data, client, state) {
- this.set('nextRequestId', data.RequestId);
- this.deltaIntervalId = setInterval(
- this.sendDelta.bind(this), this.get('deltaInterval'));
        // AllWatcherId can be hard-coded because we will only ever have one
        // client listening to the environment with the sandbox environment.
- client.receive({Response: {AllWatcherId: 42}});
+ client.receive({
+ RequestId: data.RequestId,
+ Response: {AllWatcherId: 42}
+ });
      },

      /**

[Madison Scott-Clary (makyo)]

LGTM, thanks for catching this!

https://codereview.appspot.com/14548050/

[Gary Poster (gary)]

*** Submitted:

Fix Go sandbox AllWatcher initialization

The Go sandbox AllWatcher was trying to send deltas immediately, which
does not match the juju core implementation. This fixes the behavior,
with a test.

It also fixes a user-visible bug. To qa, turn off the simulator in the
config file, and then deploy a service in the default go sandbox.
Before my branch, the unit would never appear, because the delta for
this change was lost. After my branch, the unit appears correctly.

R=matthew.scott
CC=
https://codereview.appspot.com/14548050

https://codereview.appspot.com/14548050/

[Gary Poster (gary)]

On 2013/10/11 14:29:25, matthew.scott wrote:
> LGTM, thanks for catching this!

Thanks, Matt!

https://codereview.appspot.com/14548050/

=== modified file 'app/store/env/sandbox.js'
--- app/store/env/sandbox.js	2013-10-10 17:00:09 +0000
+++ app/store/env/sandbox.js	2013-10-11 14:26:10 +0000
@@ -1066,12 +1066,12 @@
     @return {undefined} Side effects only.
     */
     handleClientWatchAll: function(data, client, state) {
-      this.set('nextRequestId', data.RequestId);
-      this.deltaIntervalId = setInterval(
-          this.sendDelta.bind(this), this.get('deltaInterval'));
       // AllWatcherId can be hard-coded because we will only ever have one
       // client listening to the environment with the sandbox environment.
-      client.receive({Response: {AllWatcherId: 42}});
+      client.receive({
+        RequestId: data.RequestId,
+        Response: {AllWatcherId: 42}
+      });
     },
 
     /**

=== modified file 'test/test_sandbox_go.js'
--- test/test_sandbox_go.js	2013-10-01 18:30:10 +0000
+++ test/test_sandbox_go.js	2013-10-11 14:26:10 +0000
@@ -164,7 +164,8 @@
       client.onmessage = function(received) {
         var receivedData = Y.JSON.parse(received.data);
         assert.equal(receivedData.Response.AllWatcherId, 42);
-        assert.equal(client.get('juju').get('nextRequestId'), 1066);
+        assert.equal(receivedData.RequestId, 1066);
+        assert.isUndefined(client.get('juju').get('nextRequestId'));
         done();
       };
       client.open();