Skip to content

Commit

Permalink
Replace custom tearDown with fixtures and cleanup.
Browse files Browse the repository at this point in the history
testtools.addCleanup is a more resilient way to perform cleanup activities,
as it will continue to clean things up even if there are unforseen problems.

Specifically, replace custom management of tempdirs with fixtures.TempDir
and replace tearDown methods that can be easily replaced with calls to
addCleanup in the setUp method. There are at least two temp dir creations that
did not have a corresponding cleanup in this patch, which is another reason
for using useFixture(fixtures.TempDir) instead of calls to mkdtemp.

Part of blueprint grizzly-testtools.

Change-Id: I4eb548010612bd5a8d30e8e2304fa66d3d5ffb7c
  • Loading branch information
emonty committed Jan 17, 2013
1 parent 3de221b commit 43f8697
Show file tree
Hide file tree
Showing 14 changed files with 117 additions and 192 deletions.
9 changes: 2 additions & 7 deletions glance/tests/functional/__init__.py
Expand Up @@ -33,6 +33,7 @@
import time
import urlparse

import fixtures
from sqlalchemy import create_engine

from glance.common import utils
Expand Down Expand Up @@ -457,7 +458,7 @@ class FunctionalTest(test_utils.BaseTestCase):

def setUp(self):
super(FunctionalTest, self).setUp()
self.test_id, self.test_dir = test_utils.get_isolated_test_env()
self.test_dir = self.useFixture(fixtures.TempDir()).path

self.api_protocol = 'http'
self.api_port = get_unused_port()
Expand Down Expand Up @@ -776,12 +777,6 @@ def stop_servers(self):
self.stop_server(self.registry_server, 'Registry server')
self.stop_server(self.scrubber_daemon, 'Scrubber daemon')

# If all went well, then just remove the test directory.
# We only want to check the logs and stuff if something
# went wrong...
if os.path.exists(self.test_dir):
shutil.rmtree(self.test_dir)

self._reset_database(self.registry_server.sql_connection)

def run_sql_cmd(self, sql):
Expand Down
9 changes: 3 additions & 6 deletions glance/tests/functional/store/test_filesystem.py
Expand Up @@ -20,7 +20,8 @@

import os
import os.path
import shutil

import fixtures
import testtools

import glance.openstack.common.cfg
Expand All @@ -37,7 +38,7 @@ class TestFilesystemStore(store_tests.BaseTestCase, testtools.TestCase):

def setUp(self):
super(TestFilesystemStore, self).setUp()
_, self.tmp_dir = glance.tests.utils.get_isolated_test_env()
self.tmp_dir = self.useFixture(fixtures.TempDir()).path

self.store_dir = os.path.join(self.tmp_dir, 'images')
os.mkdir(self.store_dir)
Expand All @@ -50,10 +51,6 @@ def setUp(self):
glance.openstack.common.cfg.CONF(default_config_files=[config_file],
args=[])

def tearDown(self):
shutil.rmtree(self.tmp_dir)
super(TestFilesystemStore, self).tearDown()

def get_store(self, **kwargs):
store = glance.store.filesystem.Store(context=kwargs.get('context'))
store.configure()
Expand Down
7 changes: 4 additions & 3 deletions glance/tests/functional/test_bin_glance_control.py
Expand Up @@ -21,9 +21,10 @@
import signal
import socket
import sys
import tempfile
import time

import fixtures

from glance.tests import functional
from glance.tests.utils import skip_if_disabled

Expand Down Expand Up @@ -90,7 +91,7 @@ def test_fallback_pidfile_uncreateable_dir(self):
"""
if os.getuid() is 0:
self.skipTest("User root can always create directories")
parent = tempfile.mkdtemp()
parent = self.useFixture(fixtures.TempDir()).path
os.chmod(parent, 0)
pid_file = os.path.join(parent, 'pids', 'api.pid')
self._do_test_fallback_pidfile(pid_file)
Expand All @@ -103,7 +104,7 @@ def test_fallback_pidfile_unwriteable_dir(self):
"""
if os.getuid() is 0:
self.skipTest("User root can always write inside directories")
parent = tempfile.mkdtemp()
parent = self.useFixture(fixtures.TempDir()).path
os.chmod(parent, 0)
pid_file = os.path.join(parent, 'api.pid')
self._do_test_fallback_pidfile(pid_file)
Expand Down
18 changes: 5 additions & 13 deletions glance/tests/unit/base.py
Expand Up @@ -19,6 +19,7 @@
import os
import shutil

import fixtures
import stubout

from glance.openstack.common import cfg
Expand All @@ -38,11 +39,7 @@ def setUp(self):
# Ensure stores + locations cleared
location.SCHEME_TO_CLS_MAP = {}
store.create_stores()

def tearDown(self):
super(StoreClearingUnitTest, self).tearDown()
# Ensure stores + locations cleared
location.SCHEME_TO_CLS_MAP = {}
self.addCleanup(setattr, location, 'SCHEME_TO_CLS_MAP', dict())


