diff --git a/project.py b/project.py index 5ffa542..7811d6b 100755 --- a/project.py +++ b/project.py @@ -134,6 +134,7 @@ class DownloadedChange(object): class ReviewableBranch(object): _commit_cache = None + _base_exists = None def __init__(self, project, branch, base): self.project = project @@ -147,14 +148,19 @@ class ReviewableBranch(object): @property def commits(self): if self._commit_cache is None: - self._commit_cache = self.project.bare_git.rev_list('--abbrev=8', - '--abbrev-commit', - '--pretty=oneline', - '--reverse', - '--date-order', - not_rev(self.base), - R_HEADS + self.name, - '--') + args = ('--abbrev=8', '--abbrev-commit', '--pretty=oneline', '--reverse', + '--date-order', not_rev(self.base), R_HEADS + self.name, '--') + try: + self._commit_cache = self.project.bare_git.rev_list(*args) + except GitError: + # We weren't able to probe the commits for this branch. Was it tracking + # a branch that no longer exists? If so, return no commits. Otherwise, + # rethrow the error as we don't know what's going on. + if self.base_exists: + raise + + self._commit_cache = [] + return self._commit_cache @property @@ -173,6 +179,23 @@ class ReviewableBranch(object): R_HEADS + self.name, '--') + @property + def base_exists(self): + """Whether the branch we're tracking exists. + + Normally it should, but sometimes branches we track can get deleted. + """ + if self._base_exists is None: + try: + self.project.bare_git.rev_parse('--verify', not_rev(self.base)) + # If we're still here, the base branch exists. + self._base_exists = True + except GitError: + # If we failed to verify, the base branch doesn't exist. + self._base_exists = False + + return self._base_exists + def UploadForReview(self, people, auto_topic=False, draft=False, diff --git a/subcmds/prune.py b/subcmds/prune.py index dc8b8c9..ff2fba1 100644 --- a/subcmds/prune.py +++ b/subcmds/prune.py @@ -51,11 +51,16 @@ class Prune(PagedCommand): out.project('project %s/' % project.relpath) out.nl() - commits = branch.commits - date = branch.date - print('%s %-33s (%2d commit%s, %s)' % ( + print('%s %-33s ' % ( branch.name == project.CurrentBranch and '*' or ' ', - branch.name, + branch.name), end='') + + if not branch.base_exists: + print('(ignoring: tracking branch is gone: %s)' % (branch.base,)) + else: + commits = branch.commits + date = branch.date + print('(%2d commit%s, %s)' % ( len(commits), len(commits) != 1 and 's' or ' ', date)) diff --git a/tests/test_project.py b/tests/test_project.py index 9b289e1..77126df 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -18,11 +18,30 @@ from __future__ import print_function +import contextlib +import os +import shutil +import subprocess +import tempfile import unittest +import git_config import project +@contextlib.contextmanager +def TempGitTree(): + """Create a new empty git checkout for testing.""" + # TODO(vapier): Convert this to tempfile.TemporaryDirectory once we drop + # Python 2 support entirely. + try: + tempdir = tempfile.mkdtemp(prefix='repo-tests') + subprocess.check_call(['git', 'init'], cwd=tempdir) + yield tempdir + finally: + shutil.rmtree(tempdir) + + class RepoHookShebang(unittest.TestCase): """Check shebang parsing in RepoHook.""" @@ -60,3 +79,58 @@ class RepoHookShebang(unittest.TestCase): for shebang, interp in DATA: self.assertEqual(project.RepoHook._ExtractInterpFromShebang(shebang), interp) + + +class FakeProject(object): + """A fake for Project for basic functionality.""" + + def __init__(self, worktree): + self.worktree = worktree + self.gitdir = os.path.join(worktree, '.git') + self.name = 'fakeproject' + self.work_git = project.Project._GitGetByExec( + self, bare=False, gitdir=self.gitdir) + self.bare_git = project.Project._GitGetByExec( + self, bare=True, gitdir=self.gitdir) + self.config = git_config.GitConfig.ForRepository(gitdir=self.gitdir) + + +class ReviewableBranchTests(unittest.TestCase): + """Check ReviewableBranch behavior.""" + + def test_smoke(self): + """A quick run through everything.""" + with TempGitTree() as tempdir: + fakeproj = FakeProject(tempdir) + + # Generate some commits. + with open(os.path.join(tempdir, 'readme'), 'w') as fp: + fp.write('txt') + fakeproj.work_git.add('readme') + fakeproj.work_git.commit('-mAdd file') + fakeproj.work_git.checkout('-b', 'work') + fakeproj.work_git.rm('-f', 'readme') + fakeproj.work_git.commit('-mDel file') + + # Start off with the normal details. + rb = project.ReviewableBranch( + fakeproj, fakeproj.config.GetBranch('work'), 'master') + self.assertEqual('work', rb.name) + self.assertEqual(1, len(rb.commits)) + self.assertIn('Del file', rb.commits[0]) + d = rb.unabbrev_commits + self.assertEqual(1, len(d)) + short, long = next(iter(d.items())) + self.assertTrue(long.startswith(short)) + self.assertTrue(rb.base_exists) + # Hard to assert anything useful about this. + self.assertTrue(rb.date) + + # Now delete the tracking branch! + fakeproj.work_git.branch('-D', 'master') + rb = project.ReviewableBranch( + fakeproj, fakeproj.config.GetBranch('work'), 'master') + self.assertEqual(0, len(rb.commits)) + self.assertFalse(rb.base_exists) + # Hard to assert anything useful about this. + self.assertTrue(rb.date)