Merge lp:~canonical-losas/charms/trusty/squid-reverseproxy/venv-testing into lp:charms/trusty/squid-reverseproxy

[Paul Collins]

Address bug#1401323 by creating the virtualenv using --system-site-packages so that apt_pkg can be accessed.

[Resubmit: Fix conflict vs. trusty.]

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-11055-results

review: Needs Fixing (automated testing)

[Adam Israel (aisrael)]

Hi Paul,

Thanks for your efforts to improve the testing story for this charm! I had the chance to review your merge proposal today. Adding virtualenv is a welcome addition here.

I ran into a few issues that I wanted to bring to your attention, though. While the venv is created, the Makefile continues to use the system python, installing packages globally. Once the venv is created, it should run commands from the `.venv/bin` directory, i.e., `.venv/bin/python`, `.venv/bin/pip`. With that, I also ran into version errors with pep8.

I reworked the Makefile to install and use the venv, and put the code here for reference:
https://code.launchpad.net/~aisrael/charms/trusty/squid-reverseproxy/rework_venv

With those issues above resolved, I see no problem getting these changes moved forward. Thanks again for your efforts! Just set this back to 'Needs Review' when you're ready for us to take another look.

review: Needs Fixing

[Marco Ceppi (marcoceppi)]

Moving this to Work In Progress, when ready for another review please set to "Needs Review"

=== modified file 'Makefile'
--- Makefile	2014-10-31 15:05:23 +0000
+++ Makefile	2015-02-23 02:12:45 +0000
@@ -21,7 +21,7 @@
 	@(charm proof $(PWD) || [ $$? -eq 100 ]) && echo OK
 	@test `cat revision` = 0 && rm revision
 
-test:
+test: .venv
 	@echo Starting tests...
 	@pip install nose testtools mock pyyaml
 	@CHARM_DIR=$(CHARM_DIR) $(TEST_PREFIX) nosetests -s $(TEST_DIR)
@@ -41,4 +41,13 @@
 		-d hooks/charmhelpers
 	@echo Do not forget to commit the updated files if any.
 
+.venv:
+	sudo apt-get install -y python-pip python-coverage python-nose python-testtools \
+		python-mock make bzr python-virtualenv
+	virtualenv .venv --system-site-packages
+ 
+clean:
+	rm -rf .venv
+
+
 .PHONY: revision proof test lint sourcedeps charm-payload

=== modified file 'hooks/install'
--- hooks/install	2013-10-10 22:50:35 +0000
+++ hooks/install	2015-02-23 02:12:45 +0000
@@ -3,7 +3,7 @@
 set -eu
 
 juju-log 'Invoking charm-pre-install hooks'
-[ -d exec.d ] && ( for f in exec.d/*/charm-pre-install; do [ -x $f ] && /bin/sh -c "$f"; done )
+[ -d exec.d ] && ( for f in exec.d/*/charm-pre-install; do [ -x $f ] && /bin/sh -c "$f"; done ) || true
 
 juju-log 'Invoking python-based install hook'
 python hooks/hooks.py install