Use a random service name for tests.

Use a random service name for tests.

This branch also changes the retry decorator to reraise the first exception it
receives instead of the last.

https://code.launchpad.net/~benji/charms/precise/juju-gui/test-failure/+merge/196775

(do not edit description out of merge proposal)

[benji, 2013-11-26 20:21:31]

Here are some comments for the reviewer.

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

https://codereview.appspot.com/33230043/diff/40001/tests/20-functional.test#n...
tests/20-functional.test:148: unit_info, self.service_name = juju_deploy(*args,
**kwargs)
Now that juju_deploy returns the service name, we have to capture it.

https://codereview.appspot.com/33230043/diff/40001/tests/deploy.py
File tests/deploy.py (right):

https://codereview.appspot.com/33230043/diff/40001/tests/deploy.py#newcode21
tests/deploy.py:21: 
Since __future__ imports are more like pragmas, I didn't see it when it wasn't
set apart, so I set it apart.

https://codereview.appspot.com/33230043/diff/40001/tests/deploy.py#newcode80
tests/deploy.py:80: service_name = make_service_name()
This is a straight-forward transformation to add random service names.

https://codereview.appspot.com/33230043/diff/40001/tests/test_deploy.py
File tests/test_deploy.py (right):

https://codereview.appspot.com/33230043/diff/40001/tests/test_deploy.py#newco...
tests/test_deploy.py:119: @mock.patch('deploy.setup_repository',
mock.Mock(return_value=REPO_PATH))
The new test needs to override the setup_repository mock, so I moved it out
here. That also meant moving cls.repo up to module level, so I made it more
constant-y.

https://codereview.appspot.com/33230043/diff/40001/tests/test_deploy.py#newco...
tests/test_deploy.py:153: self.assertEqual(deploy_call[0][0], 'deploy')
Previously there was no assertion that "deploy_call" was really a deploy.

https://codereview.appspot.com/33230043/diff/40001/tests/test_deploy.py#newco...
tests/test_deploy.py:162: self.assertIn(self.local_charm, command)
Every test in this class broke when the random service name was appended to the
juju command because they were all overspecified (verifying every element of the
command, even if it was inconsequential to the topic of the test).

I rewrote all the tests to assert only the thing the test is really about,
resulting in shorter, more focused tests.

https://codereview.appspot.com/33230043/diff/40001/tests/test_deploy.py#newco...
tests/test_deploy.py:198: def test_service_name(self):
This is the only new test in this file.

[bac, 2013-11-26 20:35:46]

nice.  LGTM.  splendid.

https://codereview.appspot.com/33230043/diff/40001/tests/20-functional.test
File tests/20-functional.test (left):

https://codereview.appspot.com/33230043/diff/40001/tests/20-functional.test#o...
tests/20-functional.test:169: juju_destroy_service(self.charm)
That clearly looked silly before.

https://codereview.appspot.com/33230043/diff/40001/tests/test_deploy.py
File tests/test_deploy.py (right):

https://codereview.appspot.com/33230043/diff/40001/tests/test_deploy.py#newco...
tests/test_deploy.py:35: def contains(needle, haystack):
docstringize

https://codereview.appspot.com/33230043/diff/40001/tests/test_helpers.py
File tests/test_helpers.py (right):

https://codereview.appspot.com/33230043/diff/40001/tests/test_helpers.py#newc...
tests/test_helpers.py:327: # reraised if all retries fail, even if a different
error is raised
re-raised?

https://codereview.appspot.com/33230043/diff/40001/tests/test_helpers.py#newc...
tests/test_helpers.py:336: # raised it reraised after all the retries have been
exhausted.
re-raised?
(i really don't know but without a hyphen it looks funny.)

[benji, 2013-11-26 20:42:40]

Docstringitization and hypheneting complete.

https://codereview.appspot.com/33230043/diff/40001/tests/test_deploy.py
File tests/test_deploy.py (right):

https://codereview.appspot.com/33230043/diff/40001/tests/test_deploy.py#newco...
tests/test_deploy.py:35: def contains(needle, haystack):
On 2013/11/26 20:35:46, bac wrote:
> docstringize

Ineed!  Done.

https://codereview.appspot.com/33230043/diff/40001/tests/test_helpers.py
File tests/test_helpers.py (right):

https://codereview.appspot.com/33230043/diff/40001/tests/test_helpers.py#newc...
tests/test_helpers.py:327: # reraised if all retries fail, even if a different
error is raised
On 2013/11/26 20:35:46, bac wrote:
> re-raised?

Changed (see below).

https://codereview.appspot.com/33230043/diff/40001/tests/test_helpers.py#newc...
tests/test_helpers.py:336: # raised it reraised after all the retries have been
exhausted.
On 2013/11/26 20:35:46, bac wrote:
> re-raised?
> (i really don't know but without a hyphen it looks funny.)

I'm not so good with the spelling, so your intuition is better than mine. 
Changed.

[benji, 2013-11-26 20:44:06]

Please take a look.

[benji, 2013-11-26 20:47:49]

*** Submitted:

Use a random service name for tests.

This branch also changes the retry decorator to reraise the first exception it
receives instead of the last.

R=bac
CC=
https://codereview.appspot.com/33230043

[benji, 2014-01-09 13:53:20]

Please take a look.