From fd1202b7745a536428cdfe1d862a07bf59a3ad43 Mon Sep 17 00:00:00 2001 From: Elrond Date: Sun, 21 Apr 2013 19:19:40 +0200 Subject: [PATCH] Cleanup storage after test, and test .delete_dir(). The storage tests work in the system's tmpdir. The python docs say, we should clean up after using things. Yes the directory should be cleaned up on reboot, but if running tests a lot, the tmpdir could fill up, so we should really cleanup. So use the new .delete_dir() on the storage interface to cleanup test dirs and get them finally removed with os.rmdir. All nicely packed into cleanup_storage(). --- mediagoblin/tests/test_storage.py | 29 +++++++++++++++++++++++------ mediagoblin/tests/test_workbench.py | 4 +++- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/mediagoblin/tests/test_storage.py b/mediagoblin/tests/test_storage.py index 749f7b07..f6f1d18f 100644 --- a/mediagoblin/tests/test_storage.py +++ b/mediagoblin/tests/test_storage.py @@ -95,6 +95,14 @@ def get_tmp_filestorage(mount_url=None, fake_remote=False): return tmpdir, this_storage +def cleanup_storage(this_storage, tmpdir, *paths): + for p in paths: + while p: + assert this_storage.delete_dir(p) == True + p.pop(-1) + os.rmdir(tmpdir) + + def test_basic_storage__resolve_filepath(): tmpdir, this_storage = get_tmp_filestorage() @@ -111,7 +119,7 @@ def test_basic_storage__resolve_filepath(): this_storage._resolve_filepath, ['../../', 'etc', 'passwd']) - os.rmdir(tmpdir) + cleanup_storage(this_storage, tmpdir) def test_basic_storage_file_exists(): @@ -127,6 +135,7 @@ def test_basic_storage_file_exists(): assert not this_storage.file_exists(['dnedir1', 'dnedir2', 'somefile.lol']) this_storage.delete_file(['dir1', 'dir2', 'filename.txt']) + cleanup_storage(this_storage, tmpdir, ['dir1', 'dir2']) def test_basic_storage_get_unique_filepath(): @@ -149,6 +158,7 @@ def test_basic_storage_get_unique_filepath(): assert new_filename == secure_filename(new_filename) os.remove(filename) + cleanup_storage(this_storage, tmpdir, ['dir1', 'dir2']) def test_basic_storage_get_file(): @@ -189,6 +199,7 @@ def test_basic_storage_get_file(): this_storage.delete_file(filepath) this_storage.delete_file(new_filepath) this_storage.delete_file(['testydir', 'testyfile.txt']) + cleanup_storage(this_storage, tmpdir, ['dir1', 'dir2'], ['testydir']) def test_basic_storage_delete_file(): @@ -204,11 +215,15 @@ def test_basic_storage_delete_file(): assert os.path.exists( os.path.join(tmpdir, 'dir1/dir2/ourfile.txt')) + assert this_storage.delete_dir(['dir1', 'dir2']) == False this_storage.delete_file(filepath) + assert this_storage.delete_dir(['dir1', 'dir2']) == True assert not os.path.exists( os.path.join(tmpdir, 'dir1/dir2/ourfile.txt')) + cleanup_storage(this_storage, tmpdir, ['dir1']) + def test_basic_storage_url_for_file(): # Not supplying a base_url should actually just bork. @@ -217,7 +232,7 @@ def test_basic_storage_url_for_file(): storage.NoWebServing, this_storage.file_url, ['dir1', 'dir2', 'filename.txt']) - os.rmdir(tmpdir) + cleanup_storage(this_storage, tmpdir) # base_url without domain tmpdir, this_storage = get_tmp_filestorage('/media/') @@ -225,7 +240,7 @@ def test_basic_storage_url_for_file(): ['dir1', 'dir2', 'filename.txt']) expected = '/media/dir1/dir2/filename.txt' assert result == expected - os.rmdir(tmpdir) + cleanup_storage(this_storage, tmpdir) # base_url with domain tmpdir, this_storage = get_tmp_filestorage( @@ -234,7 +249,7 @@ def test_basic_storage_url_for_file(): ['dir1', 'dir2', 'filename.txt']) expected = 'http://media.example.org/ourmedia/dir1/dir2/filename.txt' assert result == expected - os.rmdir(tmpdir) + cleanup_storage(this_storage, tmpdir) def test_basic_storage_get_local_path(): @@ -248,13 +263,13 @@ def test_basic_storage_get_local_path(): assert result == expected - os.rmdir(tmpdir) + cleanup_storage(this_storage, tmpdir) def test_basic_storage_is_local(): tmpdir, this_storage = get_tmp_filestorage() assert this_storage.local_storage is True - os.rmdir(tmpdir) + cleanup_storage(this_storage, tmpdir) def test_basic_storage_copy_locally(): @@ -275,6 +290,7 @@ def test_basic_storage_copy_locally(): os.remove(new_file_dest) os.rmdir(dest_tmpdir) + cleanup_storage(this_storage, tmpdir, ['dir1', 'dir2']) def _test_copy_local_to_storage_works(tmpdir, this_storage): @@ -292,6 +308,7 @@ def _test_copy_local_to_storage_works(tmpdir, this_storage): 'r').read() == 'haha' this_storage.delete_file(['dir1', 'dir2', 'copiedto.txt']) + cleanup_storage(this_storage, tmpdir, ['dir1', 'dir2']) def test_basic_storage_copy_local_to_storage(): diff --git a/mediagoblin/tests/test_workbench.py b/mediagoblin/tests/test_workbench.py index 9cd49671..6695618b 100644 --- a/mediagoblin/tests/test_workbench.py +++ b/mediagoblin/tests/test_workbench.py @@ -21,7 +21,7 @@ import tempfile from mediagoblin.tools import workbench from mediagoblin.mg_globals import setup_globals from mediagoblin.decorators import get_workbench -from mediagoblin.tests.test_storage import get_tmp_filestorage +from mediagoblin.tests.test_storage import get_tmp_filestorage, cleanup_storage class TestWorkbench(object): @@ -76,6 +76,7 @@ class TestWorkbench(object): assert filename == os.path.join( tmpdir, 'dir1/dir2/ourfile.txt') this_storage.delete_file(filepath) + cleanup_storage(this_storage, tmpdir, ['dir1', 'dir2']) # with a fake remote file storage tmpdir, this_storage = get_tmp_filestorage(fake_remote=True) @@ -102,6 +103,7 @@ class TestWorkbench(object): this_workbench.dir, 'thisfile.text') this_storage.delete_file(filepath) + cleanup_storage(this_storage, tmpdir, ['dir1', 'dir2']) this_workbench.destroy() def test_workbench_decorator(self): -- 2.25.1