On Wed, 2017-08-02 at 13:08 +0100, Radostin Stoyanov wrote:
> 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]),
ACK
--
Cedric
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
