DM-11595: Use mkdtemp for test directories #73
Conversation
timj
force-pushed
the
tickets/DM-11595
branch
4 times, most recently
from
a8129d1
to
c3fd82c
Compare
* Use assertEqual rather than assertEquals * Close a file handle that was open and use "with open"
tests/test_mapperImport.py
Outdated
| @@ -38,15 +39,16 @@ class MapperImportTestCase(unittest.TestCase): | |||
| """A test case for the data butler finding a Mapper in a root""" | |||
|
|
|||
| def setUp(self): | |||
| if os.path.exists(os.path.join(ROOT, 'root/out')): | |||
| shutil.rmtree(os.path.join(ROOT, 'root/out')) | |||
| self.testDir = os.path.join(ROOT, 'root') | |||
There was a problem hiding this comment.
nit: I think inputDir would be more accurate?
tests/test_mapperImport.py
Outdated
| shutil.rmtree(os.path.join(ROOT, 'root/out')) | ||
| self.testDir = os.path.join(ROOT, 'root') | ||
| self.rootPath = tempfile.mkdtemp(dir=self.testDir, prefix="out-") | ||
| self.outPath = os.path.split(self.rootPath)[1] |
There was a problem hiding this comment.
by removing the absolute part of the path and passing only the temp dir name to the outPath iarg, you're creating an output repository relative to the current working dir. Which would be fine, but in tearDown you're deleting self.rootPath, not self.outPath, so outpath will never get cleaned up. Right? I think you can just pass self.rootPath to the outPath iarg.
Or maybe I'm faded from travel. I need to board... off I go.
tests/test_mapperImport.py
Outdated
| self.assert_(os.path.exists( | ||
| os.path.join(ROOT, "root", "out", "foo%d.pickle" % ccd))) | ||
| self.assertTrue(os.path.exists( | ||
| os.path.join(self.rootPath, "foo%d.pickle" % ccd))) |
There was a problem hiding this comment.
I think this will fail if the current working dir is not the same as the directory above outpath?
There was a problem hiding this comment.
confirmed; running tests from inside daf_persistence/python (as an example) fails
n8pease@Nates-MacBook-Pro ~/3/lsstsw/build/daf_persistence/python[tickets/DM-11595*]$ py.test ~/3/lsstsw/build/daf_persistence/tests/test_mapperImport.py
tests/test_reposInButler.py
Outdated
| def setup(self): | ||
| self.clean() | ||
| def setUp(self): | ||
| self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='repoOfRepos-') |
There was a problem hiding this comment.
not your bug but while your here you could fix "repoOfRepos" to accurately show the test class name "reposInButler"
| @@ -374,10 +378,12 @@ class TestMultipleInputs(unittest.TestCase): | |||
| - verify dissimilar datasets with same id that were written to the repos | |||
|
|
|||
| """ | |||
| def setUp(self): | |||
| self.testDir = tempfile.mkdtemp(dir=ROOT, prefix="TestMapperInference-") | |||
There was a problem hiding this comment.
the prefix here should be "TestMultipleInputs"
tests/test_repository.py
Outdated
| self.testDir = os.path.join(ROOT, 'TestParentRepository') | ||
| if os.path.exists(self.testDir): | ||
| shutil.rmtree(self.testDir) | ||
| self.testDir = tempfile.mkdtemp(dir=ROOT, prefix="TestBasics-") |
There was a problem hiding this comment.
prefix should be "TestParentRepository"
tests/test_repositoryCfg.py
Outdated
| @@ -39,102 +39,105 @@ | |||
| class TestCfgRelationship(unittest.TestCase): | |||
|
|
|||
| def setUp(self): | |||
| self.tearDown() | |||
| self.testDir = tempfile.mkdtemp(dir=ROOT, | |||
| prefix="repositoryCfg-") | |||
There was a problem hiding this comment.
prefix should be TestCfgRelationship
tests/test_repositoryCfg.py
Outdated
| def setUp(self): | ||
| self.tearDown() | ||
| self.testDir = tempfile.mkdtemp(dir=ROOT, | ||
| prefix="repositoryCfg_TestNestedCfg-") |
There was a problem hiding this comment.
I see you just used what was already there but the prefix here can probably just be TestNestedCfg
tests/test_repositoryCfg.py
Outdated
| @@ -214,19 +216,18 @@ class TestCfgFileVersion(unittest.TestCase): | |||
| """ | |||
|
|
|||
| def setUp(self): | |||
| self.tearDown() | |||
| self.testDir = tempfile.mkdtemp(dir=ROOT, | |||
| prefix="repositoryCfg-") | |||
There was a problem hiding this comment.
prefix -> TestCfgFileVersion
tests/test_safeFileIo.py
Outdated
| @@ -45,37 +46,37 @@ def setup_module(module): | |||
| class WriteOnceCompareSameTest(unittest.TestCase): | |||
|
|
|||
| def setUp(self): | |||
| self.tearDown() | |||
| self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='safeFileIo-') | |||
There was a problem hiding this comment.
prefix -> "WriteOnceCompareSameTest"
tests/test_repository.py
Outdated
| if os.path.exists(self.testDir): | ||
| shutil.rmtree(self.testDir) | ||
| self.testDir = tempfile.mkdtemp(dir=ROOT, prefix="TestBasics-") | ||
| self.testDir = os.path.join(self.testDir, 'TestParentRepository') |
There was a problem hiding this comment.
this line shouldn't be here? It's preventing files inside TestBasics-... (which should be TestParentRepository-...)
from getting removed.
There was a problem hiding this comment.
Of course. Thanks for spotting that. I had to change many many lines...
@n8pease work in progress. In particular test_repository.TestBasics doesn't work and I don't know why...