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

[Gary Poster]

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/

[Gary Poster (gary)]

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

[Francesco Banconi (frankban)]

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/

[Francesco Banconi (frankban)]

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]

134. By Gary Poster on 2013-11-19

respond to review

135. By Gary Poster on 2013-11-19

update test per change

[Gary Poster (gary)]

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

[Gary Poster (gary)]

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/

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

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

=== modified file 'Makefile'
--- Makefile	2013-11-14 08:52:35 +0000
+++ Makefile	2013-11-19 15:08:56 +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

=== 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 @@
 # Charm tests.
 selenium==2.34.0
 xvfbwrapper==0.2.2
+
+
+## The following requirements were added by pip --freeze:
+argparse==1.2.1
+httplib2==0.8
+keyring==3.2.1
+lazr.authentication==0.1.2
+lazr.restfulclient==0.13.3
+lazr.uri==1.0.3
+mccabe==0.2.1
+oauth==1.0.1
+pep8==1.4.6
+pyflakes==0.7.3
+simplejson==3.3.1
+testresources==0.2.7
+wadllib==1.3.2
+wsgi-intercept==0.6.0
+wsgiref==0.1.2
+zope.interface==4.0.5

=== 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 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 TESTDIR=`dirname $0`
+DOWNLOADCACHE="$TESTDIR/download-cache"
 VENV="$TESTDIR/.venv"
 ACTIVATE="$VENV/bin/activate"
 TEST_REQUIREMENTS="test-requirements.pip"
@@ -26,11 +27,17 @@
 createvenv() {
     # Create a virtualenv if it does not exist, or it is older than requirements.
     retcode=0
+    updated_downloadcache=0
+    if [ ! -d "$DOWNLOADCACHE" ]; then
+        bzr co --lightweight lp:~juju-gui-charmers/juju-gui/charm-download-cache $DOWNLOADCACHE
+        updated_downloadcache=1
+    fi
     if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt "$ACTIVATE" -o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then
+        [ $updated_downloadcache -eq 0 ] && bzr up $DOWNLOADCACHE
         virtualenv --distribute $VENV
         . $ACTIVATE && \
             yes w | env BZR_PLUGIN_PATH='-user' \
-            pip install --use-mirrors -r $TEST_REQUIREMENTS --find-links deps
+            pip install -r $TEST_REQUIREMENTS --find-links deps --find-links $DOWNLOADCACHE --no-allow-external --no-index
         retcode=$?
         [ $retcode -eq 0 ] && touch $ACTIVATE || touch $TEST_REQUIREMENTS
     fi

=== 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 @@
 )
 
 
-rsync = command('rsync', '-a', '--exclude', '.bzr', '--exclude', '.venv')
+rsync = command('rsync', '-a', '--exclude', '.bzr', '--exclude', '/tests')
 
 
 def setup_repository(name, source, series='precise'):

=== 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 @@
         # unwanted directories.
         repo = setup_repository(self.name, self.source)
         charm_dir = os.path.join(repo, 'precise', self.name)
-        test_dir_name = os.path.basename(self.tests_dir)
         expected = set([
-            os.path.basename(self.root_file),
-            test_dir_name + os.path.sep,
-            os.path.join(test_dir_name, os.path.basename(self.tests_file))
+            os.path.basename(self.root_file)
         ])
         self.assert_files_equal(expected, charm_dir)