Bundle deployment support functional test.

Bundle deployment support functional test.

This branch includes a charm functional test
exercising the builtin server bundle support.

Reorganized the functional tests so that, at
least when the builtin server is used, the
deployed GUI service is reused by all tests.

Also fixed a pyJuju clean up error when the
builtin server is used to serve the GUI.

Added an helper function to retrieve, if
possible, the admin secret for the current
Juju environment, and a very simple WebSocket
client object used in tests.
Note that the websocket-client package used
by the client is now explicitly listed in
the requirements file, but it was already
installed before as a dependency of the
python juju client.

No QA required.

Tests:
run `make test JUJU_ENV="your-juju-environment"`
from the root of this branch.
Note that it is not currently possible to run
juju-test with a juju-core local provider:
with LXC juju-core requires the bootstrap and
destroy-environment commands to be run as root.
For this reason, I suggest to run the tests
using ec2; in my machine they take about 1:15h.

https://code.launchpad.net/~frankban/charms/precise/juju-gui/deployer-functional-test/+merge/187502

(do not edit description out of merge proposal)

[frankban, 2013-09-25 12:43:24]

Please take a look.

[bac, 2013-09-25 13:49:47]

LGTM with some suggestions on breaking up the long test.
My tests are still running.  I'll update when they finish.

https://codereview.appspot.com/13890044/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/13890044/diff/1/Makefile#newcode17
Makefile:17: JUJUTEST = juju-test --timeout=120m -v -e "$(JUJU_ENV)"
--upload-tools
ugh.

https://codereview.appspot.com/13890044/diff/1/tests/20-functional.test
File tests/20-functional.test (right):

https://codereview.appspot.com/13890044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:153: if options.get('builtin-server') == 'true':
should we make this test case-insensitive?  or is 'true' the one-true-way in
YAML?

https://codereview.appspot.com/13890044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:282: def test_deployer(self):
It would be nice if this super long test could be broken up into parts.  I can
see it easily being split into 4 or more tests.  Especially the error conditions
could be separate.

https://codereview.appspot.com/13890044/diff/1/tests/example.py
File tests/example.py (right):

https://codereview.appspot.com/13890044/diff/1/tests/example.py#newcode18
tests/example.py:18: 
An entry with multiple bundles would be nice for testing.

[gary.poster, 2013-09-25 14:00:28]

LGTM with suggestions.  I see now I duplicated bac's comments, except he had
some other good ones additionally. :-)

Thank you!

Gary

https://codereview.appspot.com/13890044/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/13890044/diff/1/Makefile#newcode17
Makefile:17: JUJUTEST = juju-test --timeout=120m -v -e "$(JUJU_ENV)"
--upload-tools
ugh :-)

https://codereview.appspot.com/13890044/diff/1/tests/20-functional.test
File tests/20-functional.test (right):

https://codereview.appspot.com/13890044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:282: def test_deployer(self):
Wow!

My only concern with this is that I think we could divide up the test into
smaller chunks.  Story tests like this have been historically painful to
maintain.

For example, we could put the different edge cases in separate tests.

I'm guessing there's a downside, or else you would have done just that.  Is the
downside that it will mean a lot more time lost to startup/teardown?

Please either add a comment explaining why it makes sense to combine all of the
edge cases together, or (if there isn't a good reason) divide them up.

[frankban, 2013-09-25 18:21:35]

Please take a look.

https://codereview.appspot.com/13890044/diff/1/tests/20-functional.test
File tests/20-functional.test (right):

https://codereview.appspot.com/13890044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:153: if options.get('builtin-server') == 'true':
On 2013/09/25 13:49:48, bac wrote:
> should we make this test case-insensitive?  or is 'true' the one-true-way in
> YAML?

We set that value ourselves when deploying the GUI in tests, so we know for sure
the correct spelling.
FWIW, juju-core IIRC accepts True, true, Yes and yes...

https://codereview.appspot.com/13890044/diff/1/tests/20-functional.test#newco...
tests/20-functional.test:282: def test_deployer(self):
On 2013/09/25 14:00:28, gary.poster wrote:
> Wow!
> 
> My only concern with this is that I think we could divide up the test into
> smaller chunks.  Story tests like this have been historically painful to
> maintain.
> 
> For example, we could put the different edge cases in separate tests.
> 
> I'm guessing there's a downside, or else you would have done just that.  Is
the
> downside that it will mean a lot more time lost to startup/teardown?
> 
> Please either add a comment explaining why it makes sense to combine all of
the
> edge cases together, or (if there isn't a good reason) divide them up.

Thank you both for suggesting this. I agree with you. Now we have a test for
each error case, and a test for the double deployment story.
I did a single test before because I mechanically converted a QA script I have,
but, as I mentioned, I agree we must pursue test separation.

https://codereview.appspot.com/13890044/diff/1/tests/example.py
File tests/example.py (right):

https://codereview.appspot.com/13890044/diff/1/tests/example.py#newcode18
tests/example.py:18: 
On 2013/09/25 13:49:48, bac wrote:
> An entry with multiple bundles would be nice for testing.

Yes, and you also gave me an idea for a test. I'll change this in another branch
if you agree.

[bac, 2013-09-25 20:29:35]

LGTM - I ran the tests and they took about 70 minutes and all passed.

[frankban, 2013-09-26 08:37:17]

*** Submitted:

Bundle deployment support functional test.

This branch includes a charm functional test
exercising the builtin server bundle support.

Reorganized the functional tests so that, at
least when the builtin server is used, the
deployed GUI service is reused by all tests.

Also fixed a pyJuju clean up error when the
builtin server is used to serve the GUI.

Added an helper function to retrieve, if
possible, the admin secret for the current
Juju environment, and a very simple WebSocket
client object used in tests.
Note that the websocket-client package used
by the client is now explicitly listed in
the requirements file, but it was already
installed before as a dependency of the
python juju client.

No QA required.

Tests:
run `make test JUJU_ENV="your-juju-environment"`
from the root of this branch.
Note that it is not currently possible to run
juju-test with a juju-core local provider:
with LXC juju-core requires the bootstrap and
destroy-environment commands to be run as root.
For this reason, I suggest to run the tests
using ec2; in my machine they take about 1:15h.

R=bac, gary.poster
CC=
https://codereview.appspot.com/13890044

[frankban, 2013-09-26 08:39:31]

Thank you both for the reviews!