From 9249e01b702c6fe86f089ae0a8769a0f673b4a93 Mon Sep 17 00:00:00 2001 From: kmarshall Date: Tue, 23 Aug 2016 12:02:16 -0700 Subject: [PATCH] git cl archive: Add "--dry-run" and "--notags" flags. This CL adds a couple flags to "git cl archive". dry-run: Lists the cleanup tasks, but exits before any changes are made to Git. notags: Deletes branches only; does not create archival tags. R=tandrii@chromium.org,groby@chromium.org BUG= Review-Url: https://codereview.chromium.org/2276663002 --- git_cl.py | 35 +++++++++++++++----- tests/git_cl_test.py | 79 ++++++++++++++++++++++++++++++++------------ 2 files changed, 83 insertions(+), 31 deletions(-) diff --git a/git_cl.py b/git_cl.py index 99c101e85..42a5eabc1 100755 --- a/git_cl.py +++ b/git_cl.py @@ -3265,10 +3265,17 @@ def CMDarchive(parser, args): """Archives and deletes branches associated with closed changelists.""" parser.add_option( '-j', '--maxjobs', action='store', type=int, - help='The maximum number of jobs to use when retrieving review status') + help='The maximum number of jobs to use when retrieving review status.') parser.add_option( '-f', '--force', action='store_true', help='Bypasses the confirmation prompt.') + parser.add_option( + '-d', '--dry-run', action='store_true', + help='Skip the branch tagging and removal steps.') + parser.add_option( + '-t', '--notags', action='store_true', + help='Do not tag archived branches. ' + 'Note: local commit history may be lost.') auth.add_auth_options(parser) options, args = parser.parse_args(args) @@ -3300,26 +3307,36 @@ def CMDarchive(parser, args): current_branch = GetCurrentBranch() print('\nBranches with closed issues that will be archived:\n') - print('%*s | %s' % (alignment, 'Branch name', 'Archival tag name')) - for next_item in proposal: - print('%*s %s' % (alignment, next_item[0], next_item[1])) - - if any(branch == current_branch for branch, _ in proposal): + if options.notags: + for next_item in proposal: + print(' ' + next_item[0]) + else: + print('%*s | %s' % (alignment, 'Branch name', 'Archival tag name')) + for next_item in proposal: + print('%*s %s' % (alignment, next_item[0], next_item[1])) + + # Quit now on precondition failure or if instructed by the user, either + # via an interactive prompt or by command line flags. + if options.dry_run: + print('\nNo changes were made (dry run).\n') + return 0 + elif any(branch == current_branch for branch, _ in proposal): print('You are currently on a branch \'%s\' which is associated with a ' 'closed codereview issue, so archive cannot proceed. Please ' 'checkout another branch and run this command again.' % current_branch) return 1 - - if not options.force: + elif not options.force: answer = ask_for_data('\nProceed with deletion (Y/n)? ').lower() if answer not in ('y', ''): print('Aborted.') return 1 for branch, tagname in proposal: - RunGit(['tag', tagname, branch]) + if not options.notags: + RunGit(['tag', tagname, branch]) RunGit(['branch', '-D', branch]) + print('\nJob\'s done!') return 0 diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index f2c05a0d0..47eaaf7eb 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -126,6 +126,14 @@ def CookiesAuthenticatorMockFactory(hosts_with_creds=None, same_cookie=False): return (hosts_with_creds or {}).get(host) return CookiesAuthenticatorMock +class MockChangelistWithBranchAndIssue(): + def __init__(self, branch, issue): + self.branch = branch + self.issue = issue + def GetBranch(self): + return self.branch + def GetIssue(self): + return self.issue class TestGitClBasic(unittest.TestCase): def _test_ParseIssueUrl(self, func, url, issue, patchset, hostname, fail): @@ -1701,20 +1709,11 @@ class TestGitCl(TestCase): ((['git', 'tag', 'git-cl-archived-456-foo', 'foo'],), ''), ((['git', 'branch', '-D', 'foo'],), '')] - class MockChangelist(): - def __init__(self, branch, issue): - self.branch = branch - self.issue = issue - def GetBranch(self): - return self.branch - def GetIssue(self): - return self.issue - self.mock(git_cl, 'get_cl_statuses', lambda branches, fine_grained, max_processes: - [(MockChangelist('master', 1), 'open'), - (MockChangelist('foo', 456), 'closed'), - (MockChangelist('bar', 789), 'open')]) + [(MockChangelistWithBranchAndIssue('master', 1), 'open'), + (MockChangelistWithBranchAndIssue('foo', 456), 'closed'), + (MockChangelistWithBranchAndIssue('bar', 789), 'open')]) self.assertEqual(0, git_cl.main(['archive', '-f'])) @@ -1728,21 +1727,57 @@ class TestGitCl(TestCase): ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), ((['git', 'symbolic-ref', 'HEAD'],), 'master')] - class MockChangelist(): - def __init__(self, branch, issue): - self.branch = branch - self.issue = issue - def GetBranch(self): - return self.branch - def GetIssue(self): - return self.issue - self.mock(git_cl, 'get_cl_statuses', lambda branches, fine_grained, max_processes: - [(MockChangelist('master', 1), 'closed')]) + [(MockChangelistWithBranchAndIssue('master', 1), 'closed')]) self.assertEqual(1, git_cl.main(['archive', '-f'])) + def test_archive_dry_run(self): + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) + + self.calls = \ + [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), + 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), + ((['git', 'config', 'branch.master.rietveldissue'],), '1'), + ((['git', 'config', 'rietveld.autoupdate'],), CERR1), + ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), + ((['git', 'config', 'branch.foo.rietveldissue'],), '456'), + ((['git', 'config', 'branch.bar.rietveldissue'],), CERR1), + ((['git', 'config', 'branch.bar.gerritissue'],), '789'), + ((['git', 'symbolic-ref', 'HEAD'],), 'master'),] + + self.mock(git_cl, 'get_cl_statuses', + lambda branches, fine_grained, max_processes: + [(MockChangelistWithBranchAndIssue('master', 1), 'open'), + (MockChangelistWithBranchAndIssue('foo', 456), 'closed'), + (MockChangelistWithBranchAndIssue('bar', 789), 'open')]) + + self.assertEqual(0, git_cl.main(['archive', '-f', '--dry-run'])) + + def test_archive_no_tags(self): + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) + + self.calls = \ + [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), + 'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'), + ((['git', 'config', 'branch.master.rietveldissue'],), '1'), + ((['git', 'config', 'rietveld.autoupdate'],), CERR1), + ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), + ((['git', 'config', 'branch.foo.rietveldissue'],), '456'), + ((['git', 'config', 'branch.bar.rietveldissue'],), CERR1), + ((['git', 'config', 'branch.bar.gerritissue'],), '789'), + ((['git', 'symbolic-ref', 'HEAD'],), 'master'), + ((['git', 'branch', '-D', 'foo'],), '')] + + self.mock(git_cl, 'get_cl_statuses', + lambda branches, fine_grained, max_processes: + [(MockChangelistWithBranchAndIssue('master', 1), 'open'), + (MockChangelistWithBranchAndIssue('foo', 456), 'closed'), + (MockChangelistWithBranchAndIssue('bar', 789), 'open')]) + + self.assertEqual(0, git_cl.main(['archive', '-f', '--notags'])) + def test_cmd_issue_erase_existing(self): out = StringIO.StringIO() self.mock(git_cl.sys, 'stdout', out)