Juju Charms Collection
juju-gui package

Merge lp:~gary/charms/precise/juju-gui/download-cache into lp:~juju-gui/charms/precise/juju-gui/trunk

  1. Precise Pangolin (12.04)
  2. download-cache
  3. Merge into trunk
Proposed by Gary Poster
Status: Merged
Merged at revision: 133
Proposed branch: lp:~gary/charms/precise/juju-gui/download-cache
Merge into: lp:~juju-gui/charms/precise/juju-gui/trunk
Diff against target: 136 lines (+52/-6)
7 files modified
.bzrignore (+1/-0)
HACKING.md (+21/-0)
Makefile (+1/-0)
test-requirements.pip (+19/-0)
tests/00-setup (+8/-1)
tests/deploy.py (+1/-1)
tests/test_deploy.py (+1/-4)
To merge this branch: bzr merge lp:~gary/charms/precise/juju-gui/download-cache
Reviewer Review Type Date Requested Status
charmers Pending
Review via email: mp+195700@code.launchpad.net

Description of the change

Add a download cache for test dependencies

To QA, remove the .venv in your tests dir and try running make. It is a lot faster and more reliable for me. Testing make clean would be good too.

https://codereview.appspot.com/28770043/

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :
Download full text (4.8 KiB)

Reviewers: mp+195700_code.launchpad.net,

Message:
Please take a look.

Description:
Add a download cache for test dependencies

To QA, remove the .venv in your tests dir and try running make. It is a
lot faster and more reliable for me. Testing make clean would be good
too.

https://code.launchpad.net/~gary/charms/precise/juju-gui/download-cache/+merge/195700

(do not edit description out of merge proposal)

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

Affected files (+49, -1 lines):
   M .bzrignore
   M HACKING.md
   M Makefile
   A [revision details]
   M test-requirements.pip
   M tests/00-setup

Index: .bzrignore
=== modified file '.bzrignore'
--- .bzrignore 2013-07-17 15:23:10 +0000
+++ .bzrignore 2013-11-19 02:50:54 +0000
@@ -6,3 +6,4 @@
  server/dist
  server/MANIFEST
  tests/.venv
+tests/download-cache

Index: HACKING.md
=== modified file 'HACKING.md'
--- HACKING.md 2013-11-12 17:35:42 +0000
+++ HACKING.md 2013-11-19 02:50:54 +0000
@@ -185,3 +185,24 @@
  To upgrade the dependencies, add a tarball to the `deps` directory, remove
the
  old dependency if required, and update the `server-requirements.pip` file.
  At this point, running `make` should also update the virtualenv used for
tests.
+
+## Upgrading the test dependencies ##
+
+The tests also have a number of dependencies. For speed and reliability,
these
+are also in a directory. However, they are not necessary for normal use
of the
+charm, and so, unlike the server dependencies, they are not part of the
normal
+charm branch. The 00-setup script makes a lightweight checkout of the
branch
+lp:~juju-gui-charmers/juju-gui/charm-download-cache and then uses this to
build
+the test virtual environment.
+
+To take best advantage of this approach, either use the same charm branch
+repeatedly for development; or develop the charm within a parent directory
+in which you have run `bzr init-repo`, so that the different branches can
use
+the local cache; or both.
+
+To upgrade test dependencies, add them to the `test-requirements.pip` file
and
+add the tarballs to the download-cache branch. You may need to temporarily
+disable the `--no-allow-external` and `--no-index` flags in 00-setup to get
+new transitive dependencies. Once you do, run `pip freeze` to add the
+transitive dependencies to the `test-requirements.pip` file, and make sure
to
+add those tarballs to the download cache as well.

Index: Makefile
=== modified file 'Makefile'
--- Makefile 2013-11-14 08:52:35 +0000
+++ Makefile 2013-11-19 02:50:54 +0000
@@ -62,6 +62,7 @@
  clean:
   find . -name '*.pyc' -delete
   rm -rf $(VENV)
+ rm -rf tests/download-cache

  deploy: setup
   $(VENV)/bin/python ./tests/deploy.py

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: test-requirements.pip
=== modified file 'test-requirements.pip'
--- test-requirements.pip 2013-11-12 12:15:52 +0000
+++ test-requirements.pip 2013-11-19 0...

Read more...

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

A very nice improvement Gary, thank you!
Running "make" successfully checked out the download cache branch in my
sandbox/tests/ directory.
LGTM with two comments, one below, one here:

"make deploy" is implemented in tests/deploy.py. As you can see, it
creates a temporary charm repository where the charm is copied using
rsync, and two directories are excluded: .bzr and .venv. I'd be inclined
to also exclude the download cache branch: it's ~16MB and I presume it
would sensibly slows down the deployment process. FWIW, I think it
should work also excluding the whole tests/ directory (it shouldn't be
required in that context), I'll give it a try later.

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

