Replace jitsu test with juju-test.

Replace jitsu test with juju-test.

This branch makes juju-test the default test runner
for both the unit tests and the charm functional tests.

The charm test suite now supports both pyJuju and juju-core.

The tests dependencies are now installed in a virtualenv that
is automatically created and updated by the first test
executable (00-setup). This way we don't require the user
to install all the packages, and, moreover, we make the tests
ready to be run in the charm testing infrastructure.

When the charm is deployed during tests, a temporary repository
is created where the charm is copied and then deployed.
This way we achieve two important goals:
- the developer is not forced to manually set up a local
Juju repository where to branch the Juju GUI;
- the charm can be deployed even if it contains a virtualenv, i.e.
a directory with absolute symlinks (that's because the .venv dir
is not copied in the temp repo).

The tests/helpers.py and tests/deploy.py files will be eventually
replaced by similar tools provided by the juju testing harness.

Updated the documentation where required.

Fixed unit tests exit code.

Common commands (test/lint/clean etc.) can now be easily run
using make.

Nice to have, not included in this branch:
  - collect deployment test logs;
  - reintroduce a way to test the charm specifying a
    different juju-gui-source;
  - do not require the branch to be named "juju-gui".

PS: sorry for the diff, well over 1k loc :-/

https://code.launchpad.net/~frankban/charms/precise/juju-gui/use-juju-test/+merge/168079

(do not edit description out of merge proposal)

[frankban, 2013-06-07 14:17:26]

Please take a look.

[gary.poster, 2013-06-07 16:33:34]

Wow, this looks great.  I'll give it a LGTM conditional on at least one other
person doing qa along with a review.

https://codereview.appspot.com/9874050/diff/1/HACKING.md
File HACKING.md (right):

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode19
HACKING.md:19: sudo apt-get install build-essential bzr libapt-pkg-dev
That's better :-)

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode28
HACKING.md:28: time of this writing it is not yet released.  To run it you must
first install
lol that this part didn't have to change.  Hopefully it won't stay that way.

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode36
HACKING.md:36: The current incarnation of our testing infrastructure requires
that the current
Suggest simplifying old stuff:

Our testing infrastructure requires the current directory name to match the
charm name, so you...

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode38
HACKING.md:38: directory named "juju-gui":
Agreed that removing this requirement would be nice to have for later.  Seems
like it would be pretty easy given the new approach, yeah?

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode42
HACKING.md:42: Before being able to run the suite, tests requirements need to be
installed
s/tests/test/

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode96
HACKING.md:96: If you have set up your environment to run you local development
charm,
s/run you/run your/

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode117
HACKING.md:117: created, the testing virtualenv.
Nice

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

https://codereview.appspot.com/9874050/diff/1/Makefile#newcode1
Makefile:1: # Juju GUI Makefile.
I was going to ask you to add a copyright notice, but I see that we need to do
that for the whole charm.  I'll make a card.

[teknico, 2013-06-07 17:05:59]

LGTM, very nice work, a couple trivials. Did not QA yet, will do on Monday.

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

https://codereview.appspot.com/9874050/diff/1/tests/20-functional.test#newcode14
tests/20-functional.test:14: ssh,
Swap the two lines above?

https://codereview.appspot.com/9874050/diff/1/tests/helpers.py
File tests/helpers.py (right):

https://codereview.appspot.com/9874050/diff/1/tests/helpers.py#newcode30
tests/helpers.py:30: be appeneded to the "baked in" arguments.
s/appeneded/appended/

https://codereview.appspot.com/9874050/diff/1/tests/helpers.py#newcode40
tests/helpers.py:40: """
Nice docstring. :-)

[bac, 2013-06-07 20:05:58]

LGTM but I cannot get the tests to run cleanly and I have some documentation
suggestions.

Trying to run with pyjuju I get the following failures

http://paste.ubuntu.com/5742947/

I figure it is landable if these failures can be addressed.

https://codereview.appspot.com/9874050/diff/1/HACKING.md
File HACKING.md (right):

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode67
HACKING.md:67: make unittest
Excellent

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode81
HACKING.md:81: In the command above, "myenv" is the juju environment, as it is
specified in
myenv was first introduced on line 60 but not explained until here, which is
confusing.

Also, it is not totally obvious which environment you should pick.  After some
investigation I see that it depends on the version of juju in your path.  Since
environment.yaml stanzas are different for pyjuju and juju-core you have to
carefully pick an environment that matches the juju in your path.  That should
be stated explicitly in this document.

For some makefile targets the environment is optional and the default is used. 
Can this not be applied to them all?

https://codereview.appspot.com/9874050/diff/1/tests/helpers.py
File tests/helpers.py (right):

https://codereview.appspot.com/9874050/diff/1/tests/helpers.py#newcode66
tests/helpers.py:66: return True
This check seems very fragile.  If juju-core ever grows a '--version' option
this breaks.

[frankban, 2013-06-10 09:18:53]

Please take a look.

https://codereview.appspot.com/9874050/diff/1/HACKING.md
File HACKING.md (right):

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode19
HACKING.md:19: sudo apt-get install build-essential bzr libapt-pkg-dev
On 2013/06/07 16:33:34, gary.poster wrote:
> That's better :-)

Indeed! However, I forgot two important
dependencies here: pip and virtualenv.
Fixed.

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode36
HACKING.md:36: The current incarnation of our testing infrastructure requires
that the current
On 2013/06/07 16:33:34, gary.poster wrote:
> Suggest simplifying old stuff:
> 
> Our testing infrastructure requires the current directory name to match the
> charm name, so you...

Done.

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode38
HACKING.md:38: directory named "juju-gui":
On 2013/06/07 16:33:35, gary.poster wrote:
> Agreed that removing this requirement would be nice to have for later.  Seems
> like it would be pretty easy given the new approach, yeah?

Correct, it should not be a problem.

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode42
HACKING.md:42: Before being able to run the suite, tests requirements need to be
installed
On 2013/06/07 16:33:35, gary.poster wrote:
> s/tests/test/

Done.

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode81
HACKING.md:81: In the command above, "myenv" is the juju environment, as it is
specified in
On 2013/06/07 20:05:58, bac wrote:
> myenv was first introduced on line 60 but not explained until here, which is
> confusing.

Fixed.

> Also, it is not totally obvious which environment you should pick.  After some
> investigation I see that it depends on the version of juju in your path. 
Since
> environment.yaml stanzas are different for pyjuju and juju-core you have to
> carefully pick an environment that matches the juju in your path.  

Not necessarily. Or are there problems preventing you to use 
the same environment? There are some cases where one env is not
compatible with different juju implementations (e.g. the local provider
is used, or juju-origin is specified). Other than that, AFAIK, it is
absolutely licit to boostrap, for example, the same ec2 environment from 
both pyJuju and juju-core. Am I missing something?

> That should
> be stated explicitly in this document.

The documentation already includes a link to the relevant part of
the Juju doc explaining what is and how to set up an environment.yaml.

> For some makefile targets the environment is optional and the default is used.

> Can this not be applied to them all?

This depends on juju-test explicitly requiring an environment name to 
be passed. For this reason the make targets wrapping juju-test have the
same requirement. We could parse the environment.yaml and find ourself the
name of the default env, but I believe that's out of scope for this branch,
and maybe also in general for the charm tests. I guess the ability to 
use the default env if not specified otherwise could quite easily 
be added to juju-test in the future.

https://codereview.appspot.com/9874050/diff/1/HACKING.md#newcode96
HACKING.md:96: If you have set up your environment to run you local development
charm,
On 2013/06/07 16:33:35, gary.poster wrote:
> s/run you/run your/

Done.

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

https://codereview.appspot.com/9874050/diff/1/Makefile#newcode1
Makefile:1: # Juju GUI Makefile.
On 2013/06/07 16:33:35, gary.poster wrote:
> I was going to ask you to add a copyright notice, but I see that we need to do
> that for the whole charm.  I'll make a card.

Right, thank you!

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

https://codereview.appspot.com/9874050/diff/1/tests/20-functional.test#newcode14
tests/20-functional.test:14: ssh,
On 2013/06/07 17:05:59, teknico wrote:
> Swap the two lines above?

Done.

https://codereview.appspot.com/9874050/diff/1/tests/helpers.py
File tests/helpers.py (right):

https://codereview.appspot.com/9874050/diff/1/tests/helpers.py#newcode30
tests/helpers.py:30: be appeneded to the "baked in" arguments.
On 2013/06/07 17:05:59, teknico wrote:
> s/appeneded/appended/

Done.

https://codereview.appspot.com/9874050/diff/1/tests/helpers.py#newcode66
tests/helpers.py:66: return True
On 2013/06/07 20:05:58, bac wrote:
> This check seems very fragile.  If juju-core ever grows a '--version' option
> this breaks.

You are right. A solution could be doing a version check.
I'd like to leave this for now.
Are you ok with me creating another card and working on this fix
right after this branch is merged?

[frankban, 2013-06-10 09:23:08]

On 2013/06/07 20:05:58, bac wrote:

> Trying to run with pyjuju I get the following failures
> 
> http://paste.ubuntu.com/5742947/

This is weird. When you are available, if you agree, 
I'd like to investigate this issue with you.

[teknico, 2013-06-10 11:09:30]

teknico wrote:
> Did not QA yet, will do on Monday.

QA completed successfully, on both pyjuju and juju-core. It took some more
tweaking to the juju-test command, thanks for the collaboration.

[frankban, 2013-06-10 15:28:08]

*** Submitted:

Replace jitsu test with juju-test.

This branch makes juju-test the default test runner
for both the unit tests and the charm functional tests.

The charm test suite now supports both pyJuju and juju-core.

The tests dependencies are now installed in a virtualenv that
is automatically created and updated by the first test
executable (00-setup). This way we don't require the user
to install all the packages, and, moreover, we make the tests
ready to be run in the charm testing infrastructure.

When the charm is deployed during tests, a temporary repository
is created where the charm is copied and then deployed.
This way we achieve two important goals:
- the developer is not forced to manually set up a local
Juju repository where to branch the Juju GUI;
- the charm can be deployed even if it contains a virtualenv, i.e.
a directory with absolute symlinks (that's because the .venv dir
is not copied in the temp repo).

The tests/helpers.py and tests/deploy.py files will be eventually
replaced by similar tools provided by the juju testing harness.

Updated the documentation where required.

Fixed unit tests exit code.

Common commands (test/lint/clean etc.) can now be easily run
using make.

Nice to have, not included in this branch:
  - collect deployment test logs;
  - reintroduce a way to test the charm specifying a
    different juju-gui-source;
  - do not require the branch to be named "juju-gui".

PS: sorry for the diff, well over 1k loc :-/

R=
CC=
https://codereview.appspot.com/9874050

[frankban, 2013-06-10 15:36:49]

Hi Gary, Nicola and Brad,
thanks for reviewing and QAing this branch!