Delete tmpdir with rmtree to handle Unicode paths #7572
Conversation
pytest (rather py.path.local) does not handle non-ASCII paths properly on Windows with Python 2, but Python's builtin shutil.rmtree() does. Co-authored-by: Pradyun Gedam <pradyunsg@gmail.com>
pradyunsg
added
C: tests
|
This PR restores my faith that Python 2 is evil and should be eliminated from the face of the planet... or at least our support matrix. ;P |
|
Requested review from @pfmoore because I'm sure he'll enjoy this. We spent all day on this. :) |
Gotta love those Unicode bugs :-) |
|
That was a quick review. 😆 |
|
Happened to see the email. To be fair, I haven't run the code, I don't have Python 2 installed on this PC, but that's what CI is for, isn't it? ;-) But desk checking it, it looks fine to me. |
The test for pip failed because it couldn't delete the unicode directory, then scripttest's error message generation failed, because it tried to print an error message containing the name of the directory that couldn't be deleted, because the original test had failed to delete the unicode directory. Cause: a |
|
Impressive bit of detective work! |
| @@ -103,7 +103,7 @@ def tmpdir(request, tmpdir): | |||
| # This should prevent us from needing a multiple gigabyte temporary | |||
| # directory while running the tests. | |||
| if not request.config.getoption("--keep-tmpdir"): | |||
There was a problem hiding this comment.
A comment might have been a good idea here, to explain the use of shutil.rmtree (to prevent a possible accidental revert in a distant future)
There was a problem hiding this comment.
I included the comments in a follow-up PR #7577
lock
bot
added
the
auto-locked
pytest (rather
py.path.local) does not handle non-ASCII paths properly on Windows with Python 2, but Python's builtinshutil.rmtree()does.This becomes a problem when writing a test case for #7138. The test data for it (a non-ASCII data file) would choke pytest and fail on clean-up for any tests using the test data.