Discussion:
[issue25595] test_deleted_cwd in test_importlib is failed on AIX
Serhiy Storchaka
2015-11-10 11:03:12 UTC
Permalink
New submission from Serhiy Storchaka:

test_deleted_cwd in test_importlib is failed on AIX.

http://buildbot.python.org/all/builders/PPC64%20AIX%203.x/builds/4318/steps/test/logs/stdio
======================================================================
ERROR: test_deleted_cwd (test.test_importlib.import_.test_path.Source_FinderTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/test/test_importlib/import_/test_path.py", line 169, in test_deleted_cwd
os.chdir(path)
File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/tempfile.py", line 807, in __exit__
self.cleanup()
File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/tempfile.py", line 811, in cleanup
_shutil.rmtree(self.name)
File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/shutil.py", line 478, in rmtree
onerror(os.rmdir, path, sys.exc_info())
File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/shutil.py", line 476, in rmtree
os.rmdir(path)
OSError: [Errno 16] Device busy: '/tmp/tmp2xdxtq6x'

----------------------------------------------------------------------

Proposed patch fixes the test. It also uses more verbose wording to create and remove temporary directory to be sure that caught exception was written by removing the directory, not by creating or changing CWD.

See also issue22834.

----------
components: Tests
files: test_deleted_cwd_aix.patch
keywords: patch
messages: 254438
nosy: David.Edelsohn, brett.cannon, martin.panter, serhiy.storchaka, zach.ware
priority: normal
severity: normal
stage: patch review
status: open
title: test_deleted_cwd in test_importlib is failed on AIX
type: behavior
versions: Python 3.5, Python 3.6
Added file: http://bugs.python.org/file40995/test_deleted_cwd_aix.patch

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25595>
_______________________________________
STINNER Victor
2015-11-10 11:15:21 UTC
Permalink
STINNER Victor added the comment:

-1, review on Rietveld.

----------
nosy: +haypo

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25595>
_______________________________________
Serhiy Storchaka
2015-11-10 12:55:32 UTC
Permalink
Serhiy Storchaka added the comment:

Thank you Victor for your review. Here is fixed patch that implements your suggestions.

----------
Added file: http://bugs.python.org/file40997/test_deleted_cwd_aix_2.patch

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25595>
_______________________________________
Serhiy Storchaka
2015-11-10 12:58:25 UTC
Permalink
Serhiy Storchaka added the comment:

And here is simple alternative patch based on Martin's original suggestion in msg236122.

----------
Added file: http://bugs.python.org/file40998/test_deleted_cwd_aix_alt.patch

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25595>
_______________________________________
STINNER Victor
2015-11-10 13:11:37 UTC
Permalink
STINNER Victor added the comment:

test_deleted_cwd_aix_alt.patch: I don't trust tempfile.TemporaryDirectory(), I prefer your patch.

test_deleted_cwd_aix_2.patch looks good to me.

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25595>
_______________________________________
Martin Panter
2015-11-11 00:08:00 UTC
Permalink
Martin Panter added the comment:

Wow this is tricky to get right. Victor, you are right to not trust TemporaryDirectory, because when cleanup() fails you don’t get a second chance, and it will leave the directory behind.

According to the Microsoft _mkdir() web page <https://msdn.microsoft.com/en-us/library/wt8es881.aspx>, Windows would raise ENOTEMPTY for removing the current directory. Maybe it is time to drop the win32 skip check as well?

Here is a patch based on aix_2.patch, but also removing the redundant win32 skip. And “import errno” was no longer needed. I only tested it on Linux though, which supports removing the cwd.

----------
Added file: http://bugs.python.org/file41005/test_deleted_cwd_aix_3.patch

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25595>
_______________________________________
Serhiy Storchaka
2015-11-11 06:24:17 UTC
Permalink
Serhiy Storchaka added the comment:

Martin's original code looked nicer to me. It' a pity we can't use it.

test_deleted_cwd_aix_3.patch LGTM. Thanks Martin!

----------
assignee: -> serhiy.storchaka

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25595>
_______________________________________
Roundup Robot
2015-11-11 06:27:35 UTC
Permalink
Roundup Robot added the comment:

New changeset d4dc36586f24 by Serhiy Storchaka in branch '3.5':
Issue #25595: Fixed test_deleted_cwd in test_importlib on AIX.
https://hg.python.org/cpython/rev/d4dc36586f24

New changeset 3f392050d519 by Serhiy Storchaka in branch 'default':
Issue #25595: Fixed test_deleted_cwd in test_importlib on AIX.
https://hg.python.org/cpython/rev/3f392050d519

----------
nosy: +python-dev

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25595>
_______________________________________
Serhiy Storchaka
2015-11-11 06:28:06 UTC
Permalink
Changes by Serhiy Storchaka <***@gmail.com>:


----------
resolution: -> fixed
stage: patch review -> resolved
status: open -> closed

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25595>
_______________________________________

Loading...