Charm test improvements.

Charm test improvements.

This branch includes some improvements to
the charm testing infrastructure:

Strengthened the Juju implementation check by
introducing a new juju_version helper function.

Do not require the branch to be named "juju-gui"
anymore: the charm can be tested and deployed from
an arbitrary branch.

Updated documentation.

Added the .lbox.check file: now, before
proposing/submitting at least the lint check
and the unit tests must pass.

https://code.launchpad.net/~frankban/charms/precise/juju-gui/testing-improvements/+merge/168673

(do not edit description out of merge proposal)

[frankban, 2013-06-11 14:13:06]

Please take a look.

[frankban, 2013-06-11 14:14:43]

Please take a look.

[teknico, 2013-06-11 14:38:40]

LGTM, thanks for this. Nice tests as usual. A few trivials below.

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

https://codereview.appspot.com/9714047/diff/3001/tests/20-functional.test#new...
tests/20-functional.test:74: def wait_for(self, condition, error=None,
timeout=30):
Maybe a constant at top of file for all three default timeout values?

https://codereview.appspot.com/9714047/diff/3001/tests/helpers.py
File tests/helpers.py (right):

https://codereview.appspot.com/9714047/diff/3001/tests/helpers.py#newcode137
tests/helpers.py:137: except subprocess.CalledProcessError:
This means it's goJuju, right? A comment to that effect?

https://codereview.appspot.com/9714047/diff/3001/tests/helpers.py#newcode142
tests/helpers.py:142: toint = lambda num: 0 if num is None else int(num)
This looks like a mispelling of "taint". :-) "to_int" maybe?

https://codereview.appspot.com/9714047/diff/3001/tests/requirements.pip
File tests/requirements.pip (right):

https://codereview.appspot.com/9714047/diff/3001/tests/requirements.pip#newcode1
tests/requirements.pip:1: # Juju GUI test requirements.
Oh, you can put comments in here? Good to know. :-)

https://codereview.appspot.com/9714047/diff/3001/tests/test_helpers.py
File tests/test_helpers.py (right):

https://codereview.appspot.com/9714047/diff/3001/tests/test_helpers.py#newcod...
tests/test_helpers.py:209: def test_not_semantic(self, mock_check_output):
Maybe a more explicit "test_not_semantic_versioning"?

[frankban, 2013-06-11 14:58:05]

Please take a look.

[j.c.sackett, 2013-06-11 16:26:34]

On 2013/06/11 14:58:05, frankban wrote:
> Please take a look.

LGTM.

I have nothing to add, though I do agree wtih teknico's suggestion of using a
constant for the default timeout value in 20-functional.test

[frankban, 2013-06-12 09:33:53]

*** Submitted:

Charm test improvements.

This branch includes some improvements to
the charm testing infrastructure:

Strengthened the Juju implementation check by
introducing a new juju_version helper function.

Do not require the branch to be named "juju-gui"
anymore: the charm can be tested and deployed from
an arbitrary branch.

Updated documentation.

Added the .lbox.check file: now, before
proposing/submitting at least the lint check
and the unit tests must pass.

R=teknico, j.c.sackett
CC=
https://codereview.appspot.com/9714047

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

https://codereview.appspot.com/9714047/diff/3001/tests/20-functional.test#new...
tests/20-functional.test:74: def wait_for(self, condition, error=None,
timeout=30):
On 2013/06/11 14:38:40, teknico wrote:
> Maybe a constant at top of file for all three default timeout values?

As discussed on IRC, those values are only incidentally the same, so making a
module level constant doesn't give us a lot.

https://codereview.appspot.com/9714047/diff/3001/tests/helpers.py
File tests/helpers.py (right):

https://codereview.appspot.com/9714047/diff/3001/tests/helpers.py#newcode137
tests/helpers.py:137: except subprocess.CalledProcessError:
On 2013/06/11 14:38:40, teknico wrote:
> This means it's goJuju, right? A comment to that effect?

Done.

https://codereview.appspot.com/9714047/diff/3001/tests/helpers.py#newcode142
tests/helpers.py:142: toint = lambda num: 0 if num is None else int(num)
On 2013/06/11 14:38:40, teknico wrote:
> This looks like a mispelling of "taint". :-) "to_int" maybe?

Done.

https://codereview.appspot.com/9714047/diff/3001/tests/test_helpers.py
File tests/test_helpers.py (right):

https://codereview.appspot.com/9714047/diff/3001/tests/test_helpers.py#newcod...
tests/test_helpers.py:209: def test_not_semantic(self, mock_check_output):
On 2013/06/11 14:38:40, teknico wrote:
> Maybe a more explicit "test_not_semantic_versioning"?

Done.

[frankban, 2013-06-12 09:35:02]

Thank you both for the reviews!