Merge lp:~ricardokirkner/isitdeployable/safer-unlock into lp:isitdeployable

[Ricardo Kirkner]

make sure to only attempt unlocking branch if lock was successfully acquired

[Daniel Manrique (roadmr)]

LGTM, the rearrangement of the try/except payload makes sense.

review: Approve

=== modified file 'revtracker/tasks.py'
--- revtracker/tasks.py	2017-09-11 16:17:19 +0000
+++ revtracker/tasks.py	2017-10-03 16:38:10 +0000
@@ -657,6 +657,12 @@
     try:
         branch = get_branch(url, project.name, git_branch=project.git_branch,
                             fetched_branches=fetched_branches)
+    except bzr_errors.LockContention as err:
+        print('Can not acquire lock for {}. There is already a lock at {}. '
+              'Skipping.'.format(url, err.msg))
+        return
+
+    try:
         if last_revno is None:
             last_revision = None
         elif branch.vcs == VCS.VCS_BAZAAR:
@@ -674,10 +680,6 @@
                 sourcedeps_path=project.sourcedeps_path,
                 fetched_branches=fetched_branches)
         store_revisions(project, branch, revisions)
-    except bzr_errors.LockContention as err:
-        print('Can not acquire lock for {}. There is already a lock at {}. '
-              'Skipping.'.format(url, err.msg))
-        return
     finally:
         branch.unlock()
 

=== modified file 'revtracker/tests/test_tasks.py'
--- revtracker/tests/test_tasks.py	2016-11-11 18:16:23 +0000
+++ revtracker/tests/test_tasks.py	2017-10-03 16:38:10 +0000
@@ -625,3 +625,27 @@
                 project=project, parent=rev3, revno=3, branch=dep2_url,
                 status=Revision.STATUS_UNKNOWN),
             ))
+
+    def test_do_not_attempt_to_unlock_if_failed_to_get_branch(self):
+        branch, url = self.make_commits(num_commits=2)
+        project = self.factory.make_one(Project, url=url, git_branch='master')
+        self.factory.make_one(
+            Revision, project=project, revno=1, parent=None, branch='',
+            status=Revision.STATUS_BAD, commit_message='nonsense 1')
+        tip_revision = self.factory.make_one(
+            Revision, project=project, revno=2, parent=None, branch='',
+            status=Revision.STATUS_BAD, commit_message='nonsense 2')
+        if self.vcs == VCS.VCS_GIT:
+            tip_revision.commit_sha1 = branch.revparse_single('HEAD').id.hex
+            tip_revision.save()
+
+        if self.vcs == VCS.VCS_BAZAAR:
+            branch_with_lock = tasks.get_branch(
+                url, project.name, git_branch=project.git_branch)
+            assert branch_with_lock.branch.is_locked()
+        try:
+            with captured_stdout():
+                tasks.update_revisions(project.pk)
+        finally:
+            if self.vcs == VCS.VCS_BAZAAR:
+                branch_with_lock.unlock()