almalinux/pungi
14
1
Fork 0
Code Issues Pull Requests Releases Activity

Use universal_newlines when running other commands

Browse Source 
This will automatically convert the output to unicode/str and we will
not have to worry about decoding ourselves.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
This commit is contained in:
Lubomír Sedlář 2017-09-15 09:42:33 +02:00
parent ed22e07ef9
commit ed9d7f69a6
9 changed files with 93 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
pungi/util.py
View File

@ -617,7 +617,8 @@ def run_unmount_cmd(cmd, max_retries=10, path=None, logger=None):
printed in case of failure.
"""
for i in range(max_retries):
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
universal_newlines=True)
out, err = proc.communicate()
if proc.returncode == 0:
# We were successful
@ -634,7 +635,8 @@ def run_unmount_cmd(cmd, max_retries=10, path=None, logger=None):
]
for c in commands:
try:
proc = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
proc = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
universal_newlines=True)
out, _ = proc.communicate()
logger.debug('`%s` exited with %s and following output:\n%s',
' '.join(c), proc.returncode, out)
@ -801,7 +803,7 @@ def retry(timeout=120, interval=30, wait_on=Exception):
@retry(wait_on=RuntimeError)
def git_ls_remote(baseurl, ref):
return run(['git', 'ls-remote', baseurl, ref])
return run(['git', 'ls-remote', baseurl, ref], universal_newlines=True)
def get_tz_offset():

6
pungi/wrappers/iso.py
View File

@ -198,7 +198,7 @@ def get_checkisomd5_cmd(iso_path, just_print=False):
def get_checkisomd5_data(iso_path, logger=None):
cmd = get_checkisomd5_cmd(iso_path, just_print=True)
retcode, output = run(cmd)
retcode, output = run(cmd, universal_newlines=True)
items = [line.strip().rsplit(":", 1) for line in output.splitlines()]
items = dict([(k, v.strip()) for k, v in items])
md5 = items.get(iso_path, '')
@ -235,7 +235,7 @@ def get_manifest_cmd(iso_name):
def get_volume_id(path):
cmd = ["isoinfo", "-d", "-i", path]
retcode, output = run(cmd)
retcode, output = run(cmd, universal_newlines=True)
for line in output.splitlines():
line = line.strip()
@ -414,7 +414,7 @@ def mount(image, logger=None):
with util.temp_dir(prefix='iso-mount-') as mount_dir:
env = {'LIBGUESTFS_BACKEND': 'direct', 'LIBGUESTFS_DEBUG': '1', 'LIBGUESTFS_TRACE': '1'}
cmd = ["guestmount", "-a", image, "-m", "/dev/sda", mount_dir]
ret, out = run(cmd, env=env, can_fail=True)
ret, out = run(cmd, env=env, can_fail=True, universal_newlines=True)
if ret != 0:
# The mount command failed, something is wrong. Log the output and raise an exception.
if logger:

11
pungi/wrappers/kojiwrapper.py
View File

@ -137,7 +137,8 @@ class KojiWrapper(object):
"""
task_id = None
with self.get_koji_cmd_env() as env:
retcode, output = run(command, can_fail=True, logfile=log_file, show_cmd=True, env=env)
retcode, output = run(command, can_fail=True, logfile=log_file,
show_cmd=True, env=env, universal_newlines=True)
if "--task-id" in command:
first_line = output.splitlines()[0]
if re.match(r'^\d+$', first_line):
@ -300,7 +301,7 @@ class KojiWrapper(object):
attempt = 0
while True:
retcode, output = run(cmd, can_fail=True, logfile=logfile)
retcode, output = run(cmd, can_fail=True, logfile=logfile, universal_newlines=True)
if retcode == 0 or not self._has_connection_error(output):
# Task finished for reason other than connection error.
@ -320,7 +321,8 @@ class KojiWrapper(object):
command finishes.
"""
with self.get_koji_cmd_env() as env:
retcode, output = run(command, can_fail=True, logfile=log_file, env=env)
retcode, output = run(command, can_fail=True, logfile=log_file,
env=env, universal_newlines=True)
match = re.search(r"Created task: (\d+)", output)
if not match:
@ -508,7 +510,8 @@ def get_buildroot_rpms(compose, task_id):
result.append(fmt % rpm_info)
else:
# local
retcode, output = run("rpm -qa --qf='%{name}-%{version}-%{release}.%{arch}\n'")
retcode, output = run("rpm -qa --qf='%{name}-%{version}-%{release}.%{arch}\n'",
universal_newlines=True)
for i in output.splitlines():
if not i:
continue

3
pungi/wrappers/scm.py
View File

@ -44,7 +44,8 @@ class ScmBase(kobo.log.LoggingBase):
def run_process_command(self, cwd):
if self.command:
self.log_debug('Running "%s"' % self.command)
retcode, output = run(self.command, workdir=cwd, can_fail=True)
retcode, output = run(self.command, workdir=cwd, can_fail=True,
universal_newlines=True)
if retcode != 0:
self.log_error('Output was: "%s"' % output)
raise RuntimeError('%r failed with exit code %s'

5
pungi_utils/patch_iso.py
View File

@ -28,7 +28,7 @@ from pungi.wrappers import iso
def sh(log, cmd, *args, **kwargs):
log.info('Running: %s', ' '.join(quote(x) for x in cmd))
ret, out = shortcuts.run(cmd, *args, **kwargs)
ret, out = shortcuts.run(cmd, *args, universal_newlines=True, **kwargs)
if out:
log.debug('%s', out)
return ret, out
@ -36,7 +36,8 @@ def sh(log, cmd, *args, **kwargs):
def get_lorax_dir(default='/usr/share/lorax'):
try:
_, out = shortcuts.run(['python3', '-c' 'import pylorax; print(pylorax.find_templates())'])
_, out = shortcuts.run(['python3', '-c' 'import pylorax; print(pylorax.find_templates())'],
universal_newlines=True)
return out.strip()
except Exception:
return default

6
tests/test_iso_wrapper.py
View File

@ -26,7 +26,8 @@ class TestIsoUtils(unittest.TestCase):
self.assertEqual(iso.get_implanted_md5('dummy.iso', logger=logger),
'31ff3e405e26ad01c63b62f6b11d30f6')
self.assertEqual(mock_run.call_args_list,
[mock.call(['/usr/bin/checkisomd5', '--md5sumonly', 'dummy.iso'])])
[mock.call(['/usr/bin/checkisomd5', '--md5sumonly', 'dummy.iso'],
universal_newlines=True)])
self.assertEqual(logger.mock_calls, [])
@mock.patch('pungi.wrappers.iso.run')
@ -35,7 +36,8 @@ class TestIsoUtils(unittest.TestCase):
logger = mock.Mock()
self.assertIsNone(iso.get_implanted_md5('dummy.iso', logger=logger))
self.assertEqual(mock_run.call_args_list,
[mock.call(['/usr/bin/checkisomd5', '--md5sumonly', 'dummy.iso'])])
[mock.call(['/usr/bin/checkisomd5', '--md5sumonly', 'dummy.iso'],
universal_newlines=True)])
self.assertGreater(len(logger.mock_calls), 0)
@mock.patch('pungi.util.run_unmount_cmd')

55
tests/test_koji_wrapper.py
View File

@ -391,7 +391,8 @@ class RunrootKojiWrapperTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 0, 'output': output, 'task_id': None})
self.assertEqual(
run.call_args_list,
[mock.call(cmd, can_fail=True, env=None, logfile=None, show_cmd=True)]
[mock.call(cmd, can_fail=True, env=None, logfile=None, show_cmd=True,
universal_newlines=True)]
)
@mock.patch('pungi.wrappers.kojiwrapper.run')
@ -404,7 +405,8 @@ class RunrootKojiWrapperTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 0, 'output': output, 'task_id': 1234})
self.assertEqual(
run.call_args_list,
[mock.call(cmd, can_fail=True, env=None, logfile=None, show_cmd=True)]
[mock.call(cmd, can_fail=True, env=None, logfile=None, show_cmd=True,
universal_newlines=True)]
)
@mock.patch('pungi.wrappers.kojiwrapper.run')
@ -417,7 +419,8 @@ class RunrootKojiWrapperTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 1, 'output': output, 'task_id': None})
self.assertEqual(
run.call_args_list,
[mock.call(cmd, can_fail=True, env=None, logfile=None, show_cmd=True)]
[mock.call(cmd, can_fail=True, env=None, logfile=None, show_cmd=True,
universal_newlines=True)]
)
@mock.patch('pungi.wrappers.kojiwrapper.run')
@ -430,7 +433,8 @@ class RunrootKojiWrapperTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 1, 'output': output, 'task_id': 12345})
self.assertEqual(
run.call_args_list,
[mock.call(cmd, can_fail=True, env=None, logfile=None, show_cmd=True)]
[mock.call(cmd, can_fail=True, env=None, logfile=None, show_cmd=True,
universal_newlines=True)]
)
@mock.patch.dict('os.environ', {'FOO': 'BAR'}, clear=True)
@ -450,7 +454,7 @@ class RunrootKojiWrapperTest(KojiWrapperBaseTestCase):
self.assertEqual(
run.call_args_list,
[mock.call(cmd, can_fail=True, env={'KRB5CCNAME': 'DIR:/tmp/foo', 'FOO': 'BAR'},
logfile=None, show_cmd=True)]
logfile=None, show_cmd=True, universal_newlines=True)]
)
@ -464,7 +468,8 @@ class RunBlockingCmdTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 0, 'output': output, 'task_id': 1234})
self.assertItemsEqual(run.mock_calls,
[mock.call('cmd', can_fail=True, logfile=None, env=None)])
[mock.call('cmd', can_fail=True, logfile=None, env=None,
universal_newlines=True)])
@mock.patch.dict('os.environ', {'FOO': 'BAR'}, clear=True)
@mock.patch('pungi.util.temp_dir')
@ -480,7 +485,8 @@ class RunBlockingCmdTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 0, 'output': output, 'task_id': 1234})
self.assertItemsEqual(run.mock_calls,
[mock.call('cmd', can_fail=True, logfile=None,
env={'KRB5CCNAME': 'DIR:/tmp/foo', 'FOO': 'BAR'})])
env={'KRB5CCNAME': 'DIR:/tmp/foo', 'FOO': 'BAR'},
universal_newlines=True)])
@mock.patch('pungi.wrappers.kojiwrapper.run')
def test_with_log(self, run):
@ -491,7 +497,8 @@ class RunBlockingCmdTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 0, 'output': output, 'task_id': 1234})
self.assertItemsEqual(run.mock_calls,
[mock.call('cmd', can_fail=True, logfile='logfile', env=None)])
[mock.call('cmd', can_fail=True, logfile='logfile', env=None,
universal_newlines=True)])
@mock.patch('pungi.wrappers.kojiwrapper.run')
def test_fail_with_task_id(self, run):
@ -502,7 +509,8 @@ class RunBlockingCmdTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 1, 'output': output, 'task_id': 1234})
self.assertItemsEqual(run.mock_calls,
[mock.call('cmd', can_fail=True, logfile=None, env=None)])
[mock.call('cmd', can_fail=True, logfile=None, env=None,
universal_newlines=True)])
@mock.patch('pungi.wrappers.kojiwrapper.run')
def test_fail_without_task_id(self, run):
@ -513,7 +521,8 @@ class RunBlockingCmdTest(KojiWrapperBaseTestCase):
self.koji.run_blocking_cmd('cmd')
self.assertItemsEqual(run.mock_calls,
[mock.call('cmd', can_fail=True, logfile=None, env=None)])
[mock.call('cmd', can_fail=True, logfile=None, env=None,
universal_newlines=True)])
self.assertIn('Could not find task ID', str(ctx.exception))
@mock.patch('pungi.wrappers.kojiwrapper.run')
@ -526,9 +535,10 @@ class RunBlockingCmdTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 0, 'output': retry, 'task_id': 1234})
self.assertEqual(run.mock_calls,
[mock.call('cmd', can_fail=True, logfile=None, env=None),
[mock.call('cmd', can_fail=True, logfile=None, env=None,
universal_newlines=True),
mock.call(['koji', '--profile=custom-koji', 'watch-task', '1234'],
can_fail=True, logfile=None)])
can_fail=True, logfile=None, universal_newlines=True)])
@mock.patch('pungi.wrappers.kojiwrapper.run')
def test_disconnect_and_retry_but_fail(self, run):
@ -540,9 +550,10 @@ class RunBlockingCmdTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 1, 'output': retry, 'task_id': 1234})
self.assertEqual(run.mock_calls,
[mock.call('cmd', can_fail=True, logfile=None, env=None),
[mock.call('cmd', can_fail=True, logfile=None, env=None,
universal_newlines=True),
mock.call(['koji', '--profile=custom-koji', 'watch-task', '1234'],
can_fail=True, logfile=None)])
can_fail=True, logfile=None, universal_newlines=True)])
@mock.patch('time.sleep')
@mock.patch('pungi.wrappers.kojiwrapper.run')
@ -555,13 +566,14 @@ class RunBlockingCmdTest(KojiWrapperBaseTestCase):
self.assertDictEqual(result, {'retcode': 0, 'output': retry, 'task_id': 1234})
self.assertEqual(run.mock_calls,
[mock.call('cmd', can_fail=True, logfile=None, env=None),
[mock.call('cmd', can_fail=True, logfile=None, env=None,
universal_newlines=True),
mock.call(['koji', '--profile=custom-koji', 'watch-task', '1234'],
can_fail=True, logfile=None),
can_fail=True, logfile=None, universal_newlines=True),
mock.call(['koji', '--profile=custom-koji', 'watch-task', '1234'],
can_fail=True, logfile=None),
can_fail=True, logfile=None, universal_newlines=True),
mock.call(['koji', '--profile=custom-koji', 'watch-task', '1234'],
can_fail=True, logfile=None)])
can_fail=True, logfile=None, universal_newlines=True)])
self.assertEqual(sleep.mock_calls,
[mock.call(i * 10) for i in range(1, 3)])
@ -576,11 +588,12 @@ class RunBlockingCmdTest(KojiWrapperBaseTestCase):
self.assertIn('Failed to wait', str(ctx.exception))
self.assertEqual(run.mock_calls,
[mock.call('cmd', can_fail=True, logfile=None, env=None),
[mock.call('cmd', can_fail=True, logfile=None, env=None,
universal_newlines=True),
mock.call(['koji', '--profile=custom-koji', 'watch-task', '1234'],
can_fail=True, logfile=None),
can_fail=True, logfile=None, universal_newlines=True),
mock.call(['koji', '--profile=custom-koji', 'watch-task', '1234'],
can_fail=True, logfile=None)])
can_fail=True, logfile=None, universal_newlines=True)])
self.assertEqual(sleep.mock_calls, [mock.call(i * 10) for i in range(1, 2)])

2
tests/test_patch_iso.py
View File

@ -39,7 +39,7 @@ class TestSh(unittest.TestCase):
log = mock.Mock()
patch_iso.sh(log, ['ls'], foo='bar')
self.assertEqual(mock_run.call_args_list,
[mock.call(['ls'], foo='bar')])
[mock.call(['ls'], foo='bar', universal_newlines=True)])
self.assertEqual(log.info.call_args_list,
[mock.call('Running: %s', 'ls')])
self.assertEqual(log.debug.call_args_list,

51
tests/test_util.py
View File

@ -29,7 +29,8 @@ class TestGitRefResolver(unittest.TestCase):
url = util.resolve_git_url('https://git.example.com/repo.git?somedir#HEAD')
self.assertEqual(url, 'https://git.example.com/repo.git?somedir#CAFEBABE')
run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'HEAD'])
run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'HEAD'],
universal_newlines=True)
@mock.patch('pungi.util.run')
def test_successful_resolve_branch(self, run):
@ -38,7 +39,8 @@ class TestGitRefResolver(unittest.TestCase):
url = util.resolve_git_url('https://git.example.com/repo.git?somedir#origin/f24')
self.assertEqual(url, 'https://git.example.com/repo.git?somedir#CAFEBABE')
run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'refs/heads/f24'])
run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'refs/heads/f24'],
universal_newlines=True)
@mock.patch('pungi.util.run')
def test_resolve_missing_spec(self, run):
@ -61,7 +63,8 @@ class TestGitRefResolver(unittest.TestCase):
with self.assertRaises(RuntimeError):
util.resolve_git_url('https://git.example.com/repo.git?somedir#HEAD')
run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'HEAD'])
run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'HEAD'],
universal_newlines=True)
@mock.patch('pungi.util.run')
def test_resolve_keep_empty_query_string(self, run):
@ -69,7 +72,8 @@ class TestGitRefResolver(unittest.TestCase):
url = util.resolve_git_url('https://git.example.com/repo.git?#HEAD')
run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'HEAD'])
run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'HEAD'],
universal_newlines=True)
self.assertEqual(url, 'https://git.example.com/repo.git?#CAFEBABE')
@mock.patch('pungi.util.run')
@ -78,7 +82,8 @@ class TestGitRefResolver(unittest.TestCase):
url = util.resolve_git_url('git+https://git.example.com/repo.git#HEAD')
run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'HEAD'])
run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'HEAD'],
universal_newlines=True)
self.assertEqual(url, 'git+https://git.example.com/repo.git#CAFEBABE')
@mock.patch('pungi.util.run')
@ -89,7 +94,8 @@ class TestGitRefResolver(unittest.TestCase):
util.resolve_git_url('https://git.example.com/repo.git?somedir#origin/my-branch')
run.assert_called_once_with(
['git', 'ls-remote', 'https://git.example.com/repo.git', 'refs/heads/my-branch'])
['git', 'ls-remote', 'https://git.example.com/repo.git', 'refs/heads/my-branch'],
universal_newlines=True)
self.assertIn('ref does not exist in remote repo', str(ctx.exception))
@mock.patch('time.sleep')
@ -102,7 +108,8 @@ class TestGitRefResolver(unittest.TestCase):
self.assertEqual(url, 'https://git.example.com/repo.git?somedir#CAFEBABE')
self.assertEqual(sleep.call_args_list, [mock.call(30)])
self.assertEqual(run.call_args_list,
[mock.call(['git', 'ls-remote', 'https://git.example.com/repo.git', 'HEAD'])] * 2)
[mock.call(['git', 'ls-remote', 'https://git.example.com/repo.git', 'HEAD'],
universal_newlines=True)] * 2)
class TestGetVariantData(unittest.TestCase):
@ -373,7 +380,8 @@ class TestUnmountCmd(unittest.TestCase):
mockPopen.side_effect = [self._fakeProc(0, '')]
util.run_unmount_cmd(cmd)
self.assertEqual(mockPopen.call_args_list,
[mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE)])
[mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE,
universal_newlines=True)])
@mock.patch('subprocess.Popen')
def test_unmount_cmd_fail_other_reason(self, mockPopen):
@ -384,7 +392,8 @@ class TestUnmountCmd(unittest.TestCase):
self.assertEqual(str(ctx.exception),
"Unhandled error when running 'unmount': 'It is broken'")
self.assertEqual(mockPopen.call_args_list,
[mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE)])
[mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE,
universal_newlines=True)])
@mock.patch('time.sleep')
@mock.patch('subprocess.Popen')
@ -395,7 +404,8 @@ class TestUnmountCmd(unittest.TestCase):
self._fakeProc(0, '')]
util.run_unmount_cmd(cmd)
self.assertEqual(mockPopen.call_args_list,
[mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE)] * 3)
[mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE,
universal_newlines=True)] * 3)
self.assertEqual(mock_sleep.call_args_list,
[mock.call(0), mock.call(1)])
@ -409,7 +419,8 @@ class TestUnmountCmd(unittest.TestCase):
with self.assertRaises(RuntimeError) as ctx:
util.run_unmount_cmd(cmd, max_retries=3)
self.assertEqual(mockPopen.call_args_list,
[mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE)] * 3)
[mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE,
universal_newlines=True)] * 3)
self.assertEqual(mock_sleep.call_args_list,
[mock.call(0), mock.call(1), mock.call(2)])
self.assertEqual(str(ctx.exception), "Failed to run 'unmount': Device or resource busy.")
@ -427,15 +438,21 @@ class TestUnmountCmd(unittest.TestCase):
with self.assertRaises(RuntimeError) as ctx:
util.fusermount('/path', max_retries=3, logger=logger)
cmd = ['fusermount', '-u', '/path']
expected = [mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE),
mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE),
mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE),
expected = [mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE,
universal_newlines=True),
mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE,
universal_newlines=True),
mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE,
universal_newlines=True),
mock.call(['ls', '-lA', '/path'],
stderr=subprocess.STDOUT, stdout=subprocess.PIPE),
stderr=subprocess.STDOUT, stdout=subprocess.PIPE,
universal_newlines=True),
mock.call(['fuser', '-vm', '/path'],
stderr=subprocess.STDOUT, stdout=subprocess.PIPE),
stderr=subprocess.STDOUT, stdout=subprocess.PIPE,
universal_newlines=True),
mock.call(['lsof', '+D', '/path'],
stderr=subprocess.STDOUT, stdout=subprocess.PIPE)]
stderr=subprocess.STDOUT, stdout=subprocess.PIPE,
universal_newlines=True)]
self.assertEqual(mockPopen.call_args_list, expected)
self.assertEqual(mock_sleep.call_args_list,
[mock.call(0), mock.call(1), mock.call(2)])