testing: fixed an issue with the handling of tempdirs #866
Conversation
|
This PR adds a fix for #862 |
|
Btw, integration/test_ncbi.py uses class-scope tempdirs rather than function-scope ones. Is it necessary for tests in each test class there to share a tempdir? |
|
@tomkinsc calling tempfile.gettempdir() still works: |
|
The tempdirs would ideally be function-scoped. With the changes from this PR applied that appears to be true for |
|
That's true. I just want to check that it was not important for them to be class-scoped, as they were originally. |
test/integration/test_ncbi.py
Outdated
| @@ -20,7 +20,7 @@ | |||
| log = logging.getLogger(__name__) | |||
|
|
|||
|
|
|||
| skip_test = True | |||
| skip_test = False | |||
There was a problem hiding this comment.
These tests were mostly disabled because instability in the NCBI API due to load frequently causes the tests to fail, even with the retries in genbank.py. Enabling them was mostly to illustrate the tempdir issue, so we can probably switch them off again.
| class TestCaseWithTmp(unittest.TestCase): | ||
| 'Base class for tests that use tempDir' | ||
|
|
||
| @classmethod |
There was a problem hiding this comment.
This removal fixes the issue I was having, but does it break unittest?
There was a problem hiding this comment.
@tomkinsc What mode of breakage are you thinking about? Can you maybe add a test to test for it? Current test suite doesn't seem to break.
There was a problem hiding this comment.
Incompatibility when testing under the built-in unittest module, which I suppose we don't need to worry about if we're going all-in on pytest.
There was a problem hiding this comment.
@tomkinsc I've been adding tests to the test suite that are pytest-based only, which would not be run if pytest is not used. Is there a goal of keeping the test suite runnable without pytest?
There was a problem hiding this comment.
Originally I think we had it all set up to be backwards compatible with unittest. But I think we've already crossed the bridge to being pytest-dependent. A fair amount of the metagenomics tests are not unittest compatible. Anyway I wouldn't worry about it at this point...
class tempdir in module tempdir, function tempdir in class tempdir
Fixed an issue with the handling of temporary directories during testing, caused by an interaction of a legacy unittest-based class with the newer pytest-based fixtures.