Merge lp:~frankban/juju-quickstart/add-tox into lp:juju-quickstart
- add-tox
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 116 |
Proposed branch: | lp:~frankban/juju-quickstart/add-tox |
Merge into: | lp:juju-quickstart |
Diff against target: |
765 lines (+414/-171) 9 files modified
.bzrignore (+3/-1) HACKING.rst (+85/-45) MANIFEST.in (+5/-4) Makefile (+76/-45) requirements.pip (+0/-31) setup.cfg (+20/-0) setup.py (+110/-22) test-requirements.pip (+0/-23) tox.ini (+115/-0) |
To merge this branch: | bzr merge lp:~frankban/juju-quickstart/add-tox |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email: mp+246761@code.launchpad.net |
Commit message
Description of the change
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!
Francesco Banconi (frankban) wrote : | # |
Brad Crittenden (bac) wrote : | # |
LGTM. Did not try to QA yet. Tox may be very interesting for lots of our
projects.
https:/
File HACKING.rst (right):
https:/
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:/
HACKING.rst:358: - each scenario must at least include a
"{[testenv]deps}" line in its deps:
Change the : to ; or ,
https:/
File MANIFEST.in (right):
https:/
MANIFEST.in:24: # tox.ini file, removing it from this list will break
the source distribution
Again, this could be worded better.
https:/
File setup.py (right):
https:/
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:/
setup.py:74: for requirement in deps:
Maybe add
requirement = requirement.strip()
https:/
File tox.ini (right):
https:/
tox.ini:42: basepython = python2.7
Is it required to repeat here?
- 130. By Francesco Banconi
-
Changes as per review.
Francesco Banconi (frankban) wrote : | # |
Please take a look.
https:/
File HACKING.rst (right):
https:/
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:/
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:/
File MANIFEST.in (right):
https:/
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:/
File setup.py (right):
https:/
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:/
setup.py:74: for requirement in deps:
On 2015/01/22 16:15:27, bac wrote:
> Maybe add
> requirement = requirement.strip()
Done.
https:/
File tox.ini (right):
https:/
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!
Brad Crittenden (bac) wrote : | # |
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:/
File Makefile (right):
https:/
Makefile:44:
It is a bit odd to touch the canary on an non-Debian system when the
sysdeps were not installed.
Jeff Pihach (hatch) wrote : | # |
LGTM Thanks for all that this should help simplify testing quite a bit
Francesco Banconi (frankban) wrote : | # |
Thanks for the reviews!
https:/
File Makefile (right):
https:/
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.
- 131. By Francesco Banconi
-
Improve sysdeps message in Makefile.
Francesco Banconi (frankban) wrote : | # |
*** 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:/
Preview Diff
1 | === modified file '.bzrignore' |
2 | --- .bzrignore 2013-11-06 10:16:42 +0000 |
3 | +++ .bzrignore 2015-01-30 16:35:06 +0000 |
4 | @@ -1,8 +1,10 @@ |
5 | .coverage |
6 | .DS_Store |
7 | .emacs* |
8 | -.venv |
9 | +.sysdeps-installed |
10 | +.tox |
11 | build |
12 | +devenv |
13 | dist |
14 | juju_quickstart.egg-info |
15 | MANIFEST |
16 | |
17 | === modified file 'HACKING.rst' |
18 | --- HACKING.rst 2015-01-13 12:03:15 +0000 |
19 | +++ HACKING.rst 2015-01-30 16:35:06 +0000 |
20 | @@ -11,35 +11,68 @@ |
21 | Creating a development environment |
22 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
23 | |
24 | -The development environment is created in a virtualenv. The environment |
25 | -creation requires the *make*, *pip* and *virtualenv* programs to be installed. |
26 | -To do that, run the following:: |
27 | - |
28 | - $ make sysdeps |
29 | - |
30 | -At this point, from the root of this branch, run the command:: |
31 | - |
32 | - $ make |
33 | - |
34 | -This command will create a ``.venv`` directory in the branch root, ignored |
35 | -by DVCSes, containing the development virtual environment with all the |
36 | -dependencies. |
37 | +The development environment is created using the |
38 | +`tox <https://tox.readthedocs.org/en/latest/>`_ utility. To create the |
39 | +environment, just run ``make``. The first time it is run, ``make`` also |
40 | +installs the required system dependencies. For this reason sudo privileges can |
41 | +be asked in the process. When the command completes the development environment |
42 | +is ready and placed in the ``devenv`` directory inside the project's root. |
43 | + |
44 | +At this point it is possible to start contributing to the project. The steps |
45 | +for testing the application and running Juju Quickstart in development mode are |
46 | +described below. Thanks for contributing! |
47 | |
48 | Testing and debugging the application |
49 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
50 | |
51 | -Run the tests:: |
52 | +With the development environment correctly set up (after running ``make``), it |
53 | +is easy to run the tests with the following command:: |
54 | |
55 | $ make test |
56 | |
57 | -Run the tests and lint/pep8 checks:: |
58 | +It is also possible to run a subset of the test suite by using nose directly, |
59 | +as it is already installed in the development environment, e.g.:: |
60 | + |
61 | + $ devenv/bin/nosetests quickstart/tests/test_app.py:TestWatch |
62 | + |
63 | +To run the development version of Juju Quickstart while making changes, or |
64 | +while reviewing a branch, use the following command:: |
65 | + |
66 | + $ devenv/bin/juju-quickstart |
67 | + |
68 | +The command above receives all usual Juju Quickstart arguments, so, for |
69 | +instance, to start the interactive session:: |
70 | + |
71 | + $ devenv/bin/juju-quickstart -i |
72 | + |
73 | +To run the Python linter and pep8 checks, use the following command:: |
74 | + |
75 | + $ make lint |
76 | + |
77 | +When a feature branch is ready, before proposing it, check that the code tests |
78 | +pass when Juju Quickstart is installed using the dependencies present in all |
79 | +supported platforms and Ubuntu series. To do that, run the following:: |
80 | |
81 | $ make check |
82 | |
83 | -Display help about all the available make targets, including instructions on |
84 | -setting up and running the application in the development environment:: |
85 | - |
86 | - $ make help |
87 | +The above leverages tox ability to create multiple virtual environments, each |
88 | +one with a specific set of requirements. The ``make check`` command is also |
89 | +run automatically when proposing a branch with ``lbox propose``. |
90 | + |
91 | +As a final note, having the development environment ready, it is also possible |
92 | +to use the ``tox`` command directly on the project. For instance, to run the |
93 | +tests only on the ``vivid`` scenario, use the following:: |
94 | + |
95 | + $ tox -e vivid |
96 | + |
97 | +Run ``make help`` for more information about all the available make targets. |
98 | + |
99 | +Requirements |
100 | +~~~~~~~~~~~~ |
101 | + |
102 | +The Python requirements for Juju Quickstart are dynamically generated by |
103 | +parsing the ``tox.ini`` file. To check the requirements list, run |
104 | +``make requirements``. |
105 | |
106 | Installing the application |
107 | ~~~~~~~~~~~~~~~~~~~~~~~~~~ |
108 | @@ -67,12 +100,6 @@ |
109 | |
110 | $ juju quickstart -e local |
111 | |
112 | -If you have not installed the application using ``sudo make install``, as |
113 | -described above, you can run it locally using the virtualenv's Python |
114 | -installation:: |
115 | - |
116 | - $ .venv/bin/python juju-quickstart --help |
117 | - |
118 | Project structure |
119 | ~~~~~~~~~~~~~~~~~ |
120 | |
121 | @@ -203,7 +230,8 @@ |
122 | packaging branch root. To print the version of the current Quickstart, from the |
123 | juju-quickstart branch root, run the following:: |
124 | |
125 | - $ .venv/bin/python juju-quickstart --version |
126 | + $ make |
127 | + $ devenv/bin/juju-quickstart --version |
128 | |
129 | If the ``debian/changelog`` file is outdated, install the ``devscripts`` |
130 | package and use ``dch`` to update the changelog, e.g.:: |
131 | @@ -247,8 +275,8 @@ |
132 | Creating a Homebrew release |
133 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
134 | |
135 | -The brew formula fetches its source from PyPI, so it must be done after the PyPI |
136 | -release. |
137 | +The brew formula fetches its source from PyPI, so it must be done after the |
138 | +PyPI release. |
139 | |
140 | 1. Start with a fresh brew:: |
141 | |
142 | @@ -303,8 +331,8 @@ |
143 | #. Go to https://github.com/juju/homebrew to create a pull request. |
144 | #. Copy the debian/changelog from the lp:juju-quickstart/packaging as the pull |
145 | request comment. Keep the name simple, e.g. 'juju-quickstart 1.4.0'. |
146 | -#. Watch the pull request and ensure it passes Jenkins. If changes must be made, |
147 | - rebase the branch and squash commits before pushing. |
148 | +#. Watch the pull request and ensure it passes Jenkins. If changes must be |
149 | + made, rebase the branch and squash commits before pushing. |
150 | #. If the branch makes it through CI without errors it will be accepted and |
151 | merged without human intervention. A recent branch took about two hours |
152 | from the time the pull request was made. |
153 | @@ -323,17 +351,19 @@ |
154 | Updating application and test dependencies |
155 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
156 | |
157 | -Test dependencies are listed in the ``test-requirements.pip`` file in the |
158 | -branch root, application ones in the ``requirements.pip`` file. The former |
159 | -includes the latter, so any updates to the application requirements will also |
160 | -update the test dependencies and therefore the testing virtual environment. |
161 | -Note that, since the source requirements are dynamically generated parsing |
162 | -``requirements.pip``, that file must only include ``PACKAGE==VERSION`` formatted |
163 | -dependencies, and not other pip specific requirement specifications. |
164 | - |
165 | -Also ensure, before updating the application dependencies, that those packages |
166 | -are available in the main Ubuntu repositories for the series we support (from |
167 | -precise to vivid), or in the `Juju Quickstart Beta PPA |
168 | +Test and application dependencies are listed in the ``tox.ini`` file in the |
169 | +branch root. Note that, since the source requirements are dynamically generated |
170 | +parsing ``tox.ini``, when updating the list of dependencies in ``tox.ini``, |
171 | +these rules must be followed: |
172 | + |
173 | +- each scenario must at least include a "{[testenv]deps}" line in its deps, |
174 | + this way tests can be run correctly inside the virtualenv; |
175 | +- keep each requirement in deps in its own line; |
176 | +- always use specific revisions for environments (i.e. always use "=="). |
177 | + |
178 | +Also ensure, before updating the application dependencies, that the package |
179 | +versions reflect the ones available for each supported platform and in the |
180 | +`Juju Quickstart Beta PPA |
181 | <https://launchpad.net/~juju-gui/+archive/quickstart-beta/+packages>`_. |
182 | |
183 | Please also keep up to date the possible values for the environments.yaml |
184 | @@ -341,6 +371,16 @@ |
185 | set of series supported by the Juju GUI charm |
186 | (see ``quickstart.settings.JUJU_GUI_SUPPORTED_SERIES``). |
187 | |
188 | +When introducing a new supported platform, do the following: |
189 | + |
190 | +- add a new section in the ``tox.ini`` file with a sensible name and list the |
191 | + dependencies that we expect to be found on that platform/series; |
192 | +- add the platform name to the envlist option (in the tox section of the file); |
193 | +- if required, update ``quickstart.settings.JUJU_GUI_SUPPORTED_SERIES``; |
194 | +- run ``make requirements`` and ensure that the dynamically generated list of |
195 | + requirements make sense; |
196 | +- run ``make check`` and ensure all the tests pass. |
197 | + |
198 | Debugging bundle support |
199 | ~~~~~~~~~~~~~~~~~~~~~~~~ |
200 | |
201 | @@ -356,10 +396,10 @@ |
202 | information about what is going on. The GUI builtin server exposes some bundle |
203 | information in two places: |
204 | |
205 | -- ``https://<juju-gui-url>/gui-server-info`` displays in JSON format the current |
206 | - status of all scheduled/started/completed bundle deployments; |
207 | -- ``/var/log/upstart/guiserver.log`` is the builtin server log file, which includes |
208 | - logs output from the juju-deployer library. |
209 | +- ``https://<juju-gui-url>/gui-server-info`` displays in JSON format the |
210 | + current status of all scheduled/started/completed bundle deployments; |
211 | +- ``/var/log/upstart/guiserver.log`` is the builtin server log file, which |
212 | + includes logs output from the juju-deployer library. |
213 | |
214 | Moreover, setting ``builtin-server-logging=debug`` gives more debugging |
215 | information, e.g. it prints to the log the contents of the WebSocket messages |
216 | |
217 | === modified file 'MANIFEST.in' |
218 | --- MANIFEST.in 2014-03-13 11:56:58 +0000 |
219 | +++ MANIFEST.in 2015-01-30 16:35:06 +0000 |
220 | @@ -19,8 +19,9 @@ |
221 | include MANIFEST.in |
222 | include Makefile |
223 | include README.rst |
224 | +include setup.cfg |
225 | # Note: since the source requirements are dynamically generated parsing the |
226 | -# requirements.pip file, removing it from this list will break the source |
227 | -# distribution and therefore the Debian package builds. Do not do that. |
228 | -include requirements.pip |
229 | -include test-requirements.pip |
230 | +# tox.ini file, removing the "include tox.ini" entry below from this list would |
231 | +# break the source distribution and therefore the Debian package builds. |
232 | +# Do not do that. |
233 | +include tox.ini |
234 | |
235 | === modified file 'Makefile' |
236 | --- Makefile 2014-09-05 14:54:31 +0000 |
237 | +++ Makefile 2015-01-30 16:35:06 +0000 |
238 | @@ -15,75 +15,106 @@ |
239 | # along with this program. If not, see <http://www.gnu.org/licenses/>. |
240 | |
241 | PYTHON = python |
242 | -SYSDEPS = build-essential python-dev python-pip python-virtualenv |
243 | - |
244 | -VENV = .venv |
245 | -VENV_ACTIVATE = $(VENV)/bin/activate |
246 | - |
247 | - |
248 | -$(VENV_ACTIVATE): test-requirements.pip requirements.pip |
249 | - virtualenv --distribute -p $(PYTHON) $(VENV) |
250 | - $(VENV)/bin/pip install -r test-requirements.pip || \ |
251 | - (touch test-requirements.pip; exit 1) |
252 | - @touch $(VENV_ACTIVATE) |
253 | - |
254 | -all: setup |
255 | - |
256 | -check: test lint |
257 | - |
258 | +APT_SYSDEPS = python-dev python-pip python-setuptools |
259 | +# Since the python-tox package in Ubuntu uses Python 3, use pip to install tox |
260 | +# instead. This also works on OSX where tox is not present in Homebrew. |
261 | +PIP_SYSDEPS = tox |
262 | + |
263 | +SYSDEPS_INSTALLED = .sysdeps-installed |
264 | +DEVENV = devenv |
265 | +QUICKSTART = $(DEVENV)/bin/juju-quickstart |
266 | + |
267 | +.DEFAULT_GOAL := setup |
268 | + |
269 | + |
270 | +$(QUICKSTART): juju-quickstart setup.py tox.ini |
271 | + @tox -e devenv |
272 | + |
273 | +$(SYSDEPS_INSTALLED): Makefile |
274 | +ifeq ($(shell command -v apt-get > /dev/null; echo $$?),0) |
275 | + sudo apt-get install --yes $(APT_SYSDEPS) |
276 | +else |
277 | + @echo 'System dependencies can only be installed automatically on' |
278 | + @echo 'systems with "apt-get". On OSX you can manually use Homebrew' |
279 | + @echo 'if there are missing dependencies corresponding to the following' |
280 | + @echo 'Debian packages:' |
281 | + @echo '$(APT_SYSDEPS).' |
282 | +endif |
283 | + sudo pip2 install $(PIP_SYSDEPS) |
284 | + touch $(SYSDEPS_INSTALLED) |
285 | + |
286 | + |
287 | +.PHONY: check |
288 | +check: setup |
289 | + @tox -e lint |
290 | + @tox |
291 | + |
292 | +.PHONY: clean |
293 | clean: |
294 | $(PYTHON) setup.py clean |
295 | - rm -rfv build/ dist/ juju_quickstart.egg-info MANIFEST |
296 | - rm -rfv $(VENV) |
297 | + # Remove the development environments. |
298 | + rm -rfv $(DEVENV) .tox/ |
299 | + # Remove distribution artifacts. |
300 | + rm -rfv *.egg build/ dist/ juju_quickstart.egg-info/ MANIFEST |
301 | + # Remove tests artifacts. |
302 | + rm -fv .coverage |
303 | + # Remove the sysdeps canary file. |
304 | + rm -fv $(SYSDEPS_INSTALLED) |
305 | + # Remove Python compiled bytecode. |
306 | find . -name '*.pyc' -delete |
307 | find . -name '__pycache__' -type d -delete |
308 | - |
309 | -setup: $(VENV_ACTIVATE) |
310 | - |
311 | + # Remove the virtualenv used in previous configurations. |
312 | + rm -rfv .venv/ |
313 | + |
314 | +.PHONY: help |
315 | help: |
316 | @echo -e 'Juju Quickstart - list of make targets:\n' |
317 | - @echo 'make sysdeps - Install the development environment system packages.' |
318 | @echo 'make - Set up the development and testing environment.' |
319 | @echo 'make test - Run tests.' |
320 | @echo 'make lint - Run linter and pep8.' |
321 | - @echo 'make check - Run tests, linter and pep8.' |
322 | + @echo 'make check - Run all the tests and lint in all supported scenarios.' |
323 | @echo 'make source - Create source package.' |
324 | @echo 'make install - Install on local system.' |
325 | - @echo 'make run - Run the application in the development environment.\n' |
326 | - @echo ' If "juju switch" has been used to set a default environment, that' |
327 | - @echo ' environment will be used. It is possible to override the default' |
328 | - @echo ' Juju environment by setting the JUJU_ENV environment variable,' |
329 | - @echo ' e.g.: "make run JUJU_ENV=ec2".' |
330 | - @echo 'make debug - Same as the "run" target but with the --debug flag.\n' |
331 | - @echo 'make clean - Get rid of bytecode files, build and dist dirs, venv.' |
332 | + @echo 'make clean - Get rid of bytecode files, build and dist dirs, venvs.' |
333 | @echo 'make release - Register and upload a release on PyPI.' |
334 | + @echo 'make requirements - Print out the application requirements.' |
335 | + @echo -e '\nAfter creating the development environment with "make", it is' |
336 | + @echo 'also possible to do the following:' |
337 | + @echo '- run the development version of Juju Quickstart by invoking' |
338 | + @echo ' "$(QUICKSTART)";' |
339 | + @echo '- run a specific subset of the test suite, e.g. with' |
340 | + @echo ' "$(DEVENV)/bin/nosetests quickstart/tests/test_app.py:TestWatch";' |
341 | + @echo '- use tox as usual on this project;' |
342 | + @echo ' see https://tox.readthedocs.org/en/latest/' |
343 | |
344 | +.PHONY: install |
345 | install: |
346 | $(PYTHON) setup.py install |
347 | rm -rfv ./build ./dist ./juju_quickstart.egg-info |
348 | |
349 | +.PHONY: lint |
350 | lint: setup |
351 | - @$(VENV)/bin/flake8 --show-source --exclude=$(VENV) ./quickstart |
352 | + @$(DEVENV)/bin/flake8 --show-source quickstart |
353 | |
354 | +.PHONY: release |
355 | release: check |
356 | $(PYTHON) setup.py register sdist upload |
357 | |
358 | -run: setup |
359 | - $(VENV)/bin/python ./juju-quickstart |
360 | - |
361 | -debug: setup |
362 | - $(VENV)/bin/python ./juju-quickstart --debug |
363 | - |
364 | +.PHONY: requirements |
365 | +requirements: |
366 | + $(PYTHON) setup.py requirements |
367 | + |
368 | +.PHONY: setup |
369 | +setup: $(SYSDEPS_INSTALLED) $(QUICKSTART) |
370 | + |
371 | +.PHONY: source |
372 | source: |
373 | $(PYTHON) setup.py sdist |
374 | |
375 | -sysdeps: |
376 | - sudo apt-get install --yes $(SYSDEPS) |
377 | +.PHONY: sysdeps |
378 | +sysdeps: $(SYSDEPS_INSTALLED) |
379 | |
380 | +.PHONY: test |
381 | test: setup |
382 | - @$(VENV)/bin/nosetests -s --verbosity=2 \ |
383 | - --with-coverage --cover-package=quickstart quickstart |
384 | - @rm .coverage |
385 | - |
386 | -.PHONY: all clean check debug help install lint release run setup source \ |
387 | - sysdeps test |
388 | + @$(DEVENV)/bin/nosetests \ |
389 | + --with-coverage --cover-erase --cover-package quickstart |
390 | |
391 | === removed file 'requirements.pip' |
392 | --- requirements.pip 2014-09-05 14:54:31 +0000 |
393 | +++ requirements.pip 1970-01-01 00:00:00 +0000 |
394 | @@ -1,31 +0,0 @@ |
395 | -# Juju Quickstart application requirements. |
396 | - |
397 | -# This file is part of the Juju Quickstart Plugin, which lets users set up a |
398 | -# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
399 | -# Copyright (C) 2013 Canonical Ltd. |
400 | -# |
401 | -# This program is free software: you can redistribute it and/or modify it under |
402 | -# the terms of the GNU Affero General Public License version 3, as published by |
403 | -# the Free Software Foundation. |
404 | -# |
405 | -# This program is distributed in the hope that it will be useful, but WITHOUT |
406 | -# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
407 | -# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
408 | -# Affero General Public License for more details. |
409 | -# |
410 | -# You should have received a copy of the GNU Affero General Public License |
411 | -# along with this program. If not, see <http://www.gnu.org/licenses/>. |
412 | - |
413 | -# Note: since the source requirements are dynamically generated parsing the |
414 | -# requirements.pip file, the list below must only include PACKAGE==VERSION |
415 | -# formatted dependencies. Do not include other pip specific requirement |
416 | -# specifications (e.g. -e ... or -r ...). |
417 | - |
418 | -# XXX: BradCrittenden 2014-05-23: jujuclient 0.17.5 does not specify the |
419 | -# version of websocket-client to use. Remove websocket-client as a dependency |
420 | -# here when moving to a newer jujuclient that correctly specifies the |
421 | -# version. |
422 | -websocket-client==0.18.0 |
423 | -jujuclient==0.18.4 |
424 | -PyYAML==3.11 |
425 | -urwid==1.2.1 |
426 | |
427 | === added file 'setup.cfg' |
428 | --- setup.cfg 1970-01-01 00:00:00 +0000 |
429 | +++ setup.cfg 2015-01-30 16:35:06 +0000 |
430 | @@ -0,0 +1,20 @@ |
431 | +# This file is part of the Juju Quickstart Plugin, which lets users set up a |
432 | +# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
433 | +# Copyright (C) 2015 Canonical Ltd. |
434 | +# |
435 | +# This program is free software: you can redistribute it and/or modify it under |
436 | +# the terms of the GNU Affero General Public License version 3, as published by |
437 | +# the Free Software Foundation. |
438 | +# |
439 | +# This program is distributed in the hope that it will be useful, but WITHOUT |
440 | +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
441 | +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
442 | +# Affero General Public License for more details. |
443 | +# |
444 | +# You should have received a copy of the GNU Affero General Public License |
445 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
446 | + |
447 | +[nosetests] |
448 | +detailed-errors=1 |
449 | +nocapture=1 |
450 | +verbosity=2 |
451 | |
452 | === modified file 'setup.py' |
453 | --- setup.py 2014-01-29 15:25:06 +0000 |
454 | +++ setup.py 2015-01-30 16:35:06 +0000 |
455 | @@ -16,52 +16,140 @@ |
456 | |
457 | """Juju Quickstart distribution file.""" |
458 | |
459 | +from __future__ import print_function |
460 | + |
461 | +import ConfigParser |
462 | +from distutils.core import Command |
463 | +from distutils.version import StrictVersion |
464 | import os |
465 | +import shlex |
466 | +import sys |
467 | |
468 | from setuptools import ( |
469 | find_packages, |
470 | setup, |
471 | ) |
472 | +from setuptools.command.test import test as TestCommand |
473 | |
474 | |
475 | ROOT = os.path.abspath(os.path.dirname(__file__)) |
476 | PROJECT_NAME = 'quickstart' |
477 | |
478 | +os.chdir(ROOT) |
479 | project = __import__(PROJECT_NAME) |
480 | -description_path = os.path.join(ROOT, 'README.rst') |
481 | - |
482 | -requirements_path = os.path.join(ROOT, 'requirements.pip') |
483 | -requirements = [i.strip() for i in open(requirements_path).readlines()] |
484 | -install_requires = [i for i in requirements if i and not i.startswith('#')] |
485 | - |
486 | -os.chdir(ROOT) |
487 | - |
488 | -data_files = [] |
489 | -for dirpath, dirnames, filenames in os.walk(PROJECT_NAME): |
490 | - for i, dirname in enumerate(dirnames): |
491 | - if dirname.startswith('.'): |
492 | - del dirnames[i] |
493 | - if '__init__.py' in filenames: |
494 | - continue |
495 | - elif filenames: |
496 | - for f in filenames: |
497 | - data_files.append(os.path.join( |
498 | - dirpath[len(PROJECT_NAME) + 1:], f)) |
499 | + |
500 | + |
501 | +def get_package_data(): |
502 | + """Return Juju Quickstart package data by walking through the project. |
503 | + |
504 | + The package data includes all non-python files that must be included in |
505 | + the source distribution. |
506 | + """ |
507 | + data_files = [] |
508 | + for dirpath, dirnames, filenames in os.walk(PROJECT_NAME): |
509 | + for i, dirname in enumerate(dirnames): |
510 | + if dirname.startswith('.'): |
511 | + del dirnames[i] |
512 | + if '__init__.py' in filenames: |
513 | + continue |
514 | + elif filenames: |
515 | + for f in filenames: |
516 | + data_files.append(os.path.join( |
517 | + dirpath[len(PROJECT_NAME) + 1:], f)) |
518 | + return {PROJECT_NAME: data_files} |
519 | + |
520 | + |
521 | +def get_long_description(): |
522 | + """Retrieve the project long description from the README.rst file.""" |
523 | + with open(os.path.join(ROOT, 'README.rst')) as readme_file: |
524 | + return readme_file.read() |
525 | + |
526 | + |
527 | +def get_install_requires(): |
528 | + """Dynamically generate the requirements list by parsing the tox file.""" |
529 | + config = ConfigParser.ConfigParser() |
530 | + config.read(os.path.join(ROOT, 'tox.ini')) |
531 | + all_versions = {} |
532 | + test_requirements = '{[testenv]deps}' |
533 | + platforms = config.get('tox', 'envlist').split(',') |
534 | + for platform in platforms: |
535 | + section = 'testenv:{}'.format(platform) |
536 | + deps = filter(None, config.get(section, 'deps').splitlines()) |
537 | + for requirement in deps: |
538 | + requirement = requirement.strip() |
539 | + if requirement.startswith('#') or requirement == test_requirements: |
540 | + continue |
541 | + name, version = requirement.split('==') |
542 | + versions = all_versions.setdefault(name, set()) |
543 | + versions.add(version) |
544 | + requirements = [] |
545 | + for name, versions in all_versions.items(): |
546 | + if len(versions) == 1: |
547 | + requirements.append('{}=={}'.format(name, versions.pop())) |
548 | + continue |
549 | + versions = list(sorted(versions, key=StrictVersion)) |
550 | + requirements.append( |
551 | + '{}>={},<={}'.format(name, versions[0], versions[-1])) |
552 | + return requirements |
553 | + |
554 | + |
555 | +class RequirementsCommand(Command): |
556 | + """Set up the requirements command.""" |
557 | + |
558 | + description = 'output program requirements' |
559 | + user_options = [] |
560 | + |
561 | + def initialize_options(self): |
562 | + pass |
563 | + |
564 | + def finalize_options(self): |
565 | + pass |
566 | + |
567 | + def run(self): |
568 | + requirements = get_install_requires() |
569 | + print('\n{}'.format('\n'.join(requirements))) |
570 | + |
571 | + |
572 | +class ToxCommand(TestCommand): |
573 | + """Set up the tox testing command.""" |
574 | + |
575 | + user_options = [('tox-args=', 'a', 'Arguments to pass to tox')] |
576 | + |
577 | + def initialize_options(self): |
578 | + TestCommand.initialize_options(self) |
579 | + self.tox_args = None |
580 | + |
581 | + def finalize_options(self): |
582 | + TestCommand.finalize_options(self) |
583 | + self.test_args = [] |
584 | + self.test_suite = True |
585 | + |
586 | + def run_tests(self): |
587 | + # Import here, cause outside the eggs aren't loaded. |
588 | + import tox |
589 | + args = [] if self.tox_args is None else shlex.split(self.tox_args) |
590 | + errno = tox.cmdline(args=args) |
591 | + sys.exit(errno) |
592 | |
593 | |
594 | setup( |
595 | name='juju-quickstart', |
596 | version=project.get_version(), |
597 | description=project.__doc__, |
598 | - long_description=open(description_path).read(), |
599 | + long_description=get_long_description(), |
600 | author='The Juju GUI team', |
601 | author_email='juju-gui@lists.ubuntu.com', |
602 | url='https://launchpad.net/juju-quickstart', |
603 | keywords='juju quickstart plugin', |
604 | packages=find_packages(), |
605 | - package_data={PROJECT_NAME: data_files}, |
606 | + package_data=get_package_data(), |
607 | scripts=['juju-quickstart'], |
608 | - install_requires=install_requires, |
609 | + install_requires=get_install_requires(), |
610 | + tests_require=['tox'], |
611 | + cmdclass = { |
612 | + 'requirements': RequirementsCommand, |
613 | + 'test': ToxCommand, |
614 | + }, |
615 | zip_safe=False, |
616 | classifiers=[ |
617 | 'Development Status :: 5 - Production/Stable', |
618 | |
619 | === removed file 'test-requirements.pip' |
620 | --- test-requirements.pip 2014-09-05 14:54:31 +0000 |
621 | +++ test-requirements.pip 1970-01-01 00:00:00 +0000 |
622 | @@ -1,23 +0,0 @@ |
623 | -# Juju Quickstart test requirements. |
624 | - |
625 | -# This file is part of the Juju Quickstart Plugin, which lets users set up a |
626 | -# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
627 | -# Copyright (C) 2013 Canonical Ltd. |
628 | -# |
629 | -# This program is free software: you can redistribute it and/or modify it under |
630 | -# the terms of the GNU Affero General Public License version 3, as published by |
631 | -# the Free Software Foundation. |
632 | -# |
633 | -# This program is distributed in the hope that it will be useful, but WITHOUT |
634 | -# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
635 | -# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
636 | -# Affero General Public License for more details. |
637 | -# |
638 | -# You should have received a copy of the GNU Affero General Public License |
639 | -# along with this program. If not, see <http://www.gnu.org/licenses/>. |
640 | - |
641 | -coverage==3.7.1 |
642 | -flake8==2.2.3 |
643 | -mock==1.0.1 |
644 | -nose==1.3.4 |
645 | --r requirements.pip |
646 | |
647 | === added file 'tox.ini' |
648 | --- tox.ini 1970-01-01 00:00:00 +0000 |
649 | +++ tox.ini 2015-01-30 16:35:06 +0000 |
650 | @@ -0,0 +1,115 @@ |
651 | +# This file is part of the Juju Quickstart Plugin, which lets users set up a |
652 | +# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
653 | +# Copyright (C) 2015 Canonical Ltd. |
654 | +# |
655 | +# This program is free software: you can redistribute it and/or modify it under |
656 | +# the terms of the GNU Affero General Public License version 3, as published by |
657 | +# the Free Software Foundation. |
658 | +# |
659 | +# This program is distributed in the hope that it will be useful, but WITHOUT |
660 | +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
661 | +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
662 | +# Affero General Public License for more details. |
663 | +# |
664 | +# You should have received a copy of the GNU Affero General Public License |
665 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
666 | + |
667 | +[tox] |
668 | +# The envlist option must only include platform scenarios. In essence, those |
669 | +# are used to test Juju Quickstart with the specific dependencies present in |
670 | +# each supported OS platform. |
671 | +envlist = pypi,ppa,vivid |
672 | +# XXX frankban: currently tests do not pass on trusty and utopic scenarios. |
673 | +# Fix the tests, then remove the line above and uncomment the line below. |
674 | +# envlist = pypi,ppa,trusty,utopic,vivid |
675 | + |
676 | + |
677 | +# Define the base requirements and commands to use when testing the program. |
678 | + |
679 | +[testenv] |
680 | +basepython = python2.7 |
681 | +deps = |
682 | + mock==1.0.1 |
683 | + nose==1.3.4 |
684 | +commands = |
685 | + nosetests {posargs:--quiet} |
686 | + |
687 | + |
688 | +# Set up the development environment. |
689 | + |
690 | +[testenv:devenv] |
691 | +envdir = devenv |
692 | +usedevelop = True |
693 | +deps = |
694 | + {[testenv]deps} |
695 | + {[testenv:lint]deps} |
696 | + coverage==3.7.1 |
697 | +commands = |
698 | + |
699 | + |
700 | +# Set up different scenarios for each Juju Quickstart source repository. |
701 | +# Note that the setup.py distribution file calculates Juju Quickstart |
702 | +# requirements based on this file. For this reason, when updating dependencies, |
703 | +# respect the following rules: |
704 | +# - all scenarios must be listed in the envlist above (in the tox section); |
705 | +# - each scenario must at least include a "{[testenv]deps}" line in its deps; |
706 | +# - keep each requirement in deps in its own line; |
707 | +# - always use specific revisions for environments (i.e. use "=="). |
708 | +# After updating this file, run "make requirements" and ensure the general |
709 | +# dependency rules in the output make sense. |
710 | + |
711 | +[testenv:pypi] |
712 | +# Always recreate the virtualenv here so that old installed dependencies are |
713 | +# not reused and a fresh pip install can be accurately simulated. |
714 | +recreate = True |
715 | +deps = |
716 | + {[testenv]deps} |
717 | + # All the dependencies are installed by Juju Quickstart itself, using |
718 | + # the dynamically generated "install_requires" setup field. |
719 | + # Note that, since Homebrew internally uses pip when installing Quickstart, |
720 | + # this scenario also covers OSX installations. |
721 | + |
722 | +[testenv:ppa] |
723 | +deps = |
724 | + {[testenv]deps} |
725 | + # Dependencies present in ppa:juju/stable. |
726 | + # See https://launchpad.net/~juju/+archive/ubuntu/stable. |
727 | + websocket-client==0.18.0 |
728 | + jujuclient==0.18.4 |
729 | + urwid==1.2.1 |
730 | + # The distribution PyYAML requirement is used in this case. |
731 | + |
732 | +[testenv:trusty] |
733 | +deps = |
734 | + {[testenv]deps} |
735 | + # Ubuntu 14.04 (trusty) distro dependencies. |
736 | + websocket-client==0.12.0 |
737 | + jujuclient==0.17.5 |
738 | + PyYAML==3.10 |
739 | + urwid==1.1.1 |
740 | + |
741 | +[testenv:utopic] |
742 | +deps = |
743 | + {[testenv]deps} |
744 | + # Ubuntu 14.10 (utopic) distro dependencies. |
745 | + websocket-client==0.12.0 |
746 | + jujuclient==0.17.5 |
747 | + PyYAML==3.11 |
748 | + urwid==1.2.1 |
749 | + |
750 | +[testenv:vivid] |
751 | +deps = |
752 | + {[testenv]deps} |
753 | + # Ubuntu 15.04 (vivid) distro dependencies. |
754 | + websocket-client==0.18.0 |
755 | + jujuclient==0.18.5 |
756 | + PyYAML==3.11 |
757 | + urwid==1.2.1 |
758 | + |
759 | + |
760 | +# Define additional tox targets to be used to validate and lint the code. |
761 | + |
762 | +[testenv:lint] |
763 | +usedevelop = True |
764 | +deps = flake8==2.3.0 |
765 | +commands = flake8 --show-source 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): ts.pip
M .bzrignore
M HACKING.rst
M MANIFEST.in
M Makefile
A [revision details]
D requirements.pip
A setup.cfg
M setup.py
D test-requiremen
A tox.ini