https://codereview.appspot.com/28770043/diff/1/tests/00-setup#newcode33
tests/00-setup:33: if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt
"$ACTIVATE" -o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then
What do you think about updating the download cache branch when at least
one of the requirement files is outdated?

IIUC, if user X updated the requirements, it is likely that also the
download cache has been updated. In this case, user Y will just run make
to get the new cache branch revision. Subsequently "pip install" can
proceed correctly.

https://codereview.appspot.com/28770043/

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

FWIW, this diff here seems to work:
http://pastebin.ubuntu.com/6442036/

Note the leading slash in '/tests': that's important in order to only
exclude the tests directory in the branch root, and not, e.g., the
guiserver tests (which are required to setup.py install the builtin
server).

As mentioned, another possibility is to just exclude the download cache
directory. Another idea could be running "make clean" in the target
repository, so that the process is slightly automated. Unfortunately,
this would leave the .bzr directory in the temporary repository. <shrug>

https://codereview.appspot.com/28770043/

lp:~gary/charms/precise/juju-gui/download-cache updated
134. By Gary Poster

respond to review

135. By Gary Poster

update test per change

Revision history for this message
Gary Poster (gary) wrote :

*** Submitted:

Add a download cache for test dependencies

To QA, remove the .venv in your tests dir and try running make. It is a
lot faster and more reliable for me. Testing make clean would be good
too.

R=frankban
CC=
https://codereview.appspot.com/28770043

https://codereview.appspot.com/28770043/

Revision history for this message
Gary Poster (gary) wrote :

Thank you for the great review and suggestions, Francesco. I
implemented both, choosing to use the /tests approach you mentioned for
the first one.