class IsolatedUnitTest(StoreClearingUnitTest):
Expand All @@ -53,7 +50,8 @@ class IsolatedUnitTest(StoreClearingUnitTest):
"""

def setUp(self):
self.test_id, self.test_dir = test_utils.get_isolated_test_env()
super(IsolatedUnitTest, self).setUp()
self.test_dir = self.useFixture(fixtures.TempDir()).path
self.stubs = stubout.StubOutForTesting()
policy_file = self._copy_data_file('policy.json', self.test_dir)
self.config(sql_connection='sqlite://',
Expand All @@ -62,8 +60,8 @@ def setUp(self):
default_store='filesystem',
filesystem_store_datadir=os.path.join(self.test_dir),
policy_file=policy_file)
super(IsolatedUnitTest, self).setUp()
stubs.stub_out_registry_and_store_server(self.stubs, self.test_dir)
self.addCleanup(self.stubs.UnsetAll)

def _copy_data_file(self, file_name, dst_dir):
src_file_name = os.path.join('glance/tests/etc', file_name)
Expand All @@ -75,9 +73,3 @@ def set_policy_rules(self, rules):
fap = open(CONF.policy_file, 'w')
fap.write(json.dumps(rules))
fap.close()

def tearDown(self):
super(IsolatedUnitTest, self).tearDown()
self.stubs.UnsetAll()
if os.path.exists(self.test_dir):
shutil.rmtree(self.test_dir)
5 changes: 1 addition & 4 deletions glance/tests/unit/test_auth.py
Expand Up @@ -120,10 +120,7 @@ class TestKeystoneAuthPlugin(utils.BaseTestCase):
def setUp(self):
super(TestKeystoneAuthPlugin, self).setUp()
self.stubs = stubout.StubOutForTesting()

def tearDown(self):
super(TestKeystoneAuthPlugin, self).tearDown()
self.stubs.UnsetAll()
self.addCleanup(self.stubs.UnsetAll)

def test_required_creds(self):
"""
Expand Down
5 changes: 1 addition & 4 deletions glance/tests/unit/test_cache_middleware.py
Expand Up @@ -137,10 +137,7 @@ class TestCacheMiddlewareProcessRequest(testtools.TestCase):
def setUp(self):
super(TestCacheMiddlewareProcessRequest, self).setUp()
self.stubs = stubout.StubOutForTesting()

def tearDown(self):
super(TestCacheMiddlewareProcessRequest, self).tearDown()
self.stubs.UnsetAll()
self.addCleanup(self.stubs.UnsetAll)

def test_v1_deleted_image_fetch(self):
"""
Expand Down
31 changes: 13 additions & 18 deletions glance/tests/unit/test_config.py
Expand Up @@ -17,8 +17,8 @@

import os.path
import shutil
import tempfile

import fixtures
import stubout

from glance.api.middleware import context
Expand All @@ -31,10 +31,7 @@ class TestPasteApp(test_utils.BaseTestCase):
def setUp(self):
super(TestPasteApp, self).setUp()
self.stubs = stubout.StubOutForTesting()

def tearDown(self):
super(TestPasteApp, self).tearDown()
self.stubs.UnsetAll()
self.addCleanup(self.stubs.UnsetAll)

def _do_test_load_paste_app(self,
expected_app_type,
Expand All @@ -58,24 +55,22 @@ def _appendto(orig, copy, str):
config_file=paste_config_file,
group='paste_deploy')

temp_file = os.path.join(tempfile.mkdtemp(), 'testcfg.conf')
temp_dir = self.useFixture(fixtures.TempDir()).path
temp_file = os.path.join(temp_dir, 'testcfg.conf')

try:
_writeto(temp_file, '[DEFAULT]\n')
_writeto(temp_file, '[DEFAULT]\n')

config.parse_args(['--config-file', temp_file])
config.parse_args(['--config-file', temp_file])

paste_to = temp_file.replace('.conf', '-paste.ini')
if not paste_config_file and make_paste_file:
paste_from = os.path.join(os.getcwd(),
'etc/glance-registry-paste.ini')
_appendto(paste_from, paste_to, paste_append)
paste_to = temp_file.replace('.conf', '-paste.ini')
if not paste_config_file and make_paste_file:
paste_from = os.path.join(os.getcwd(),
'etc/glance-registry-paste.ini')
_appendto(paste_from, paste_to, paste_append)

app = config.load_paste_app('glance-registry')
app = config.load_paste_app('glance-registry')

self.assertEquals(expected_app_type, type(app))
finally:
shutil.rmtree(os.path.dirname(temp_file))
self.assertEquals(expected_app_type, type(app))

def test_load_paste_app(self):
expected_middleware = context.UnauthenticatedContextMiddleware
Expand Down

0 comments on commit 43f8697

Please sign in to comment.