openstack
/
ironic-python-agent
Code Issues Proposed changes

Merge "Improve the catching of calls to 'execute' related functions"

This commit is contained in:
Zuul 2017-10-24 08:47:59 +00:00 committed by Gerrit Code Review
parent 9ea63a40bd ef838dd6bb
commit 65a1cbf4f4
2 changed files with 30 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
ironic_python_agent/tests/unit/base.py
View File

@ -15,8 +15,11 @@
"""Common utilities and classes across all unit tests.""" """Common utilities and classes across all unit tests."""
import mock import subprocess
import ironic_lib
import mock
from oslo_concurrency import processutils
from oslotest import base as test_base from oslotest import base as test_base
from ironic_python_agent import utils from ironic_python_agent import utils
@ -25,19 +28,33 @@ from ironic_python_agent import utils
class IronicAgentTest(test_base.BaseTestCase): class IronicAgentTest(test_base.BaseTestCase):
"""Extends the base test to provide common features across agent tests.""" """Extends the base test to provide common features across agent tests."""
# By default block execution of utils.execute() and related functions.
block_execute = True
def setUp(self): def setUp(self):
super(IronicAgentTest, self).setUp() super(IronicAgentTest, self).setUp()
"""Add a blanket ban on running external processes via utils.execute(). """Ban running external processes via 'execute' like functions
`self` will grow a property called _exec_patch which is the Mock `self` will grow a property called _exec_patch which is the Mock
that replaces utils.execute. that replaces all the 'execute' related functions.
If the mock is called, an exception is raised to warn the tester. If the mock is called, an exception is raised to warn the tester.
""" """
# NOTE(bigjools): Not using a decorator on tests because I don't # NOTE(bigjools): Not using a decorator on tests because I don't
# want to force every test method to accept a new arg. Instead, they # want to force every test method to accept a new arg. Instead, they
# can override or examine this self._exec_patch Mock as needed. # can override or examine this self._exec_patch Mock as needed.
self._exec_patch = mock.Mock() if self.block_execute:
self._exec_patch.side_effect = Exception( self._exec_patch = mock.Mock()
"Don't call utils.execute in tests!") self._exec_patch.side_effect = Exception(
self.patch(utils, 'execute', self._exec_patch) "Don't call ironic_lib.utils.execute() / "
"processutils.execute() or similar functions in tests!")
# NOTE(jlvillal): pyudev.Context() calls ctypes.find_library()
# which calls subprocess.Popen(). So not blocking
# subprocess.Popen()
self.patch(ironic_lib.utils, 'execute', self._exec_patch)
self.patch(processutils, 'execute', self._exec_patch)
self.patch(subprocess, 'call', self._exec_patch)
self.patch(subprocess, 'check_call', self._exec_patch)
self.patch(subprocess, 'check_output', self._exec_patch)
self.patch(utils, 'execute', self._exec_patch)

12
ironic_python_agent/tests/unit/test_utils.py
View File

@ -26,7 +26,6 @@ from ironic_lib import utils as ironic_utils
import mock import mock
from oslo_concurrency import processutils from oslo_concurrency import processutils
from oslo_serialization import base64 from oslo_serialization import base64
from oslotest import base as test_base
import six import six
import testtools import testtools
@ -35,12 +34,13 @@ from ironic_python_agent.tests.unit import base as ironic_agent_base
from ironic_python_agent import utils from ironic_python_agent import utils
# Normally we'd use the IronicAgentTest base class which mocks out class ExecuteTestCase(ironic_agent_base.IronicAgentTest):
# any use of utils.execute() to prevent accidental processes. However # This test case does call utils.execute(), so don't block access to the
# this test is checking the upcall to ironic_lib's execute(), so that # execute calls.
# is mocked out instead. block_execute = False
class ExecuteTestCase(test_base.BaseTestCase):
# We do mock out the call to ironic_utils.execute() so we don't actually
# 'execute' anything, as utils.execute() calls ironic_utils.execute()
@mock.patch.object(ironic_utils, 'execute', autospec=True) @mock.patch.object(ironic_utils, 'execute', autospec=True)
def test_execute(self, mock_execute): def test_execute(self, mock_execute):
utils.execute('/usr/bin/env', 'false', check_exit_code=False) utils.execute('/usr/bin/env', 'false', check_exit_code=False)