https://codereview.appspot.com/28770043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '.bzrignore'
--- .bzrignore 2013-07-17 15:23:10 +0000
+++ .bzrignore 2013-11-19 15:08:56 +0000
@@ -6,3 +6,4 @@
6server/dist6server/dist
7server/MANIFEST7server/MANIFEST
8tests/.venv8tests/.venv
9tests/download-cache
910
=== modified file 'HACKING.md'
--- HACKING.md 2013-11-12 17:35:42 +0000
+++ HACKING.md 2013-11-19 15:08:56 +0000
@@ -185,3 +185,24 @@
185To upgrade the dependencies, add a tarball to the `deps` directory, remove the185To upgrade the dependencies, add a tarball to the `deps` directory, remove the
186old dependency if required, and update the `server-requirements.pip` file.186old dependency if required, and update the `server-requirements.pip` file.
187At this point, running `make` should also update the virtualenv used for tests.187At this point, running `make` should also update the virtualenv used for tests.
188
189## Upgrading the test dependencies ##
190
191The tests also have a number of dependencies. For speed and reliability, these
192are also in a directory. However, they are not necessary for normal use of the
193charm, and so, unlike the server dependencies, they are not part of the normal
194charm branch. The 00-setup script makes a lightweight checkout of the branch
195lp:~juju-gui-charmers/juju-gui/charm-download-cache and then uses this to build
196the test virtual environment.
197
198To take best advantage of this approach, either use the same charm branch
199repeatedly for development; or develop the charm within a parent directory
200in which you have run `bzr init-repo`, so that the different branches can use
201the local cache; or both.
202
203To upgrade test dependencies, add them to the `test-requirements.pip` file and
204add the tarballs to the download-cache branch. You may need to temporarily
205disable the `--no-allow-external` and `--no-index` flags in 00-setup to get
206new transitive dependencies. Once you do, run `pip freeze` to add the
207transitive dependencies to the `test-requirements.pip` file, and make sure to
208add those tarballs to the download cache as well.
188209
=== modified file 'Makefile'
--- Makefile 2013-11-14 08:52:35 +0000
+++ Makefile 2013-11-19 15:08:56 +0000
@@ -62,6 +62,7 @@
62clean:62clean:
63 find . -name '*.pyc' -delete63 find . -name '*.pyc' -delete
64 rm -rf $(VENV)64 rm -rf $(VENV)
65 rm -rf tests/download-cache
6566
66deploy: setup67deploy: setup
67 $(VENV)/bin/python ./tests/deploy.py68 $(VENV)/bin/python ./tests/deploy.py
6869
=== modified file 'test-requirements.pip'
--- test-requirements.pip 2013-11-12 12:15:52 +0000
+++ test-requirements.pip 2013-11-19 15:08:56 +0000
@@ -45,3 +45,22 @@
45# Charm tests.45# Charm tests.
46selenium==2.34.046selenium==2.34.0
47xvfbwrapper==0.2.247xvfbwrapper==0.2.2
48
49
50## The following requirements were added by pip --freeze:
51argparse==1.2.1
52httplib2==0.8
53keyring==3.2.1
54lazr.authentication==0.1.2
55lazr.restfulclient==0.13.3
56lazr.uri==1.0.3
57mccabe==0.2.1
58oauth==1.0.1
59pep8==1.4.6
60pyflakes==0.7.3
61simplejson==3.3.1
62testresources==0.2.7
63wadllib==1.3.2
64wsgi-intercept==0.6.0
65wsgiref==0.1.2
66zope.interface==4.0.5
4867
=== modified file 'tests/00-setup'
--- tests/00-setup 2013-11-14 12:20:12 +0000
+++ tests/00-setup 2013-11-19 15:08:56 +0000
@@ -17,6 +17,7 @@
17# along with this program. If not, see <http://www.gnu.org/licenses/>.17# along with this program. If not, see <http://www.gnu.org/licenses/>.
1818
19TESTDIR=`dirname $0`19TESTDIR=`dirname $0`
20DOWNLOADCACHE="$TESTDIR/download-cache"
20VENV="$TESTDIR/.venv"21VENV="$TESTDIR/.venv"
21ACTIVATE="$VENV/bin/activate"22ACTIVATE="$VENV/bin/activate"
22TEST_REQUIREMENTS="test-requirements.pip"23TEST_REQUIREMENTS="test-requirements.pip"
@@ -26,11 +27,17 @@
26createvenv() {27createvenv() {
27 # Create a virtualenv if it does not exist, or it is older than requirements.28 # Create a virtualenv if it does not exist, or it is older than requirements.
28 retcode=029 retcode=0
30 updated_downloadcache=0
31 if [ ! -d "$DOWNLOADCACHE" ]; then
32 bzr co --lightweight lp:~juju-gui-charmers/juju-gui/charm-download-cache $DOWNLOADCACHE
33 updated_downloadcache=1
34 fi
29 if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt "$ACTIVATE" -o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then35 if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt "$ACTIVATE" -o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then
36 [ $updated_downloadcache -eq 0 ] && bzr up $DOWNLOADCACHE
30 virtualenv --distribute $VENV37 virtualenv --distribute $VENV
31 . $ACTIVATE && \38 . $ACTIVATE && \
32 yes w | env BZR_PLUGIN_PATH='-user' \39 yes w | env BZR_PLUGIN_PATH='-user' \
33 pip install --use-mirrors -r $TEST_REQUIREMENTS --find-links deps40 pip install -r $TEST_REQUIREMENTS --find-links deps --find-links $DOWNLOADCACHE --no-allow-external --no-index
34 retcode=$?41 retcode=$?
35 [ $retcode -eq 0 ] && touch $ACTIVATE || touch $TEST_REQUIREMENTS42 [ $retcode -eq 0 ] && touch $ACTIVATE || touch $TEST_REQUIREMENTS
36 fi43 fi
3744
=== modified file 'tests/deploy.py'
--- tests/deploy.py 2013-08-08 13:09:10 +0000
+++ tests/deploy.py 2013-11-19 15:08:56 +0000
@@ -30,7 +30,7 @@
30)30)
3131
3232
33rsync = command('rsync', '-a', '--exclude', '.bzr', '--exclude', '.venv')33rsync = command('rsync', '-a', '--exclude', '.bzr', '--exclude', '/tests')
3434
3535
36def setup_repository(name, source, series='precise'):36def setup_repository(name, source, series='precise'):
3737
=== modified file 'tests/test_deploy.py'
--- tests/test_deploy.py 2013-08-08 13:53:54 +0000
+++ tests/test_deploy.py 2013-11-19 15:08:56 +0000
@@ -94,11 +94,8 @@
94 # unwanted directories.94 # unwanted directories.
95 repo = setup_repository(self.name, self.source)95 repo = setup_repository(self.name, self.source)
96 charm_dir = os.path.join(repo, 'precise', self.name)96 charm_dir = os.path.join(repo, 'precise', self.name)
97 test_dir_name = os.path.basename(self.tests_dir)
98 expected = set([97 expected = set([
99 os.path.basename(self.root_file),98 os.path.basename(self.root_file)
100 test_dir_name + os.path.sep,
101 os.path.join(test_dir_name, os.path.basename(self.tests_file))
102 ])99 ])
103 self.assert_files_equal(expected, charm_dir)100 self.assert_files_equal(expected, charm_dir)
104101

Subscribers

People subscribed via source and target branches

to all changes: