Merge lp:~gnuoy/charm-helpers/add-vcsdirs-module into lp:charm-helpers

[Liam Young]

Code to manage a payload from a version control system by creating
a directory for each revision and using a symlink to point at the
desired directory which an application server can then reference
e.g.
Point apache/gunicorn etc at /srv/code/my-project and deploy
payload from charm:

deploy_base = VcsDirs('/srv/code', 'my-project')
deploy_base.deploy_dir('/var/lib/juju/charm/new-code')

If r8 was in /var/lib/juju/charm/new-code then this would result in:
$ ls -l /srv/code
total 4
lrwxrwxrwx 1 liam liam 23 Jul 31 16:51 my-project -> /srv/code/my-project-8
drwxrwxr-x 3 liam liam 4096 Jul 31 16:41 my-project-8

Old directories can be removed with prune_dirs e.g.
deploy_base.prune_dirs(keep_dirs=2, order_by_revno=True)

[Matthew Wedgwood (mew)]

This looks awesome, Liam. Thanks! As this is a contrib feature, I'm game for merging it as-is, but I see a few things that are worth discussing first.

In _get_vcs_identifier, I think it would be best to catch exceptions a bit more specifically. If the user doesn't have the git module, for example, they might get back a generic "Exception" with no description of what went wrong. It would be helpful to provide a description when you throw the exception (e.g. Exception("Something bad happened")). Also, if the srcdir is neither a git nor bzr repo, the function simply returns None. That looks problematic in the places it's used.

In VcsDirs.deploy_dir() (line 174), I see:

   if check_newer and current_branchid >= int(candidate_branchid):

I know git isn't your focus, but I'm pretty sure that the branchids won't be comparable as ints. If fixing that isn't trivial, I certainly wouldn't begrudge you to remove git support for this merge. Also (trivial), I believe the "check_newer" boolean in this line is redundant.

As for the body of this conditional, did you intend to do anything besides report? I would have expected it to do the deploy only if the new code is newer.

