classification
Title: Expand test coverage in posixpath
Type: behavior Stage: resolved
Components: Tests Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: brian.curtin, ev, michael.foord, python-dev
Priority: normal Keywords: needs review, patch

Created on 2011-03-14 16:58 by ev, last changed 2011-03-16 21:18 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
test_posixpath_with_same_device.patch ev, 2011-03-14 18:50 updated with setting inodes review
test_posixpath_with_same_device.patch ev, 2011-03-15 17:50 review
Messages (13)
msg130856 - (view) Author: Evan Dandrea (ev) * Date: 2011-03-14 16:58
I've expanded the coverage of the posixpath test.  The following scenarios have been added:
 - lexists with a non-existent file.
 - ismount with binary data.
 - ismount with a directory that is not a mountpoint.
 - ismount with a non-existent file.
 - ismount with a symlink.
 - expanduser with $HOME unset.
 - realpath with a relative path.
 - sameopenfile with a basic test.

I have tested it on Ubuntu natty (20110311) and Mac OSX 10.6.6.
msg130870 - (view) Author: Evan Dandrea (ev) * Date: 2011-03-14 18:22
Fixed a typo in the previous patch.
msg130872 - (view) Author: Evan Dandrea (ev) * Date: 2011-03-14 18:40
Added an updated patch that includes testing whether ismount would succeed by faking path being on a different device from its parent.
msg130875 - (view) Author: Evan Dandrea (ev) * Date: 2011-03-14 18:50
It's probably best to give the fake stats inode numbers, so if the code does fail to check the st_dev fields, it will fail the test.  I've updated the patch with this.
msg130986 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2011-03-15 16:10
The patch includes an unconditional "import posix" that will fail on Windows.

posixpath is available on windows (although not *all* its functionality makes sense), so the whole test should not be skipped - but it is reasonable to skip just the new tests using posix.

Something like:

try:
    import posix
except ImportError:
    posix = None

Then decorated tests that use posix with:

@unittest.skipIf(posix is None, "Test requires posix module")
msg130987 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2011-03-15 16:15
Note that instead of try finally constructs you can create a cleanup function and call self.addCleanup(...). This reduces extra levels of indentation in your test code.

In the new code in "test_ismount", from "# Non-existent mountpoint" on, I would turn this into a new test. (Does os.symlink *always* exist - if it is platform dependent you should skip the test if it isn't available.)
msg130989 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2011-03-15 16:16
Scratch the comment about symlink - in test_mount. I see you already protect that code with "if support.can_symlink()".
msg130990 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2011-03-15 16:18
Will posixpath.sameopenfile work on Windows? That may need skipping.
msg131003 - (view) Author: Evan Dandrea (ev) * Date: 2011-03-15 17:50
Updated the patch to address Michael's concerns.
msg131005 - (view) Author: Evan Dandrea (ev) * Date: 2011-03-15 17:51
I've looked at the sameopenfile code, and can see no reason why it would not work on Windows.  I've asked Brian to verify this.
msg131006 - (view) Author: Evan Dandrea (ev) * Date: 2011-03-15 17:53
I haven't used addCleanup here, but have noted it for the future.  In this case, using it would require adding another function to handle the reassignment, which I think is a bit more messy than the extra bit of indentation.
msg131014 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2011-03-15 18:27
Tested the patch on Windows -- all tests pass.
msg131178 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-16 21:18
New changeset f11da6cecffd by Michael Foord in branch '3.2':
Closes issue 11503. Improves test coverage of posixpath.
http://hg.python.org/cpython/rev/f11da6cecffd
History
Date User Action Args
2011-03-16 21:18:24python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg131178

resolution: fixed
stage: patch review -> resolved
2011-03-15 18:27:23brian.curtinsetnosy: michael.foord, brian.curtin, ev
messages: + msg131014
2011-03-15 17:53:10evsetnosy: michael.foord, brian.curtin, ev
messages: + msg131006
2011-03-15 17:51:27evsetnosy: michael.foord, brian.curtin, ev
messages: + msg131005
2011-03-15 17:50:42evsetfiles: + test_posixpath_with_same_device.patch
nosy: michael.foord, brian.curtin, ev
messages: + msg131003
2011-03-15 16:18:04michael.foordsetnosy: michael.foord, brian.curtin, ev
messages: + msg130990
2011-03-15 16:16:41michael.foordsetnosy: michael.foord, brian.curtin, ev
messages: + msg130989
2011-03-15 16:15:29michael.foordsetnosy: michael.foord, brian.curtin, ev
messages: + msg130987
2011-03-15 16:10:59michael.foordsetnosy: michael.foord, brian.curtin, ev
messages: + msg130986
2011-03-15 15:59:06michael.foordsetfiles: - test_posixpath.patch
nosy: michael.foord, brian.curtin, ev
2011-03-15 15:59:02michael.foordsetfiles: - test_posixpath.patch
nosy: michael.foord, brian.curtin, ev
2011-03-15 15:58:57michael.foordsetfiles: - test_posixpath_with_same_device.patch
nosy: michael.foord, brian.curtin, ev
2011-03-14 22:12:42brian.curtinsetkeywords: + needs review
nosy: + brian.curtin

stage: patch review
2011-03-14 18:50:30evsetfiles: + test_posixpath_with_same_device.patch
nosy: michael.foord, ev
messages: + msg130875
2011-03-14 18:40:58evsetfiles: + test_posixpath_with_same_device.patch
nosy: michael.foord, ev
messages: + msg130872
2011-03-14 18:22:29evsetfiles: + test_posixpath.patch
nosy: michael.foord, ev
messages: + msg130870
2011-03-14 17:08:18evsetnosy: + michael.foord
2011-03-14 16:58:13evcreate