From 090b1a166bd19bdb98b0311d58b85582bd1676ed Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 8 Nov 2017 12:41:36 +0000 Subject: Replace subprocess "check_output" with "Popen" "check_output" is a blocking wrapper for "Popen" which returns the output of the command execution or raises an exception in case of error. "Popen" is a non-blocking function that allows to create asynchronous tasks. It returns any possible execution error but doesn't raise an exception; this is delegated to the developer. This code is used in the Yardstick CLI base test class. Change-Id: Ie3e1228b2d40cb306354447653678bf581bc9697 Signed-off-by: Rodolfo Alonso Hernandez --- tests/functional/utils.py | 44 +++++----------- tests/unit/common/test_process.py | 106 +++++++++++++++++++++++++++++++++++++- tests/unit/common/test_utils.py | 33 ++++++++---- 3 files changed, 142 insertions(+), 41 deletions(-) (limited to 'tests') diff --git a/tests/functional/utils.py b/tests/functional/utils.py index b96d2dd50..d889c0dfa 100755 --- a/tests/functional/utils.py +++ b/tests/functional/utils.py @@ -7,14 +7,12 @@ # http://www.apache.org/licenses/LICENSE-2.0 ############################################################################## -from __future__ import absolute_import - import copy import os -import subprocess from oslo_serialization import jsonutils -from oslo_utils import encodeutils + +from yardstick.common import process class Yardstick(object): @@ -26,38 +24,22 @@ class Yardstick(object): """ - def __init__(self, fake=False): - - self.args = ["yardstick"] + def __init__(self): + self._args = ["yardstick"] self.env = copy.deepcopy(os.environ) - def __del__(self): - pass - - def __call__(self, cmd, getjson=False, report_path=None, raw=False, - suffix=None, extension=None, keep_old=False, - write_report=False): + def __call__(self, cmd, getjson=False): """Call yardstick in the shell - :param cmd: yardstick command - :param getjson: in cases, when yardstick prints JSON, you can catch - output deserialized - TO DO: - :param report_path: if present, yardstick command and its output will - be written to file with passed file name - :param raw: don't write command itself to report file. Only output - will be written + :param cmd: Yardstick command. + :param getjson: If the output is a JSON object, it's deserialized. + :return Command output string. """ if not isinstance(cmd, list): cmd = cmd.split(" ") - try: - output = encodeutils.safe_decode(subprocess.check_output( - self.args + cmd, stderr=subprocess.STDOUT, env=self.env), - 'utf-8') - - if getjson: - return jsonutils.loads(output) - return output - except subprocess.CalledProcessError as e: - raise e + cmd = self._args + cmd + output = process.execute(cmd=cmd) + if getjson: + return jsonutils.loads(output) + return output diff --git a/tests/unit/common/test_process.py b/tests/unit/common/test_process.py index 5eee55bcc..1c6dfec27 100644 --- a/tests/unit/common/test_process.py +++ b/tests/unit/common/test_process.py @@ -11,10 +11,13 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import unittest import mock +import unittest + +from oslo_utils import encodeutils +from yardstick.common import exceptions from yardstick.common import process @@ -44,3 +47,104 @@ class TerminateChildrenTestcase(unittest.TestCase): def test_no_children(self, mock_multiprocessing): mock_multiprocessing.active_children.return_value = [] process.terminate_children() + + +class ExecuteTestCase(unittest.TestCase): + + RET_CODE_OK = 0 + RET_CODE_WRONG = 1 + + def setUp(self): + self._mock_create_process = mock.patch.object(process, + 'create_process') + self.mock_create_process = self._mock_create_process.start() + self.obj = mock.Mock() + self.cmd = mock.Mock() + self.obj.communicate = mock.Mock() + self.stdout = 'std out' + self.stderr = 'std err' + self.obj.communicate.return_value = (self.stdout, self.stderr) + self.mock_create_process.return_value = (self.obj, self.cmd) + self.input_cmd = 'input cmd' + self.additional_env = mock.Mock() + + def test_execute_with_input(self): + process_input = 'process input' + self.obj.returncode = self.RET_CODE_OK + out = process.execute(self.input_cmd, process_input=process_input, + additional_env=self.additional_env) + self.obj.communicate.assert_called_once_with( + encodeutils.to_utf8(process_input)) + self.mock_create_process.assert_called_once_with( + self.input_cmd, run_as_root=False, + additional_env=self.additional_env) + self.assertEqual(self.stdout, out) + + def test_execute_no_input(self): + self.obj.returncode = self.RET_CODE_OK + out = process.execute(self.input_cmd, + additional_env=self.additional_env) + self.obj.communicate.assert_called_once_with(None) + self.mock_create_process.assert_called_once_with( + self.input_cmd, run_as_root=False, + additional_env=self.additional_env) + self.assertEqual(self.stdout, out) + + def test_execute_exception(self): + self.obj.returncode = self.RET_CODE_WRONG + self.assertRaises(exceptions.ProcessExecutionError, process.execute, + self.input_cmd, additional_env=self.additional_env) + self.obj.communicate.assert_called_once_with(None) + + def test_execute_with_extra_code(self): + self.obj.returncode = self.RET_CODE_WRONG + out = process.execute(self.input_cmd, + additional_env=self.additional_env, + extra_ok_codes=[self.RET_CODE_WRONG]) + self.obj.communicate.assert_called_once_with(None) + self.mock_create_process.assert_called_once_with( + self.input_cmd, run_as_root=False, + additional_env=self.additional_env) + self.assertEqual(self.stdout, out) + + def test_execute_exception_no_check(self): + self.obj.returncode = self.RET_CODE_WRONG + out = process.execute(self.input_cmd, + additional_env=self.additional_env, + check_exit_code=False) + self.obj.communicate.assert_called_once_with(None) + self.mock_create_process.assert_called_once_with( + self.input_cmd, run_as_root=False, + additional_env=self.additional_env) + self.assertEqual(self.stdout, out) + + +class CreateProcessTestCase(unittest.TestCase): + + @mock.patch.object(process, 'subprocess_popen') + def test_process_string_command(self, mock_subprocess_popen): + cmd = 'command' + obj = mock.Mock() + mock_subprocess_popen.return_value = obj + out1, out2 = process.create_process(cmd) + self.assertEqual(obj, out1) + self.assertEqual([cmd], out2) + + @mock.patch.object(process, 'subprocess_popen') + def test_process_list_command(self, mock_subprocess_popen): + cmd = ['command'] + obj = mock.Mock() + mock_subprocess_popen.return_value = obj + out1, out2 = process.create_process(cmd) + self.assertEqual(obj, out1) + self.assertEqual(cmd, out2) + + @mock.patch.object(process, 'subprocess_popen') + def test_process_with_env(self, mock_subprocess_popen): + cmd = ['command'] + obj = mock.Mock() + additional_env = {'var1': 'value1'} + mock_subprocess_popen.return_value = obj + out1, out2 = process.create_process(cmd, additional_env=additional_env) + self.assertEqual(obj, out1) + self.assertEqual(['env', 'var1=value1'] + cmd, out2) diff --git a/tests/unit/common/test_utils.py b/tests/unit/common/test_utils.py index 42b75d1f0..452b93a56 100644 --- a/tests/unit/common/test_utils.py +++ b/tests/unit/common/test_utils.py @@ -11,15 +11,15 @@ from __future__ import absolute_import -import ipaddress -import os -import unittest from copy import deepcopy -from itertools import product, chain - import errno +import ipaddress +from itertools import product, chain import mock +import os +import six from six.moves import configparser +import unittest import yardstick from yardstick.common import utils @@ -775,7 +775,8 @@ class RemoveFileTestCase(unittest.TestCase): def test_remove_file(self): try: utils.remove_file('notexistfile.txt') - except Exception as e: + except Exception as e: # pylint: disable=broad-except + # NOTE(ralonsoh): to narrow the scope of this exception. self.assertTrue(isinstance(e, OSError)) @@ -997,7 +998,8 @@ class TestUtilsIpAddrMethods(unittest.TestCase): self.assertEqual(utils.safe_ip_address(addr), expected, addr) @mock.patch("yardstick.common.utils.logging") - def test_safe_ip_address_negative(self, mock_logging): + def test_safe_ip_address_negative(self, *args): + # NOTE(ralonsoh): check the calls to mocked functions. for value in self.INVALID_IP_ADDRESS_STR_LIST: self.assertIsNone(utils.safe_ip_address(value), value) @@ -1026,7 +1028,8 @@ class TestUtilsIpAddrMethods(unittest.TestCase): self.assertEqual(utils.get_ip_version(addr), 6, addr) @mock.patch("yardstick.common.utils.logging") - def test_get_ip_version_negative(self, mock_logging): + def test_get_ip_version_negative(self, *args): + # NOTE(ralonsoh): check the calls to mocked functions. for value in self.INVALID_IP_ADDRESS_STR_LIST: self.assertIsNone(utils.get_ip_version(value), value) @@ -1055,7 +1058,8 @@ class TestUtilsIpAddrMethods(unittest.TestCase): self.assertEqual(utils.ip_to_hex(value), value) @mock.patch("yardstick.common.utils.logging") - def test_ip_to_hex_negative(self, mock_logging): + def test_ip_to_hex_negative(self, *args): + # NOTE(ralonsoh): check the calls to mocked functions. addr_list = self.GOOD_IP_V4_ADDRESS_STR_LIST mask_list = self.GOOD_IP_V4_MASK_STR_LIST value_iter = (''.join(pair) for pair in product(addr_list, mask_list)) @@ -1063,6 +1067,17 @@ class TestUtilsIpAddrMethods(unittest.TestCase): self.assertEqual(utils.ip_to_hex(value), value) +class SafeDecodeUtf8TestCase(unittest.TestCase): + + @unittest.skipIf(six.PY2, + 'This test should only be launched with Python 3.x') + def test_safe_decode_utf8(self): + _bytes = b'this is a byte array' + out = utils.safe_decode_utf8(_bytes) + self.assertIs(type(out), str) + self.assertEqual('this is a byte array', out) + + def main(): unittest.main() -- cgit 1.2.3-korg