Finally, I see that you move the code from its original place rather than copying (or preferably rsync'ing) it. I wonder if that might be somewhat surprising to users. I think it bears mentioning in the docs at least.

review: Needs Information

[Liam Young (gnuoy)]

> This looks awesome, Liam. Thanks! As this is a contrib feature, I'm game for
> merging it as-is, but I see a few things that are worth discussing first.
>
> In _get_vcs_identifier, I think it would be best to catch exceptions a bit
> more specifically. If the user doesn't have the git module, for example, they
> might get back a generic "Exception" with no description of what went wrong.
> It would be helpful to provide a description when you throw the exception
> (e.g. Exception("Something bad happened")). Also, if the srcdir is neither a
> git nor bzr repo, the function simply returns None. That looks problematic in
> the places it's used.
>

This was intentional as _get_vcs_identifier tries to autodetect which vcs system is in use. If it gets an exception because a particular vcs module isn't available or because the directory is not under the control of that vcs system then it moves on and tries the next one, hence not raising an exception back to the user. Do you think this approach is flawed ? or would it be ok to just make sure that if no vcs system is detected an exception, with useful message, is raised ?

> In VcsDirs.deploy_dir() (line 174), I see:
>
> if check_newer and current_branchid >= int(candidate_branchid):
>
> I know git isn't your focus, but I'm pretty sure that the branchids won't be
> comparable as ints. If fixing that isn't trivial, I certainly wouldn't
> begrudge you to remove git support for this merge. Also (trivial), I believe
> the "check_newer" boolean in this line is redundant.

I left it up to the user to decide whether to compare the branchids based on the vcs system they're using. If the caller sets check_newer to be true when using git it will fail ( and I agree current_branchid >= int(candidate_branchid) does not look massively useful). Perhaps I need to take the decision about whether to compare ids away from the user and have it be dictated by the vcs system (bzr => yes, git => no).

>
> As for the body of this conditional, did you intend to do anything besides
> report? I would have expected it to do the deploy only if the new code is
> newer.

If using bzr I'd expect check_newer to be set to true and so only a newer revno of the code is deployed. As for git, I decided not to worry and just deploy what is given

>
> Finally, I see that you move the code from its original place rather than
> copying (or preferably rsync'ing) it. I wonder if that might be somewhat
> surprising to users. I think it bears mentioning in the docs at least.

That makes sense, I'll take a look at rsync'ing it.

[Matthew Wedgwood (mew)]

> > In _get_vcs_identifier, I think it would be best to catch exceptions a bit
> > more specifically. If the user doesn't have the git module, for example, they
> > might get back a generic "Exception" with no description of what went wrong.
> > It would be helpful to provide a description when you throw the exception
> > (e.g. Exception("Something bad happened")). Also, if the srcdir is neither a
> > git nor bzr repo, the function simply returns None. That looks problematic in
> > the places it's used.
>
> This was intentional as _get_vcs_identifier tries to autodetect which vcs
> system is in use. If it gets an exception because a particular vcs module
> isn't available or because the directory is not under the control of that vcs
> system then it moves on and tries the next one, hence not raising an exception
> back to the user. Do you think this approach is flawed ? or would it be ok to
> just make sure that if no vcs system is detected an exception, with useful
> message, is raised ?

I guess my main concern is that there will be no way to distinguish between those errors. They'll simply come out as a traceback ending in "raise Exception" necessitating a look at the code to figure out what went wrong. What I would expect is a message along the lines of "<dir> is not a VCS repository. Tried <repo types>."

> > In VcsDirs.deploy_dir() (line 174), I see:
> >
> > if check_newer and current_branchid >= int(candidate_branchid):
> >
> > I know git isn't your focus, but I'm pretty sure that the branchids won't be
> > comparable as ints. If fixing that isn't trivial, I certainly wouldn't
> > begrudge you to remove git support for this merge. Also (trivial), I believe
> > the "check_newer" boolean in this line is redundant.
>
> I left it up to the user to decide whether to compare the branchids based on
> the vcs system they're using. If the caller sets check_newer to be true when
> using git it will fail ( and I agree current_branchid >=
> int(candidate_branchid) does not look massively useful). Perhaps I need to
> take the decision about whether to compare ids away from the user and have it
> be dictated by the vcs system (bzr => yes, git => no).

I think it's useful to provide the option, as it seems quite helpful to have the library refuse to activate an older version unless it's explicitly asked to. It's still possible to compare branch IDs in git, but you have to use 'git rev-list' and work it out.

[lp:~gnuoy/charm-helpers/add-vcsdirs-module]

66. By Liam Young on 2013-08-28

Added support for comparing Git directories by the committed date of the head commit

67. By Liam Young on 2013-08-28

Switched to using rsync rather than move for copying src directories

[Liam Young (gnuoy)]

I've added the following:
1) A message with the exception if _get_vcs_identifier fails to find the vcs system for the directory
2) Added support for comparing Git directories by the committed date of the head commit
3) Switched to using rsync for populating directories rather than move.

review: Needs Resubmitting

Unmerged revisions

67. By Liam Young on 2013-08-28

Switched to using rsync rather than move for copying src directories

66. By Liam Young on 2013-08-28

Added support for comparing Git directories by the committed date of the head commit

65. By Liam Young on 2013-07-31

Add VcsDirs module for managing a payload from a version control system by creating a directory for each revision

=== added directory 'charmhelpers/contrib/vcsdir'
=== added file 'charmhelpers/contrib/vcsdir/__init__.py'
--- charmhelpers/contrib/vcsdir/__init__.py	1970-01-01 00:00:00 +0000
+++ charmhelpers/contrib/vcsdir/__init__.py	2013-08-28 15:57:15 +0000
@@ -0,0 +1,208 @@
+#
+# Code to manage a payload from a version control system by creating
+# a directory for each revision and using a symlink to point at the
+# desired directory which an application server can then reference
+# e.g.
+# Point apache/gunicorn etc at /srv/code/my-project and deploy
+# payload from charm:
+#
+# deploy_base = VcsDirs('/srv/code', 'my-project')
+# deploy_base.deploy_dir('/var/lib/juju/charm/new-code')
+#
+# If r8 was in /var/lib/juju/charm/new-code then this would result in:
+# $ ls -l /srv/code
+# total 4
+# lrwxrwxrwx 1 liam liam   23 Jul 31 16:51 my-project -> /srv/code/my-project-8
+# drwxrwxr-x 3 liam liam 4096 Jul 31 16:41 my-project-8
+#
+# Old directories can be removed with prune_dirs e.g.
+# deploy_base.prune_dirs(keep_dirs=2, order_by_revno=True)
+
+
+import glob
+import os
+import shutil
+from charmhelpers.core import host
+
+
+def _get_vcs_identifier(source):
+    """Return identifier for branch and a value which can be
+       used to inspect chronological order of branches"""
+    try:
+        from git import Repo
+        repo = Repo(source)
+        # Return sha for the branch id and use commited date for ordering with
+        # other branches
+        return (repo.head.commit.hexsha, repo.head.commit.committed_date)
+    except:
+        pass
+    try:
+        from bzrlib.branch import Branch
+        current_branch = Branch.open(source)
+        # Return branch revno for both the branch id and also use it for
+        # ordering with other branches
+        return (current_branch.revno(), current_branch.revno())
+    except:
+        raise Exception(source + " is not a VCS controlled repository. Tried Git and Bzr")
+
+
+class VcsDir():
+    """A VcsDir is a directory which is managed by a vcs system and has a name
+       in format codename-branchid. Where branchid is the id for the
+       branch e.g. if using bzr it is the bzr revno."""
+    def __init__(self, dir):
+        self.fqdir = dir
+        self.basename = os.path.basename(self.fqdir)
+        self.dirname = os.path.dirname(self.fqdir)
+        if os.path.exists(self.fqdir) and not os.path.isdir(self.fqdir):
+            raise Exception
+
+    def get_fqdir(self):
+        """Get the full path to the directory"""
+        return self.fqdir
+
+    def get_basename(self):
+        """Return directory without path"""
+        return self.basename
+
+    def get_codename(self):
+        """Return the project codename"""
+        return self.basename.rpartition('-')[0]
+
+    def get_branchid(self):
+        """Return the vcs identifier for the branch"""
+        return self.basename.rsplit('-')[-1]
+
+    def get_comparator(self):
+        """Return the vcs comparator for the branch"""
+        (_, vcs_comparator) = _get_vcs_identifier(self.fqdir)
+        return vcs_comparator
+
+    def validate_name(self):
+        """Check the branchid in the dir name matches the code branchid"""
+        (vcs_identifier, _) = _get_vcs_identifier(self.fqdir)
+        if str(self.get_branchid()) == str(vcs_identifier):
+            return True
+        else:
+            return False
+
+
+class VcsDirs():
+    """A collection of VcsDir directories with a symlink pointing at the
+       desired directory which an application (apache, gunicorn etc) can
+       use"""
+    def __init__(self, dir, code_name):
+        self.fqdir = dir
+        self.code_name = code_name
+        self.dir_glob = os.path.join(self.fqdir, code_name + '-*')
+
+    def add_code_symlink(self, branchid):
+        """Create a symlink pointing at the requestred vcsdir"""
+        symlink = self.get_currentcode_symlink()
+        source_dir = self.get_directory_by_branchid(branchid)
+        if os.path.islink(symlink):
+            os.remove(symlink)
+        os.symlink(source_dir, symlink)
+
+    def get_currentcode_symlink(self):
+        """Return full path to currentcode symlink"""
+        return os.path.join(self.fqdir, self.code_name)
+
+    def get_currentcode_symlink_target(self):
+        """Return the target of the currentcode symlink"""
+        return os.path.realpath(self.get_currentcode_symlink())
+
+    def get_currentcode_branchid(self):
+        """Return the branchid of the vcsdir that the currentcode symlinki is
+           currently pointing at"""
+        new_vcsdir = VcsDir(self.get_currentcode_symlink_target())
+        branchid = new_vcsdir.get_branchid()
+        return branchid
+
+    def get_branchids(self):
+        """Get a list of all the branchids available"""
+        branchids = set()
+        for direc in glob.glob(self.dir_glob):
+            if os.path.isdir(direc):
+                branchids.add(VcsDir(direc).get_branchid())
+        return branchids
+
+    def get_directories_key_branchid(self):
+        """Get a hash of all the vcs directories with branchid as the key"""
+        branch_ids = {}
+        for direc in glob.glob(self.dir_glob):
+            if os.path.isdir(direc):
+                new_vcsdir = VcsDir(direc)
+                new_vcsdir.validate_name()
+                branchid = new_vcsdir.get_branchid()
+                if branchid in branch_ids:
+                    continue
+                else:
+                    branch_ids[branchid] = direc
+        return branch_ids
+
+    def get_directories_key_comparator(self):
+        """Get the comparator of all the vcs directories with branchid as
+           the key"""
+        branch_comps = {}
+        for direc in glob.glob(self.dir_glob):
+            if os.path.isdir(direc):
+                new_vcsdir = VcsDir(direc)
+                branch_comp = new_vcsdir.get_comparator()
+                if branch_comp in branch_comps:
+                    continue
+                else:
+                    branch_comps[branch_comp] = direc
+        return branch_comps
+
+    def get_directory_by_branchid(self, branchid):
+        """Return the directory for a given branchid"""
+        branch_ids = self.get_directories_key_branchid()
+        if branchid in branch_ids:
+            return branch_ids[branchid]
+        return None
+
+    def is_branchid_present(self, branchid):
+        """Is a directory containing a particular branchid present """
+        if branchid in self.get_branchids():
+            return True
+        return False
+
+    def prune_dirs(self, keep_dirs=3, order_by_revno=False):
+        """Delete the oldest 'keep_dirs' number of directories. Delete the
+           oldest by directory mtime unless order_by_revno is True"""
+        delete_count = 0
+        direc = self.get_directories_key_comparator()
+        number_to_delete = len(direc) - keep_dirs
+        if order_by_revno:
+            sorted_ids = sorted(direc.iterkeys())
+        else:
+            branchid_dates = [(branchid, os.stat(direc[branchid]).st_mtime)
+                              for branchid in direc.keys()]
+            sorted_ids = [r[0]
+                          for r in sorted(branchid_dates, key=lambda r: r[1])]
+        for branchid in sorted_ids:
+            if delete_count < number_to_delete:
+                if direc[branchid] != self.get_currentcode_symlink_target():
+                    shutil.rmtree(direc[branchid])
+                    delete_count += 1
+
+    def deploy_dir(self, srcdir, check_newer=False):
+        """Move a new directory in and update the currentcode symlink to point
+           at it. check_newer assumes vcs type supports rA > rb"""
+        (candidate_branchid, candidate_comparator) = _get_vcs_identifier(srcdir)
+        branchids = self.get_branchids()
+        if candidate_branchid in branchids:
+            print "Directory containing branchid already exists"
+            return
+        if check_newer and os.path.exists(self.get_currentcode_symlink()):
+            (_, current_comparator) = _get_vcs_identifier(self.get_currentcode_symlink_target())
+            if check_newer and current_comparator >= int(candidate_comparator):
+                print "ERROR. Not deploying branch. Live branch (%s) >= candidate branch(%s)" % \
+                      (current_comparator, candidate_comparator)
+                return
+        # Move new directiry in and point currenctcode symlink at it
+        host.rsync(srcdir + os.sep, os.path.join(self.fqdir,
+                                                 self.code_name +
+                                                 "-" + str(candidate_branchid)))
+        self.add_code_symlink(branchid=str(candidate_branchid))

=== added directory 'tests/contrib/vcsdir'
=== added file 'tests/contrib/vcsdir/__init__.py'
=== added file 'tests/contrib/vcsdir/test_vcsdir.py'
--- tests/contrib/vcsdir/test_vcsdir.py	1970-01-01 00:00:00 +0000
+++ tests/contrib/vcsdir/test_vcsdir.py	2013-08-28 15:57:15 +0000
@@ -0,0 +1,290 @@
+from testtools import TestCase
+import glob
+from mock import patch, call, Mock
+import charmhelpers.contrib.vcsdir as vcsdir
+from charmhelpers.core import host
+import os
+import shutil
+
+DISTINCT_BRANCHIDS = ['aa', 'bb', 'cc']
+PROJECT_NAME = 'my-super-whizzy-project'
+BASE_DIR = '/srv'
+
+tmpbranchids = DISTINCT_BRANCHIDS * 2
+FAKE_VCSDIRS = []
+
+for branchid in tmpbranchids:
+    directory = os.path.join(BASE_DIR,
+                             PROJECT_NAME + '-' + branchid)
+    FAKE_VCSDIRS.append([directory, branchid])
+TESTDIR = {}
+TESTDIR['direc'] = FAKE_VCSDIRS[0][0]
+TESTDIR['branchid'] = FAKE_VCSDIRS[0][1]
+NEWDIR = {}
+NEWDIR['direc'] = os.path.join(BASE_DIR, 'new-dir')
+NEWDIR['branchid'] = 'zz'
+
+
+class mock_git():
+    class Repo():
+        def __init__(self, source):
+            self.head.commit.hexsha = "gittest"
+            self.head.commit.committed_date = 1374676968
+
+        class head():
+            class commit():
+                pass
+
+
+class BzrBranch():
+    def revno(self):
+        return 10
+
+
+class mock_bzrlib_branch():
+    class Branch():
+        @staticmethod
+        def open(source):
+            return BzrBranch()
+
+
+class vcsidentifierTest(TestCase):
+    def test_get_vcs_identifier_git_exception(self):
+        self.assertRaises(Exception, lambda: vcsdir._get_vcs_identifier('tmp'))
+
+    @patch.dict('sys.modules', {'git': mock_git})
+    def test_get_vcs_identifier_git(self):
+        self.assertEqual(vcsdir._get_vcs_identifier('tmp'), ("gittest", 1374676968))
+
+    @patch.dict('sys.modules', {'bzrlib.branch': mock_bzrlib_branch})
+    def test_get_vcs_identifier_bzr(self):
+        self.assertEqual(vcsdir._get_vcs_identifier('tmp'), (10, 10))
+
+
+class VcsDirTest(TestCase):
+    @patch.object(os.path, 'isdir')
+    @patch.object(os.path, 'exists')
+    def test_fail_nodir(self, mock_pathexists, mock_pathisdir):
+        mock_pathexists.return_value = True
+        mock_pathisdir.return_value = False
+        self.assertRaises(Exception, lambda: vcsdir.VcsDir('/tmp'))
+
+    def test_get_fqdir(self):
+        vcstestdir = vcsdir.VcsDir(TESTDIR['direc'])
+        self.assertEqual(TESTDIR['direc'], vcstestdir.get_fqdir())
+
+    def test_get_basename(self):
+        vcstestdir = vcsdir.VcsDir(TESTDIR['direc'])
+        self.assertEqual(TESTDIR['direc'].replace(BASE_DIR + os.sep, ''),
+                         vcstestdir.get_basename())
+
+    def test_get_codename(self):
+        vcstestdir = vcsdir.VcsDir(TESTDIR['direc'])
+        self.assertEqual(PROJECT_NAME, vcstestdir.get_codename())
+
+    def test_get_branchid(self):
+        vcstestdir = vcsdir.VcsDir(TESTDIR['direc'])
+        self.assertEqual('aa', vcstestdir.get_branchid())
+
+    @patch.object(vcsdir, '_get_vcs_identifier')
+    def test_get_comparator(self, mock_get_vcs_identifier):
+        vcstestdir = vcsdir.VcsDir(TESTDIR['direc'])
+        mock_get_vcs_identifier.return_value = (DISTINCT_BRANCHIDS[0], DISTINCT_BRANCHIDS[0])
+        self.assertEqual('aa', vcstestdir.get_comparator())
+
+    @patch.object(vcsdir, '_get_vcs_identifier')
+    def test_validate_name(self, mock_get_vcs_identifier):
+        vcstestdir = vcsdir.VcsDir(TESTDIR['direc'])
+        mock_get_vcs_identifier.return_value = (DISTINCT_BRANCHIDS[0], DISTINCT_BRANCHIDS[0])
+        self.assertEqual(True, vcstestdir.validate_name())
+        mock_get_vcs_identifier.return_value = (DISTINCT_BRANCHIDS[1], DISTINCT_BRANCHIDS[1])
+        self.assertEqual(False, vcstestdir.validate_name())
+
+
+class VcsDirsTest(TestCase):
+    @patch.object(os, 'remove')
+    @patch.object(os.path, 'islink')
+    @patch.object(os, 'symlink')
+    @patch.object(vcsdir.VcsDirs, 'get_directory_by_branchid')
+    def test_add_code_symlink(self, mock_get_directory_by_branchid,
+                              mock_symlink, mock_pathislink, mock_remove):
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        code_symlink = os.path.join(BASE_DIR, PROJECT_NAME)
+        mock_pathislink.return_value = True
+        mock_get_directory_by_branchid.return_value = TESTDIR['direc']
+        testdir.add_code_symlink(branchid=TESTDIR['branchid'])
+        mock_symlink.assert_called_with(TESTDIR['direc'], code_symlink)
+
+    @patch.object(vcsdir.VcsDir, 'get_branchid')
+    @patch.object(vcsdir.VcsDirs, 'get_currentcode_symlink')
+    def test_get_currentcode_branchid(self, mock_get_currentcode_symlink,
+                                      mock_get_branchid):
+        mock_get_currentcode_symlink.return_value = os.path.join(BASE_DIR,
+                                                                 PROJECT_NAME)
+        mock_get_branchid.return_value = TESTDIR['branchid']
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        self.assertEqual(testdir.get_currentcode_branchid(),
+                         TESTDIR['branchid'])
+
+    @patch.object(os.path, 'isdir')
+    @patch.object(vcsdir.VcsDir, 'get_branchid')
+    @patch.object(glob, 'glob')
+    def test_get_branchids(self, mock_glob, mock_get_branchid,
+                           mock_ospathisdir):
+        branchids = [x[1] for x in FAKE_VCSDIRS]
+        directories = [x[0] for x in FAKE_VCSDIRS]
+
+        def branchid_side_effect():
+            return branchids.pop()
+        mock_get_branchid.side_effect = branchid_side_effect
+        mock_glob.return_value = directories
+        mock_ospathisdir.return_value = True
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        branchids = testdir.get_branchids()
+        # Check we have all the distinct branchids
+        self.assertEqual(branchids, set([x[1] for x in FAKE_VCSDIRS]))
+
+    @patch.object(os.path, 'isdir')
+    @patch.object(vcsdir.VcsDir, 'validate_name')
+    @patch.object(vcsdir.VcsDir, 'get_branchid')
+    @patch.object(glob, 'glob')
+    def test_get_directories_key_branchid(self, mock_glob,
+                                          mock_get_branchid,
+                                          mock_validate_name,
+                                          mock_ospathisdir):
+        vcsdirs = [x[0] for x in FAKE_VCSDIRS]
+        branchids = [x[1] for x in FAKE_VCSDIRS]
+        mock_glob.return_value = vcsdirs
+        mock_validate_name.return_value = True
+        mock_ospathisdir.return_value = True
+
+        def branchid_side_effect():
+            return branchids.pop(0)
+        mock_get_branchid.side_effect = branchid_side_effect
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        branchids = testdir.get_directories_key_branchid()
+        self.assertEqual(branchids[TESTDIR['branchid']], TESTDIR['direc'])
+
+    @patch.object(os.path, 'isdir')
+    @patch.object(vcsdir.VcsDir, 'get_comparator')
+    @patch.object(glob, 'glob')
+    def test_get_directories_key_comparator(self, mock_glob,
+                                            mock_get_comparator,
+                                            mock_ospathisdir):
+        vcsdirs = [x[0] for x in FAKE_VCSDIRS]
+        branchids = [x[1] for x in FAKE_VCSDIRS]
+        mock_glob.return_value = vcsdirs
+        mock_ospathisdir.return_value = True
+
+        def branchid_side_effect():
+            return branchids.pop(0)
+        mock_get_comparator.side_effect = branchid_side_effect
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        branchids = testdir.get_directories_key_comparator()
+        self.assertEqual(branchids[TESTDIR['branchid']], TESTDIR['direc'])
+
+    @patch.object(vcsdir.VcsDirs, 'get_branchids')
+    def test_is_branchid_present(self, mock_get_branchids):
+        branchids = [x[1] for x in FAKE_VCSDIRS]
+        mock_get_branchids.return_value = branchids
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        self.assertEqual(True, testdir.is_branchid_present(branchids[0]))
+        self.assertEqual(False,
+                         testdir.is_branchid_present('nonexistantbranchid'))
+
+    @patch.object(vcsdir.VcsDirs, 'get_directories_key_branchid')
+    def test_get_directory_by_branchid(self,
+                                       mock_get_directories_key_branchid):
+        branchids = {}
+        for branch in FAKE_VCSDIRS:
+            branchids[branch[1]] = branch[0]
+        mock_get_directories_key_branchid.return_value = branchids
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        self.assertEqual(FAKE_VCSDIRS[0][0],
+                         testdir.get_directory_by_branchid(
+                             FAKE_VCSDIRS[0][1]))
+        self.assertEqual(None,
+                         testdir.get_directory_by_branchid('nonexistantid'))
+
+    @patch.object(vcsdir.VcsDirs, 'get_directories_key_comparator')
+    @patch.object(shutil, 'rmtree')
+    @patch.object(os, 'stat')
+    def test_prune_dirs_by_ctime(self, mock_osstat, mock_rmtree,
+                                 mock_get_directories_key_comparator):
+        branchids = {}
+        dirctimes = {}
+        base_time = 1200000000
+        for branch in FAKE_VCSDIRS:
+            branchids[branch[1]] = branch[0]
+            base_time = base_time + 1
+            dirctimes[branch[0]] = str(base_time) + ".00000"
+        mock_get_directories_key_comparator.return_value = branchids
+
+        def osstat_side_effect(bob):
+            mtime = dirctimes[bob]
+            return Mock(st_mtime=mtime)
+        mock_osstat.side_effect = osstat_side_effect
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        testdir.prune_dirs(keep_dirs=1)
+        calls = [call(FAKE_VCSDIRS[0][0]), call(FAKE_VCSDIRS[1][0])]
+        mock_rmtree.assert_has_calls(calls)
+
+    @patch.object(vcsdir.VcsDirs, 'get_directories_key_comparator')
+    @patch.object(shutil, 'rmtree')
+    def test_prune_dirs_by_revno(self, mock_rmtree,
+                                 mock_get_directories_key_comparator):
+        branch_comps = {}
+        for branch in FAKE_VCSDIRS:
+            branch_comps[branch[1]] = branch[0]
+
+        mock_get_directories_key_comparator.return_value = branch_comps
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        testdir.prune_dirs(keep_dirs=1, order_by_revno=True)
+        calls = [call(FAKE_VCSDIRS[0][0]), call(FAKE_VCSDIRS[1][0])]
+        mock_rmtree.assert_has_calls(calls)
+
+    @patch.object(vcsdir.VcsDirs, 'add_code_symlink')
+    @patch.object(vcsdir.VcsDirs, 'get_branchids')
+    @patch.object(vcsdir, '_get_vcs_identifier')
+    @patch.object(host, 'rsync')
+    def test_deploy_dir_newdir(self, mock_rsync, mock_get_vcs_identifier,
+                               mock_get_branchids, mock_add_code_symlink):
+        branchids = [x[1] for x in FAKE_VCSDIRS]
+        mock_get_branchids.return_value = branchids
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        mock_get_vcs_identifier.return_value = (NEWDIR['branchid'], NEWDIR['branchid'])
+        testdir.deploy_dir(NEWDIR['direc'])
+        new_vcsdir = os.path.join(BASE_DIR,
+                                  PROJECT_NAME + '-' + NEWDIR['branchid'])
+        mock_rsync.assert_called_with(NEWDIR['direc'] + os.sep, new_vcsdir)
+
+    @patch.object(vcsdir.VcsDirs, 'add_code_symlink')
+    @patch.object(vcsdir.VcsDirs, 'get_branchids')
+    @patch.object(vcsdir, '_get_vcs_identifier')
+    @patch.object(host, 'rsync')
+    def test_deploy_dir_same_branchid(self, mock_rsync,
+                                      mock_get_vcs_identifier,
+                                      mock_get_branchids,
+                                      mock_add_code_symlink):
+        mock_get_vcs_identifier.return_value = (FAKE_VCSDIRS[0][1], FAKE_VCSDIRS[0][1])
+        branchids = [x[1] for x in FAKE_VCSDIRS]
+        mock_get_branchids.return_value = branchids
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        testdir.deploy_dir(NEWDIR['direc'])
+        assert not mock_rsync.called
+        assert not mock_add_code_symlink.called
+
+    @patch.object(vcsdir.VcsDirs, 'get_currentcode_branchid')
+    @patch.object(os.path, 'exists')
+    @patch.object(vcsdir, '_get_vcs_identifier')
+    @patch.object(host, 'rsync')
+    def test_deploy_dir_lower_branchid(self, mock_rsync,
+                                       mock_get_vcs_identifier,
+                                       mock_ospathexists,
+                                       mock_get_currentcode_branchid):
+        mock_ospathexists.return_value = True
+        mock_get_vcs_identifier.return_value = ('1', '1')
+        mock_get_currentcode_branchid.return_value = '2'
+        testdir = vcsdir.VcsDirs(BASE_DIR, PROJECT_NAME)
+        testdir.deploy_dir(NEWDIR['direc'], check_newer=True)
+        assert not mock_rsync.called