Commit 906bfde9 authored by Josip's avatar Josip Committed by LUCI CQ

Use logger where unit test has coverage

Change-Id: I0a4d16fef3aed6bba0c6015fe861a2f11558e11b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2029114
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: 's avatarAnthony Polito <apolito@google.com>
parent 42a51f6a
...@@ -140,9 +140,8 @@ def DieWithError(message, change_desc=None): ...@@ -140,9 +140,8 @@ def DieWithError(message, change_desc=None):
def SaveDescriptionBackup(change_desc): def SaveDescriptionBackup(change_desc):
backup_path = os.path.join(DEPOT_TOOLS, DESCRIPTION_BACKUP_FILE) backup_path = os.path.join(DEPOT_TOOLS, DESCRIPTION_BACKUP_FILE)
print('\nsaving CL description to %s\n' % backup_path) print('\nsaving CL description to %s\n' % backup_path)
backup_file = open(backup_path, 'w') with open(backup_path, 'w') as backup_file:
backup_file.write(change_desc.description) backup_file.write(change_desc.description)
backup_file.close()
def GetNoGitPagerEnv(): def GetNoGitPagerEnv():
...@@ -862,6 +861,7 @@ class Settings(object): ...@@ -862,6 +861,7 @@ class Settings(object):
cr_settings_file = FindCodereviewSettingsFile() cr_settings_file = FindCodereviewSettingsFile()
if autoupdate != 'false' and cr_settings_file: if autoupdate != 'false' and cr_settings_file:
LoadCodereviewSettingsFromFile(cr_settings_file) LoadCodereviewSettingsFromFile(cr_settings_file)
cr_settings_file.close()
self.updated = True self.updated = True
@staticmethod @staticmethod
...@@ -1344,8 +1344,9 @@ class Changelist(object): ...@@ -1344,8 +1344,9 @@ class Changelist(object):
'Interpreting it as a local directory.', url) 'Interpreting it as a local directory.', url)
if not os.path.isdir(url): if not os.path.isdir(url):
logging.error( logging.error(
'Remote "%s" for branch "%s" points to "%s", but it doesn\'t exist.', 'Remote "%(remote)s" for branch "%(branch)s" points to "%(url)s", '
remote, self.GetBranch(), url) 'but it doesn\'t exist.',
{'remote': remote, 'branch': self.GetBranch(), 'url': url})
return None return None
cache_path = url cache_path = url
...@@ -1703,7 +1704,7 @@ class Changelist(object): ...@@ -1703,7 +1704,7 @@ class Changelist(object):
"""Returns Gerrit project name based on remote git URL.""" """Returns Gerrit project name based on remote git URL."""
remote_url = self.GetRemoteUrl() remote_url = self.GetRemoteUrl()
if remote_url is None: if remote_url is None:
logging.warn('can\'t detect Gerrit project.') logging.warning('can\'t detect Gerrit project.')
return None return None
project = urllib.parse.urlparse(remote_url).path.strip('/') project = urllib.parse.urlparse(remote_url).path.strip('/')
if project.endswith('.git'): if project.endswith('.git'):
...@@ -1756,11 +1757,14 @@ class Changelist(object): ...@@ -1756,11 +1757,14 @@ class Changelist(object):
remote_url = self.GetRemoteUrl() remote_url = self.GetRemoteUrl()
if remote_url is None: if remote_url is None:
print('WARNING: invalid remote') logging.warning('invalid remote')
return return
if urllib.parse.urlparse(remote_url).scheme != 'https': if urllib.parse.urlparse(remote_url).scheme != 'https':
print('WARNING: Ignoring branch %s with non-https remote %s' % logging.warning('Ignoring branch %(branch)s with non-https remote '
(self.branch, self.GetRemoteUrl())) '%(remote)s', {
'branch': self.branch,
'remote': self.GetRemoteUrl()
})
return return
# Lazy-loader to identify Gerrit and Git hosts. # Lazy-loader to identify Gerrit and Git hosts.
......
...@@ -2094,9 +2094,17 @@ class TestGitCl(TestCase): ...@@ -2094,9 +2094,17 @@ class TestGitCl(TestCase):
((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'), ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'),
((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'branch.master.remote'],), 'origin'),
((['git', 'config', 'remote.origin.url'],), 'custom-scheme://repo'), ((['git', 'config', 'remote.origin.url'],), 'custom-scheme://repo'),
(('logging.warning',
'Ignoring branch %(branch)s with non-https remote '
'%(remote)s', {
'branch': 'master',
'remote': 'custom-scheme://repo'}
), None),
] ]
self.mock(git_cl.gerrit_util, 'CookiesAuthenticator', self.mock(git_cl.gerrit_util, 'CookiesAuthenticator',
CookiesAuthenticatorMockFactory(hosts_with_creds={})) CookiesAuthenticatorMockFactory(hosts_with_creds={}))
self.mock(logging, 'warning',
lambda *a: self._mocked_call('logging.warning', *a))
cl = git_cl.Changelist() cl = git_cl.Changelist()
cl.branch = 'master' cl.branch = 'master'
cl.branchref = 'refs/heads/master' cl.branchref = 'refs/heads/master'
...@@ -2111,9 +2119,18 @@ class TestGitCl(TestCase): ...@@ -2111,9 +2119,18 @@ class TestGitCl(TestCase):
((['git', 'config', 'branch.master.remote'], ), 'origin'), ((['git', 'config', 'branch.master.remote'], ), 'origin'),
((['git', 'config', 'remote.origin.url'], ), ((['git', 'config', 'remote.origin.url'], ),
'git@somehost.example:foo/bar.git'), 'git@somehost.example:foo/bar.git'),
(('logging.error',
'Remote "%(remote)s" for branch "%(branch)s" points to "%(url)s", '
'but it doesn\'t exist.', {
'remote': 'origin',
'branch': 'master',
'url': 'git@somehost.example:foo/bar.git'}
), None),
] ]
self.mock(git_cl.gerrit_util, 'CookiesAuthenticator', self.mock(git_cl.gerrit_util, 'CookiesAuthenticator',
CookiesAuthenticatorMockFactory(hosts_with_creds={})) CookiesAuthenticatorMockFactory(hosts_with_creds={}))
self.mock(logging, 'error',
lambda *a: self._mocked_call('logging.error', *a))
cl = git_cl.Changelist() cl = git_cl.Changelist()
cl.branch = 'master' cl.branch = 'master'
cl.branchref = 'refs/heads/master' cl.branchref = 'refs/heads/master'
...@@ -3031,7 +3048,7 @@ class TestGitCl(TestCase): ...@@ -3031,7 +3048,7 @@ class TestGitCl(TestCase):
self.mock(os.path, 'isdir', selective_os_path_isdir_mock) self.mock(os.path, 'isdir', selective_os_path_isdir_mock)
self.mock(logging, 'error', self.mock(logging, 'error',
lambda fmt, *a: self._mocked_call('logging.error', fmt % a)) lambda *a: self._mocked_call('logging.error', *a))
self.calls = [ self.calls = [
((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
...@@ -3042,8 +3059,12 @@ class TestGitCl(TestCase): ...@@ -3042,8 +3059,12 @@ class TestGitCl(TestCase):
(('os.path.isdir', '/cache/this-dir-doesnt-exist'), (('os.path.isdir', '/cache/this-dir-doesnt-exist'),
False), False),
(('logging.error', (('logging.error',
'Remote "origin" for branch "master" points to' 'Remote "%(remote)s" for branch "%(branch)s" points to "%(url)s", '
' "/cache/this-dir-doesnt-exist", but it doesn\'t exist.'), None), 'but it doesn\'t exist.', {
'remote': 'origin',
'branch': 'master',
'url': '/cache/this-dir-doesnt-exist'}
), None),
] ]
cl = git_cl.Changelist(issue=1) cl = git_cl.Changelist(issue=1)
self.assertIsNone(cl.GetRemoteUrl()) self.assertIsNone(cl.GetRemoteUrl())
...@@ -3094,11 +3115,21 @@ class TestGitCl(TestCase): ...@@ -3094,11 +3115,21 @@ class TestGitCl(TestCase):
self.assertEqual(cl._GerritChangeIdentifier(), 'my%2Frepo~123456') self.assertEqual(cl._GerritChangeIdentifier(), 'my%2Frepo~123456')
def test_gerrit_change_identifier_without_project(self): def test_gerrit_change_identifier_without_project(self):
self.mock(logging, 'error',
lambda *a: self._mocked_call('logging.error', *a))
self.calls = [ self.calls = [
((['git', 'symbolic-ref', 'HEAD'],), 'master'), ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
((['git', 'config', 'branch.master.merge'],), 'master'), ((['git', 'config', 'branch.master.merge'],), 'master'),
((['git', 'config', 'branch.master.remote'],), 'origin'), ((['git', 'config', 'branch.master.remote'],), 'origin'),
((['git', 'config', 'remote.origin.url'],), CERR1), ((['git', 'config', 'remote.origin.url'],), CERR1),
(('logging.error',
'Remote "%(remote)s" for branch "%(branch)s" points to "%(url)s", '
'but it doesn\'t exist.', {
'remote': 'origin',
'branch': 'master',
'url': ''}
), None),
] ]
cl = git_cl.Changelist(issue=123456) cl = git_cl.Changelist(issue=123456)
self.assertEqual(cl._GerritChangeIdentifier(), '123456') self.assertEqual(cl._GerritChangeIdentifier(), '123456')
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment