[Python-Dev] Adding test.support.safe_rmpath()

[Richard Levasseur]

On Fri, Feb 15, 2019 at 10:02 AM Zachary Ware <zachary.ware+pydev at gmail.com>
wrote:

> On Fri, Feb 15, 2019 at 11:44 AM Steve Dower <steve.dower at python.org>
> wrote:
> > That said, I'd love to have a context manager that we can use to make
> > this easier. Really, none of us should be having to decide "how am I
> > going to use a temporary location on the file system in my test",
> > because we should have one obvious (and easy!) way to do it.
>
> I found an old rejected issue [1] for adding a `tmpdir` method to
> unittest.TestCase, which is actually a solution that we've
> independently developed and use frequently for work.  It basically
> works by registering a cleanup function before returning the path to
> the temporary directory, so you just call `self.tmpdir()`, use the
> path, forget about cleanup, and don't lose a level of indentation to a
> context manager.  I think it would be worthwhile to reconsider this
> addition to unittest, or add it as a standard base test class in
> test.support (though either way it would need a cleaner and more
> robust implementation than is offered in that issue).
>

(Sorry if this starts to veer off the original topic a bit)

I added something similar (though more robust) in the absl testing framework
<https://github.com/abseil/abseil-py/blob/master/absl/testing/absltest.py#L539>.
Tests can just call self.create_tempfile() or self.create_tempdir() and not
have to worry about cleanup, prior state, or the low-level details of where
and how the file gets created. I have re-implemented the same logic quite a
few times, and seen it code reviews even more times, but *rarely* have I
seen it done *correctly* -- it turns out its not easy to do entirely right.

tl;dr: I agree: it would be nice if unittest provided some help here.

I apologize for the length here. I've had to answer "just use tempfile,
whats wrong with that?" a few times, so I've got a whole enumerated list of
points :).

While adding this conceptually simple feature to absl, I unexpectedly found
it to be kinda complicated. I'll try to keep it short and to the point. To
be clear, this is all about needing a named file on disk that can be used
with e.g. open() by the code-under-test. I wouldn't call this incredibly
common overall, but its not uncommon.

There's basically 3 problems that have a bit of overlap.

First: The tempfile module is a poor fit for testing (don't get me wrong,
it works, but its not *nice for use in tests*)*.* This is because:
1. Using it as a context manager is distracting. The indentation signifies
a conceptual scope the reader needs to be aware of, but in a test context,
its usually not useful. At worst, it covers most of the test. At best, its
constrained to a block at the start.
2. tempfile defaults to binary mode instead of text; just another thing to
bite you.
3. On windows, you can't reopen the file, so for cross-platform stuff, you
can't even use it for this case.
4. You pretty much always have to pass delete=False, which *kinda* defeats
the point of e.g. using a context manager

Second: The various file/test apis to do setup and cleanup are awkward.
This is because:
1. Are you deleting a file, directory tree, just a directory (is it
empty?)? Make sure to call the proper function, otherwise you'll get an
error.
2. Creating a directory tree? Make sure to call makedirs() with the right
parameters, otherwise you'll get an error.
3. Using tearDown? Make sure to handle errors, lest other tearDown logic
not run and leave a dirty post-test state that might inconsistently break a
following test.
4. Using setUp? Make sure to not assume a clean state because of (3).
5. Did you write a helper function to e.g., making creating
"foo/bar/baz.txt" easy? Now you have to implement logic to split up the
path, create dirs, etc. Not hard, admittedly, but its the ~9th thing in
this list so far -- "I just want to create a temp file for testing"
6. Are you using mkstemp? Remember to close the FD it returns, even though
its "just a test"
7. Are you using tempfile.gettempdir (or some other manual scheme)? Make
sure to give each test a unique location within it, otherwise collisions
can happen.

Third: This is a bit more opinion: I'm *really* into optimizing my
edit-debug cycle latency, so random file/dir names are really annoying
because they slow me down. They're absolutely necessary for running a
finished test suite, but get in the way for debugging specific tests (i.e.
where a dev spends the majority of their time when dealing with a failing
test).
This is because:
1. Command history is lost. I can't up-arrow-enter to re-run e.g. a grep
over the file I'm interested in.
2. The only way to inspect a file is to set_trace() before it gets deleted,
but after the logic I need to check has run. Then run some expression
that'll print the filename.




> [1] https://bugs.python.org/issue2156
>
> --
> Zach
> _______________________________________________
> Python-Dev mailing list
> Python-Dev at python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/richardlev%40gmail.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20190216/0cc7fc46/attachment.html>