Merge lp://staging/~frankban/charms/precise/juju-gui/venv-fixes into lp://staging/~juju-gui/charms/precise/juju-gui/trunk

Reviewers: mp+195199_code.launchpad.net,

Message:
Please take a look.

Description:
Improve charm test venv creation.

Also added missing SYSDEP.

https://code.launchpad.net/~frankban/charms/precise/juju-gui/venv-fixes/+merge/195199

(do not edit description out of merge proposal)

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

Affected files (+10, -4 lines):
   M Makefile
   A [revision details]
   M tests/00-setup

Index: Makefile
=== modified file 'Makefile'
--- Makefile 2013-11-13 16:35:19 +0000
+++ Makefile 2013-11-14 08:52:35 +0000
@@ -16,8 +16,9 @@

  JUJUTEST = juju-test --timeout=120m -v -e "$(JUJU_ENV)"
  VENV = ./tests/.venv
-SYSDEPS = build-essential bzr libapt-pkg-dev python-pip python-virtualenv
xvfb \
- libpython-dev
+SYSDEPS = build-essential bzr libapt-pkg-dev libpython-dev python-pip \
+ python-virtualenv rsync xvfb
+

  all: setup

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision:
<email address hidden>

Index: tests/00-setup
=== modified file 'tests/00-setup'
--- tests/00-setup 2013-11-12 13:48:53 +0000
+++ tests/00-setup 2013-11-14 08:52:35 +0000
@@ -25,13 +25,16 @@

  createvenv() {
      # Create a virtualenv if it does not exist, or it is older than
requirements.
+ retcode=0
      if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt "$ACTIVATE"
-o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then
          virtualenv --distribute $VENV
- . $VENV/bin/activate && \
+ . $ACTIVATE && \
              yes w | env BZR_PLUGIN_PATH='-user' \
              pip install --use-mirrors -r $TEST_REQUIREMENTS --find-links
deps
- touch $VENV/bin/activate
+ retcode=$?
+ [ $retcode -eq 0 ] && touch $ACTIVATE || rm -f $ACTIVATE
      fi
+ return $retcode
  }

Please take a look.

https://codereview.appspot.com/26530043/

LGTM with consideration of comment.

https://codereview.appspot.com/26530043/diff/20001/tests/00-setup
File tests/00-setup (right):

https://codereview.appspot.com/26530043/diff/20001/tests/00-setup#newcode35
tests/00-setup:35: [ $retcode -eq 0 ] && touch $ACTIVATE || rm -f
$ACTIVATE
The only downside to this is that, since the $ACTIVATE is removed, you
can't use it to diagnose issues. What would you think of touching
$TEST_REQUIREMENTS in the failure case, instead?

https://codereview.appspot.com/26530043/

*** Submitted:

Improve charm test venv creation.

Also added missing SYSDEP.

R=gary.poster
CC=
https://codereview.appspot.com/26530043

https://codereview.appspot.com/26530043/diff/20001/tests/00-setup
File tests/00-setup (right):

https://codereview.appspot.com/26530043/diff/20001/tests/00-setup#newcode35
tests/00-setup:35: [ $retcode -eq 0 ] && touch $ACTIVATE || rm -f
$ACTIVATE
On 2013/11/14 11:57:56, gary.poster wrote:
> The only downside to this is that, since the $ACTIVATE is removed, you
can't use
> it to diagnose issues. What would you think of touching
$TEST_REQUIREMENTS in
> the failure case, instead?

Great suggestion! Done.

https://codereview.appspot.com/26530043/