From 0753970616860440282ad2c4aeabd1003945c756 Mon Sep 17 00:00:00 2001 From: Brant Knudson Date: Fri, 3 Jan 2014 10:43:24 -0600 Subject: [PATCH] Tests use cleanUp rather than tearDown The tests were putting clean-up code in tearDown. Using tearDown causes problems with developing and testing because if there's an exception raised in setUp then tearDown is not called and this typically affects other tests. For example, you'll get several tests failing because one test's setUp failed and now the other tests can't load fixtures. Also, this should make it easier to switch to using fixtures since fixtures use cleanUp. Change-Id: I0d0d14ec34ebbeb9dbf0fe2fe3dd304b868bcf93 --- keystone/tests/core.py | 73 ++++++++++--------- keystone/tests/rest.py | 7 +- ...st_associate_project_endpoint_extension.py | 7 +- keystone/tests/test_backend_ldap.py | 33 +++++---- keystone/tests/test_backend_sql.py | 8 +- keystone/tests/test_cert_setup.py | 15 ++-- keystone/tests/test_injection.py | 6 +- keystone/tests/test_no_admin_token_auth.py | 10 +-- keystone/tests/test_policy.py | 24 +++--- keystone/tests/test_v3.py | 19 +++-- keystone/tests/test_v3_protection.py | 18 ++--- keystone/tests/test_wsgi.py | 6 +- 12 files changed, 112 insertions(+), 114 deletions(-) diff --git a/keystone/tests/core.py b/keystone/tests/core.py index 2627fce529..9a47671555 100644 --- a/keystone/tests/core.py +++ b/keystone/tests/core.py @@ -252,16 +252,18 @@ class TestClient(object): class NoModule(object): """A mixin class to provide support for unloading/disabling modules.""" - def __init__(self, *args, **kw): - super(NoModule, self).__init__(*args, **kw) - self._finders = [] - self._cleared_modules = {} + def setUp(self): + super(NoModule, self).setUp() - def tearDown(self): - super(NoModule, self).tearDown() - for finder in self._finders: - sys.meta_path.remove(finder) - sys.modules.update(self._cleared_modules) + self._finders = [] + + def cleanup_finders(): + for finder in self._finders: + sys.meta_path.remove(finder) + self.addCleanup(cleanup_finders) + + self._cleared_modules = {} + self.addCleanup(sys.modules.update, self._cleared_modules) def clear_module(self, module): cleared_modules = {} @@ -288,9 +290,17 @@ class NoModule(object): class TestCase(testtools.TestCase): - def __init__(self, *args, **kw): - super(TestCase, self).__init__(*args, **kw) + def setUp(self): + super(TestCase, self).setUp() + self._paths = [] + + def _cleanup_paths(): + for path in self._paths: + if path in sys.path: + sys.path.remove(path) + self.addCleanup(_cleanup_paths) + self._memo = {} self._overrides = [] self._group_overrides = {} @@ -298,40 +308,35 @@ class TestCase(testtools.TestCase): # show complete diffs on failure self.maxDiff = None - def setUp(self): - super(TestCase, self).setUp() + self.addCleanup(CONF.reset) + self.config([dirs.etc('keystone.conf.sample'), dirs.tests('test_overrides.conf')]) + + self.opt(policy_file=dirs.etc('policy.json')) + + # NOTE(morganfainberg): The only way to reconfigure the + # CacheRegion object on each setUp() call is to remove the + # .backend property. + self.addCleanup(delattr, cache.REGION, 'backend') + # ensure the cache region instance is setup cache.configure_cache_region(cache.REGION) - self.opt(policy_file=dirs.etc('policy.json')) self.logger = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG)) warnings.filterwarnings('ignore', category=DeprecationWarning) + # Clear the registry of providers so that providers from previous + # tests aren't used. + self.addCleanup(dependency.reset) + + self.addCleanup(kvs.INMEMDB.clear) + + self.addCleanup(timeutils.clear_time_override) + def config(self, config_files): CONF(args=[], project='keystone', default_config_files=config_files) - def tearDown(self): - try: - timeutils.clear_time_override() - # NOTE(morganfainberg): The only way to reconfigure the - # CacheRegion object on each setUp() call is to remove the - # .backend property. - del cache.REGION.backend - super(TestCase, self).tearDown() - finally: - for path in self._paths: - if path in sys.path: - sys.path.remove(path) - - # Clear the registry of providers so that providers from previous - # tests aren't used. - dependency.reset() - - kvs.INMEMDB.clear() - CONF.reset() - def opt_in_group(self, group, **kw): for k, v in kw.iteritems(): CONF.set_override(k, v, group) diff --git a/keystone/tests/rest.py b/keystone/tests/rest.py index 77fbb624db..6f0bdb02b1 100644 --- a/keystone/tests/rest.py +++ b/keystone/tests/rest.py @@ -63,13 +63,10 @@ class RestfulTestCase(tests.TestCase): self.public_app = webtest.TestApp( self.loadapp(app_conf, name='main')) + self.addCleanup(delattr, self, 'public_app') self.admin_app = webtest.TestApp( self.loadapp(app_conf, name='admin')) - - def tearDown(self): - self.public_app = None - self.admin_app = None - super(RestfulTestCase, self).tearDown() + self.addCleanup(delattr, self, 'admin_app') def request(self, app, path, body=None, headers=None, token=None, expected_status=None, **kwargs): diff --git a/keystone/tests/test_associate_project_endpoint_extension.py b/keystone/tests/test_associate_project_endpoint_extension.py index 0d7ef68b84..7e4e78260a 100644 --- a/keystone/tests/test_associate_project_endpoint_extension.py +++ b/keystone/tests/test_associate_project_endpoint_extension.py @@ -44,16 +44,15 @@ class TestExtensionCase(test_v3.RestfulTestCase): def setUp(self): super(TestExtensionCase, self).setUp() + self.addCleanup(self.conf_files.pop) + # TODO(blk-u): check if the above is necessary and remove if not. + self.default_request_url = ( '/OS-EP-FILTER/projects/%(project_id)s' '/endpoints/%(endpoint_id)s' % { 'project_id': self.default_domain_project_id, 'endpoint_id': self.endpoint_id}) - def tearDown(self): - super(TestExtensionCase, self).tearDown() - self.conf_files.pop() - class AssociateEndpointProjectFilterCRUDTestCase(TestExtensionCase): """Test OS-EP-FILTER endpoint to project associations extension.""" diff --git a/keystone/tests/test_backend_ldap.py b/keystone/tests/test_backend_ldap.py index f2688c97c9..0153b3caf5 100644 --- a/keystone/tests/test_backend_ldap.py +++ b/keystone/tests/test_backend_ldap.py @@ -1019,16 +1019,15 @@ class LdapIdentitySqlAssignment(sql.Base, tests.TestCase, BaseLDAPIdentity): self.load_backends() cache.configure_cache_region(cache.REGION) self.engine = session.get_engine() + self.addCleanup(session.cleanup) + sql.ModelBase.metadata.create_all(bind=self.engine) + self.addCleanup(sql.ModelBase.metadata.drop_all, bind=self.engine) + self.load_fixtures(default_fixtures) #defaulted by the data load self.user_foo['enabled'] = True - def tearDown(self): - sql.ModelBase.metadata.drop_all(bind=self.engine) - session.cleanup() - super(LdapIdentitySqlAssignment, self).tearDown() - def test_domain_crud(self): pass @@ -1068,8 +1067,13 @@ class MultiLDAPandSQLIdentity(sql.Base, tests.TestCase, BaseLDAPIdentity): self._set_config() self.load_backends() + self.engine = session.get_engine() + self.addCleanup(session.cleanup) + sql.ModelBase.metadata.create_all(bind=self.engine) + self.addCleanup(sql.ModelBase.metadata.drop_all, bind=self.engine) + self._setup_domain_test_data() # All initial domain data setup complete, time to switch on support @@ -1077,24 +1081,23 @@ class MultiLDAPandSQLIdentity(sql.Base, tests.TestCase, BaseLDAPIdentity): self.orig_config_domains_enabled = ( config.CONF.identity.domain_specific_drivers_enabled) + self.addCleanup( + self.opt_in_group, 'identity', + domain_specific_drivers_enabled=self.orig_config_domains_enabled) self.opt_in_group('identity', domain_specific_drivers_enabled=True) + self.orig_config_dir = ( config.CONF.identity.domain_config_dir) + self.addCleanup(self.opt_in_group, 'identity', + domain_config_dir=self.orig_config_dir) self.opt_in_group('identity', domain_config_dir=tests.TESTSDIR) + self._set_domain_configs() self.clear_database() self.load_fixtures(default_fixtures) - def tearDown(self): - super(MultiLDAPandSQLIdentity, self).tearDown() - self.opt_in_group( - 'identity', - domain_config_dir=self.orig_config_dir) - self.opt_in_group( - 'identity', - domain_specific_drivers_enabled=self.orig_config_domains_enabled) - sql.ModelBase.metadata.drop_all(bind=self.engine) - session.cleanup() + # TODO(blk-u): the addCleanup of config options above is probably + # unnecessary since TestCase resets config. def _set_config(self): self.config([tests.dirs.etc('keystone.conf.sample'), diff --git a/keystone/tests/test_backend_sql.py b/keystone/tests/test_backend_sql.py index f59921f57f..1a6df3abc2 100644 --- a/keystone/tests/test_backend_sql.py +++ b/keystone/tests/test_backend_sql.py @@ -47,18 +47,16 @@ class SqlTests(tests.TestCase, sql.Base): # create tables and keep an engine reference for cleanup. # this must be done after the models are loaded by the managers. self.engine = session.get_engine() + self.addCleanup(session.cleanup) + sql.ModelBase.metadata.create_all(bind=self.engine) + self.addCleanup(sql.ModelBase.metadata.drop_all, bind=self.engine) # populate the engine with tables & fixtures self.load_fixtures(default_fixtures) #defaulted by the data load self.user_foo['enabled'] = True - def tearDown(self): - sql.ModelBase.metadata.drop_all(bind=self.engine) - session.cleanup() - super(SqlTests, self).tearDown() - class SqlModels(SqlTests): def setUp(self): diff --git a/keystone/tests/test_cert_setup.py b/keystone/tests/test_cert_setup.py index 150fd58b87..7cf690f5a0 100644 --- a/keystone/tests/test_cert_setup.py +++ b/keystone/tests/test_cert_setup.py @@ -53,6 +53,14 @@ class CertSetupTestCase(tests.TestCase): self.load_fixtures(default_fixtures) self.controller = token.controllers.Auth() + def cleanup_ssldir(): + try: + shutil.rmtree(SSLDIR) + except OSError: + pass + + self.addCleanup(cleanup_ssldir) + def test_can_handle_missing_certs(self): self.opt_in_group('signing', certfile='invalid') user = { @@ -85,10 +93,3 @@ class CertSetupTestCase(tests.TestCase): self.assertTrue(os.path.exists(CONF.ssl.ca_certs)) self.assertTrue(os.path.exists(CONF.ssl.certfile)) self.assertTrue(os.path.exists(CONF.ssl.keyfile)) - - def tearDown(self): - try: - shutil.rmtree(SSLDIR) - except OSError: - pass - super(CertSetupTestCase, self).tearDown() diff --git a/keystone/tests/test_injection.py b/keystone/tests/test_injection.py index c18917a149..34250a11cd 100644 --- a/keystone/tests/test_injection.py +++ b/keystone/tests/test_injection.py @@ -21,9 +21,9 @@ from keystone.common import dependency class TestDependencyInjection(testtools.TestCase): - def tearDown(self): - dependency.reset() - super(TestDependencyInjection, self).tearDown() + def setUp(self): + super(TestDependencyInjection, self).setUp() + self.addCleanup(dependency.reset) def test_dependency_injection(self): class Interface(object): diff --git a/keystone/tests/test_no_admin_token_auth.py b/keystone/tests/test_no_admin_token_auth.py index cfa18a71e0..20fb661e45 100644 --- a/keystone/tests/test_no_admin_token_auth.py +++ b/keystone/tests/test_no_admin_token_auth.py @@ -39,15 +39,15 @@ class TestNoAdminTokenAuth(tests.TestCase): self.load_backends() _generate_paste_config() + self.addCleanup(os.remove, + tests.dirs.tmp('no_admin_token_auth-paste.ini')) + # TODO(blk-u): Make _generate_paste_config a member function and have + # it also do addCleanup. self.admin_app = webtest.TestApp( self.loadapp(tests.dirs.tmp('no_admin_token_auth'), name='admin'), extra_environ=dict(REMOTE_ADDR='127.0.0.1')) - - def tearDown(self): - self.admin_app = None - os.remove(tests.dirs.tmp('no_admin_token_auth-paste.ini')) - super(TestNoAdminTokenAuth, self).tearDown() + self.addCleanup(setattr, self, 'admin_app', None) def test_request_no_admin_token_auth(self): # This test verifies that if the admin_token_auth middleware isn't diff --git a/keystone/tests/test_policy.py b/keystone/tests/test_policy.py index 66cf1a01e4..7213e5f761 100644 --- a/keystone/tests/test_policy.py +++ b/keystone/tests/test_policy.py @@ -36,17 +36,18 @@ CONF = config.CONF class PolicyFileTestCase(tests.TestCase): def setUp(self): super(PolicyFileTestCase, self).setUp() + self.orig_policy_file = CONF.policy_file + # TODO(blk-u): Resetting the conf value here is probably unnecessary + # since TestCase clears conf. + self.addCleanup(self.opt, policy_file=self.orig_policy_file) + rules.reset() + self.addCleanup(rules.reset) _unused, self.tmpfilename = tempfile.mkstemp() self.opt(policy_file=self.tmpfilename) self.target = {} - def tearDown(self): - super(PolicyFileTestCase, self).tearDown() - rules.reset() - self.opt(policy_file=self.orig_policy_file) - def test_modified_policy_reloads(self): action = "example:test" empty_credentials = {} @@ -65,6 +66,7 @@ class PolicyTestCase(tests.TestCase): def setUp(self): super(PolicyTestCase, self).setUp() rules.reset() + self.addCleanup(rules.reset) # NOTE(vish): preload rules to circumvent reloading from file rules.init() self.rules = { @@ -94,10 +96,6 @@ class PolicyTestCase(tests.TestCase): for k, v in self.rules.items())) rules._ENFORCER.set_rules(these_rules) - def tearDown(self): - rules.reset() - super(PolicyTestCase, self).tearDown() - def test_enforce_nonexistent_action_throws(self): action = "example:noexist" self.assertRaises(exception.ForbiddenAction, rules.enforce, @@ -165,6 +163,7 @@ class DefaultPolicyTestCase(tests.TestCase): def setUp(self): super(DefaultPolicyTestCase, self).setUp() rules.reset() + self.addCleanup(rules.reset) rules.init() self.rules = { @@ -181,6 +180,8 @@ class DefaultPolicyTestCase(tests.TestCase): # Oslo policy as we shoudn't have to reload the rules if they have # already been set using set_rules(). self._old_load_rules = rules._ENFORCER.load_rules + self.addCleanup(setattr, rules._ENFORCER, 'load_rules', + self._old_load_rules) setattr(rules._ENFORCER, 'load_rules', lambda *args, **kwargs: None) def _set_rules(self, default_rule): @@ -189,11 +190,6 @@ class DefaultPolicyTestCase(tests.TestCase): for k, v in self.rules.items()), default_rule) rules._ENFORCER.set_rules(these_rules) - def tearDown(self): - super(DefaultPolicyTestCase, self).tearDown() - rules._ENFORCER.load_rules = self._old_load_rules - rules.reset() - def test_policy_called(self): self.assertRaises(exception.ForbiddenAction, rules.enforce, self.credentials, "example:exist", {}) diff --git a/keystone/tests/test_v3.py b/keystone/tests/test_v3.py index ed77d12b16..b10f3b6cfb 100644 --- a/keystone/tests/test_v3.py +++ b/keystone/tests/test_v3.py @@ -73,6 +73,7 @@ class RestfulTestCase(rest.RestfulTestCase): """ new_paste_file = self.generate_paste_config() + self.addCleanup(self.remove_generated_paste_config) if new_paste_file: app_conf = 'config:%s' % (new_paste_file) @@ -80,15 +81,17 @@ class RestfulTestCase(rest.RestfulTestCase): self.empty_context = {'environment': {}} - def tearDown(self): - self.teardown_database() - self.remove_generated_paste_config() - # need to reset the plug-ins - auth.controllers.AUTH_METHODS = {} #drop the policy rules - CONF.reset() - rules.reset() - super(RestfulTestCase, self).tearDown() + self.addCleanup(rules.reset) + + # TODO(blk-u): check if the following line is necessary and remove if + # it's not. TestCase already does CONF.reset. + self.addCleanup(CONF.reset) + + # need to reset the plug-ins + self.addCleanup(setattr, auth.controllers, 'AUTH_METHODS', {}) + + self.addCleanup(self.teardown_database) def load_backends(self): self.config(self.config_files()) diff --git a/keystone/tests/test_v3_protection.py b/keystone/tests/test_v3_protection.py index be78789413..434b934f20 100644 --- a/keystone/tests/test_v3_protection.py +++ b/keystone/tests/test_v3_protection.py @@ -57,8 +57,12 @@ class IdentityTestProtectedCase(test_v3.RestfulTestCase): # Initialize the policy engine and allow us to write to a temp # file in each test to create the policies self.orig_policy_file = CONF.policy_file + self.addCleanup(rules.reset) rules.reset() _unused, self.tmpfilename = tempfile.mkstemp() + + #TODO(blk-u): This seems unnecessary since TestCase resets conf. + self.addCleanup(self.opt, policy_file=self.orig_policy_file) self.opt(policy_file=self.tmpfilename) # A default auth request we can use - un-scoped user token @@ -66,11 +70,6 @@ class IdentityTestProtectedCase(test_v3.RestfulTestCase): user_id=self.user1['id'], password=self.user1['password']) - def tearDown(self): - super(IdentityTestProtectedCase, self).tearDown() - rules.reset() - self.opt(policy_file=self.orig_policy_file) - def load_sample_data(self): # Start by creating a couple of domains self.domainA = self.new_domain_ref() @@ -413,6 +412,10 @@ class IdentityTestv3CloudPolicySample(test_v3.RestfulTestCase): # Finally, switch to the v3 sample policy file self.orig_policy_file = CONF.policy_file + self.addCleanup(self.opt, policy_file=self.orig_policy_file) + # TODO(blk-u): Resetting the conf setting is probably unnecessary since + # TestCase does it. + self.addCleanup(rules.reset) rules.reset() self.opt(policy_file=tests.dirs.etc('policy.v3cloudsample.json')) @@ -475,11 +478,6 @@ class IdentityTestv3CloudPolicySample(test_v3.RestfulTestCase): user_id=self.just_a_user['id'], project_id=self.project['id']) - def tearDown(self): - super(IdentityTestv3CloudPolicySample, self).tearDown() - rules.reset() - self.opt(policy_file=self.orig_policy_file) - def _stati(self, expected_status): # Return the expected return codes for APIs with and without data # with any specified status overriding the normal values diff --git a/keystone/tests/test_wsgi.py b/keystone/tests/test_wsgi.py index 4da50c1d19..fcd00b191e 100644 --- a/keystone/tests/test_wsgi.py +++ b/keystone/tests/test_wsgi.py @@ -190,15 +190,13 @@ class MiddlewareTest(BaseWSGITest): class LocalizedResponseTest(tests.TestCase): def setUp(self): super(LocalizedResponseTest, self).setUp() + gettextutils._AVAILABLE_LANGUAGES.clear() + self.addCleanup(gettextutils._AVAILABLE_LANGUAGES.clear) fixture = self.useFixture(moxstubout.MoxStubout()) self.stubs = fixture.stubs - def tearDown(self): - gettextutils._AVAILABLE_LANGUAGES.clear() - super(LocalizedResponseTest, self).tearDown() - def _set_expected_languages(self, all_locales=[], avail_locales=None): # Override localedata.locale_identifiers to return some locales. def returns_some_locales(*args, **kwargs):