Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-11595: Use mkdtemp for test directories #73

Merged
merged 6 commits into from
Aug 18, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion tests/test_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import lsst.utils.tests
import os
import shutil
import tempfile

ROOT = os.path.abspath(os.path.dirname(__file__))

Expand All @@ -39,7 +40,8 @@ def setup_module(module):
class ButlerTest(unittest.TestCase):
"""Test case for basic Butler operations."""

testDir = os.path.join(ROOT, 'ButlerTest')
def setUp(self):
self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='ButlerTest-')

def tearDown(self):
if os.path.exists(self.testDir):
Expand Down
11 changes: 4 additions & 7 deletions tests/test_butlerProxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import os
import shutil
import unittest
import tempfile
import lsst.utils.tests

import lsst.daf.persistence as dafPersist
Expand All @@ -40,20 +41,16 @@ class ButlerProxyTestCase(unittest.TestCase):
"""A test case for the data butler finding a Mapper in a root"""

inputDir = os.path.join(ROOT, 'root')
outputDir = os.path.join(ROOT, 'ButlerProxyTestCase')

def removeTestDir(self):
if os.path.exists(self.outputDir):
shutil.rmtree(self.outputDir)

def setUp(self):
self.removeTestDir()
self.outputDir = tempfile.mkdtemp(dir=ROOT, prefix='ButlerProxyTestCase-')
self.butler = dafPersist.Butler(self.inputDir,
outPath=os.path.join(self.outputDir, "proxyOut"))

def tearDown(self):
del self.butler
self.removeTestDir()
if os.path.exists(self.outputDir):
shutil.rmtree(self.outputDir)

