Juju Charms Collection
redis package

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

  1. Trusty Tahr (14.04)
  2. tsting-fixes
  3. Merge into trunk
Proposed by Francesco Banconi
Status: Merged
Merged at revision: 5
Proposed branch: lp:~frankban/charms/trusty/redis/tsting-fixes
Merge into: lp:~juju-gui/charms/trusty/redis/trunk
Diff against target: 96 lines (+20/-14)
1 file modified
Makefile (+20/-14)
To merge this branch: bzr merge lp:~frankban/charms/trusty/redis/tsting-fixes
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+262062@code.launchpad.net

Description of the change

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://codereview.appspot.com/248930043/

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :
Download full text (4.2 KiB)

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...

Read more...

Revision history for this message
Martin Hilton (martin-hilton) wrote :

LGTM

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

Revision history for this message
Brad Crittenden (bac) wrote :

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

review: Approve (code)
Revision history for this message
Francesco Banconi (frankban) wrote :

Thank you both for the reviews!

lp:~frankban/charms/trusty/redis/tsting-fixes updated
6. By Francesco Banconi

Changes as per review.

Revision history for this message
Francesco Banconi (frankban) wrote :

Please take a look.

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

Revision history for this message
Francesco Banconi (frankban) wrote :

*** 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/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Makefile'
--- Makefile 2015-06-05 09:15:51 +0000
+++ Makefile 2015-06-16 12:50:54 +0000
@@ -7,7 +7,6 @@
7# Override those values to run functional tests with a different environment7# Override those values to run functional tests with a different environment
8# or Juju series.8# or Juju series.
9SERIES=trusty9SERIES=trusty
10JUJU_ENV=local
1110
12# Define system Debian dependencies: run "make sysdeps" to install them.11# Define system Debian dependencies: run "make sysdeps" to install them.
13# Please keep them in alphabetical order.12# Please keep them in alphabetical order.
@@ -21,7 +20,7 @@
21PIP = $(VENV)/bin/pip20PIP = $(VENV)/bin/pip
22FTESTS=$(shell find -L tests -type f -executable | sort)21FTESTS=$(shell find -L tests -type f -executable | sort)
2322
24.DEFAULT_GOAL := setup23.DEFAULT_GOAL := $(VENV_ACTIVATE)
2524
26.PHONY: help25.PHONY: help
27help:26help:
@@ -38,9 +37,10 @@
38 @echo 'with the SERIES environment variable and the service name with '37 @echo 'with the SERIES environment variable and the service name with '
39 @echo 'the JUJU_SERVICE_NAME environment variable, for instance:'38 @echo 'the JUJU_SERVICE_NAME environment variable, for instance:'
40 @echo ' make deploy SERIES=precise JUJU_SERVICE_NAME=myredis'39 @echo ' make deploy SERIES=precise JUJU_SERVICE_NAME=myredis'
4140 @echo -e '\nWhen using "make deploy" or "make test" or "make ftest" it '
42.PHONY: setup41 @echo 'is possible to override the Juju environment that will be used '
43setup: $(VENV_ACTIVATE)42 @echo 'with the JUJU_ENV environment variable, for instance:'
43 @echo ' make test JUJU_ENV=ec2'
4444
45.PHONY: sysdeps45.PHONY: sysdeps
46sysdeps:46sysdeps:
@@ -53,7 +53,7 @@
53 @touch $(VENV_ACTIVATE)53 @touch $(VENV_ACTIVATE)
5454
55.PHONY: bundletester55.PHONY: bundletester
56bundletester: setup56bundletester: $(VENV_ACTIVATE)
57 $(PIP) install bundletester57 $(PIP) install bundletester
58 $(VENV)/bin/bundletester -l DEBUG58 $(VENV)/bin/bundletester -l DEBUG
5959
@@ -67,13 +67,17 @@
67 juju charm proof67 juju charm proof
6868
69.PHONY: lint69.PHONY: lint
70lint: setup70lint: $(VENV_ACTIVATE)
71 @$(VENV)/bin/flake8 --show-source --exclude=$(VENV) \71 @$(VENV)/bin/flake8 --show-source --exclude=$(VENV) \
72 --filename *.py,install,generic-hook \72 --filename *.py,install,generic-hook \
73 hooks/ tests/ unit_tests/73 hooks/ tests/ unit_tests/
7474
75.PHONY: jenv
76jenv:
77 $(eval JENV := $(shell JUJU_ENV=$(JUJU_ENV) juju switch))
78
75.PHONY: deploy79.PHONY: deploy
76deploy:80deploy: jenv
77 @# The use of readlink below is required for OS X.81 @# The use of readlink below is required for OS X.
78 @$(eval export JUJU_REPOSITORY:=$(shell mktemp -d `readlink -f /tmp`/temp.XXXX))82 @$(eval export JUJU_REPOSITORY:=$(shell mktemp -d `readlink -f /tmp`/temp.XXXX))
79 @echo "JUJU_REPOSITORY is $(JUJU_REPOSITORY)"83 @echo "JUJU_REPOSITORY is $(JUJU_REPOSITORY)"
@@ -82,24 +86,26 @@
82 @rsync -a . $(JUJU_REPOSITORY)/${SERIES}/$(JUJU_TEST_CHARM) \86 @rsync -a . $(JUJU_REPOSITORY)/${SERIES}/$(JUJU_TEST_CHARM) \
83 --exclude .git --exclude .bzr --exclude tests --exclude unit_tests87 --exclude .git --exclude .bzr --exclude tests --exclude unit_tests
84 @# Deploying the charm.88 @# Deploying the charm.
85 juju deploy local:${SERIES}/$(JUJU_TEST_CHARM) $(JUJU_SERVICE_NAME)89 juju deploy -e $(JENV) local:${SERIES}/$(JUJU_TEST_CHARM) $(JUJU_SERVICE_NAME)
8690
87.PHONY: test91.PHONY: test
88test: unittest ftest92test: unittest ftest
8993
90.PHONY: ftest94.PHONY: ftest
91ftest: setup95ftest: $(VENV_ACTIVATE) jenv
92 juju bootstrap -e $(JUJU_ENV) --upload-tools96 @juju bootstrap -e $(JENV) --upload-tools || \
97 echo "reusing already bootstrapped $(JENV) environment"
93 @ # Wait for the environment to be ready.98 @ # Wait for the environment to be ready.
94 juju status -e $(JUJU_ENV)99 @juju status -e $(JENV)
95 @# Setting the path is required because internally amulet calls100 @# Setting the path is required because internally amulet calls
96 @# juju-deployer using subprocess.101 @# juju-deployer using subprocess.
97 PATH="$(VENV)/bin:$(PATH)" $(NOSE) --verbosity 2 -s -x $(FTESTS)102 PATH="$(VENV)/bin:$(PATH)" $(NOSE) --verbosity 2 -s -x $(FTESTS)
98 juju destroy-environment $(JUJU_ENV) -y
99 @# Clean up deployer garbage.103 @# Clean up deployer garbage.
100 @-$(RM) -rf trusty104 @-$(RM) -rf trusty
105 @echo "Done! Remember to destroy the environment with:"
106 @echo "juju destroy-environment $(JENV) -y"
101107
102.PHONY: unittest108.PHONY: unittest
103unittest: setup109unittest: $(VENV_ACTIVATE)
104 $(NOSE) --verbosity 2 -s -w unit_tests \110 $(NOSE) --verbosity 2 -s -w unit_tests \
105 --with-coverage --cover-package hooks --cover-erase111 --with-coverage --cover-package hooks --cover-erase

Subscribers

People subscribed via source and target branches