Add add_units, expose, & unexpose to Go Sandbox.

Add add_units, expose, & unexpose to Go Sandbox.



https://code.launchpad.net/~bac/juju-gui/go-sandbox-addunits-expose-unexpose/+merge/171155

(do not edit description out of merge proposal)

[bac, 2013-06-24 19:29:16]

Please take a look.

[jeff.pihach, 2013-06-24 19:56:08]

LGTM - Thanks for this. just a few trivials below.

https://codereview.appspot.com/10522043/diff/1/app/store/env/sandbox.js
File app/store/env/sandbox.js (right):

https://codereview.appspot.com/10522043/diff/1/app/store/env/sandbox.js#newco...
app/store/env/sandbox.js:767: //client.receiveNow(data);
Remove

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js
File test/test_fakebackend.js (right):

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:331: fakebackend.logout();
Just to be on the safe side we should probably check to make sure it was logged
in first.

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:389: fakebackend.logout();
again as above, should probably check to make sure we are logged in.

https://codereview.appspot.com/10522043/diff/1/test/test_sandbox.js
File test/test_sandbox.js (right):

https://codereview.appspot.com/10522043/diff/1/test/test_sandbox.js#newcode1436
test/test_sandbox.js:1436: 
remove :)

[frankban, 2013-06-25 09:45:11]

LGTM, thank you.

[teknico, 2013-06-25 11:09:40]

LGTM. Again, thanks for adapting those support functions from the PyJuju sandbox
test suite.

See a few comments below. Also, a number of comment sentences are missing the
closing punctuation.

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js
File test/test_fakebackend.js (right):

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:347: '`Je ne suis pas un service` is an invalid service
name.',
Slightly unusual usage of backquotes. Maybe double quotes instead?

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:359: 'Service `wordpress` was already exposed.',
As above re: backquotes.

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:407: '`Je ne suis pas un service` is an invalid service
name.',
As above re: backquotes.

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:418: 'Service `wordpress` is not exposed.',
As above re: backquotes.

https://codereview.appspot.com/10522043/diff/1/test/test_sandbox.js
File test/test_sandbox.js (right):

https://codereview.appspot.com/10522043/diff/1/test/test_sandbox.js#newcode1810
test/test_sandbox.js:1810: }
Please add a blank line here.

Thanks for adapting these functions from the PyJuju sandbox test suite. I was
about to do the same, thankfully I looked at your branch first. :-)

[bac, 2013-06-25 13:06:05]

*** Submitted:

Add add_units, expose, & unexpose to Go Sandbox.

R=jeff.pihach, frankban, teknico
CC=
https://codereview.appspot.com/10522043

https://codereview.appspot.com/10522043/diff/1/app/store/env/sandbox.js
File app/store/env/sandbox.js (right):

https://codereview.appspot.com/10522043/diff/1/app/store/env/sandbox.js#newco...
app/store/env/sandbox.js:767: //client.receiveNow(data);
On 2013/06/24 19:56:08, jeff.pihach wrote:
> Remove

Done.

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js
File test/test_fakebackend.js (right):

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:331: fakebackend.logout();
Boo!  Hiss!

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:347: '`Je ne suis pas un service` is an invalid service
name.',
The test is merely testing what the fakebackend was issuing as an error message,
so the backquotes here were required.

It is sufficiently odd that I changed fakebackend.js to use single quotes
instead and updated these tests.

https://codereview.appspot.com/10522043/diff/1/test/test_sandbox.js
File test/test_sandbox.js (right):

https://codereview.appspot.com/10522043/diff/1/test/test_sandbox.js#newcode1436
test/test_sandbox.js:1436: 
On 2013/06/24 19:56:08, jeff.pihach wrote:
> remove :)

Done.