Repository: jmcgeheeiv/pyfakefs
Pull request #491 (2019-08-03 16:01:47)

Fix scandir DirEntry.path for relative path or fd
The documentation for os.DirEntry.path [1] says that the path attribute
of a DirEntry object is only absolute if scandir()'s path argument was
absolute, and simply the name attribute if scandir()'s path argument was
a filedescriptor. This patch makes pyfakefs do this, and adjusts the
tests.

Relative symlinks are now also covered in the scandir tests.

RealScandirFdTest.test_path_like was failing for me in Python 3.7.3 on
Debian Bullseye.

[1] https://docs.python.org/3/library/os.html#os.DirEntry.path

nesciens: Arg. `pyfakefs.tests.fake_os_test.FakeScandirRelTest` fails on Windows in the AppVeyor build. It seems that the fake scandir gives 0 files when doing scandir with a relative path instead of 2 or 6 depending on the Python version. `pyfakefs.tests.fake_os_test.RealScandirRelTest` (with `Real` instead of `Fake`) succeeds though, so this seems to be something where pyfakefs does not agree with the real behavior on Windows.

Unfortunately I don't have a Windows environment to reproduce this on. I might try under Wine, but might not have enough time to spend on that.
mrbean-bremen: Thanks for the fix! I'm working under Windows, so I may check and hopefully fix the Windows problem. If you don't mind, I can do this directly on your PR branch, though I'm not sure if I will get to this today.
mrbean-bremen: Ok, I had a quick look, and the problem seems to be with `relpath`/`abspath` under Windows, where the drive is not correctly taken into account. This is actually a weak point with the Windows implementation in pyfakefs and has to do with the fact that both `C:\` and `\` are handled as root.
I will see what I can do, though this may take a while...

Patch:
diff --git a/pyfakefs/fake_scandir.py b/pyfakefs/fake_scandir.py
index 03df0ff..3b9271e 100644
--- a/pyfakefs/fake_scandir.py
+++ b/pyfakefs/fake_scandir.py
@@ -40,6 +40,7 @@ class DirEntry(BaseClass):
         self._filesystem = filesystem
         self.name = ''
         self.path = ''
+        self._abspath = ''
         self._inode = None
         self._islink = False
         self._isdir = False
@@ -93,14 +94,14 @@ class DirEntry(BaseClass):
         """
         if follow_symlinks:
             if self._statresult_symlink is None:
-                file_object = self._filesystem.resolve(self.path)
+                file_object = self._filesystem.resolve(self._abspath)
                 if self._filesystem.is_windows_fs:
                     file_object.st_nlink = 0
                 self._statresult_symlink = file_object.stat_result.copy()
             return self._statresult_symlink
 
         if self._statresult is None:
-            file_object = self._filesystem.lresolve(self.path)
+            file_object = self._filesystem.lresolve(self._abspath)
             self._inode = file_object.st_ino
             if self._filesystem.is_windows_fs:
                 file_object.st_nlink = 0
@@ -125,12 +126,15 @@ class ScanDirIter:
                 raise NotImplementedError(
                     'scandir does not support file descriptor '
                     'path argument')
-            path = self.filesystem.get_open_file(path).get_object().path
-
-        self.path = self.filesystem.absnormpath(path)
+            self.abspath = self.filesystem.absnormpath(
+                self.filesystem.get_open_file(path).get_object().path)
+            self.path = ''
+        else:
+            self.abspath = self.filesystem.absnormpath(path)
+            self.path = path
         contents = {}
         try:
-            contents = self.filesystem.confirmdir(path).contents
+            contents = self.filesystem.confirmdir(self.abspath).contents
         except OSError:
             pass
         self.contents_iter = iter(contents)
@@ -147,8 +151,10 @@ class ScanDirIter:
         dir_entry.name = entry
         dir_entry.path = self.filesystem.joinpaths(self.path,
                                                    dir_entry.name)
-        dir_entry._isdir = self.filesystem.isdir(dir_entry.path)
-        dir_entry._islink = self.filesystem.islink(dir_entry.path)
+        dir_entry._abspath = self.filesystem.joinpaths(self.abspath,
+                                                       dir_entry.name)
+        dir_entry._isdir = self.filesystem.isdir(dir_entry._abspath)
+        dir_entry._islink = self.filesystem.islink(dir_entry._abspath)
         return dir_entry
 
     # satisfy both Python 2 and 3


Test patch:
diff --git a/pyfakefs/tests/fake_os_test.py b/pyfakefs/tests/fake_os_test.py
index b4bd787..49cb2b2 100644
--- a/pyfakefs/tests/fake_os_test.py
+++ b/pyfakefs/tests/fake_os_test.py
@@ -4714,6 +4714,9 @@ class StatPropagationTest(TestCase):
 
 @unittest.skipIf(not use_scandir, 'only run if scandir is available')
 class FakeScandirTest(FakeOsModuleTestBase):
+    FILE_SIZE = 50
+    LINKED_FILE_SIZE = 10
+
     def setUp(self):
         super(FakeScandirTest, self).setUp()
         self.supports_symlinks = (not self.is_windows or
@@ -4737,35 +4740,70 @@ class FakeScandirTest(FakeOsModuleTestBase):
         link_dir = self.make_path('linked', 'plugh')
         self.linked_file_path = self.os.path.join(link_dir, 'file')
         self.linked_dir_path = self.os.path.join(link_dir, 'dir')
-
-        self.create_dir(self.linked_dir_path)
-        self.create_file(self.linked_file_path, contents=b'a' * 10)
+        self.rel_linked_dir_path = (
+            self.os.path.join('..', '..', 'linked', 'plugh', 'dir'))
+        self.rel_linked_file_path = (
+            self.os.path.join('..', '..', 'linked', 'plugh', 'file'))
         self.dir_path = self.os.path.join(self.directory, 'dir')
-        self.create_dir(self.dir_path)
         self.file_path = self.os.path.join(self.directory, 'file')
-        self.create_file(self.file_path, contents=b'b' * 50)
         self.file_link_path = self.os.path.join(self.directory, 'link_file')
+        self.dir_link_path = self.os.path.join(self.directory, 'link_dir')
+        self.file_rel_link_path = self.os.path.join(self.directory,
+                                                    'rel_link_file')
+        self.dir_rel_link_path = self.os.path.join(self.directory,
+                                                   'rel_link_dir')
+
+        self.create_dir(self.dir_path)
+        self.create_file(self.file_path, contents=b'b' * self.FILE_SIZE)
         if self.supports_symlinks:
-            self.create_symlink(self.file_link_path, self.linked_file_path)
-            self.dir_link_path = self.os.path.join(self.directory, 'link_dir')
+            self.create_dir(self.linked_dir_path)
+            self.create_file(self.linked_file_path,
+                             contents=b'a' * self.LINKED_FILE_SIZE),
             self.create_symlink(self.dir_link_path, self.linked_dir_path)
-        self.dir_entries = [entry for entry in self.scandir(self.directory)]
-        self.dir_entries = sorted(self.dir_entries,
-                                  key=lambda entry: entry.name)
+            self.create_symlink(self.file_link_path, self.linked_file_path)
+            self.create_symlink(self.dir_rel_link_path,
+                                self.rel_linked_dir_path)
+            self.create_symlink(self.file_rel_link_path,
+                                self.rel_linked_file_path)
+
+        # Changing the working directory below is to make sure relative paths
+        # to the files and directories created above are reasonable.
+        # Corner-cases about relative paths are better checked in tests created
+        # for that purpose.
+        #
+        # WARNING: This is self.pretest_cwd and not self.cwd as the latter is
+        # used by superclass RealFsTestCase.
+        self.pretest_cwd = self.os.getcwd()
+        self.os.chdir(self.base_path)
+
+        self.dir_entries = list(self.do_scandir())
+        self.dir_entries.sort(key=lambda entry: entry.name)
+
+    def tearDown(self):
+        self.os.chdir(self.pretest_cwd)
+        super(FakeScandirTest, self).tearDown()
+
+    def do_scandir(self):
+        """Hook to override how scandir is called."""
+        return self.scandir(self.directory)
+
+    def scandir_path(self):
+        """Hook to override the expected scandir() path in DirEntry.path."""
+        return self.directory
 
     def test_paths(self):
         sorted_names = ['dir', 'file']
         if self.supports_symlinks:
-            sorted_names.extend(['link_dir', 'link_file'])
+            sorted_names.extend(['link_dir', 'link_file', 'rel_link_dir',
+                                 'rel_link_file'])
 
         self.assertEqual(len(sorted_names), len(self.dir_entries))
         self.assertEqual(sorted_names,
                          [entry.name for entry in self.dir_entries])
-
-        self.skip_real_fs()
-        # this one does not work in the real fs - will wait for
-        # final 3.7 release to check into this
-        self.assertEqual(self.dir_path, self.dir_entries[0].path)
+        sorted_paths = [self.os.path.join(self.scandir_path(), name)
+                        for name in sorted_names]
+        self.assertEqual(sorted_paths,
+                         [entry.path for entry in self.dir_entries])
 
     def test_isfile(self):
         self.assertFalse(self.dir_entries[0].is_file())
@@ -4777,6 +4815,12 @@ class FakeScandirTest(FakeOsModuleTestBase):
             self.assertTrue(self.dir_entries[3].is_file())
             self.assertFalse(
                 self.dir_entries[3].is_file(follow_symlinks=False))
+            self.assertFalse(self.dir_entries[4].is_file())
+            self.assertFalse(
+                self.dir_entries[4].is_file(follow_symlinks=False))
+            self.assertTrue(self.dir_entries[5].is_file())
+            self.assertFalse(
+                self.dir_entries[5].is_file(follow_symlinks=False))
 
     def test_isdir(self):
         self.assertTrue(self.dir_entries[0].is_dir())
@@ -4786,6 +4830,10 @@ class FakeScandirTest(FakeOsModuleTestBase):
             self.assertFalse(self.dir_entries[2].is_dir(follow_symlinks=False))
             self.assertFalse(self.dir_entries[3].is_dir())
             self.assertFalse(self.dir_entries[3].is_dir(follow_symlinks=False))
+            self.assertTrue(self.dir_entries[4].is_dir())
+            self.assertFalse(self.dir_entries[4].is_dir(follow_symlinks=False))
+            self.assertFalse(self.dir_entries[5].is_dir())
+            self.assertFalse(self.dir_entries[5].is_dir(follow_symlinks=False))
 
     def test_is_link(self):
         if self.supports_symlinks:
@@ -4793,6 +4841,8 @@ class FakeScandirTest(FakeOsModuleTestBase):
             self.assertFalse(self.dir_entries[1].is_symlink())
             self.assertTrue(self.dir_entries[2].is_symlink())
             self.assertTrue(self.dir_entries[3].is_symlink())
+            self.assertTrue(self.dir_entries[4].is_symlink())
+            self.assertTrue(self.dir_entries[5].is_symlink())
 
     def test_path_links_not_resolved(self):
         # regression test for #350
@@ -4821,29 +4871,44 @@ class FakeScandirTest(FakeOsModuleTestBase):
                              self.dir_entries[2].inode())
             self.assertEqual(self.os.lstat(self.file_link_path).st_ino,
                              self.dir_entries[3].inode())
-
-    def check_stat(self, expected_size):
-        self.assertEqual(50, self.dir_entries[1].stat().st_size)
+            self.assertEqual(self.os.lstat(self.dir_rel_link_path).st_ino,
+                             self.dir_entries[4].inode())
+            self.assertEqual(self.os.lstat(self.file_rel_link_path).st_ino,
+                             self.dir_entries[5].inode())
+
+    def check_stat(self, absolute_symlink_expected_size,
+                   relative_symlink_expected_size):
+        self.assertEqual(self.FILE_SIZE, self.dir_entries[1].stat().st_size)
         self.assertEqual(
             self.os.stat(self.dir_path).st_ctime,
             self.dir_entries[0].stat().st_ctime)
 
         if self.supports_symlinks:
-            self.assertEqual(10, self.dir_entries[3].stat().st_size)
-            self.assertEqual(expected_size,
+            self.assertEqual(self.LINKED_FILE_SIZE,
+                             self.dir_entries[3].stat().st_size)
+            self.assertEqual(absolute_symlink_expected_size,
                              self.dir_entries[3].stat(
                                  follow_symlinks=False).st_size)
             self.assertEqual(
                 self.os.stat(self.linked_dir_path).st_mtime,
                 self.dir_entries[2].stat().st_mtime)
+            self.assertEqual(self.LINKED_FILE_SIZE,
+                             self.dir_entries[5].stat().st_size)
+            self.assertEqual(relative_symlink_expected_size,
+                             self.dir_entries[5].stat(
+                                 follow_symlinks=False).st_size)
+            self.assertEqual(
+                self.os.stat(self.linked_dir_path).st_mtime,
+                self.dir_entries[4].stat().st_mtime)
 
     @unittest.skipIf(TestCase.is_windows, 'POSIX specific behavior')
     def test_stat_posix(self):
-        self.check_stat(len(self.linked_file_path))
+        self.check_stat(len(self.linked_file_path),
+                        len(self.rel_linked_file_path))
 
     @unittest.skipIf(not TestCase.is_windows, 'Windows specific behavior')
     def test_stat_windows(self):
-        self.check_stat(0)
+        self.check_stat(0, 0)
 
     def test_index_access_to_stat_times_returns_int(self):
         self.assertEqual(self.os.stat(self.dir_path)[stat.ST_CTIME],
@@ -4851,6 +4916,8 @@ class FakeScandirTest(FakeOsModuleTestBase):
         if self.supports_symlinks:
             self.assertEqual(self.os.stat(self.linked_dir_path)[stat.ST_MTIME],
                              int(self.dir_entries[2].stat().st_mtime))
+            self.assertEqual(self.os.stat(self.linked_dir_path)[stat.ST_MTIME],
+                             int(self.dir_entries[4].stat().st_mtime))
 
     def test_stat_ino_dev(self):
         if self.supports_symlinks:
@@ -4859,13 +4926,19 @@ class FakeScandirTest(FakeOsModuleTestBase):
                              self.dir_entries[3].stat().st_ino)
             self.assertEqual(file_stat.st_dev,
                              self.dir_entries[3].stat().st_dev)
+            self.assertEqual(file_stat.st_ino,
+                             self.dir_entries[5].stat().st_ino)
+            self.assertEqual(file_stat.st_dev,
+                             self.dir_entries[5].stat().st_dev)
 
     @unittest.skipIf(sys.version_info < (3, 6) or not use_builtin_scandir,
                      'Path-like objects have been introduced in Python 3.6')
     def test_path_like(self):
         self.assertTrue(isinstance(self.dir_entries[0], os.PathLike))
-        self.assertEqual(self.dir_path, os.fspath(self.dir_entries[0]))
-        self.assertEqual(self.file_path, os.fspath(self.dir_entries[1]))
+        self.assertEqual(self.os.path.join(self.scandir_path(), 'dir'),
+                         os.fspath(self.dir_entries[0]))
+        self.assertEqual(self.os.path.join(self.scandir_path(), 'file'),
+                         os.fspath(self.dir_entries[1]))
 
 
 class RealScandirTest(FakeScandirTest):
@@ -4873,26 +4946,55 @@ class RealScandirTest(FakeScandirTest):
         return True
 
 
+class FakeScandirRelTest(FakeScandirTest):
+    def scandir_path(self):
+        # When scandir is called with a relative path, that relative path is
+        # used in the path attribute of the DirEntry objects.
+        return self.os.path.relpath(self.directory)
+
+    def do_scandir(self):
+      return self.scandir(self.os.path.relpath(self.directory))
+
+
+class RealScandirRelTest(FakeScandirRelTest):
+    def use_real_fs(self):
+        return True
+
+
 @unittest.skipIf(sys.version_info < (3, 7) or TestCase.is_windows,
                  'dir_fd support for os.scandir was introduced in Python 3.7')
 class FakeScandirFdTest(FakeScandirTest):
-    def setUp(self):
-        super(FakeScandirFdTest, self).setUp()
-        self.dir_fd = self.os.open(self.directory, os.O_RDONLY)
-        self.dir_entries = [entry for entry in self.scandir(self.dir_fd)]
-        self.dir_entries = sorted(self.dir_entries,
-                                  key=lambda entry: entry.name)
-
     def tearDown(self):
         self.os.close(self.dir_fd)
         super(FakeScandirFdTest, self).tearDown()
 
+    def scandir_path(self):
+        # When scandir is called with a filedescriptor, only the name of the entry
+        # is returned in the path attribute of the DirEntry objects.
+        return ''
+
+    def do_scandir(self):
+      self.dir_fd = self.os.open(self.directory, os.O_RDONLY)
+      return self.scandir(self.dir_fd)
+
 
 class RealScandirFdTest(FakeScandirFdTest):
     def use_real_fs(self):
         return True
 
 
+class FakeScandirFdRelTest(FakeScandirFdTest):
+    def do_scandir(self):
+      self.dir_fd = self.os.open(self.os.path.relpath(self.directory),
+                                 os.O_RDONLY)
+      return self.scandir(self.dir_fd)
+
+
+class RealScandirFdRelTest(FakeScandirFdRelTest):
+    def use_real_fs(self):
+        return True
+
+
 @unittest.skipIf(TestCase.is_python2,
                  reason='Xattr only supported in Linux/Python 3')
 class FakeExtendedAttributeTest(FakeOsModuleTestBase):