test_walk fails on Linux #4

Closed
benhoyt opened this Issue May 15, 2013 · 15 comments

Comments

Projects
None yet
3 participants
Owner

benhoyt commented May 15, 2013

======================================================================
FAIL: test_traversal (test_walk.WalkTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Work\scandir\tests\test_walk.py", line 66, in test_traversal
    self.assertEqual(all[3 - 2 * flipped], sub2_tree)
AssertionError: Tuples differ: ('C:\\Work\\scandir\\tests\\te... != ('C:\\Work\\scandir\\tests\\te...

First differing element 1:
[]
['link']

- ('C:\\Work\\scandir\\tests\\temp\\TEST1\\SUB2', [], ['link', 'tmp3'])
?                                                 ----

+ ('C:\\Work\\scandir\\tests\\temp\\TEST1\\SUB2', ['link'], ['tmp3'])
?                                                        +  +

Malizor commented Jun 6, 2013

Same problem on Ubuntu 13.04.

Also, the test does not work on python2:

ERROR: test_traversal (test_walk.WalkTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/nicolas/Bureau/scandir/tests/test_walk.py", line 48, in test_traversal
    os.symlink(os.path.abspath(t2_path), link_path, True)
TypeError: symlink() takes exactly 2 arguments (3 given)

moloney added a commit to moloney/scandir that referenced this issue Feb 25, 2014

BF: Fix is_dir/is_file for symlinks (Issue #4) in Linux
We need to do an os.stat call for symlinks to see if they point
at a file or directory. I am not sure if there is a more efficient
way to do this in windows.

I went ahead and added caching of the os.stat result too.

@moloney moloney referenced this issue Feb 25, 2014

Closed

Test fixes #15

Owner

benhoyt commented May 30, 2014

The original issue doesn't happen anymore, closing. @Malizor, if the issue you brought up still happens on Ubuntu, please open a new issue.

@benhoyt benhoyt closed this May 30, 2014

@benhoyt benhoyt reopened this May 30, 2014

Owner

benhoyt commented May 30, 2014

Okay, the original issue as described does still happen on Linux, reopening. I'm going to edit the description of this issue to match the current symptoms.

@benhoyt benhoyt changed the title from Symlink support in Python 3 doesn't work (at least in Windows) to test_walk fails on Linux May 30, 2014

Contributor

htoothrot commented Jul 4, 2014

What is the status of this issue? As moloney found, it seems the only way around this is to fallback to a symlink-following stat() call when d_type == DT_LNK (in is_dir and is_file).

Is there still some kind of decision to made about this?

Owner

benhoyt commented Jul 5, 2014

I need to review and test the fix. I'm back working on scandir (particularly PEP 471 at the moment), so I'll try to get to this in the next week or two.

Owner

benhoyt commented Jul 8, 2014

Note to self: this is easy to repro on Windows just by setting sys.platform to something bogus, like sys.platform = 'FOOBAR'.

Owner

benhoyt commented Jul 9, 2014

This is actually happening because scandir.py's definition of walk() is not putting symlinks to directories in the right list. See also https://mail.python.org/pipermail/python-dev/2014-July/135401.html

Fix should be simple enough -- I'll commit in the next few days.

Contributor

htoothrot commented Jul 9, 2014

Maybe I should've fleshed out my previous comment a little more.. I began to note it was down to the difference between lstat and stat but I thought you were already aware. Ideally paying the price for a full stat() would only happen when it's a symlink.

Owner

benhoyt commented Jul 12, 2014

Fixed in e601121

@benhoyt benhoyt closed this Jul 12, 2014

Contributor

htoothrot commented Jul 12, 2014

Ben, with that change scandir.walk will now put symlinks to files in the directories list when followlinks=False. This does not match what os.walk does.

As a side note, I think I might be of the opinion that is_file/is_dir should always use stat instead of lstat when the file is a symlink, which would match os.path.isdir/isfile. What is the reason to the current is_dir/is_file implementation?

@benhoyt benhoyt reopened this Jul 12, 2014

Owner

benhoyt commented Jul 12, 2014

You're dead right. Thank you.

benhoyt added a commit that referenced this issue Jul 12, 2014

Issue #4: Make scandir.walk() *actually* mimic os.walk() in all symli…
…nk cases.

Add explicit unit tests for this.
Owner

benhoyt commented Jul 12, 2014

I realized I was overthinking it. To mimic os.walk(), I just need to use entry.is_symlink() in conjunction with os.path.isdir() in the top loop -- as per os.walk(), no followlinks test needed there.

I've fixed this in 4907dc7 and added explicit unit tests to catch this.

Tested on Python 3.4 on Windows, about about to test on Python 2.6 on Linux.

Does this look right to you?

Owner

benhoyt commented Jul 12, 2014

Yep, works on Linux on python 2.6 fine too. Haven't tested Python 3 on Linux, but hoping for the best. :-)

Contributor

htoothrot commented Jul 12, 2014

8 passed in 0.27 seconds (from 4907dc7) on both:

platform linux2 -- Python 3.2.3 -- py-1.4.20 -- pytest-2.5.2
platform linux -- Python 3.4.1 -- py-1.4.20 -- pytest-2.5.2

Owner

benhoyt commented Jul 22, 2014

I think this is all fixed now, closing.

@benhoyt benhoyt closed this Jul 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment