openstack
/
neutron
Code Issues Proposed changes

Improve fixture usage. 

There were two broad issues with fixtures.

Firstly, the 'SafeFixture' workaround for resource leaks in fixtures
<1.3 is not needed if we depend on fixtures>=1.3.1. While testtools
may raise a TypeError when trying to query a fixture that failed to
setup, this is only ever a cascading failure - it will not cause
tests to fail, cause leaks, or cause tests to incorrectly pass. That
will be fixed in testtools soon to stop it happening (but as it cannot
affect whether a test passes or fails or leaks happen there is no
reason to wait for that). Leaks are seen with fixtures 1.3.0 still
because eventlet raises a BaseException subclass rather than an
Exception subclass, and fixtures 1.3.0 didn't handle that - 1.3.1 does.

Secondly, some of the fixtures had race conditions where things were
started and then cleanups scheduled. Where possible I've fixed those,
but some of them require more significant work to fully address.

Change-Id: I3290712f7274970defda19263f4955e3c78e5ed6
Depends-On: I8c01506894ec0a92b53bc0e4ad14767f2dd6a6b3
Closes-bug: #1453888
This commit is contained in:
Robert Collins 2015-06-30 09:40:17 +12:00
parent f96cfca940
commit 7344e3ab8e
12 changed files with 73 additions and 139 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
neutron/tests/base.py
View File

