From fe86f5e4ae8aee42d685dcedb574b95a5c5ce85a Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Tue, 17 Sep 2019 15:54:42 -0500 Subject: [PATCH] Some test cleanup - Tests are based on oslotest.base.BaseTestCase, which uses the NestedTempFile fixture, which uses the TempDir fixture, which adds a cleanup routine to remove the base temporary directory it creates. There's therefore no need for tests to clean up this directory, so all the code that does that is removed. - The eventlet incarnation of tests was trying to make use of the `env` external without whitelisting it, resulting in an ugly red deprecation warning. This commit adds `env` to whitelist_externals in [testenv]. - A handful of typos in the Beowulf quote are corrected. Change-Id: I91cc52e00e0a918dadd2a3a771bd322b0f165ed2 --- oslo_concurrency/tests/unit/test_lockutils.py | 165 ++++++------------ .../tests/unit/test_lockutils_eventlet.py | 46 +++-- .../tests/unit/test_processutils.py | 10 +- tox.ini | 1 + 4 files changed, 84 insertions(+), 138 deletions(-) diff --git a/oslo_concurrency/tests/unit/test_lockutils.py b/oslo_concurrency/tests/unit/test_lockutils.py index a2ce7b7..b124963 100644 --- a/oslo_concurrency/tests/unit/test_lockutils.py +++ b/oslo_concurrency/tests/unit/test_lockutils.py @@ -15,7 +15,6 @@ import collections import multiprocessing import os -import shutil import signal import subprocess import sys @@ -188,65 +187,45 @@ class LockTestCase(test_base.BaseTestCase): def test_nested_synchronized_external_works(self): """We can nest external syncs.""" - tempdir = tempfile.mkdtemp() - try: - self.config(lock_path=tempdir, group='oslo_concurrency') - sentinel = object() + self.config(lock_path=tempfile.mkdtemp(), group='oslo_concurrency') + sentinel = object() - @lockutils.synchronized('testlock1', 'test-', external=True) - def outer_lock(): + @lockutils.synchronized('testlock1', 'test-', external=True) + def outer_lock(): - @lockutils.synchronized('testlock2', 'test-', external=True) - def inner_lock(): - return sentinel - return inner_lock() + @lockutils.synchronized('testlock2', 'test-', external=True) + def inner_lock(): + return sentinel + return inner_lock() - self.assertEqual(sentinel, outer_lock()) - - finally: - if os.path.exists(tempdir): - shutil.rmtree(tempdir) + self.assertEqual(sentinel, outer_lock()) def _do_test_lock_externally(self): """We can lock across multiple processes.""" - handles_dir = tempfile.mkdtemp() - try: - children = [] - for n in range(50): - queue = multiprocessing.Queue() - proc = multiprocessing.Process( - target=lock_files, - args=(handles_dir, queue)) - proc.start() - children.append((proc, queue)) - for child, queue in children: - child.join() - count = queue.get(block=False) - self.assertEqual(50, count) - finally: - if os.path.exists(handles_dir): - shutil.rmtree(handles_dir, ignore_errors=True) + children = [] + for n in range(50): + queue = multiprocessing.Queue() + proc = multiprocessing.Process( + target=lock_files, + args=(tempfile.mkdtemp(), queue)) + proc.start() + children.append((proc, queue)) + for child, queue in children: + child.join() + count = queue.get(block=False) + self.assertEqual(50, count) def test_lock_externally(self): - lock_dir = tempfile.mkdtemp() - self.config(lock_path=lock_dir, group='oslo_concurrency') + self.config(lock_path=tempfile.mkdtemp(), group='oslo_concurrency') - try: - self._do_test_lock_externally() - finally: - if os.path.exists(lock_dir): - shutil.rmtree(lock_dir, ignore_errors=True) + self._do_test_lock_externally() def test_lock_externally_lock_dir_not_exist(self): lock_dir = tempfile.mkdtemp() os.rmdir(lock_dir) self.config(lock_path=lock_dir, group='oslo_concurrency') - try: - self._do_test_lock_externally() - finally: - if os.path.exists(lock_dir): - shutil.rmtree(lock_dir, ignore_errors=True) + self._do_test_lock_externally() def test_synchronized_with_prefix(self): lock_name = 'mylock' @@ -264,88 +243,64 @@ class LockTestCase(test_base.BaseTestCase): self.assertTrue(bar(lock_dir, lock_pfix, lock_name)) def test_synchronized_without_prefix(self): - lock_dir = tempfile.mkdtemp() - self.config(lock_path=lock_dir, group='oslo_concurrency') + self.config(lock_path=tempfile.mkdtemp(), group='oslo_concurrency') @lockutils.synchronized('lock', external=True) def test_without_prefix(): # We can't check much pass - try: - test_without_prefix() - finally: - if os.path.exists(lock_dir): - shutil.rmtree(lock_dir, ignore_errors=True) + test_without_prefix() def test_synchronized_prefix_without_hypen(self): - lock_dir = tempfile.mkdtemp() - self.config(lock_path=lock_dir, group='oslo_concurrency') + self.config(lock_path=tempfile.mkdtemp(), group='oslo_concurrency') @lockutils.synchronized('lock', 'hypen', True) def test_without_hypen(): # We can't check much pass - try: - test_without_hypen() - finally: - if os.path.exists(lock_dir): - shutil.rmtree(lock_dir, ignore_errors=True) + test_without_hypen() def test_contextlock(self): - lock_dir = tempfile.mkdtemp() - self.config(lock_path=lock_dir, group='oslo_concurrency') + self.config(lock_path=tempfile.mkdtemp(), group='oslo_concurrency') - try: - # Note(flaper87): Lock is not external, which means - # a semaphore will be yielded - with lockutils.lock("test") as sem: - if six.PY2: - self.assertIsInstance(sem, threading._Semaphore) - else: - self.assertIsInstance(sem, threading.Semaphore) + # Note(flaper87): Lock is not external, which means + # a semaphore will be yielded + with lockutils.lock("test") as sem: + if six.PY2: + self.assertIsInstance(sem, threading._Semaphore) + else: + self.assertIsInstance(sem, threading.Semaphore) - # NOTE(flaper87): Lock is external so an InterProcessLock - # will be yielded. - with lockutils.lock("test2", external=True) as lock: - self.assertTrue(lock.exists()) + # NOTE(flaper87): Lock is external so an InterProcessLock + # will be yielded. + with lockutils.lock("test2", external=True) as lock: + self.assertTrue(lock.exists()) - with lockutils.lock("test1", - external=True) as lock1: - self.assertIsInstance(lock1, - lockutils.InterProcessLock) - finally: - if os.path.exists(lock_dir): - shutil.rmtree(lock_dir, ignore_errors=True) + with lockutils.lock("test1", external=True) as lock1: + self.assertIsInstance(lock1, lockutils.InterProcessLock) def test_contextlock_unlocks(self): - lock_dir = tempfile.mkdtemp() - self.config(lock_path=lock_dir, group='oslo_concurrency') + self.config(lock_path=tempfile.mkdtemp(), group='oslo_concurrency') - sem = None + with lockutils.lock("test") as sem: + if six.PY2: + self.assertIsInstance(sem, threading._Semaphore) + else: + self.assertIsInstance(sem, threading.Semaphore) - try: - with lockutils.lock("test") as sem: - if six.PY2: - self.assertIsInstance(sem, threading._Semaphore) - else: - self.assertIsInstance(sem, threading.Semaphore) - - with lockutils.lock("test2", external=True) as lock: - self.assertTrue(lock.exists()) - - # NOTE(flaper87): Lock should be free - with lockutils.lock("test2", external=True) as lock: - self.assertTrue(lock.exists()) + with lockutils.lock("test2", external=True) as lock: + self.assertTrue(lock.exists()) # NOTE(flaper87): Lock should be free - # but semaphore should already exist. - with lockutils.lock("test") as sem2: - self.assertEqual(sem, sem2) - finally: - if os.path.exists(lock_dir): - shutil.rmtree(lock_dir, ignore_errors=True) + with lockutils.lock("test2", external=True) as lock: + self.assertTrue(lock.exists()) + + # NOTE(flaper87): Lock should be free + # but semaphore should already exist. + with lockutils.lock("test") as sem2: + self.assertEqual(sem, sem2) def _test_remove_lock_external_file(self, lock_dir, use_external=False): lock_name = 'mylock' @@ -361,17 +316,13 @@ class LockTestCase(test_base.BaseTestCase): for ent in os.listdir(lock_dir): self.assertRaises(OSError, ent.startswith, lock_pfix) - if os.path.exists(lock_dir): - shutil.rmtree(lock_dir, ignore_errors=True) - def test_remove_lock_external_file(self): lock_dir = tempfile.mkdtemp() self.config(lock_path=lock_dir, group='oslo_concurrency') self._test_remove_lock_external_file(lock_dir) def test_remove_lock_external_file_lock_path(self): - lock_dir = tempfile.mkdtemp() - self._test_remove_lock_external_file(lock_dir, + self._test_remove_lock_external_file(tempfile.mkdtemp(), use_external=True) def test_no_slash_in_b64(self): diff --git a/oslo_concurrency/tests/unit/test_lockutils_eventlet.py b/oslo_concurrency/tests/unit/test_lockutils_eventlet.py index bb333d7..cb09298 100644 --- a/oslo_concurrency/tests/unit/test_lockutils_eventlet.py +++ b/oslo_concurrency/tests/unit/test_lockutils_eventlet.py @@ -13,7 +13,6 @@ # under the License. import os -import shutil import tempfile import eventlet @@ -28,32 +27,27 @@ class TestFileLocks(test_base.BaseTestCase): def test_concurrent_green_lock_succeeds(self): """Verify spawn_n greenthreads with two locks run concurrently.""" tmpdir = tempfile.mkdtemp() - try: - self.completed = False + self.completed = False - def locka(wait): - a = lockutils.InterProcessLock(os.path.join(tmpdir, 'a')) - with a: - wait.wait() - self.completed = True + def locka(wait): + a = lockutils.InterProcessLock(os.path.join(tmpdir, 'a')) + with a: + wait.wait() + self.completed = True - def lockb(wait): - b = lockutils.InterProcessLock(os.path.join(tmpdir, 'b')) - with b: - wait.wait() + def lockb(wait): + b = lockutils.InterProcessLock(os.path.join(tmpdir, 'b')) + with b: + wait.wait() - wait1 = eventlet.event.Event() - wait2 = eventlet.event.Event() - pool = greenpool.GreenPool() - pool.spawn_n(locka, wait1) - pool.spawn_n(lockb, wait2) - wait2.send() - eventlet.sleep(0) - wait1.send() - pool.waitall() + wait1 = eventlet.event.Event() + wait2 = eventlet.event.Event() + pool = greenpool.GreenPool() + pool.spawn_n(locka, wait1) + pool.spawn_n(lockb, wait2) + wait2.send() + eventlet.sleep(0) + wait1.send() + pool.waitall() - self.assertTrue(self.completed) - - finally: - if os.path.exists(tmpdir): - shutil.rmtree(tmpdir) + self.assertTrue(self.completed) diff --git a/oslo_concurrency/tests/unit/test_processutils.py b/oslo_concurrency/tests/unit/test_processutils.py index 820839f..982d293 100644 --- a/oslo_concurrency/tests/unit/test_processutils.py +++ b/oslo_concurrency/tests/unit/test_processutils.py @@ -190,18 +190,18 @@ class ProcessExecutionErrorTest(test_base.BaseTestCase): stdout = """ Lo, praise of the prowess of people-kings of spear-armed Danes, in days long sped, - we have heard, and what honot the athelings won! + we have heard, and what honor the athelings won! Oft Scyld the Scefing from squadroned foes, from many a tribe, the mead-bench tore, - awing the earls. Since erse he lay + awing the earls. Since erst he lay friendless, a foundling, fate repaid him: - for he waxed under welkin, in wealth he trove, + for he waxed under welkin, in wealth he throve, till before him the folk, both far and near, who house by the whale-path, heard his mandate, - gabe him gits: a good king he! + gave him gifts: a good king he! To him an heir was afterward born, a son in his halls, whom heaven sent - to favor the fol, feeling their woe + to favor the folk, feeling their woe that erst they had lacked an earl for leader so long a while; the Lord endowed him, the Wielder of Wonder, with world's renown. diff --git a/tox.ini b/tox.ini index 00f953f..ba5ebd6 100644 --- a/tox.ini +++ b/tox.ini @@ -9,6 +9,7 @@ deps = -r{toxinidir}/test-requirements.txt -r{toxinidir}/requirements.txt # We want to support both vanilla stdlib and eventlet monkey patched +whitelist_externals = env commands = lockutils-wrapper stestr run --slowest {posargs} env TEST_EVENTLET=1 lockutils-wrapper stestr run --slowest {posargs}