Merge lp://qastaging/~frankban/charms/trusty/redis/tsting-fixes into lp://qastaging/~juju-gui/charms/trusty/redis/trunk

Reviewers: mp+262062_code.launchpad.net,

Message:
Please take a look.

Description:
Makefile fixes for bundletester.

- The "local" env is no longer used by default when
   JUJU_ENV is not specified. Now the current env is
   used instead.

- The environment is no longer destroyed when ftests
   complete.

- Do not exit if bootstrap returns an error: assume
   the environment to be already bootstrapped.

- Removed the setup phony target to work around a
   bundletester bug.

For more info, see review comments at
https://bugs.launchpad.net/charms/+bug/1459345

https://code.launchpad.net/~frankban/charms/trusty/redis/tsting-fixes/+merge/262062

(do not edit description out of merge proposal)

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

Affected files (+19, -13 lines):
   M Makefile
   A [revision details]

Index: Makefile
=== modified file 'Makefile'
--- Makefile 2015-06-05 09:15:51 +0000
+++ Makefile 2015-06-16 09:51:39 +0000
@@ -7,7 +7,6 @@
  # Override those values to run functional tests with a different
environment
  # or Juju series.
  SERIES=trusty
-JUJU_ENV=local

  # Define system Debian dependencies: run "make sysdeps" to install them.
  # Please keep them in alphabetical order.
@@ -21,7 +20,7 @@
  PIP = $(VENV)/bin/pip
  FTESTS=$(shell find -L tests -type f -executable | sort)

-.DEFAULT_GOAL := setup
+.DEFAULT_GOAL := $(VENV_ACTIVATE)

  .PHONY: help
  help:
@@ -38,9 +37,10 @@
   @echo 'with the SERIES environment variable and the service name with '
   @echo 'the JUJU_SERVICE_NAME environment variable, for instance:'
   @echo ' make deploy SERIES=precise JUJU_SERVICE_NAME=myredis'
-
-.PHONY: setup
-setup: $(VENV_ACTIVATE)
+ @echo -e '\nWhen using "make deploy" or "make test" or "make ftest" it '
+ @echo 'is possible to override the Juju environement that will be used '
+ @echo 'with the JUJU_ENV environment variable, for instance:'
+ @echo ' make test JUJU_ENV=ec2'

  .PHONY: sysdeps
  sysdeps:
@@ -53,7 +53,7 @@
   @touch $(VENV_ACTIVATE)

  .PHONY: bundletester
-bundletester: setup
+bundletester: $(VENV_ACTIVATE)
   $(PIP) install bundletester
   $(VENV)/bin/bundletester -l DEBUG

@@ -67,13 +67,14 @@
   juju charm proof

  .PHONY: lint
-lint: setup
+lint: $(VENV_ACTIVATE)
   @$(VENV)/bin/flake8 --show-source --exclude=$(VENV) \
    --filename *.py,install,generic-hook \
    hooks/ tests/ unit_tests/

  .PHONY: deploy
  deploy:
+ $(eval JENV := $(shell JUJU_ENV=$(JUJU_ENV) juju switch))
   @# The use of readlink below is required for OS X.
   @$(eval export JUJU_REPOSITORY:=$(shell mktemp -d `readlink -f
/tmp`/temp.XXXX))
   @echo "JUJU_REPOSITORY is $(JUJU_REPOSITORY)"
@@ -82,24 +83,27 @@
   @rsync -a . $(JUJU_REPOSITORY)/${SERIES}/$(JUJU_TEST_CHARM) \
    --exclude .git --exclude .bzr --exclude tests --exclude unit_tests
   @# Deploying the charm.
- juju deploy local:${SERIES}/$(JUJU_TEST_CHARM) $(JUJU_SERVICE_NAME)
+ juju deploy -e $(JENV) local:${SERIES}/$(JUJU_TEST_CHARM)
$(JUJU_SERVICE_NAME)

  .PHONY: test
  test: unittest ftest

  .PHONY: ftest
-ftest: setup
- juju bootstrap -e $(JUJU_ENV) --upload-tools
+ftest: $(VENV_ACTIVATE)
+ $(eval JENV := $(shell JUJU_ENV=$(JUJU_ENV) juju switch))
+ @j...

LGTM

https://codereview.appspot.com/248930043/

LGTM with typo fixes and possible refactoring. No need for re-review.

Thank you both for the reviews!

Please take a look.

https://codereview.appspot.com/248930043/

*** Submitted:

Makefile fixes for bundletester.

- The "local" env is no longer used by default when
   JUJU_ENV is not specified. Now the current env is
   used instead.

- The environment is no longer destroyed when ftests
   complete.

- Do not exit if bootstrap returns an error: assume
   the environment to be already bootstrapped.

- Removed the setup phony target to work around a
   bundletester bug.

For more info, see review comments at
https://bugs.launchpad.net/charms/+bug/1459345

R=martin.hilton
CC=
https://codereview.appspot.com/248930043

https://codereview.appspot.com/248930043/