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
Add a context manager to change cwd in test.test_support and run the test suite in a temp dir. #51961
Comments
To simplify the tests that require a different CWD I wrote a context manager that allows to change the working directory. I used it on bpo-3426 to test os.path.abspath() with ASCII and non-ASCII CWDs and realized that it can also be used to fix bpo-5684 where a zipfile fails to extract the content of the archive in the CWD if it is read-only. Patch attached. |
A patch which provides a context manager and a decorator. Sample usages: with temp_cwd() as cwd:
assert cwd == os.getcwd()
@writablecwd
def test_zipfile():
# do something useful
print os.path.abspath('.') |
Changed, after review from Ezio and other developpers. |
with_writable_cwd should probably use functools.wraps. |
The patch looks good, I'd just move _test_cwd inside the function and drop the [:-3] from TESTFN, but apart from that it's OK. I also agree that functools.wraps should be added. To summarize the discussion we had on #python-dev:
The result will be something like '@test_xxxx_tmp_cwd/dirname'. |
Different approach, after some other talks with Ezio and David. Now the directory is changed before running any test. Additionally, TESTFN always contains a single filename (without path). The decorator becomes useless. |
Here is a patch based on the previous patches and some discussion. |
It looks fine. Few comments:
|
Patch which fixes the "relative import" issue. ~ $ cd Lib/ Note: There are some 2-spaces indents for patch readability. |
Slightly more readable, without 2-spaces indent. |
There were syntax errors in the previous patch. Sorry. |
Removed the dummy CM hack. |
Let's fix some other tests. |
Almost completed patch, the code should be OK, I just have to add a few comments and check that it works fine in all the situations. |
Final version of the patch, with the temp_cwd context manager added to test_support and used in test_regrtest to run the test suite in a temporary directory. It also includes a fix for test_subprocess that was failing when the tests are not run in the original cwd. |
With the latest patch I get one failure: test_bufio. I piped that test to a file and attached the results here. There are numerous "Permission denied: @test" IOErrors. |
Brian, is it the only one failing? Do you reproduce the error when running this test alone? |
Yep, that's the only one failing. The output I attached is the result of running the test alone. |
Ok, it may be not related directly with this patch. Can you diagnose if it something like bpo-7443? |
And you could try the patch attached to bpo-7443, and see if it fixes the test_bufio issue. |
The command line used to run the tests is important. Variants: And another full run: Try some of these variants, it may help diagnose the windows issue. (Note: maybe some antivirus or other software may be related to the race condition described on bpo-7443) |
With Ezio's latest patch (sent via IRC), test_bufio still fails and additionally test_mailbox fails. If I apply the patch on bpo-7443 along with Ezio's patch, everything looks fine. I haven't thoroughly looked at that issue, but on the surface it looks similar (same setup, same result). |
Fixed on trunk with r78136. Before closing this issue, we may apply additional cleanup on regrtest:
|
I'm not sure it's safe to remove those hacks, they might be necessary in some corner case. |
"Complex is better than complicated... Special cases aren't special enough to break the rules." The module "regrtest" is complex enough. We don't need to keep useless hacks inside. Eventually, if we keep these hacks around, it may hide real bugs. |
Ported to py3k in r78214. I will think about the cleanups later, they are not so important right now. |
Hello One advice against using __file__: http://lists.debian.org/debian-python/2010/01/msg00172.html Regards |
The cleanups have been committed in r78719 (trunk) and r78723 (py3k). |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: