From 15a9b8cc5ddc0262ef293b2c419d609875c32b22 Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Thu, 13 Feb 2020 00:52:30 +0000 Subject: [PATCH] git-cl: Use scm.GIT.FetchUpstreamTuple. scm: - Add methods to deal with git configs and tests for them. Will make it easier to mock git config calls as opposed to all git calls. - Add tests to FetchUpstreamTuple git-cl: - Mock out settings.GetChangeRoot() - Use scm.GIT.FetchUpstreamTuple() to reduce code duplication, and mock it out on tests. Bug: 1051631 Change-Id: I1a3220b4c0f3e6221b3c00550259a433c2775798 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2052552 Commit-Queue: Edward Lesmes Reviewed-by: Anthony Polito --- git_cl.py | 33 ++++--------- scm.py | 90 ++++++++++++++++++++--------------- tests/git_cl_test.py | 96 ++++++++++++------------------------- tests/scm_unittest.py | 108 ++++++++++++++++++++++++++++++++++++++---- 4 files changed, 188 insertions(+), 139 deletions(-) diff --git a/git_cl.py b/git_cl.py index e53e8afdc..78ca488ee 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1194,30 +1194,15 @@ class Changelist(object): """Returns a tuple containing remote and remote ref, e.g. 'origin', 'refs/heads/master' """ - remote = '.' - upstream_branch = _git_get_branch_config_value('merge', branch=branch) - - if upstream_branch: - remote = _git_get_branch_config_value('remote', branch=branch) - else: - upstream_branch = RunGit(['config', 'rietveld.upstream-branch'], - error_ok=True).strip() - if upstream_branch: - remote = RunGit(['config', 'rietveld.upstream-remote']).strip() - else: - # Else, try to guess the origin remote. - remote_branches = RunGit(['branch', '-r']).split() - if 'origin/master' in remote_branches: - # Fall back on origin/master if it exits. - remote = 'origin' - upstream_branch = 'refs/heads/master' - else: - DieWithError( - 'Unable to determine default branch to diff against.\n' - 'Either pass complete "git diff"-style arguments, like\n' - ' git cl upload origin/master\n' - 'or verify this branch is set up to track another \n' - '(via the --track argument to "git checkout -b ...").') + remote, upstream_branch = scm.GIT.FetchUpstreamTuple( + settings.GetRoot(), branch) + if not remote or not upstream_branch: + DieWithError( + 'Unable to determine default branch to diff against.\n' + 'Either pass complete "git diff"-style arguments, like\n' + ' git cl upload origin/master\n' + 'or verify this branch is set up to track another \n' + '(via the --track argument to "git checkout -b ...").') return remote, upstream_branch diff --git a/scm.py b/scm.py index f81768bdf..3b34c5e68 100644 --- a/scm.py +++ b/scm.py @@ -143,6 +143,33 @@ class GIT(object): results.append(('%s ' % m.group(1)[0], m.group(2))) return results + @staticmethod + def GetConfig(cwd, key, default=None): + try: + return GIT.Capture(['config', key], cwd=cwd) + except subprocess2.CalledProcessError: + return default + + @staticmethod + def GetBranchConfig(cwd, branch, key, default=None): + assert branch, 'A branch must be given' + key = 'branch.%s.%s' % (branch, key) + return GIT.GetConfig(cwd, key, default) + + @staticmethod + def SetConfig(cwd, key, value=None): + if value is None: + args = ['config', '--unset', key] + else: + args = ['config', key, value] + GIT.Capture(args, cwd=cwd) + + @staticmethod + def SetBranchConfig(cwd, branch, key, value=None): + assert branch, 'A branch must be given' + key = 'branch.%s.%s' % (branch, key) + GIT.SetConfig(cwd, key, value) + @staticmethod def IsWorkTreeDirty(cwd): return GIT.Capture(['status', '-s'], cwd=cwd) != '' @@ -150,10 +177,7 @@ class GIT(object): @staticmethod def GetEmail(cwd): """Retrieves the user email address if known.""" - try: - return GIT.Capture(['config', 'user.email'], cwd=cwd) - except subprocess2.CalledProcessError: - return '' + return GIT.GetConfig(cwd, 'user.email', '') @staticmethod def ShortBranchName(branch): @@ -171,47 +195,35 @@ class GIT(object): return GIT.ShortBranchName(GIT.GetBranchRef(cwd)) @staticmethod - def FetchUpstreamTuple(cwd): + def GetRemoteBranches(cwd): + return GIT.Capture(['branch', '-r'], cwd=cwd).split() + + @staticmethod + def FetchUpstreamTuple(cwd, branch=None): """Returns a tuple containg remote and remote ref, e.g. 'origin', 'refs/heads/master' """ - remote = '.' - branch = GIT.GetBranch(cwd) try: - upstream_branch = GIT.Capture( - ['config', '--local', 'branch.%s.merge' % branch], cwd=cwd) + branch = branch or GIT.GetBranch(cwd) except subprocess2.CalledProcessError: - upstream_branch = None - if upstream_branch: - try: - remote = GIT.Capture( - ['config', '--local', 'branch.%s.remote' % branch], cwd=cwd) - except subprocess2.CalledProcessError: - pass - else: - try: - upstream_branch = GIT.Capture( - ['config', '--local', 'rietveld.upstream-branch'], cwd=cwd) - except subprocess2.CalledProcessError: - upstream_branch = None + pass + if branch: + upstream_branch = GIT.GetBranchConfig(cwd, branch, 'merge') if upstream_branch: - try: - remote = GIT.Capture( - ['config', '--local', 'rietveld.upstream-remote'], cwd=cwd) - except subprocess2.CalledProcessError: - pass - else: - # Else, try to guess the origin remote. - remote_branches = GIT.Capture(['branch', '-r'], cwd=cwd).split() - if 'origin/master' in remote_branches: - # Fall back on origin/master if it exits. - remote = 'origin' - upstream_branch = 'refs/heads/master' - else: - # Give up. - remote = None - upstream_branch = None - return remote, upstream_branch + remote = GIT.GetBranchConfig(cwd, branch, 'remote', '.') + return remote, upstream_branch + + upstream_branch = GIT.GetConfig(cwd, 'rietveld.upstream-branch') + if upstream_branch: + remote = GIT.GetConfig(cwd, 'rietveld.upstream-remote', '.') + return remote, upstream_branch + + # Else, try to guess the origin remote. + if 'origin/master' in GIT.GetRemoteBranches(cwd): + # Fall back on origin/master if it exits. + return 'origin', 'refs/heads/master' + + return None, None @staticmethod def RefToRemoteRef(ref, remote): diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 64a729e87..04d012f82 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -627,6 +627,10 @@ class TestGitCl(unittest.TestCase): mock.patch( 'git_cl.DieWithError', lambda msg, change=None: self._mocked_call('DieWithError', msg)).start() + mock.patch('git_cl.Settings.GetRoot', return_value='').start() + mock.patch( + 'git_cl.Changelist.FetchUpstreamTuple', + return_value=('origin', 'refs/heads/master')).start() # It's important to reset settings to not have inter-tests interference. git_cl.settings = None self.addCleanup(mock.patch.stopall) @@ -775,8 +779,6 @@ class TestGitCl(unittest.TestCase): ] calls.extend([ - ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'), - ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), 'https://%s.googlesource.com/my/repo' % short_hostname), ]) @@ -803,14 +805,13 @@ class TestGitCl(unittest.TestCase): ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ] + if custom_cl_base: ancestor_revision = custom_cl_base else: # Determine ancestor_revision to be merge base. ancestor_revision = 'fake_ancestor_sha' calls += [ - ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'), - ((['git', 'config', 'branch.master.remote'],), 'origin'), (('get_or_create_merge_base', 'master', 'refs/remotes/origin/master'), ancestor_revision), ] @@ -851,7 +852,6 @@ class TestGitCl(unittest.TestCase): calls += cls._git_sanity_checks(ancestor_revision, 'master', get_remote_branch=False) calls += [ - ((['git', 'rev-parse', '--show-cdup'],), ''), ((['git', 'rev-parse', 'HEAD'],), '12345'), ((['git', '-c', 'core.quotePath=false', 'diff', '--name-status', @@ -968,8 +968,6 @@ class TestGitCl(unittest.TestCase): ] ref_to_push = 'abcdef0123456789' calls += [ - ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'), - ((['git', 'config', 'branch.master.remote'],), 'origin'), ] if custom_cl_base is None: @@ -1754,7 +1752,7 @@ class TestGitCl(unittest.TestCase): self.assertNotEqual(git_cl.main(['patch', '123456']), 0) @staticmethod - def _get_gerrit_codereview_server_calls(branch, value=None, + def _get_gerrit_codereview_server_calls(value=None, git_short_host='host', detect_branch=True, detect_server=True): @@ -1764,16 +1762,12 @@ class TestGitCl(unittest.TestCase): """ calls = [] if detect_branch: - calls.append(((['git', 'symbolic-ref', 'HEAD'],), branch)) + calls.append(((['git', 'symbolic-ref', 'HEAD'],), 'master')) if detect_server: - calls.append(((['git', 'config', 'branch.' + branch + '.gerritserver'],), + calls.append(((['git', 'config', 'branch.master.gerritserver'],), CERR1 if value is None else value)) if value is None: calls += [ - ((['git', 'config', 'branch.' + branch + '.merge'],), - 'refs/heads' + branch), - ((['git', 'config', 'branch.' + branch + '.remote'],), - 'origin'), ((['git', 'config', 'remote.origin.url'],), 'https://%s.googlesource.com/my/repo' % git_short_host), ] @@ -1791,7 +1785,6 @@ class TestGitCl(unittest.TestCase): if codereview_in_url and actual_codereview == 'rietveld': self.calls += [ - ((['git', 'rev-parse', '--show-cdup'],), ''), ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ] @@ -1802,7 +1795,7 @@ class TestGitCl(unittest.TestCase): ] if detect_gerrit_server: self.calls += self._get_gerrit_codereview_server_calls( - 'master', git_short_host=git_short_host, + git_short_host=git_short_host, detect_branch=not new_branch and force_codereview) actual_codereview = 'gerrit' @@ -1886,7 +1879,7 @@ class TestGitCl(unittest.TestCase): def test_patch_gerrit_guess_by_url(self): self.calls += self._get_gerrit_codereview_server_calls( - 'master', git_short_host='else', detect_server=False) + git_short_host='else', detect_server=False) self._patch_common( actual_codereview='gerrit', git_short_host='else', codereview_in_url=True, detect_gerrit_server=False) @@ -1908,7 +1901,7 @@ class TestGitCl(unittest.TestCase): def test_patch_gerrit_guess_by_url_with_repo(self): self.calls += self._get_gerrit_codereview_server_calls( - 'master', git_short_host='else', detect_server=False) + git_short_host='else', detect_server=False) self._patch_common( actual_codereview='gerrit', git_short_host='else', codereview_in_url=True, detect_gerrit_server=False) @@ -1948,8 +1941,6 @@ class TestGitCl(unittest.TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'config', 'branch.master.gerritserver'],), CERR1), - ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'), - ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), 'https://chromium.googlesource.com/my/repo'), (('DieWithError', @@ -2051,8 +2042,6 @@ class TestGitCl(unittest.TestCase): self.calls = [ ((['git', 'config', '--bool', 'gerrit.skip-ensure-authenticated'],), CERR1), - ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'), - ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), 'custom-scheme://repo'), (('logging.warning', 'Ignoring branch %(branch)s with non-https remote ' @@ -2075,8 +2064,6 @@ class TestGitCl(unittest.TestCase): self.calls = [ ((['git', 'config', '--bool', 'gerrit.skip-ensure-authenticated'], ), CERR1), - ((['git', 'config', 'branch.master.merge'], ), 'refs/heads/master'), - ((['git', 'config', 'branch.master.remote'], ), 'origin'), ((['git', 'config', 'remote.origin.url'], ), 'git@somehost.example:foo/bar.git'), (('logging.error', @@ -2103,8 +2090,6 @@ class TestGitCl(unittest.TestCase): ((['git', 'config', 'branch.feature.gerritissue'],), '123'), ((['git', 'config', 'branch.feature.gerritserver'],), 'https://chromium-review.googlesource.com'), - ((['git', 'config', 'branch.feature.merge'],), 'refs/heads/master'), - ((['git', 'config', 'branch.feature.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), 'https://chromium.googlesource.com/infra/infra.git'), (('SetReview', 'chromium-review.googlesource.com', @@ -2165,8 +2150,6 @@ class TestGitCl(unittest.TestCase): def test_description(self): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.merge'],), 'feature'), - ((['git', 'config', 'branch.feature.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), 'https://chromium.googlesource.com/my/repo'), (('GetChangeDetail', 'chromium-review.googlesource.com', @@ -2412,25 +2395,25 @@ class TestGitCl(unittest.TestCase): self.assertEqual(0, git_cl.main(['issue', '--json', 'output.json'])) def _common_GerritCommitMsgHookCheck(self): - mock.patch('git_cl.os.path.abspath', - lambda path: self._mocked_call(['abspath', path])).start() - mock.patch('git_cl.os.path.exists', - lambda path: self._mocked_call(['exists', path])).start() - mock.patch('git_cl.gclient_utils.FileRead', - lambda path: self._mocked_call(['FileRead', path])).start() - mock.patch('git_cl.gclient_utils.rm_file_or_tree', - lambda path: self._mocked_call(['rm_file_or_tree', path])).start() - self.calls = [ - ((['git', 'rev-parse', '--show-cdup'],), '../'), - ((['abspath', '../'],), '/abs/git_repo_root'), - ] + mock.patch( + 'git_cl.os.path.abspath', + lambda path: self._mocked_call(['abspath', path])).start() + mock.patch( + 'git_cl.os.path.exists', + lambda path: self._mocked_call(['exists', path])).start() + mock.patch( + 'git_cl.gclient_utils.FileRead', + lambda path: self._mocked_call(['FileRead', path])).start() + mock.patch( + 'git_cl.gclient_utils.rm_file_or_tree', + lambda path: self._mocked_call(['rm_file_or_tree', path])).start() return git_cl.Changelist(issue=123) def test_GerritCommitMsgHookCheck_custom_hook(self): cl = self._common_GerritCommitMsgHookCheck() self.calls += [ - ((['exists', '/abs/git_repo_root/.git/hooks/commit-msg'],), True), - ((['FileRead', '/abs/git_repo_root/.git/hooks/commit-msg'],), + ((['exists', '.git/hooks/commit-msg'],), True), + ((['FileRead', '.git/hooks/commit-msg'],), '#!/bin/sh\necho "custom hook"') ] cl._GerritCommitMsgHookCheck(offer_removal=True) @@ -2438,18 +2421,18 @@ class TestGitCl(unittest.TestCase): def test_GerritCommitMsgHookCheck_not_exists(self): cl = self._common_GerritCommitMsgHookCheck() self.calls += [ - ((['exists', '/abs/git_repo_root/.git/hooks/commit-msg'],), False), + ((['exists', '.git/hooks/commit-msg'],), False), ] cl._GerritCommitMsgHookCheck(offer_removal=True) def test_GerritCommitMsgHookCheck(self): cl = self._common_GerritCommitMsgHookCheck() self.calls += [ - ((['exists', '/abs/git_repo_root/.git/hooks/commit-msg'],), True), - ((['FileRead', '/abs/git_repo_root/.git/hooks/commit-msg'],), + ((['exists', '.git/hooks/commit-msg'],), True), + ((['FileRead', '.git/hooks/commit-msg'],), '...\n# From Gerrit Code Review\n...\nadd_ChangeId()\n'), (('ask_for_data', 'Do you want to remove it now? [Yes/No]: '), 'Yes'), - ((['rm_file_or_tree', '/abs/git_repo_root/.git/hooks/commit-msg'],), + ((['rm_file_or_tree', '.git/hooks/commit-msg'],), ''), ] cl._GerritCommitMsgHookCheck(offer_removal=True) @@ -2645,9 +2628,6 @@ class TestGitCl(unittest.TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), CERR1), ((['git', 'symbolic-ref', 'HEAD'],), CERR1), - ((['git', 'config', 'rietveld.upstream-branch'],), CERR1), - ((['git', 'branch', '-r'],), 'origin/HEAD -> origin/master\n' - 'origin/master'), ((['git', 'config', 'remote.origin.url'],), 'https://chromium.googlesource.com/infra/infra'), (('SetReview', 'chromium-review.googlesource.com', 'infra%2Finfra~10', @@ -2660,10 +2640,6 @@ class TestGitCl(unittest.TestCase): def test_git_cl_comments_fetch_gerrit(self, *_mocks): self.calls = [ ((['git', 'config', 'branch.foo.gerritserver'],), ''), - ((['git', 'config', 'branch.foo.merge'],), ''), - ((['git', 'config', 'rietveld.upstream-branch'],), CERR1), - ((['git', 'branch', '-r'],), 'origin/HEAD -> origin/master\n' - 'origin/master'), ((['git', 'config', 'remote.origin.url'],), 'https://chromium.googlesource.com/infra/infra'), (('GetChangeDetail', 'chromium-review.googlesource.com', @@ -2809,10 +2785,6 @@ class TestGitCl(unittest.TestCase): # comments from the latest patchset are shown. self.calls = [ ((['git', 'config', 'branch.foo.gerritserver'],), ''), - ((['git', 'config', 'branch.foo.merge'],), ''), - ((['git', 'config', 'rietveld.upstream-branch'],), CERR1), - ((['git', 'branch', '-r'],), 'origin/HEAD -> origin/master\n' - 'origin/master'), ((['git', 'config', 'remote.origin.url'],), 'https://chromium.googlesource.com/infra/infra'), (('GetChangeDetail', 'chromium-review.googlesource.com', @@ -2927,8 +2899,6 @@ class TestGitCl(unittest.TestCase): url = 'https://chromium.googlesource.com/my/repo' self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.merge'],), 'master'), - ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), '/cache/this-dir-exists'), (('os.path.isdir', '/cache/this-dir-exists'), @@ -2955,8 +2925,6 @@ class TestGitCl(unittest.TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.merge'],), 'master'), - ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), '/cache/this-dir-doesnt-exist'), (('os.path.isdir', '/cache/this-dir-doesnt-exist'), @@ -2986,8 +2954,6 @@ class TestGitCl(unittest.TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.merge'],), 'master'), - ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), '/cache/this-dir-exists'), (('os.path.isdir', '/cache/this-dir-exists'), True), @@ -3009,8 +2975,6 @@ class TestGitCl(unittest.TestCase): def test_gerrit_change_identifier_with_project(self): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.merge'],), 'master'), - ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), 'https://chromium.googlesource.com/a/my/repo.git/'), ] @@ -3023,8 +2987,6 @@ class TestGitCl(unittest.TestCase): self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'master'), - ((['git', 'config', 'branch.master.merge'],), 'master'), - ((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'remote.origin.url'],), CERR1), (('logging.error', 'Remote "%(remote)s" for branch "%(branch)s" points to "%(url)s", ' diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 03fc43291..53ac6205f 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -23,6 +23,10 @@ import scm import subprocess2 +def callError(code=1, cmd='', cwd='', stdout=b'', stderr=b''): + return subprocess2.CalledProcessError(code, cmd, cwd, stdout, stderr) + + class GitWrapperTestCase(unittest.TestCase): def setUp(self): super(GitWrapperTestCase, self).setUp() @@ -84,29 +88,115 @@ class RealGitTest(fake_repos.FakeReposTestBase): super(RealGitTest, self).setUp() self.enabled = self.FAKE_REPOS.set_up_git() if self.enabled: - self.clone_dir = scm.os.path.join(self.FAKE_REPOS.git_base, 'repo_1') + self.cwd = scm.os.path.join(self.FAKE_REPOS.git_base, 'repo_1') else: self.skipTest('git fake repos not available') def testIsValidRevision(self): # Sha1's are [0-9a-z]{32}, so starting with a 'z' or 'r' should always fail. - self.assertFalse(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev='zebra')) - self.assertFalse(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev='r123456')) + self.assertFalse(scm.GIT.IsValidRevision(cwd=self.cwd, rev='zebra')) + self.assertFalse(scm.GIT.IsValidRevision(cwd=self.cwd, rev='r123456')) # Valid cases first_rev = self.githash('repo_1', 1) - self.assertTrue(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev=first_rev)) - self.assertTrue(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev='HEAD')) + self.assertTrue(scm.GIT.IsValidRevision(cwd=self.cwd, rev=first_rev)) + self.assertTrue(scm.GIT.IsValidRevision(cwd=self.cwd, rev='HEAD')) def testIsAncestor(self): self.assertTrue(scm.GIT.IsAncestor( - self.clone_dir, self.githash('repo_1', 1), self.githash('repo_1', 2))) + self.cwd, self.githash('repo_1', 1), self.githash('repo_1', 2))) self.assertFalse(scm.GIT.IsAncestor( - self.clone_dir, self.githash('repo_1', 2), self.githash('repo_1', 1))) + self.cwd, self.githash('repo_1', 2), self.githash('repo_1', 1))) self.assertFalse(scm.GIT.IsAncestor( - self.clone_dir, self.githash('repo_1', 1), 'zebra')) + self.cwd, self.githash('repo_1', 1), 'zebra')) def testGetAllFiles(self): - self.assertEqual(['DEPS','origin'], scm.GIT.GetAllFiles(self.clone_dir)) + self.assertEqual(['DEPS','origin'], scm.GIT.GetAllFiles(self.cwd)) + + def testGetSetConfig(self): + key = 'scm.test-key' + + self.assertIsNone(scm.GIT.GetConfig(self.cwd, key)) + self.assertEqual( + 'default-value', scm.GIT.GetConfig(self.cwd, key, 'default-value')) + + scm.GIT.SetConfig(self.cwd, key, 'set-value') + self.assertEqual('set-value', scm.GIT.GetConfig(self.cwd, key)) + self.assertEqual( + 'set-value', scm.GIT.GetConfig(self.cwd, key, 'default-value')) + + scm.GIT.SetConfig(self.cwd, key) + self.assertIsNone(scm.GIT.GetConfig(self.cwd, key)) + self.assertEqual( + 'default-value', scm.GIT.GetConfig(self.cwd, key, 'default-value')) + + def testGetSetBranchConfig(self): + branch = scm.GIT.GetBranch(self.cwd) + key = 'scm.test-key' + + self.assertIsNone(scm.GIT.GetBranchConfig(self.cwd, branch, key)) + self.assertEqual( + 'default-value', + scm.GIT.GetBranchConfig(self.cwd, branch, key, 'default-value')) + + scm.GIT.SetBranchConfig(self.cwd, branch, key, 'set-value') + self.assertEqual( + 'set-value', scm.GIT.GetBranchConfig(self.cwd, branch, key)) + self.assertEqual( + 'set-value', + scm.GIT.GetBranchConfig(self.cwd, branch, key, 'default-value')) + self.assertEqual( + 'set-value', + scm.GIT.GetConfig(self.cwd, 'branch.%s.%s' % (branch, key))) + + scm.GIT.SetBranchConfig(self.cwd, branch, key) + self.assertIsNone(scm.GIT.GetBranchConfig(self.cwd, branch, key)) + + def testFetchUpstreamTuple_NoUpstreamFound(self): + self.assertEqual( + (None, None), scm.GIT.FetchUpstreamTuple(self.cwd)) + + @mock.patch('scm.GIT.GetRemoteBranches', return_value=['origin/master']) + def testFetchUpstreamTuple_GuessOriginMaster(self, _mockGetRemoteBranches): + self.assertEqual( + ('origin', 'refs/heads/master'), scm.GIT.FetchUpstreamTuple(self.cwd)) + + def testFetchUpstreamTuple_RietveldUpstreamConfig(self): + scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-branch', 'rietveld-upstream') + scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-remote', 'rietveld-remote') + self.assertEqual( + ('rietveld-remote', 'rietveld-upstream'), + scm.GIT.FetchUpstreamTuple(self.cwd)) + scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-branch') + scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-remote') + + @mock.patch('scm.GIT.GetBranch', side_effect=callError()) + def testFetchUpstreamTuple_NotOnBranch(self, _mockGetBranch): + scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-branch', 'rietveld-upstream') + scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-remote', 'rietveld-remote') + self.assertEqual( + ('rietveld-remote', 'rietveld-upstream'), + scm.GIT.FetchUpstreamTuple(self.cwd)) + scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-branch') + scm.GIT.SetConfig(self.cwd, 'rietveld.upstream-remote') + + def testFetchUpstreamTuple_BranchConfig(self): + branch = scm.GIT.GetBranch(self.cwd) + scm.GIT.SetBranchConfig(self.cwd, branch, 'merge', 'branch-merge') + scm.GIT.SetBranchConfig(self.cwd, branch, 'remote', 'branch-remote') + self.assertEqual( + ('branch-remote', 'branch-merge'), scm.GIT.FetchUpstreamTuple(self.cwd)) + scm.GIT.SetBranchConfig(self.cwd, branch, 'merge') + scm.GIT.SetBranchConfig(self.cwd, branch, 'remote') + + def testFetchUpstreamTuple_AnotherBranchConfig(self): + branch = 'scm-test-branch' + scm.GIT.SetBranchConfig(self.cwd, branch, 'merge', 'other-merge') + scm.GIT.SetBranchConfig(self.cwd, branch, 'remote', 'other-remote') + self.assertEqual( + ('other-remote', 'other-merge'), + scm.GIT.FetchUpstreamTuple(self.cwd, branch)) + scm.GIT.SetBranchConfig(self.cwd, branch, 'merge') + scm.GIT.SetBranchConfig(self.cwd, branch, 'remote') if __name__ == '__main__':