Reduce the number of import statements and improve readability. Update the unit tests to match these changes. --- setup.py | 13 ++++++------- src/virtBootstrap/sources.py | 11 ++++++++--- src/virtBootstrap/utils.py | 32 ++++++++++++++++++++++-------- tests/test_docker_source.py | 4 ++-- tests/test_utils.py | 46 ++++++++++++++++++++++++++------------------ 5 files changed, 67 insertions(+), 39 deletions(-) diff --git a/setup.py b/setup.py index 2f299b6..2d06144 100755 --- a/setup.py +++ b/setup.py @@ -9,9 +9,8 @@ based on setuptools. import codecs import os import sys -from subprocess import call -from setuptools import Command -from setuptools import setup +import subprocess +import setuptools def read(fname): @@ -23,7 +22,7 @@ def read(fname): return fobj.read() -class CheckPylint(Command): +class CheckPylint(setuptools.Command): """ Check python source files with pylint and pycodestyle. """ @@ -55,7 +54,7 @@ class CheckPylint(Command): print(">>> Running pycodestyle ...") cmd = "pycodestyle " - if (call(cmd + files, shell=True) != 0): + if (subprocess.call(cmd + files, shell=True) != 0): res = 1 print(">>> Running pylint ...") @@ -63,13 +62,13 @@ class CheckPylint(Command): if self.errors_only: args = "-E" cmd = "pylint %s --output-format=%s " % (args, format(output_format)) - if (call(cmd + files, shell=True) != 0): + if (subprocess.call(cmd + files, shell=True) != 0): res = 1 sys.exit(res) -setup( +setuptools.setup( name='virt-bootstrap', version='0.1.0', author='Cedric Bosdonnat', diff --git a/src/virtBootstrap/sources.py b/src/virtBootstrap/sources.py index f4bae72..40b66f9 100644 --- a/src/virtBootstrap/sources.py +++ b/src/virtBootstrap/sources.py @@ -25,7 +25,7 @@ import shutil import getpass import os import logging -from subprocess import CalledProcessError, PIPE, Popen +import subprocess from virtBootstrap import utils @@ -259,12 +259,17 @@ class DockerSource(object): """ Parse the output from skopeo copy to track download progress. """ - proc = Popen(cmd, stdout=PIPE, stderr=PIPE, universal_newlines=True) + proc = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True + ) # Without `make_async`, `fd.read` in `read_async` blocks. utils.make_async(proc.stdout) if not self.parse_output(proc): - raise CalledProcessError(proc.returncode, ' '.join(cmd)) + raise subprocess.CalledProcessError(proc.returncode, ' '.join(cmd)) def validate_image_layers(self): """ diff --git a/src/virtBootstrap/utils.py b/src/virtBootstrap/utils.py index dbe4677..63ef57a 100644 --- a/src/virtBootstrap/utils.py +++ b/src/virtBootstrap/utils.py @@ -27,12 +27,12 @@ import fcntl import hashlib import json import os +import subprocess import sys import tempfile import logging import re -from subprocess import CalledProcessError, PIPE, Popen import passlib.hosts # pylint: disable=invalid-name @@ -80,7 +80,11 @@ def execute(cmd): cmd_str = ' '.join(cmd) logger.debug("Call command:\n%s", cmd_str) - proc = Popen(cmd, stdout=PIPE, stderr=PIPE) + proc = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE + ) output, err = proc.communicate() if output: @@ -89,7 +93,7 @@ def execute(cmd): logger.debug("Stderr:\n%s", err.decode('utf-8')) if proc.returncode != 0: - raise CalledProcessError(proc.returncode, cmd_str) + raise subprocess.CalledProcessError(proc.returncode, cmd_str) def safe_untar(src, dest): @@ -170,8 +174,12 @@ def get_mime_type(path): """ Get the mime type of a file. """ - return (Popen(["/usr/bin/file", "--mime-type", path], stdout=PIPE) - .stdout.read().decode('utf-8').split()[1]) + return ( + subprocess.Popen( + ["/usr/bin/file", "--mime-type", path], + stdout=subprocess.PIPE + ).stdout.read().decode('utf-8').split()[1] + ) def create_qcow2(tar_file, layer_file, backing_file=None, size=DEF_QCOW2_SIZE): @@ -273,7 +281,11 @@ def get_image_details(src, raw=False, cmd.append('--tls-verify=false') if username and password: cmd.append("--creds=%s:%s" % (username, password)) - proc = Popen(cmd, stdout=PIPE, stderr=PIPE) + proc = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE + ) output, error = proc.communicate() if error: raise ValueError("Image could not be retrieved:", @@ -396,8 +408,12 @@ def write_progress(prog): """ # Get terminal width try: - terminal_width = int(Popen(["stty", "size"], stdout=PIPE).stdout - .read().decode('utf-8').split()[1]) + terminal_width = int( + subprocess.Popen( + ["stty", "size"], + stdout=subprocess.PIPE + ).stdout.read().decode('utf-8').split()[1] + ) except Exception: terminal_width = 80 # Prepare message diff --git a/tests/test_docker_source.py b/tests/test_docker_source.py index 8108e31..4c52173 100644 --- a/tests/test_docker_source.py +++ b/tests/test_docker_source.py @@ -375,7 +375,7 @@ class TestDockerSource(unittest.TestCase): """ m_self = mock.Mock(spec=sources.DockerSource) m_self.parse_output.return_value = parse_output_return - with mock.patch.multiple('virtBootstrap.sources', + with mock.patch.multiple('virtBootstrap.sources.subprocess', Popen=mock.DEFAULT, PIPE=mock.DEFAULT) as mocked: with mock.patch('virtBootstrap.utils.make_async') as m_make_async: @@ -402,7 +402,7 @@ class TestDockerSource(unittest.TestCase): Ensures that read_skopeo_progress() raise CalledProcessError when parse_output() returns false. """ - with self.assertRaises(sources.CalledProcessError): + with self.assertRaises(sources.subprocess.CalledProcessError): self._mock_read_skopeo_progress('test', False) ################################### diff --git a/tests/test_utils.py b/tests/test_utils.py index aa9bdc2..0b6ccc0 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -90,12 +90,12 @@ class TestUtils(unittest.TestCase): """ with mock.patch.multiple(utils, logger=mock.DEFAULT, - Popen=mock.DEFAULT) as mocked: + subprocess=mock.DEFAULT) as mocked: cmd = ['foo'] output, err = 'test_out', 'test_err' - mocked['Popen'].return_value.returncode = 0 - (mocked['Popen'].return_value + mocked['subprocess'].Popen.return_value.returncode = 0 + (mocked['subprocess'].Popen.return_value .communicate.return_value) = (output.encode(), err.encode()) utils.execute(cmd) @@ -108,10 +108,10 @@ class TestUtils(unittest.TestCase): Ensures that execute() raise CalledProcessError exception when the exit code of process is not 0. """ - with mock.patch('virtBootstrap.utils.Popen') as m_popen: + with mock.patch('virtBootstrap.utils.subprocess.Popen') as m_popen: m_popen.return_value.returncode = 1 m_popen.return_value.communicate.return_value = (b'output', b'err') - with self.assertRaises(utils.CalledProcessError): + with self.assertRaises(utils.subprocess.CalledProcessError): utils.execute(['foo']) ################################### @@ -191,7 +191,7 @@ class TestUtils(unittest.TestCase): ################################### # Tests for: get_mime_type() ################################### - @mock.patch('virtBootstrap.utils.Popen') + @mock.patch('virtBootstrap.utils.subprocess.Popen') def test_utils_get_mime_type(self, m_popen): """ Ensures that get_mime_type() returns the detected MIME type @@ -202,8 +202,10 @@ class TestUtils(unittest.TestCase): stdout = ('%s: %s' % (path, mime)).encode() m_popen.return_value.stdout.read.return_value = stdout self.assertEqual(utils.get_mime_type(path), mime) - m_popen.assert_called_once_with(["/usr/bin/file", "--mime-type", path], - stdout=utils.PIPE) + m_popen.assert_called_once_with( + ["/usr/bin/file", "--mime-type", path], + stdout=utils.subprocess.PIPE + ) ################################### # Tests for: untar_layers() @@ -356,7 +358,7 @@ class TestUtils(unittest.TestCase): ################################### # Tests for: get_image_details() ################################### - @mock.patch('virtBootstrap.utils.Popen') + @mock.patch('virtBootstrap.utils.subprocess.Popen') def test_utils_get_image_details_raise_error_on_fail(self, m_popen): """ Ensures that get_image_details() throws ValueError exception @@ -367,7 +369,7 @@ class TestUtils(unittest.TestCase): with self.assertRaises(ValueError): utils.get_image_details(src) - @mock.patch('virtBootstrap.utils.Popen') + @mock.patch('virtBootstrap.utils.subprocess.Popen') def test_utils_get_image_details_return_json_obj_on_success(self, m_popen): """ Ensures that get_image_details() returns python dictionary which @@ -393,7 +395,7 @@ class TestUtils(unittest.TestCase): '--tls-verify=false', "--creds=%s:%s" % (username, password)] - with mock.patch.multiple(utils, + with mock.patch.multiple(utils.subprocess, Popen=mock.DEFAULT, PIPE=mock.DEFAULT) as mocked: mocked['Popen'].return_value.communicate.return_value = [b'{}', @@ -457,7 +459,11 @@ class TestUtils(unittest.TestCase): Ensures that make_async() sets O_NONBLOCK flag on PIPE. """ - pipe = utils.Popen(["echo"], stdout=utils.PIPE).stdout + pipe = utils.subprocess.Popen( + ["echo"], + stdout=utils.subprocess.PIPE + ).stdout + fd = pipe.fileno() F_GETFL = utils.fcntl.F_GETFL O_NONBLOCK = utils.os.O_NONBLOCK @@ -661,17 +667,18 @@ class TestUtils(unittest.TestCase): terminal_width = 120 prog = {'status': 'status', 'value': 0} with mock.patch.multiple(utils, - Popen=mock.DEFAULT, - PIPE=mock.DEFAULT, + subprocess=mock.DEFAULT, sys=mock.DEFAULT) as mocked: - (mocked['Popen'].return_value.stdout + (mocked['subprocess'].Popen.return_value.stdout .read.return_value) = ("20 %s" % terminal_width).encode() utils.write_progress(prog) - mocked['Popen'].assert_called_once_with(["stty", "size"], - stdout=mocked['PIPE']) + mocked['subprocess'].Popen.assert_called_once_with( + ["stty", "size"], + stdout=mocked['subprocess'].PIPE + ) output_message = mocked['sys'].stdout.write.call_args[0][0] mocked['sys'].stdout.write.assert_called_once() self.assertEqual(len(output_message), terminal_width + 1) @@ -685,9 +692,10 @@ class TestUtils(unittest.TestCase): """ default_terminal_width = 80 prog = {'status': 'status', 'value': 0} - with mock.patch.multiple(utils, Popen=mock.DEFAULT, + with mock.patch.multiple(utils, + subprocess=mock.DEFAULT, sys=mock.DEFAULT) as mocked: - mocked['Popen'].side_effect = Exception() + mocked['subprocess'].Popen.side_effect = Exception() utils.write_progress(prog) self.assertEqual(len(mocked['sys'].stdout.write.call_args[0][0]), -- 2.13.3 _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list