openstack
/
ironic-lib
Code Issues Proposed changes

Prevent tests from using utils.execute() 

This change introduces a new base test class that mocks out
utils.execute() and similar functions and forces an exception if it
gets called. If utils.execute() is mocked by the test case then it
will work. What this does is prevent un-mocked access to
utils.execute() / processutils.execute() and similar subprocess
library functions.

Inspired by Julian Edwards' patch to ironic-python-agent

Change-Id: Ie5bda8f4c65cf4dfb40b91c11b61485c954addde
This commit is contained in:
John L. Villalovos 2017-05-11 13:22:28 -07:00
parent e36a11a7c9
commit 61a0c2582b
7 changed files with 92 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
ironic_lib/tests/base.py Normal file
View File

@ -0,0 +1,60 @@
# Copyright 2017 Cisco Systems, Inc
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# 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.
"""Common utilities and classes across all unit tests."""
import subprocess
import mock
from oslo_concurrency import processutils
from oslotest import base as test_base
from ironic_lib import utils
class IronicLibTestCase(test_base.BaseTestCase):
"""Test case base class for all unit tests except callers of utils.execute.
This test class prevents calls to the utils.execute() /
processutils.execute() and similar functions.
"""
block_execute = True
def setUp(self):
super(IronicLibTestCase, self).setUp()
"""Add a blanket ban on running external processes via utils.execute().
`self` will create a property called _exec_patch which is the Mock that
replaces utils.execute.
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
# want to force every test method to accept a new arg. Instead, they
# can override or examine this self._exec_patch Mock as needed.
if self.block_execute:
self._exec_patch = mock.Mock()
self._exec_patch.side_effect = Exception(
"Don't call ironic_lib.utils.execute() / "
"processutils.execute() or similar functions in tests!")
self.patch(processutils, 'execute', self._exec_patch)
self.patch(subprocess, 'Popen', 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)

4
ironic_lib/tests/test_disk_partitioner.py
View File

@ -16,15 +16,15 @@
import eventlet
import mock
from oslotest import base as test_base
from testtools.matchers import HasLength
from ironic_lib import disk_partitioner
from ironic_lib import exception
from ironic_lib.tests import base
from ironic_lib import utils
class DiskPartitionerTestCase(test_base.BaseTestCase):
class DiskPartitionerTestCase(base.IronicLibTestCase):
def test_add_partition(self):
dp = disk_partitioner.DiskPartitioner('/dev/fake')

27
ironic_lib/tests/test_disk_utils.py
View File

@ -25,19 +25,19 @@ from oslo_config import cfg
from oslo_serialization import base64
from oslo_service import loopingcall
from oslo_utils import imageutils
from oslotest import base as test_base
import requests
from ironic_lib import disk_partitioner
from ironic_lib import disk_utils
from ironic_lib import exception
from ironic_lib.tests import base
from ironic_lib import utils
CONF = cfg.CONF
@mock.patch.object(utils, 'execute', autospec=True)
class ListPartitionsTestCase(test_base.BaseTestCase):
class ListPartitionsTestCase(base.IronicLibTestCase):
def test_correct(self, execute_mock):
output = """
@ -72,7 +72,7 @@ BYT;
@mock.patch.object(disk_partitioner.DiskPartitioner, 'commit', lambda _: None)
class WorkOnDiskTestCase(test_base.BaseTestCase):
class WorkOnDiskTestCase(base.IronicLibTestCase):
def setUp(self):
super(WorkOnDiskTestCase, self).setUp()
@ -294,7 +294,7 @@ class WorkOnDiskTestCase(test_base.BaseTestCase):
@mock.patch.object(utils, 'execute', autospec=True)
class MakePartitionsTestCase(test_base.BaseTestCase):
class MakePartitionsTestCase(base.IronicLibTestCase):
def setUp(self):
super(MakePartitionsTestCase, self).setUp()
@ -443,7 +443,7 @@ class MakePartitionsTestCase(test_base.BaseTestCase):
@mock.patch.object(utils, 'execute', autospec=True)
class DestroyMetaDataTestCase(test_base.BaseTestCase):
class DestroyMetaDataTestCase(base.IronicLibTestCase):
def setUp(self):
super(DestroyMetaDataTestCase, self).setUp()
@ -485,7 +485,7 @@ class DestroyMetaDataTestCase(test_base.BaseTestCase):
@mock.patch.object(utils, 'execute', autospec=True)
class GetDeviceBlockSizeTestCase(test_base.BaseTestCase):
class GetDeviceBlockSizeTestCase(base.IronicLibTestCase):
def setUp(self):
super(GetDeviceBlockSizeTestCase, self).setUp()
@ -503,7 +503,7 @@ class GetDeviceBlockSizeTestCase(test_base.BaseTestCase):
@mock.patch.object(disk_utils, 'dd', autospec=True)
@mock.patch.object(disk_utils, 'qemu_img_info', autospec=True)
@mock.patch.object(disk_utils, 'convert_image', autospec=True)
class PopulateImageTestCase(test_base.BaseTestCase):
class PopulateImageTestCase(base.IronicLibTestCase):
def setUp(self):
super(PopulateImageTestCase, self).setUp()
@ -537,7 +537,7 @@ def _looping_call_done(*args, **kwargs):
@mock.patch.object(utils, 'mkfs', lambda fs, path, label=None: None)
# NOTE(dtantsur): destroy_disk_metadata resets file size, disabling it
@mock.patch.object(disk_utils, 'destroy_disk_metadata', lambda *_: None)
class RealFilePartitioningTestCase(test_base.BaseTestCase):
class RealFilePartitioningTestCase(base.IronicLibTestCase):
"""This test applies some real-world partitioning scenario to a file.
This test covers the whole partitioning, mocking everything not possible
@ -545,6 +545,9 @@ class RealFilePartitioningTestCase(test_base.BaseTestCase):
and also conducts integration testing of DiskPartitioner.
"""
# Allow calls to utils.execute() and related functions
block_execute = False
def setUp(self):
super(RealFilePartitioningTestCase, self).setUp()
# NOTE(dtantsur): no parted utility on gate-ironic-python26
@ -608,7 +611,7 @@ class RealFilePartitioningTestCase(test_base.BaseTestCase):
@mock.patch.object(shutil, 'copyfileobj', autospec=True)
@mock.patch.object(requests, 'get', autospec=True)
class GetConfigdriveTestCase(test_base.BaseTestCase):
class GetConfigdriveTestCase(base.IronicLibTestCase):
@mock.patch.object(gzip, 'GzipFile', autospec=True)
def test_get_configdrive(self, mock_gzip, mock_requests, mock_copy):
@ -664,7 +667,7 @@ class GetConfigdriveTestCase(test_base.BaseTestCase):
@mock.patch('time.sleep', lambda sec: None)
class OtherFunctionTestCase(test_base.BaseTestCase):
class OtherFunctionTestCase(base.IronicLibTestCase):
@mock.patch.object(os, 'stat', autospec=True)
@mock.patch.object(stat, 'S_ISBLK', autospec=True)
@ -772,7 +775,7 @@ class OtherFunctionTestCase(test_base.BaseTestCase):
@mock.patch.object(utils, 'execute', autospec=True)
class WholeDiskPartitionTestCases(test_base.BaseTestCase):
class WholeDiskPartitionTestCases(base.IronicLibTestCase):
def setUp(self):
super(WholeDiskPartitionTestCases, self).setUp()
@ -947,7 +950,7 @@ or continue with the current setting?
self.assertEqual(1, mock_log.call_count)
class WholeDiskConfigDriveTestCases(test_base.BaseTestCase):
class WholeDiskConfigDriveTestCases(base.IronicLibTestCase):
def setUp(self):
super(WholeDiskConfigDriveTestCases, self).setUp()

4
ironic_lib/tests/test_metrics.py
View File

@ -16,9 +16,9 @@
import types
import mock
from oslotest import base as test_base
from ironic_lib import metrics as metricslib
from ironic_lib.tests import base
class MockedMetricLogger(metricslib.MetricLogger):
@ -27,7 +27,7 @@ class MockedMetricLogger(metricslib.MetricLogger):
_timer = mock.Mock(spec_set=types.FunctionType)
class TestMetricLogger(test_base.BaseTestCase):
class TestMetricLogger(base.IronicLibTestCase):
def setUp(self):
super(TestMetricLogger, self).setUp()
self.ml = MockedMetricLogger('prefix', '.')

4
ironic_lib/tests/test_metrics_statsd.py
View File

@ -16,9 +16,9 @@
import socket
import mock
from oslotest import base as test_base
from ironic_lib import metrics_statsd
from ironic_lib.tests import base
def connect(family=None, type=None, proto=None):
@ -26,7 +26,7 @@ def connect(family=None, type=None, proto=None):
pass
class TestStatsdMetricLogger(test_base.BaseTestCase):
class TestStatsdMetricLogger(base.IronicLibTestCase):
def setUp(self):
super(TestStatsdMetricLogger, self).setUp()
self.ml = metrics_statsd.StatsdMetricLogger('prefix', '.', 'test-host',

5
ironic_lib/tests/test_metrics_utils.py
View File

@ -13,19 +13,18 @@
# License for the specific language governing permissions and limitations
# under the License.
from oslotest import base as test_base
from oslo_config import cfg
from ironic_lib import exception
from ironic_lib import metrics as metricslib
from ironic_lib import metrics_statsd
from ironic_lib import metrics_utils
from ironic_lib.tests import base
CONF = cfg.CONF
class TestGetLogger(test_base.BaseTestCase):
class TestGetLogger(base.IronicLibTestCase):
def setUp(self):
super(TestGetLogger, self).setUp()

16
ironic_lib/tests/test_utils.py
View File

@ -21,15 +21,15 @@ import os.path
import mock
from oslo_concurrency import processutils
from oslo_config import cfg
from oslotest import base as test_base
from ironic_lib import exception
from ironic_lib.tests import base
from ironic_lib import utils
CONF = cfg.CONF
class BareMetalUtilsTestCase(test_base.BaseTestCase):
class BareMetalUtilsTestCase(base.IronicLibTestCase):
def test_unlink(self):
with mock.patch.object(os, "unlink", autospec=True) as unlink_mock:
@ -44,7 +44,9 @@ class BareMetalUtilsTestCase(test_base.BaseTestCase):
unlink_mock.assert_called_once_with("/fake/path")
class ExecuteTestCase(test_base.BaseTestCase):
class ExecuteTestCase(base.IronicLibTestCase):
# Allow calls to utils.execute() and related functions
block_execute = False
@mock.patch.object(processutils, 'execute', autospec=True)
@mock.patch.object(os.environ, 'copy', return_value={}, autospec=True)
@ -126,7 +128,7 @@ class ExecuteTestCase(test_base.BaseTestCase):
self._test_execute_with_log_stdout(log_stdout=False)
class MkfsTestCase(test_base.BaseTestCase):
class MkfsTestCase(base.IronicLibTestCase):
@mock.patch.object(utils, 'execute', autospec=True)
def test_mkfs(self, execute_mock):
@ -177,7 +179,7 @@ class MkfsTestCase(test_base.BaseTestCase):
'ext4', '/my/block/dev', 'ext4-vol')
class IsHttpUrlTestCase(test_base.BaseTestCase):
class IsHttpUrlTestCase(base.IronicLibTestCase):
def test_is_http_url(self):
self.assertTrue(utils.is_http_url('http://127.0.0.1'))
@ -188,7 +190,7 @@ class IsHttpUrlTestCase(test_base.BaseTestCase):
self.assertFalse(utils.is_http_url('11111111'))
class ParseRootDeviceTestCase(test_base.BaseTestCase):
class ParseRootDeviceTestCase(base.IronicLibTestCase):
def test_parse_root_device_hints_without_operators(self):
root_device = {
@ -391,7 +393,7 @@ class ParseRootDeviceTestCase(test_base.BaseTestCase):
ValueError, utils._normalize_hint_expression, '', 'size')
class MatchRootDeviceTestCase(test_base.BaseTestCase):
class MatchRootDeviceTestCase(base.IronicLibTestCase):
def setUp(self):
super(MatchRootDeviceTestCase, self).setUp()