def testCheckProxy(self):
"""Attempt to cycle a DateTime object through the butler
Expand Down
16 changes: 9 additions & 7 deletions tests/test_mapperImport.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import pickle
import shutil
import unittest
import tempfile
import lsst.utils.tests

import lsst.daf.persistence as dafPersist
Expand All @@ -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')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think inputDir would be more accurate?

self.rootPath = tempfile.mkdtemp(dir=self.testDir, prefix="out-")
self.outPath = os.path.split(self.rootPath)[1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


self.butler = dafPersist.Butler(os.path.join(ROOT, "root"), outPath="out")
self.butler = dafPersist.Butler(self.testDir, outPath=self.outPath)

def tearDown(self):
del self.butler
if os.path.exists(os.path.join(ROOT, 'root/out')):
shutil.rmtree(os.path.join(ROOT, 'root/out'))
if os.path.exists(self.rootPath):
shutil.rmtree(self.rootPath)

def testMapperClass(self):
repository = self.butler._repos.outputs()[0].repo
Expand All @@ -56,8 +58,8 @@ def checkIO(self, butler, bbox, ccd):
butler.put(bbox, "x", ccd=ccd)
y = butler.get("x", ccd=ccd, immediate=True)
self.assertEqual(bbox, y)
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)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will fail if the current working dir is not the same as the directory above outpath?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 


def testIO(self):
bbox = [[3, 4], [5, 6]]
Expand Down
19 changes: 9 additions & 10 deletions tests/test_pexPolicyToButlerPolicy.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,28 +24,27 @@
import os
import shutil
import unittest
import tempfile

from past.builtins import basestring

import lsst.daf.persistence
import lsst.pex.policy
import lsst.utils.tests
from lsst.utils import getPackageDir

pafPolicyPath = os.path.join(os.path.dirname(__file__), 'pexToButlerPolicy', 'policy.paf')
packageDir = getPackageDir('daf_persistence')
ROOT = os.path.abspath(os.path.dirname(__file__))


class PolicyTestCase(unittest.TestCase):
"""A test case for the butler policy to verify that it can load a pex policy properly."""

def setUp(self):
self.testData = os.path.join(packageDir, 'tests', 'testPexPolicyToButlerPolicy')
self.tearDown()
os.makedirs(self.testData)
self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='testPexPolicyToButlerPolicy-')

def tearDown(self):
if os.path.exists(self.testData):
shutil.rmtree(self.testData)
if os.path.exists(self.testDir):
shutil.rmtree(self.testDir)

def testPafReader(self):
"""Test that Butler Policy can read a paf file and the keys compare the same as when the same file is
Expand Down Expand Up @@ -90,9 +89,9 @@ def testDumpAndLoad(self):
as a pex policy, verify they compare equal.
"""
policy = lsst.daf.persistence.Policy(pafPolicyPath)
yamlPolicyFile = os.path.join(self.testData, 'policy.yaml')
policy.dumpToFile(os.path.join(self.testData, 'policy.yaml'))
self.assertTrue(os.path.exists(os.path.join(self.testData, 'policy.yaml')))
yamlPolicyFile = os.path.join(self.testDir, 'policy.yaml')
policy.dumpToFile(os.path.join(self.testDir, 'policy.yaml'))
self.assertTrue(os.path.exists(os.path.join(self.testDir, 'policy.yaml')))

# test that the data went through the entire wringer correctly - verify the
# original pex data matches the lsst.daf.persistence.Policy data
Expand Down
45 changes: 23 additions & 22 deletions tests/test_posixParentSearch.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
from lsst.utils import getPackageDir
import lsst.utils.tests
import shutil
import tempfile


ROOT = os.path.abspath(os.path.dirname(__file__))


def setup_module(module):
Expand All @@ -35,47 +39,44 @@ def setup_module(module):
class PosixParentSearch(unittest.TestCase):
"""A test case for parentSearch in PosixStorage."""

testDir = os.path.relpath(os.path.join(getPackageDir('daf_persistence'), 'tests', 'PosixParentSearch'))

def setUp(self):
self.tearDown()
os.makedirs(PosixParentSearch.testDir)
self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='PosixParentSearch-')

def tearDown(self):
if os.path.exists(PosixParentSearch.testDir):
shutil.rmtree(PosixParentSearch.testDir)
if os.path.exists(self.testDir):
shutil.rmtree(self.testDir)

def testFilePath(self):
"""Test that a file can be found; when root is part of the path then root is returned with the path
result. When root is not part of the path then root is not returned with the path result."""
with open(os.path.join(PosixParentSearch.testDir, 'foo.txt'), 'w') as f:
with open(os.path.join(self.testDir, 'foo.txt'), 'w') as f:
f.write('abc')
storage = dafPersist.PosixStorage(uri=PosixParentSearch.testDir,
storage = dafPersist.PosixStorage(uri=self.testDir,
create=True)
foundName = storage.search(storage.root, 'foo.txt', searchParents=True)
self.assertEqual(foundName, ['foo.txt'])

searchFor = os.path.join(PosixParentSearch.testDir, 'foo.txt')
searchFor = os.path.join(self.testDir, 'foo.txt')
foundName = storage.search(storage.root, searchFor, searchParents=True)
self.assertEqual(foundName, [searchFor])

def testFilePathWithHeaderExt(self):
"""Find a file with a search string that includes a FITS-style header extension."""
with open(os.path.join(PosixParentSearch.testDir, 'foo.txt'), 'w') as f:
with open(os.path.join(self.testDir, 'foo.txt'), 'w') as f:
f.write('abc')
storage = dafPersist.PosixStorage(uri=PosixParentSearch.testDir,
storage = dafPersist.PosixStorage(uri=self.testDir,
create=True)
foundName = storage.search(storage.root, 'foo.txt[0]', searchParents=True)
self.assertEqual(foundName, ['foo.txt[0]'])

searchFor = os.path.join(PosixParentSearch.testDir, 'foo.txt[0]')
searchFor = os.path.join(self.testDir, 'foo.txt[0]')
foundName = storage.search(storage.root, searchFor, searchParents=True)
self.assertEqual(foundName, [searchFor])

def testFilePathInParent(self):
"""Find a file in a repo that is a grandchild of the repo that has the file"""
parentDir = os.path.join(PosixParentSearch.testDir, 'a')
childDir = os.path.join(PosixParentSearch.testDir, 'b')
parentDir = os.path.join(self.testDir, 'a')
childDir = os.path.join(self.testDir, 'b')
for d in (parentDir, childDir):
os.makedirs(d)
with open(os.path.join(parentDir, 'foo.txt'), 'w') as f:
Expand All @@ -95,9 +96,9 @@ def testFilePathInParent(self):

def testFilePathIn2ndParentParent(self):
"""Find a file in a repo that is the parent of a parent of the root repo."""
grandParentDir = os.path.join(PosixParentSearch.testDir, 'a')
parentDir = os.path.join(PosixParentSearch.testDir, 'b')
childDir = os.path.join(PosixParentSearch.testDir, 'c')
grandParentDir = os.path.join(self.testDir, 'a')
parentDir = os.path.join(self.testDir, 'b')
childDir = os.path.join(self.testDir, 'c')
for d in (grandParentDir, parentDir, childDir):
os.makedirs(d)
for name in ('foo.txt', 'bar.txt'):
Expand All @@ -121,19 +122,19 @@ def testFilePathIn2ndParentParent(self):

def testDoSearchParentFlag(self):
"""Test that parent search can be told to follow _parent symlink (or not) when searching."""
parentDir = os.path.join(PosixParentSearch.testDir, 'a')
childDir = os.path.join(PosixParentSearch.testDir, 'b')
parentDir = os.path.join(self.testDir, 'a')
childDir = os.path.join(self.testDir, 'b')
for d in (parentDir, childDir):
os.makedirs(d)
with open(os.path.join(parentDir, 'foo.txt'), 'w') as f:
f.write('abc')
os.symlink('../a', os.path.join(childDir, '_parent'))
storage = dafPersist.PosixStorage(uri=childDir, create=True)
self.assertEquals(storage.search(storage.root, 'foo.txt', searchParents=True), ['_parent/foo.txt'])
self.assertEquals(storage.search(storage.root, 'foo.txt', searchParents=False), None)
self.assertEqual(storage.search(storage.root, 'foo.txt', searchParents=True), ['_parent/foo.txt'])
self.assertEqual(storage.search(storage.root, 'foo.txt', searchParents=False), None)

def testNoResults(self):
storage = dafPersist.PosixStorage(uri=PosixParentSearch.testDir, create=True)
storage = dafPersist.PosixStorage(uri=self.testDir, create=True)
self.assertIsNone(storage.search(storage.root, 'fileThatDoesNotExist.txt', searchParents=True))


Expand Down
38 changes: 15 additions & 23 deletions tests/test_posixStorage.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
import os
import unittest
import lsst.daf.persistence as dp
from lsst.utils import getPackageDir
import lsst.utils.tests
import shutil
import tempfile

try:
FileType = file
Expand All @@ -34,7 +34,7 @@
FileType = IOBase


packageDir = os.path.join(getPackageDir('daf_persistence'))
ROOT = os.path.abspath(os.path.dirname(__file__))


def setup_module(module):
Expand All @@ -43,21 +43,19 @@ def setup_module(module):
class GetParentFromSymlink(unittest.TestCase):
"""A test case for getting the relative path to parent from a symlink in PosixStorage."""

testDir = os.path.join(packageDir, 'tests', 'GetParentFromSymlink')

def setUp(self):
self.tearDown()
self.parentFolderPath = os.path.join(GetParentFromSymlink.testDir, "theParent")
self.childFolderPath = os.path.join(GetParentFromSymlink.testDir, "theChild")
self.parentlessFolderPath = os.path.join(GetParentFromSymlink.testDir, "parentlessRepo")
self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='GetParentFromSymlink-')
self.parentFolderPath = os.path.join(self.testDir, "theParent")
self.childFolderPath = os.path.join(self.testDir, "theChild")
self.parentlessFolderPath = os.path.join(self.testDir, "parentlessRepo")
for p in (self.parentFolderPath, self.childFolderPath, self.parentlessFolderPath):
os.makedirs(p)
relpath = os.path.relpath(self.parentFolderPath, self.childFolderPath)
os.symlink(relpath, os.path.join(self.childFolderPath, '_parent'))

def tearDown(self):
if os.path.exists(GetParentFromSymlink.testDir):
shutil.rmtree(GetParentFromSymlink.testDir)
if os.path.exists(self.testDir):
shutil.rmtree(self.testDir)

def testV1RepoWithParen(self):
parentPath = dp.PosixStorage.getParentSymlinkPath(self.childFolderPath)
Expand All @@ -71,10 +69,8 @@ def testV1RepoWithoutParent(self):
class TestRelativePath(unittest.TestCase):
"""A test case for the PosixStorage.relativePath function."""

testDir = os.path.join(packageDir, 'tests', 'TestRelativePath')

def setUp(self):
self.tearDown()
self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='TestRelativePath-')

def tearDown(self):
if os.path.exists(self.testDir):
Expand All @@ -100,10 +96,8 @@ def testRelativePath(self):
class TestAbsolutePath(unittest.TestCase):
"""A test case for the PosixStorage.absolutePath function."""

testDir = os.path.join(packageDir, 'tests', 'TestAbsolutePath')

def setUp(self):
self.tearDown()
self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='TestAbsolutePath-')

def tearDown(self):
if os.path.exists(self.testDir):
Expand All @@ -126,10 +120,8 @@ def testAbsolutePath(self):
class TestGetLocalFile(unittest.TestCase):
"""A test case for the PosixStorage.getLocalFile function."""

testDir = os.path.join(packageDir, 'tests', 'TestGetLocalFile')

def setUp(self):
self.tearDown()
self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='TestGetLocalFile-')

def tearDown(self):
if os.path.exists(self.testDir):
Expand All @@ -138,15 +130,15 @@ def tearDown(self):
def testAbsolutePath(self):
"""Tests that GetLocalFile returns a file when it exists and returns
None when it does not exist."""
storage = dp.PosixStorage(TestGetLocalFile.testDir, create=True)
storage = dp.PosixStorage(self.testDir, create=True)
self.assertIsNone(storage.getLocalFile('foo.txt'))
f = open(os.path.join(TestGetLocalFile.testDir, 'foo.txt'), 'w')
f.write('foobarbaz')
f.close()
with open(os.path.join(self.testDir, 'foo.txt'), 'w') as f:
f.write('foobarbaz')
del f
f = storage.getLocalFile('foo.txt')
self.assertIsInstance(f, FileType)
self.assertEqual(f.read(), 'foobarbaz')
f.close()


class MemoryTester(lsst.utils.tests.MemoryTestCase):
Expand Down
17 changes: 7 additions & 10 deletions tests/test_reposInButler.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import os
import shutil
import unittest
import tempfile

import yaml

Expand Down Expand Up @@ -98,15 +99,12 @@ def map_str(self, dataId, write):

class ReposInButler(unittest.TestCase):

def clean(self):
if os.path.exists(os.path.join(ROOT, 'repoOfRepos')):
shutil.rmtree(os.path.join(ROOT, 'repoOfRepos'))

def setup(self):
self.clean()
def setUp(self):
self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='repoOfRepos-')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not your bug but while your here you could fix "repoOfRepos" to accurately show the test class name "reposInButler"


def tearDown(self):
self.clean()
if os.path.exists(self.testDir):
shutil.rmtree(self.testDir)

# disable this test until we work more on repo of repos; starting with DM-6227
@unittest.expectedFailure
Expand All @@ -122,15 +120,14 @@ def test(self):
}

# create a cfg of a repository for our repositories
storageCfg = dp.PosixStorage.cfg(root=os.path.join(ROOT, 'repoOfRepos'))
storageCfg = dp.PosixStorage.cfg(root=self.testDir)
accessCfg = dp.Access.cfg(storageCfg=storageCfg)
mapperCfg = dp.RepositoryMapper.cfg(policy=repoMapperPolicy)
# Note that right now a repo is either input OR output, there is no input-output repo, this design
# is result of butler design conversations. Right now, if a user wants to write to and then read from
# a repo, a repo can have a parent repo with the same access (and mapper) parameters as itself.
repoOfRepoCfg = dp.Repository.cfg(mode='rw',
storageCfg=dp.PosixStorage.cfg(root=os.path.join(ROOT,
'repoOfRepos')),
storageCfg=dp.PosixStorage.cfg(root=self.testDir),
mapper=dp.RepositoryMapper.cfg(policy=repoMapperPolicy))

repoButler = dp.Butler(outputs=repoOfRepoCfg)
Expand Down