pytest error: tempfile.py:368: FileNotFoundError

[tomkinsc]

This may be related to changes introduced in #840.

When running pytest, the fixtures responsible for creating temp directories can attempt to create a temp directory within a containing directory that does not exist, resulting in an error from tempfile.py: tempfile.py:368: FileNotFoundError

How to reproduce (once the expected test output has been updated to what GenBank now returns):
Within test/integration/test_ncbi.py, change skip_test = True to skip_test = False.
Then run:
pytest test/integration/test_ncbi.py
Note that the issue does not appear when calling one of the test classes directly.
(ex. pytest test/integration/test_ncbi.py::TestFastaFetch).

Potentially relevant from the pytest changelog is a change in pytest 3.3.0 related to testdir /tmpdir: pytest-dev/pytest#2751
Maybe with the upgrade to pytest 3.6.3 in #856, we no longer need the custom tmpdir fixtures in conftest.py?

[dpark01]

If you think our custom fixtures could be eliminated by using newly available features in the latest pytest, I’m all for reducing our custom test code.

[tomkinsc]

It's worth trying, but since the custom fixtures were added to squash a bug that appeared only intermittently it would be helpful to have @notestaff take a look since he was able to reliably reproduce the issue (by running pytest many times?).

[dpark01]

Yeah I actually don’t remember why we have these custom fixtures.. I just remember we tweaked them recently.

[yesimon]

pytest doesn't have great handling for ordering tmpfile generation, which was the original purpose to order the module vs function vs session level tmpfiles.

[notestaff]

also, pytest doesn't use mkdtemp, which is the safer way to create temdirs.
also, using own fixtures lets us preserve the tempdirs during testing by setting an env var.

i'll push the PR fixing this shortly, the changes are at https://github.com/broadinstitute/viral-ngs/compare/is-fix-test-tempfiles

[tomkinsc]

How should we access the tempdir within a test class method, tempfile.gettempdir()? Do we really need to add an ugly pytest fixture for it?

[notestaff]

I'm not sure I understand. In what way is adding a test method parameter uglier than adding a line of code that calls tempfile.gettempdir()?

[tomkinsc]

IMHO, pytest's param-like fixtures can be ambiguous since it is not always clear where they are defined, especially if in a different scope. The nativeunittest module has setUp() setUpClass(), which are at least in-scope.

[notestaff]

While fixtures can get implicitly imported from conftest.py, which can be at the root or in a local dir, it's not the same level of bad as "from X import *", since if it's not local it's in one of the few conftest.py .