[virt-tools-list] [virt-bootstrap] [PATCH v3 04/24] Use explicit import

Radostin Stoyanov rstoyanov1 at gmail.com
Wed Aug 2 12:08:18 UTC 2017


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




More information about the virt-tools-list mailing list