@ -244,10 +244,10 @@ class DietTestCase(testtools.TestCase):
{'key': k, 'exp': v, 'act': actual_superset[k]})
class ProcessMonitorFixture(tools.SafeFixture):
class ProcessMonitorFixture(fixtures.Fixture):
"""Test fixture to capture and cleanup any spawn process monitor."""
def setUp(self):
super(ProcessMonitorFixture, self).setUp()
def _setUp(self):
self.old_callable = (
external_process.ProcessMonitor._spawn_checking_thread)
p = mock.patch("neutron.agent.linux.external_process.ProcessMonitor."
@ -410,14 +410,13 @@ class BaseTestCase(DietTestCase):
cfg.CONF.set_override("notification_driver", notification_driver)
class PluginFixture(tools.SafeFixture):
class PluginFixture(fixtures.Fixture):
def __init__(self, core_plugin=None):
super(PluginFixture, self).__init__()
self.core_plugin = core_plugin
def setUp(self):
super(PluginFixture, self).setUp()
def _setUp(self):
self.dhcp_periodic_p = mock.patch(
'neutron.db.agentschedulers_db.DhcpAgentSchedulerDbMixin.'
'start_periodic_dhcp_agent_status_check')

13
neutron/tests/common/machine_fixtures.py
View File

@ -13,12 +13,13 @@
# under the License.
#
import fixtures
from neutron.agent.linux import ip_lib
from neutron.tests.common import net_helpers
from neutron.tests import tools
class FakeMachine(tools.SafeFixture):
class FakeMachine(fixtures.Fixture):
"""Create a fake machine.
:ivar bridge: bridge on which the fake machine is bound
@ -42,8 +43,7 @@ class FakeMachine(tools.SafeFixture):
self.ip = self.ip_cidr.partition('/')[0]
self.gateway_ip = gateway_ip
def setUp(self):
super(FakeMachine, self).setUp()
def _setUp(self):
ns_fixture = self.useFixture(
net_helpers.NamespaceFixture())
self.namespace = ns_fixture.name
@ -66,7 +66,7 @@ class FakeMachine(tools.SafeFixture):
net_helpers.assert_no_ping(self.namespace, dst_ip)
class PeerMachines(tools.SafeFixture):
class PeerMachines(fixtures.Fixture):
"""Create 'amount' peered machines on an ip_cidr.
:ivar bridge: bridge on which peer machines are bound
@ -85,8 +85,7 @@ class PeerMachines(tools.SafeFixture):
self.ip_cidr = ip_cidr or self.CIDR
self.gateway_ip = gateway_ip
def setUp(self):
super(PeerMachines, self).setUp()
def _setUp(self):
self.machines = []
for index in range(self.AMOUNT):

47
neutron/tests/common/net_helpers.py
View File

@ -23,6 +23,7 @@ import shlex
import signal
import subprocess
import fixtures
import netaddr
from oslo_utils import uuidutils
import six
@ -328,7 +329,7 @@ class NetcatTester(object):
setattr(self, proc_attr, None)
class NamespaceFixture(tools.SafeFixture):
class NamespaceFixture(fixtures.Fixture):
"""Create a namespace.
:ivar ip_wrapper: created namespace
@ -341,27 +342,25 @@ class NamespaceFixture(tools.SafeFixture):
super(NamespaceFixture, self).__init__()
self.prefix = prefix
def setUp(self):
super(NamespaceFixture, self).setUp()
def _setUp(self):
ip = ip_lib.IPWrapper()
self.name = self.prefix + uuidutils.generate_uuid()
self.ip_wrapper = ip.ensure_namespace(self.name)
self.addCleanup(self.destroy)
self.ip_wrapper = ip.ensure_namespace(self.name)
def destroy(self):
if self.ip_wrapper.netns.exists(self.name):
self.ip_wrapper.netns.delete(self.name)
class VethFixture(tools.SafeFixture):
class VethFixture(fixtures.Fixture):
"""Create a veth.
:ivar ports: created veth ports
:type ports: IPDevice 2-uplet
"""
def setUp(self):
super(VethFixture, self).setUp()
def _setUp(self):
ip_wrapper = ip_lib.IPWrapper()
self.ports = common_base.create_resource(
@ -392,7 +391,7 @@ class VethFixture(tools.SafeFixture):
@six.add_metaclass(abc.ABCMeta)
class PortFixture(tools.SafeFixture):
class PortFixture(fixtures.Fixture):
"""Create a port.
:ivar port: created port
@ -410,8 +409,8 @@ class PortFixture(tools.SafeFixture):
pass
@abc.abstractmethod
def setUp(self):
super(PortFixture, self).setUp()
def _setUp(self):
super(PortFixture, self)._setUp()
if not self.bridge:
self.bridge = self.useFixture(self._create_bridge_fixture()).bridge
@ -427,7 +426,7 @@ class PortFixture(tools.SafeFixture):
tools.fail('Unexpected bridge type: %s' % type(bridge))
class OVSBridgeFixture(tools.SafeFixture):
class OVSBridgeFixture(fixtures.Fixture):
"""Create an OVS bridge.
:ivar prefix: bridge name prefix
@ -440,8 +439,7 @@ class OVSBridgeFixture(tools.SafeFixture):
super(OVSBridgeFixture, self).__init__()
self.prefix = prefix
def setUp(self):
super(OVSBridgeFixture, self).setUp()
def _setUp(self):
ovs = ovs_lib.BaseOVS()
self.bridge = common_base.create_resource(self.prefix, ovs.add_bridge)
self.addCleanup(self.bridge.destroy)
@ -458,8 +456,8 @@ class OVSPortFixture(PortFixture):
def _create_bridge_fixture(self):
return OVSBridgeFixture()
def setUp(self):
super(OVSPortFixture, self).setUp()
def _setUp(self):
super(OVSPortFixture, self)._setUp()
port_name = common_base.create_resource(PORT_PREFIX, self.create_port)
self.addCleanup(self.bridge.delete_port, port_name)
@ -475,7 +473,7 @@ class OVSPortFixture(PortFixture):
return name
class LinuxBridgeFixture(tools.SafeFixture):
class LinuxBridgeFixture(fixtures.Fixture):
"""Create a linux bridge.
:ivar bridge: created bridge
@ -484,9 +482,7 @@ class LinuxBridgeFixture(tools.SafeFixture):
:type namespace: str
"""
def setUp(self):
super(LinuxBridgeFixture, self).setUp()
def _setUp(self):
self.namespace = self.useFixture(NamespaceFixture()).name
self.bridge = common_base.create_resource(
BR_PREFIX,
@ -509,8 +505,8 @@ class LinuxBridgePortFixture(PortFixture):
def _create_bridge_fixture(self):
return LinuxBridgeFixture()
def setUp(self):
super(LinuxBridgePortFixture, self).setUp()
def _setUp(self):
super(LinuxBridgePortFixture, self)._setUp()
self.port, self.br_port = self.useFixture(VethFixture()).ports
# bridge side
@ -539,15 +535,14 @@ class VethBridge(object):
len(self.ports))
class VethBridgeFixture(tools.SafeFixture):
class VethBridgeFixture(fixtures.Fixture):
"""Simulate a bridge with a veth.
:ivar bridge: created bridge
:type bridge: FakeBridge
"""
def setUp(self):
super(VethBridgeFixture, self).setUp()
def _setUp(self):
ports = self.useFixture(VethFixture()).ports
self.bridge = VethBridge(ports)
@ -562,8 +557,8 @@ class VethPortFixture(PortFixture):
def _create_bridge_fixture(self):
return VethBridgeFixture()
def setUp(self):
super(VethPortFixture, self).setUp()
def _setUp(self):
super(VethPortFixture, self)._setUp()
self.port = self.bridge.allocate_port()
ns_ip_wrapper = ip_lib.IPWrapper(self.namespace)

12
neutron/tests/fullstack/config_fixtures.py
View File

@ -15,13 +15,13 @@
import os.path
import tempfile
import fixtures
import six
from neutron.common import constants
from neutron.tests import base
from neutron.tests.common import helpers as c_helpers
from neutron.tests.common import net_helpers
from neutron.tests import tools
class ConfigDict(base.AttributeDict):
@ -41,7 +41,7 @@ class ConfigDict(base.AttributeDict):
self.convert_to_attr_dict(value)
class ConfigFileFixture(tools.SafeFixture):
class ConfigFileFixture(fixtures.Fixture):
"""A fixture that knows how to translate configurations to files.
:param base_filename: the filename to use on disk.
@ -55,8 +55,7 @@ class ConfigFileFixture(tools.SafeFixture):
self.config = config
self.temp_dir = temp_dir
def setUp(self):
super(ConfigFileFixture, self).setUp()
def _setUp(self):
config_parser = self.dict_to_config_parser(self.config)
# Need to randomly generate a unique folder to put the file in
self.filename = os.path.join(self.temp_dir, self.base_filename)
@ -74,7 +73,7 @@ class ConfigFileFixture(tools.SafeFixture):
return config_parser
class ConfigFixture(tools.SafeFixture):
class ConfigFixture(fixtures.Fixture):
"""A fixture that holds an actual Neutron configuration.
Note that 'self.config' is intended to only be updated once, during
@ -88,8 +87,7 @@ class ConfigFixture(tools.SafeFixture):
self.temp_dir = temp_dir
self.base_filename = base_filename
def setUp(self):
super(ConfigFixture, self).setUp()
def _setUp(self):
cfg_fixture = ConfigFileFixture(
self.base_filename, self.config, self.temp_dir)
self.useFixture(cfg_fixture)

40
neutron/tests/fullstack/fullstack_fixtures.py
View File

@ -28,7 +28,6 @@ from neutron.agent.linux import utils
from neutron.tests import base
from neutron.tests.common import net_helpers
from neutron.tests.fullstack import config_fixtures
from neutron.tests import tools
LOG = logging.getLogger(__name__)
@ -36,7 +35,7 @@ LOG = logging.getLogger(__name__)
DEFAULT_LOG_DIR = '/tmp/fullstack-logs/'
class ProcessFixture(tools.SafeFixture):
class ProcessFixture(fixtures.Fixture):
def __init__(self, test_name, process_name, exec_name, config_filenames):
super(ProcessFixture, self).__init__()
self.test_name = test_name
@ -45,8 +44,8 @@ class ProcessFixture(tools.SafeFixture):
self.config_filenames = config_filenames
self.process = None
def setUp(self):
super(ProcessFixture, self).setUp()
def _setUp(self):
self.addCleanup(self.stop)
self.start()
def start(self):
@ -65,15 +64,10 @@ class ProcessFixture(tools.SafeFixture):
def stop(self):
self.process.stop(block=True)
def cleanUp(self, *args, **kwargs):
self.stop()
super(ProcessFixture, self).cleanUp(*args, **kwargs)
class RabbitmqEnvironmentFixture(fixtures.Fixture):
class RabbitmqEnvironmentFixture(tools.SafeFixture):
def setUp(self):
super(RabbitmqEnvironmentFixture, self).setUp()
def _setUp(self):
self.user = base.get_rand_name(prefix='user')
self.password = base.get_rand_name(prefix='pass')
self.vhost = base.get_rand_name(prefix='vhost')
@ -93,14 +87,12 @@ class RabbitmqEnvironmentFixture(tools.SafeFixture):
utils.execute(cmd, run_as_root=True)
class FullstackFixture(tools.SafeFixture):
class FullstackFixture(fixtures.Fixture):
def __init__(self):
super(FullstackFixture, self).__init__()
self.test_name = None
def setUp(self):
super(FullstackFixture, self).setUp()
def _setUp(self):
self.temp_dir = self.useFixture(fixtures.TempDir()).path
rabbitmq_environment = self.useFixture(RabbitmqEnvironmentFixture())
@ -120,7 +112,7 @@ class FullstackFixture(tools.SafeFixture):
return False
class NeutronServerFixture(tools.SafeFixture):
class NeutronServerFixture(fixtures.Fixture):
NEUTRON_SERVER = "neutron-server"
@ -130,9 +122,7 @@ class NeutronServerFixture(tools.SafeFixture):
self.temp_dir = temp_dir
self.rabbitmq_environment = rabbitmq_environment
def setUp(self):
super(NeutronServerFixture, self).setUp()
def _setUp(self):
self.neutron_cfg_fixture = config_fixtures.NeutronConfigFixture(
self.temp_dir, cfg.CONF.database.connection,
self.rabbitmq_environment)
@ -169,7 +159,7 @@ class NeutronServerFixture(tools.SafeFixture):
return client.Client(auth_strategy="noauth", endpoint_url=url)
class OVSAgentFixture(tools.SafeFixture):
class OVSAgentFixture(fixtures.Fixture):
NEUTRON_OVS_AGENT = "neutron-openvswitch-agent"
@ -182,9 +172,7 @@ class OVSAgentFixture(tools.SafeFixture):
self.neutron_config = self.neutron_cfg_fixture.config
self.plugin_config = self.plugin_cfg_fixture.config
def setUp(self):
super(OVSAgentFixture, self).setUp()
def _setUp(self):
self.useFixture(net_helpers.OVSBridgeFixture(self._get_br_int_name()))
self.useFixture(net_helpers.OVSBridgeFixture(self._get_br_phys_name()))
@ -204,7 +192,7 @@ class OVSAgentFixture(tools.SafeFixture):
return self.plugin_config.ovs.bridge_mappings.split(':')[1]
class L3AgentFixture(tools.SafeFixture):
class L3AgentFixture(fixtures.Fixture):
NEUTRON_L3_AGENT = "neutron-l3-agent"
@ -217,9 +205,7 @@ class L3AgentFixture(tools.SafeFixture):
self.neutron_config = self.neutron_cfg_fixture.config
self.integration_bridge_name = integration_bridge_name
def setUp(self):
super(L3AgentFixture, self).setUp()
def _setUp(self):
self.plugin_cfg_fixture = config_fixtures.L3ConfigFixture(
self.temp_dir, self.integration_bridge_name)
self.useFixture(self.plugin_cfg_fixture)

4
neutron/tests/fullstack/test_l3_agent.py
View File

@ -23,8 +23,8 @@ from neutron.tests.fullstack import fullstack_fixtures as f_fixtures
class SingleNodeEnvironment(f_fixtures.FullstackFixture):
def setUp(self):
super(SingleNodeEnvironment, self).setUp()
def _setUp(self):
super(SingleNodeEnvironment, self)._setUp()
neutron_config = self.neutron_server.neutron_cfg_fixture
ml2_config = self.neutron_server.plugin_cfg_fixture

7
neutron/tests/functional/agent/linux/helpers.py
View File

@ -14,10 +14,10 @@
# under the License.
import os
from neutron.tests import tools
import fixtures
class RecursivePermDirFixture(tools.SafeFixture):
class RecursivePermDirFixture(fixtures.Fixture):
"""Ensure at least perms permissions on directory and ancestors."""
def __init__(self, directory, perms):
@ -25,8 +25,7 @@ class RecursivePermDirFixture(tools.SafeFixture):
self.directory = directory
self.least_perms = perms
def setUp(self):
super(RecursivePermDirFixture, self).setUp()
def _setUp(self):
previous_directory = None
current_directory = self.directory
while previous_directory != current_directory:

8
neutron/tests/retargetable/client_fixtures.py
View File

@ -17,18 +17,18 @@ Neutron API via different methods.
import abc
import fixtures
import six
from neutron.common import exceptions as q_exc
from neutron import context
from neutron import manager
from neutron.tests import base
from neutron.tests import tools
from neutron.tests.unit import testlib_api
@six.add_metaclass(abc.ABCMeta)
class AbstractClientFixture(tools.SafeFixture):
class AbstractClientFixture(fixtures.Fixture):
"""
Base class for a client that can interact the neutron api in some
manner.
@ -71,8 +71,8 @@ class PluginClientFixture(AbstractClientFixture):
super(PluginClientFixture, self).__init__()
self.plugin_conf = plugin_conf
def setUp(self):
super(PluginClientFixture, self).setUp()
def _setUp(self):
super(PluginClientFixture, self)._setUp()
self.useFixture(testlib_api.SqlFixture())
self.useFixture(self.plugin_conf)
self.useFixture(base.PluginFixture(self.plugin_conf.plugin_name))

53
neutron/tests/tools.py
View File

@ -16,52 +16,12 @@
import warnings
import fixtures
from oslo_utils import excutils
import six
from neutron.api.v2 import attributes
class SafeFixture(fixtures.Fixture):
"""Base Fixture ensuring cleanups are done even if setUp fails.
Required until testtools/fixtures bugs #1456353 #1456370 are solved.
"""
def __init__(self):
unsafe_setup = self.setUp
self.setUp = lambda: self.safe_setUp(unsafe_setup)
self.initialized = True
def setUp(self):
assert getattr(self, 'initialized', True)
super(SafeFixture, self).setUp()
def safe_setUp(self, unsafe_setup):
"""Ensure cleanup is done even if setUp fails."""
try:
unsafe_setup()
except Exception:
with excutils.save_and_reraise_exception():
self.safe_cleanUp()
def safe_cleanUp(self):
"""Perform cleanUp if required.
Fixture.addCleanup/cleanUp can be called only after Fixture.setUp
successful call. It implies we cannot and don't need to call cleanUp
if Fixture.setUp fails or is not called.
This method assumes Fixture.setUp was called successfully if
self._detail_sources is defined (Fixture.setUp last action).
"""
root_setup_succeed = hasattr(self, '_detail_sources')
if root_setup_succeed:
self.cleanUp()
class AttributeMapMemento(SafeFixture):
class AttributeMapMemento(fixtures.Fixture):
"""Create a copy of the resource attribute map so it can be restored during
test cleanup.
@ -75,13 +35,13 @@ class AttributeMapMemento(SafeFixture):
- Inheritance is a bit of overkill for this facility and it's a
stretch to rationalize the "is a" criteria.
"""
def setUp(self):
def _setUp(self):
# Shallow copy is not a proper choice for keeping a backup copy as
# the RESOURCE_ATTRIBUTE_MAP map is modified in place through the
# 0th level keys. Ideally deepcopy() would be used but this seems
# to result in test failures. A compromise is to copy one level
# deeper than a shallow copy.
super(AttributeMapMemento, self).setUp()
self.contents_backup = {}
for res, attrs in six.iteritems(attributes.RESOURCE_ATTRIBUTE_MAP):
self.contents_backup[res] = attrs.copy()
@ -91,19 +51,18 @@ class AttributeMapMemento(SafeFixture):
attributes.RESOURCE_ATTRIBUTE_MAP = self.contents_backup
class WarningsFixture(SafeFixture):
class WarningsFixture(fixtures.Fixture):
"""Filters out warnings during test runs."""
warning_types = (
DeprecationWarning, PendingDeprecationWarning, ImportWarning
)
def setUp(self):
super(WarningsFixture, self).setUp()
def _setUp(self):
self.addCleanup(warnings.resetwarnings)
for wtype in self.warning_types:
warnings.filterwarnings(
"always", category=wtype, module='^neutron\\.')
self.addCleanup(warnings.resetwarnings)
"""setup_mock_calls and verify_mock_calls are convenient methods

8
neutron/tests/unit/plugins/ml2/test_plugin.py
View File

@ -14,6 +14,8 @@
# under the License.
import functools
import fixtures
import mock
import six
import testtools
@ -48,7 +50,6 @@ from neutron.plugins.ml2.drivers import type_vlan
from neutron.plugins.ml2 import models
from neutron.plugins.ml2 import plugin as ml2_plugin
from neutron.tests import base
from neutron.tests import tools
from neutron.tests.unit import _test_extension_portbindings as test_bindings
from neutron.tests.unit.agent import test_securitygroups_rpc as test_sg_rpc
from neutron.tests.unit.db import test_allowedaddresspairs_db as test_pair
@ -71,7 +72,7 @@ HOST = 'fake_host'
# TODO(marun) - Move to somewhere common for reuse
class PluginConfFixture(tools.SafeFixture):
class PluginConfFixture(fixtures.Fixture):
"""Plugin configuration shared across the unit and functional tests."""
def __init__(self, plugin_name, parent_setup=None):
@ -79,8 +80,7 @@ class PluginConfFixture(tools.SafeFixture):
self.plugin_name = plugin_name
self.parent_setup = parent_setup
def setUp(self):
super(PluginConfFixture, self).setUp()
def _setUp(self):
if self.parent_setup:
self.parent_setup()

7
neutron/tests/unit/testlib_api.py
View File

@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import fixtures
import six
import testtools
@ -21,7 +22,6 @@ from neutron.db import api as db_api
from neutron.db.migration.models import head # noqa
from neutron.db import model_base
from neutron.tests import base
from neutron.tests import tools
from neutron import wsgi
@ -57,13 +57,12 @@ def create_request(path, body, content_type, method='GET',
return req
class SqlFixture(tools.SafeFixture):
class SqlFixture(fixtures.Fixture):
# flag to indicate that the models have been loaded
_TABLES_ESTABLISHED = False
def setUp(self):
super(SqlFixture, self).setUp()
def _setUp(self):
# Register all data models
engine = db_api.get_engine()
if not SqlFixture._TABLES_ESTABLISHED:

2
test-requirements.txt
View File

@ -5,7 +5,7 @@ hacking<0.11,>=0.10.0
cliff>=1.13.0 # Apache-2.0
coverage>=3.6
fixtures>=0.3.14
fixtures>=1.3.1
mock>=1.0
python-subunit>=0.0.18
requests-mock>=0.6.0 # Apache-2.0