Merge lp://staging/~frankban/juju-quickstart/add-tox into lp://staging/juju-quickstart

Reviewers: mp+246761_code.launchpad.net,

Message:
Please take a look.

Description:
Introduce tox for testing multiple sets of deps.

Quickstart must be run on several platforms/Ubuntu series,
each one with its own versions of the packages that
Quickstart depends on. The tox utility is used for both:
- handling those kind of separate scenarios;
- creating the development virtual environments.
The latter was previously done using virtualenv directly
in the Makefile. The new infrastructure should be
more flexible.

Update the relevant parts of the Makefile, also handling
the automatic installation of system dependencies.

Implement the ability to calculate the requirements
for the PyPI package automatically by parsing the
tox.ini file. I am not really sure about the method I
used (take a look at the setup.py file), but the
alternative is to manually update those and violate DRY.
I think we could give it a try and fall back to the
manual approach if required.

Also implement a correct behavior for
"python setup.py test", which now runs all the tests.

To test and QA this branch, run `make clean` and then
follow what described in the HACKING.rst file, in the
following paragraphs:
- Creating a development environment
- Testing and debugging the application
- Requirements
- Updating application and test dependencies

Thank you!

https://code.launchpad.net/~frankban/juju-quickstart/add-tox/+merge/246761

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/189580044/

Affected files (+409, -171 lines):
   M .bzrignore
   M HACKING.rst
   M MANIFEST.in
   M Makefile
   A [revision details]
   D requirements.pip
   A setup.cfg
   M setup.py
   D test-requirements.pip
   A tox.ini

LGTM. Did not try to QA yet. Tox may be very interesting for lots of our
projects.

https://codereview.appspot.com/189580044/diff/1/HACKING.rst
File HACKING.rst (right):

https://codereview.appspot.com/189580044/diff/1/HACKING.rst#newcode356
HACKING.rst:356: parsing ``tox.ini``, dependencies must be listed with
the following rules:
Could you try to re-word that sentence? I'm not sure what you're saying.

https://codereview.appspot.com/189580044/diff/1/HACKING.rst#newcode358
HACKING.rst:358: - each scenario must at least include a
"{[testenv]deps}" line in its deps:
Change the : to ; or ,

https://codereview.appspot.com/189580044/diff/1/MANIFEST.in
File MANIFEST.in (right):

https://codereview.appspot.com/189580044/diff/1/MANIFEST.in#newcode24
MANIFEST.in:24: # tox.ini file, removing it from this list will break
the source distribution
Again, this could be worded better.

https://codereview.appspot.com/189580044/diff/1/setup.py
File setup.py (right):

https://codereview.appspot.com/189580044/diff/1/setup.py#newcode43
setup.py:43: """Return Juju Quickstart package data by walking through
the project."""
A better explanation of what "package data" is would be nice.

https://codereview.appspot.com/189580044/diff/1/setup.py#newcode74
setup.py:74: for requirement in deps:
Maybe add

requirement = requirement.strip()

https://codereview.appspot.com/189580044/diff/1/tox.ini
File tox.ini (right):

https://codereview.appspot.com/189580044/diff/1/tox.ini#newcode42
tox.ini:42: basepython = python2.7
Is it required to repeat here?

https://codereview.appspot.com/189580044/

Please take a look.

https://codereview.appspot.com/189580044/diff/1/HACKING.rst
File HACKING.rst (right):

https://codereview.appspot.com/189580044/diff/1/HACKING.rst#newcode356
HACKING.rst:356: parsing ``tox.ini``, dependencies must be listed with
the following rules:
On 2015/01/22 16:15:27, bac wrote:
> Could you try to re-word that sentence? I'm not sure what you're
saying.

Done.

https://codereview.appspot.com/189580044/diff/1/HACKING.rst#newcode358
HACKING.rst:358: - each scenario must at least include a
"{[testenv]deps}" line in its deps:
On 2015/01/22 16:15:27, bac wrote:
> Change the : to ; or ,

Done.

https://codereview.appspot.com/189580044/diff/1/MANIFEST.in
File MANIFEST.in (right):

https://codereview.appspot.com/189580044/diff/1/MANIFEST.in#newcode24
MANIFEST.in:24: # tox.ini file, removing it from this list will break
the source distribution
On 2015/01/22 16:15:27, bac wrote:
> Again, this could be worded better.

Done.

https://codereview.appspot.com/189580044/diff/1/setup.py
File setup.py (right):

https://codereview.appspot.com/189580044/diff/1/setup.py#newcode43
setup.py:43: """Return Juju Quickstart package data by walking through
the project."""
On 2015/01/22 16:15:27, bac wrote:
> A better explanation of what "package data" is would be nice.

Done.

https://codereview.appspot.com/189580044/diff/1/setup.py#newcode74
setup.py:74: for requirement in deps:
On 2015/01/22 16:15:27, bac wrote:
> Maybe add

> requirement = requirement.strip()

Done.

https://codereview.appspot.com/189580044/diff/1/tox.ini
File tox.ini (right):

https://codereview.appspot.com/189580044/diff/1/tox.ini#newcode42
tox.ini:42: basepython = python2.7
On 2015/01/22 16:15:27, bac wrote:
> Is it required to repeat here?

No it's not. Good catch!

https://codereview.appspot.com/189580044/

Code is LGTM with the one suggestion for the Makefile.

QA OK on trusty

On OS X, the 'make sysdeps' suggests you use brew to install python-dev
python-setuptools and python-pip. Those package names are not the same
in brew and pip is installed via 'brew install python'. I'm not sure if
the equivalent of python-dev gets installed but 'make check' ran fine.

Perhaps you can adjust the message to be less misleading.

https://codereview.appspot.com/189580044/diff/20001/Makefile
File Makefile (right):

https://codereview.appspot.com/189580044/diff/20001/Makefile#newcode44
Makefile:44:
It is a bit odd to touch the canary on an non-Debian system when the
sysdeps were not installed.

https://codereview.appspot.com/189580044/

LGTM Thanks for all that this should help simplify testing quite a bit

https://codereview.appspot.com/189580044/

Thanks for the reviews!

https://codereview.appspot.com/189580044/diff/20001/Makefile
File Makefile (right):

https://codereview.appspot.com/189580044/diff/20001/Makefile#newcode44
Makefile:44:
On 2015/01/30 15:38:52, bac wrote:
> It is a bit odd to touch the canary on an non-Debian system when the
sysdeps
> were not installed.

I modified the message to be more clear. We need to touch the file so
that OSX users having all the required sysdeps installed can still
proceed without seeing the message each time.

https://codereview.appspot.com/189580044/

*** Submitted:

Introduce tox for testing multiple sets of deps.

Quickstart must be run on several platforms/Ubuntu series,
each one with its own versions of the packages that
Quickstart depends on. The tox utility is used for both:
- handling those kind of separate scenarios;
- creating the development virtual environments.
The latter was previously done using virtualenv directly
in the Makefile. The new infrastructure should be
more flexible.

Update the relevant parts of the Makefile, also handling
the automatic installation of system dependencies.

Implement the ability to calculate the requirements
for the PyPI package automatically by parsing the
tox.ini file. I am not really sure about the method I
used (take a look at the setup.py file), but the
alternative is to manually update those and violate DRY.
I think we could give it a try and fall back to the
manual approach if required.

Also implement a correct behavior for
"python setup.py test", which now runs all the tests.

To test and QA this branch, run `make clean` and then
follow what described in the HACKING.rst file, in the
following paragraphs:
- Creating a development environment
- Testing and debugging the application
- Requirements
- Updating application and test dependencies

Thank you!

R=bac, benjamin.saller, jeff.pihach
CC=
https://codereview.appspot.com/189580044

https://codereview.appspot.com/189580044/