Repository: PyFilesystem/pyfilesystem2
Pull request #134 (2018-01-25 17:22:18)

Walk iterators
This modifies the walk code to use iterators over lists where possible.

This is so that the `info`, `files` and `dirs` methods don't need to load an entire directory worth before they start yielding results--which should also be more memory efficient.

See https://github.com/PyFilesystem/pyfilesystem2/issues/112

Also includes some optimisations for file copying which will benefit S3FS and potentially other network filesystems that implement `getfile`.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/15221202/badge)](https://coveralls.io/builds/15221202)

Coverage decreased (-0.07%) to 99.878% when pulling **fd7224f942929c951e3b97de33e3342ea09799b3 on iter-walk** into **ba49ad73ab0a3ddb13661cab970c4f0131cf9364 on master**.


Patch:
diff --git a/.travis.yml b/.travis.yml
index 4ccb64e..e175042 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -9,7 +9,6 @@ python:
 
 os:
   - linux
-  - osx
 
 before_install:
   - pip install setuptools pip -U
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8c485cc..9023f7c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - fs.getfile function
 
+### Changed
+
+- Modified walk to use iterators internally (for more efficient walking)
+- Modified fs.copy to use getfile
+
 ## [2.0.17] - 2017-11-20
 
 ### Fixed
diff --git a/fs/compress.py b/fs/compress.py
index 93fd619..987e42b 100644
--- a/fs/compress.py
+++ b/fs/compress.py
@@ -102,7 +102,6 @@ def write_zip(src_fs,
                     _zip.write(sys_path, zip_name)
 
 
-
 def write_tar(src_fs,
               file,
               compression=None,
diff --git a/fs/copy.py b/fs/copy.py
index 601a8a9..b6239a9 100644
--- a/fs/copy.py
+++ b/fs/copy.py
@@ -103,10 +103,39 @@ def copy_file(src_fs, src_path, dst_fs, dst_path):
             else:
                 # Standard copy
                 with src_fs.lock(), dst_fs.lock():
-                    with src_fs.openbin(src_path) as read_file:
-                        # There may be an optimized copy available on
-                        # dst_fs
-                        dst_fs.setbinfile(dst_path, read_file)
+                    if dst_fs.hassyspath(dst_path):
+                        with dst_fs.openbin(dst_path, 'w') as write_file:
+                            src_fs.getfile(src_path, write_file)
+                    else:
+                        with src_fs.openbin(src_path) as read_file:
+                            dst_fs.setbinfile(dst_path, read_file)
+
+
+def copy_file_internal(src_fs, src_path, dst_fs, dst_path):
+    """Low level copy, that doesn't call manage_fs or lock.
+
+    If the destination exists, and is a file, it will be first truncated.
+
+    This method exists to optimize copying in loops. In general you
+    should prefer `copy_file`.
+
+    Arguments:
+        src_fs (FS or str): Source filesystem.
+        src_path (str): Path to a file on the source filesystem.
+        dst_fs (FS or str): Destination filesystem.
+        dst_path (str): Path to a file on the destination filesystem.
+
+    """
+    if src_fs is dst_fs:
+        # Same filesystem, so we can do a potentially optimized
+        # copy
+        src_fs.copy(src_path, dst_path, overwrite=True)
+    elif dst_fs.hassyspath(dst_path):
+        with dst_fs.openbin(dst_path, 'w') as write_file:
+            src_fs.getfile(src_path, write_file)
+    else:
+        with src_fs.openbin(src_path) as read_file:
+            dst_fs.setbinfile(dst_path, read_file)
 
 
 def copy_file_if_newer(src_fs, src_path, dst_fs, dst_path):
@@ -143,11 +172,8 @@ def copy_file_if_newer(src_fs, src_path, dst_fs, dst_path):
                 with src_fs.lock(), dst_fs.lock():
                     if _source_is_newer(src_fs, src_path,
                                         dst_fs, dst_path):
-                        with src_fs.openbin(src_path) as read_file:
-                            # There may be an optimized copy available
-                            # on dst_fs
-                            dst_fs.setbinfile(dst_path, read_file)
-                            return True
+                        copy_file_internal(src_fs, src_path, dst_fs, dst_path)
+                        return True
                     else:
                         return False
 
@@ -209,7 +235,7 @@ def copy_dir(src_fs, src_path, dst_fs, dst_path,
                     for info in files:
                         src_path = info.make_path(dir_path)
                         dst_path = info.make_path(copy_path)
-                        copy_file(
+                        copy_file_internal(
                             src_fs,
                             src_path,
                             dst_fs,
@@ -276,5 +302,5 @@ def copy_dir_if_newer(src_fs, src_path, dst_fs, dst_path,
                             src_modified > dst_state[dir_path].modified
                         )
                         if do_copy:
-                            copy_file(src_fs, dir_path, dst_fs, copy_path)
+                            copy_file_internal(src_fs, dir_path, dst_fs, copy_path)
                             on_copy(src_fs, dir_path, dst_fs, copy_path)
diff --git a/fs/mirror.py b/fs/mirror.py
index 80d1528..066813d 100644
--- a/fs/mirror.py
+++ b/fs/mirror.py
@@ -20,7 +20,7 @@ from __future__ import print_function
 from __future__ import unicode_literals
 
 
-from .copy import copy_file
+from .copy import copy_file_internal
 from .errors import ResourceNotFound
 from .walk import Walker
 from .opener import manage_fs
@@ -92,7 +92,7 @@ def _mirror(src_fs, dst_fs, walker=None, copy_if_newer=True):
                     # Compare file info
                     if copy_if_newer and not _compare(_file, dst_file):
                         continue
-            copy_file(src_fs, _path, dst_fs, _path)
+            copy_file_internal(src_fs, _path, dst_fs, _path)
 
         # Make directories
         for _dir in dirs:
diff --git a/fs/mountfs.py b/fs/mountfs.py
index fd3ca66..e243c50 100644
--- a/fs/mountfs.py
+++ b/fs/mountfs.py
@@ -159,6 +159,10 @@ class MountFS(FS):
         fs, _path = self._delegate(path)
         return fs.getbytes(_path)
 
+    def getfile(self, path, file, chunk_size=None, **options):
+        fs, _path = self._delegate(path)
+        return fs.getfile(_path, file, chunk_size=chunk_size, **options)
+
     def gettext(self, path, encoding=None, errors=None, newline=None):
         self.check()
         fs, _path = self._delegate(path)
diff --git a/fs/multifs.py b/fs/multifs.py
index c4ada4c..3860520 100644
--- a/fs/multifs.py
+++ b/fs/multifs.py
@@ -259,6 +259,10 @@ class MultiFS(FS):
             raise errors.ResourceNotFound(path)
         return fs.getbytes(path)
 
+    def getfile(self, path, file, chunk_size=None, **options):
+        fs = self._delegate_required(path)
+        return fs.getfile(path, file, chunk_size=chunk_size, **options)
+
     def gettext(self, path, encoding=None, errors=None, newline=''):
         self.check()
         fs = self._delegate_required(path)
@@ -291,13 +295,13 @@ class MultiFS(FS):
 
     def hassyspath(self, path):
         self.check()
-        fs = self._delegate_required(path)
-        return fs.hassyspath(path)
+        fs = self._delegate(path)
+        return fs is not None and fs.hassyspath(path)
 
     def hasurl(self, path, purpose='download'):
         self.check()
-        fs = self._delegate_required(path)
-        return fs.hasurl(path, purpose=purpose)
+        fs = self._delegate(path)
+        return fs is not None and fs.hasurl(path, purpose=purpose)
 
     def isdir(self, path):
         self.check()
diff --git a/fs/walk.py b/fs/walk.py
index 94d9e2e..6a524a6 100644
--- a/fs/walk.py
+++ b/fs/walk.py
@@ -7,8 +7,7 @@ searching etc. See :ref:`walking` for details.
 
 from __future__ import unicode_literals
 
-import itertools
-
+from collections import defaultdict
 from collections import deque
 from collections import namedtuple
 
@@ -26,86 +25,7 @@ Step = namedtuple('Step', 'path, dirs, files')
 """
 
 
-class WalkerBase(object):
-    """The base class for a Walker.
-
-    To create a custom walker, implement `~fs.walk.WalkerBase.walk`
-    in a sub-class.
-
-    See `~fs.walk.Walker` for a fully featured walker object that
-    should be adequate for all but your most exotic directory walking
-    needs.
-
-    """
-
-    def walk(self, fs, path='/', namespaces=None):
-        """Walk the directory structure of a filesystem.
-
-        Arguments:
-            fs (FS): A filesystem instance.
-            path (str, optional): A path to a directory on the filesystem.
-            namespaces(list, optional): A list of additional namespaces
-                to add to the `~fs.info.Info` objects.
-
-        Returns:
-            ~collections.Iterator: iterator of `~fs.walk.Step` named tuples.
-
-        """
-        raise NotImplementedError
-
-    def files(self, fs, path='/'):
-        """Walk a filesystem, yielding absolute paths to files.
-
-        Arguments:
-            fs (FS): A filesystem instance.
-            path (str, optional): A path to a directory on the filesystem.
-
-        Yields:
-            str: absolute path to files on the filesystem found
-            recursively within the given directory.
-
-        """
-        iter_walk = iter(self.walk(fs, path=path))
-        for _path, _, files in iter_walk:
-            for info in files:
-                yield join(_path, info.name)
-
-    def dirs(self, fs, path='/'):
-        """Walk a filesystem, yielding absolute paths to directories.
-
-        Arguments:
-            fs (FS): A filesystem instance.
-            path (str, optional): A path to a directory on the filesystem.
-
-        Yields:
-            str: absolute path to directories on the filesystem found
-            recursively within the given directory.
-
-        """
-        for path, dirs, _ in self.walk(fs, path=path):
-            for info in dirs:
-                yield join(path, info.name)
-
-    def info(self, fs, path='/', namespaces=None):
-        """Walk a filesystem, yielding tuples of ``(<path>, <info>)``.
-
-        Arguments:
-            fs (FS): A filesystem instance.
-            path (str, optional): A path to a directory on the filesystem.
-            namespaces (list, optional): A list of additional namespaces
-                to add to the `Info` objects.
-
-        Yields:
-            (str, Info): a tuple of ``(<absolute path>, <resource info>)``.
-
-        """
-        _walk = self.walk(fs, path=path, namespaces=namespaces)
-        for _path, dirs, files in _walk:
-            for info in itertools.chain(dirs, files):
-                yield join(_path, info.name), info
-
-
-class Walker(WalkerBase):
+class Walker(object):
     """A walker object recursively lists directories in a filesystem.
 
     Arguments:
@@ -122,7 +42,8 @@ class Walker(WalkerBase):
             list of filename patterns, e.g. ``['*.py']``. Files will only
             be returned if the final component matches one of the patterns.
         exclude_dirs (list, optional): A list of patterns that will be used
-            to filter out directories from the walk. e.g. ``['*.svn', '*.git']``.
+            to filter out directories from the walk. e.g.
+            ``['*.svn', '*.git']``.
         max_depth (int, optional): Maximum directory depth to walk.
 
     """
@@ -223,32 +144,18 @@ class Walker(WalkerBase):
             max_depth=(self.max_depth, None)
         )
 
-    def filter_files(self, fs, infos):
-        """Filter a sequence of resource `Info` objects.
-
-        The default implementation filters those files for which
-        `~fs.walk.Walker.check_file` returns `True`.
-
-        Arguments:
-            fs (FS): a filesystem instance.
-            infos (list): A list of `~fs.info.Info` instances.
-
-        Returns:
-            list: a list of `Info` objects passing the ``check_file``
-            validation.
-
-        """
-        _check_file = self.check_file
-        return [
-            info for info in infos
-            if _check_file(fs, info)
-        ]
+    def _iter_walk(self, fs, path, namespaces=None):
+        """Get the walk generator."""
+        if self.search == 'breadth':
+            return self._walk_breadth(fs, path, namespaces=namespaces)
+        else:
+            return self._walk_depth(fs, path, namespaces=namespaces)
 
     def _check_open_dir(self, fs, path, info):
         """Check if a directory should be considered in the walk.
         """
         if (self.exclude_dirs is not None and
-            fs.match(self.exclude_dirs, info.name)):
+                fs.match(self.exclude_dirs, info.name)):
             return False
         return self.check_open_dir(fs, path, info)
 
@@ -357,13 +264,68 @@ class Walker(WalkerBase):
 
         """
         _path = abspath(normpath(path))
+        dir_info = defaultdict(list)
+        _walk = self._iter_walk(fs, _path, namespaces=namespaces)
+        for dir_path, info in _walk:
+            if info is None:
+                dirs = []
+                files = []
+                for _info in dir_info[dir_path]:
+                    (dirs if _info.is_dir else files).append(_info)
+                yield Step(dir_path, dirs, files)
+                del dir_info[dir_path]
+            else:
+                dir_info[dir_path].append(info)
 
-        if self.search == 'breadth':
-            do_walk = self._walk_breadth
-        elif self.search == 'depth':
-            do_walk = self._walk_depth
+    def files(self, fs, path='/'):
+        """Walk a filesystem, yielding absolute paths to files.
+
+        Arguments:
+            fs (FS): A filesystem instance.
+            path (str, optional): A path to a directory on the filesystem.
+
+        Yields:
+            str: absolute path to files on the filesystem found
+            recursively within the given directory.
 
-        return do_walk(fs, _path, namespaces=namespaces)
+        """
+        for _path, info in self._iter_walk(fs, path=path):
+            if info is not None and not info.is_dir:
+                yield join(_path, info.name)
+
+    def dirs(self, fs, path='/'):
+        """Walk a filesystem, yielding absolute paths to directories.
+
+        Arguments:
+            fs (FS): A filesystem instance.
+            path (str, optional): A path to a directory on the filesystem.
+
+        Yields:
+            str: absolute path to directories on the filesystem found
+            recursively within the given directory.
+
+        """
+        for _path, info in self._iter_walk(fs, path=path):
+            if info is not None and info.is_dir:
+                yield join(_path, info.name)
+
+    def info(self, fs, path='/', namespaces=None):
+        """Walk a filesystem, yielding tuples of ``(<path>, <info>)``.
+
+        Arguments:
+            fs (FS): A filesystem instance.
+            path (str, optional): A path to a directory on the filesystem.
+            namespaces (list, optional): A list of additional namespaces
+                to add to the `Info` objects.
+
+        Yields:
+            (str, Info): a tuple of ``(<absolute path>, <resource info>)``.
+
+        """
+        _walk = self._iter_walk(fs, path=path, namespaces=namespaces)
+        for _path, info in _walk:
+            if info is not None:
+                yield join(_path, info.name), info
 
     def _walk_breadth(self, fs, path, namespaces=None):
         """Walk files using a *breadth first* search.
@@ -375,22 +337,17 @@ class Walker(WalkerBase):
 
         while queue:
             dir_path = pop()
-            dirs = []
-            files = []
             for info in self._scan(fs, dir_path, namespaces=namespaces):
                 if info.is_dir:
                     _depth = self._calculate_depth(dir_path) - depth + 1
                     if self._check_open_dir(fs, dir_path, info):
-                        dirs.append(info)
+                        yield dir_path, info  # Opened a directory
                         if self._check_scan_dir(fs, dir_path, info, _depth):
                             push(join(dir_path, info.name))
                 else:
-                    files.append(info)
-            yield Step(
-                dir_path,
-                dirs,
-                self.filter_files(fs, files)
-            )
+                    if self.check_file(fs, info):
+                        yield dir_path, info  # Found a file
+            yield dir_path, None  # End of directory
 
     def _walk_depth(self, fs, path, namespaces=None):
         """Walk files using a *depth first* search.
@@ -402,34 +359,28 @@ class Walker(WalkerBase):
             return self._scan(fs, path, namespaces=namespaces)
 
         depth = self._calculate_depth(path)
-        stack = [(
-            path, scan(path), [], []
-        )]
+        stack = [(path, scan(path), None)]
         push = stack.append
 
         while stack:
-            dir_path, iter_files, dirs, files = stack[-1]
-            try:
-                info = next(iter_files)
-            except StopIteration:
-                yield Step(
-                    dir_path,
-                    dirs,
-                    self.filter_files(fs, files)
-                )
+            dir_path, iter_files, parent = stack[-1]
+            info = next(iter_files, None)
+            if info is None:
+                if parent is not None:
+                    yield parent
+                yield dir_path, None
                 del stack[-1]
+            elif info.is_dir:
+                _depth = self._calculate_depth(dir_path) - depth + 1
+                if self._check_open_dir(fs, dir_path, info):
+                    if self._check_scan_dir(fs, dir_path, info, _depth):
+                        _path = join(dir_path, info.name)
+                        push((_path, scan(_path), (dir_path, info)))
+                    else:
+                        yield dir_path, info
             else:
-                if info.is_dir:
-                    _depth = self._calculate_depth(dir_path) - depth + 1
-                    if self._check_open_dir(fs, dir_path, info):
-                        dirs.append(info)
-                        if self._check_scan_dir(fs, dir_path, info, _depth):
-                            _path = join(dir_path, info.name)
-                            push((
-                                _path, scan(_path), [], []
-                            ))
-                else:
-                    files.append(info)
+                if self.check_file(fs, info):
+                    yield dir_path, info
 
 
 class BoundWalker(object):


Test patch:
diff --git a/fs/test.py b/fs/test.py
index db399b2..dbdc0a6 100644
--- a/fs/test.py
+++ b/fs/test.py
@@ -401,6 +401,8 @@ class FSTestCases(object):
             self.assertFalse(self.fs.hassyspath('foo'))
         else:
             self.assertTrue(self.fs.hassyspath('foo'))
+        # Should not throw an error
+        self.fs.hassyspath('a/b/c/foo/bar')
 
     def test_geturl(self):
         self.fs.create('foo')
@@ -410,6 +412,8 @@ class FSTestCases(object):
             self.assertFalse(self.fs.hasurl('foo'))
         else:
             self.assertTrue(self.fs.hasurl('foo'))
+        # Should not throw an error
+        self.fs.hasurl('a/b/c/foo/bar')
 
     def test_geturl_purpose(self):
         """Check an unknown purpose raises a NoURL error.
@@ -1075,8 +1079,14 @@ class FSTestCases(object):
 
     def test_removetree(self):
         self.fs.makedirs('foo/bar/baz')
+        self.fs.makedirs('foo/egg')
+        self.fs.makedirs('foo/a/b/c/d/e')
         self.fs.create('foo/egg.txt')
         self.fs.create('foo/bar/egg.bin')
+        self.fs.create('foo/bar/baz/egg.txt')
+        self.fs.create('foo/a/b/c/1.txt')
+        self.fs.create('foo/a/b/c/2.txt')
+        self.fs.create('foo/a/b/c/3.txt')
 
         self.assert_exists('foo/egg.txt')
         self.assert_exists('foo/bar/egg.bin')
diff --git a/tests/test_walk.py b/tests/test_walk.py
index fafee98..9aaf6fe 100644
--- a/tests/test_walk.py
+++ b/tests/test_walk.py
@@ -9,14 +9,6 @@ from fs.wrap import read_only
 import six
 
 
-class TestWalkerBase(unittest.TestCase):
-
-    def test_not_implemented(self):
-        walker = walk.WalkerBase()
-        with self.assertRaises(NotImplementedError):
-            walker.walk(None, path='/')
-
-
 class TestWalker(unittest.TestCase):
 
     def setUp(self):
@@ -45,7 +37,7 @@ class TestWalk(unittest.TestCase):
         self.fs.makedir('foo2/bar2')
         self.fs.makedir('foo2/bar2/bar3')
         self.fs.create('foo2/bar2/bar3/test.txt')
-        self.fs.create('foo2/top3.txt')
+        self.fs.create('foo2/top3.bin')
 
     def test_invalid(self):
         with self.assertRaises(ValueError):
@@ -64,7 +56,20 @@ class TestWalk(unittest.TestCase):
                 [info.name for info in dirs],
                 [info.name for info in files]
             ))
-        expected = [(u'/', [u'foo1', u'foo2', u'foo3'], []), (u'/foo1', [u'bar1'], [u'top1.txt', u'top2.txt']), (u'/foo2', [u'bar2'], [u'top3.txt']), (u'/foo3', [], []), (u'/foo1/bar1', [], []), (u'/foo2/bar2', [u'bar3'], []), (u'/foo2/bar2/bar3', [], [u'test.txt'])]
+        expected = [(u'/', [u'foo1', u'foo2', u'foo3'], []), (u'/foo1', [u'bar1'], [u'top1.txt', u'top2.txt']), (u'/foo2', [u'bar2'], [u'top3.bin']), (u'/foo3', [], []), (u'/foo1/bar1', [], []), (u'/foo2/bar2', [u'bar3'], []), (u'/foo2/bar2/bar3', [], [u'test.txt'])]
+        self.assertEqual(_walk, expected)
+
+    def test_walk_depth(self):
+        _walk = []
+        for step in self.fs.walk(search='depth'):
+            self.assertIsInstance(step, walk.Step)
+            path, dirs, files = step
+            _walk.append((
+                path,
+                [info.name for info in dirs],
+                [info.name for info in files]
+            ))
+        expected = [(u'/foo1/bar1', [], []), (u'/foo1', [u'bar1'], [u'top1.txt', u'top2.txt']), (u'/foo2/bar2/bar3', [], [u'test.txt']), (u'/foo2/bar2', [u'bar3'], []), (u'/foo2', [u'bar2'], [u'top3.bin']), (u'/foo3', [], []), (u'/', [u'foo1', u'foo2', u'foo3'], [])]
         self.assertEqual(_walk, expected)
 
     def test_walk_directory(self):
@@ -77,7 +82,7 @@ class TestWalk(unittest.TestCase):
                 [info.name for info in dirs],
                 [info.name for info in files]
             ))
-        expected = [(u'/foo2', [u'bar2'], [u'top3.txt']), (u'/foo2/bar2', [u'bar3'], []), (u'/foo2/bar2/bar3', [], [u'test.txt'])]
+        expected = [(u'/foo2', [u'bar2'], [u'top3.bin']), (u'/foo2/bar2', [u'bar3'], []), (u'/foo2/bar2/bar3', [], [u'test.txt'])]
         self.assertEqual(_walk, expected)
 
     def test_walk_levels_1(self):
@@ -88,6 +93,15 @@ class TestWalk(unittest.TestCase):
         files = sorted(info.name for info in results[0].files)
         self.assertEqual(files, [])
 
+    def test_walk_levels_1_depth(self):
+        results = list(self.fs.walk(max_depth=1, search='depth'))
+        self.assertEqual(len(results), 1)
+        dirs = sorted(info.name for info in results[0].dirs)
+        self.assertEqual(dirs, ['foo1', 'foo2', 'foo3'])
+        files = sorted(info.name for info in results[0].files)
+        self.assertEqual(files, [])
+
+
     def test_walk_levels_2(self):
         _walk = []
         for step in self.fs.walk(max_depth=2):
@@ -98,7 +112,7 @@ class TestWalk(unittest.TestCase):
                 sorted(info.name for info in dirs),
                 sorted(info.name for info in files)
             ))
-        expected = [(u'/', [u'foo1', u'foo2', u'foo3'], []), (u'/foo1', [u'bar1'], [u'top1.txt', u'top2.txt']), (u'/foo2', [u'bar2'], [u'top3.txt']), (u'/foo3', [], [])]
+        expected = [(u'/', [u'foo1', u'foo2', u'foo3'], []), (u'/foo1', [u'bar1'], [u'top1.txt', u'top2.txt']), (u'/foo2', [u'bar2'], [u'top3.bin']), (u'/foo3', [], [])]
         self.assertEqual(_walk, expected)
 
     def test_walk_files(self):
@@ -109,7 +123,7 @@ class TestWalk(unittest.TestCase):
             [
                 '/foo1/top1.txt',
                 '/foo1/top2.txt',
-                '/foo2/top3.txt',
+                '/foo2/top3.bin',
                 '/foo2/bar2/bar3/test.txt',
             ]
         )
@@ -121,7 +135,7 @@ class TestWalk(unittest.TestCase):
                 '/foo1/top1.txt',
                 '/foo1/top2.txt',
                 '/foo2/bar2/bar3/test.txt',
-                '/foo2/top3.txt',
+                '/foo2/top3.bin',
             ]
         )
 
@@ -144,9 +158,9 @@ class TestWalk(unittest.TestCase):
             dirs,
             [
                 '/foo1/bar1',
+                '/foo1',
                 '/foo2/bar2/bar3',
                 '/foo2/bar2',
-                '/foo1',
                 '/foo2',
                 '/foo3'
             ]
@@ -162,11 +176,49 @@ class TestWalk(unittest.TestCase):
             ]
         )
 
+    def test_walk_files_filter(self):
+        files = list(self.fs.walk.files(filter=['*.txt']))
+
+        self.assertEqual(
+            files,
+            [
+                '/foo1/top1.txt',
+                '/foo1/top2.txt',
+                '/foo2/bar2/bar3/test.txt',
+            ]
+        )
+
+        files = list(self.fs.walk.files(search="depth", filter=['*.txt']))
+        self.assertEqual(
+            files,
+            [
+                '/foo1/top1.txt',
+                '/foo1/top2.txt',
+                '/foo2/bar2/bar3/test.txt',
+            ]
+        )
+
+        files = list(self.fs.walk.files(filter=['*.bin']))
+
+        self.assertEqual(
+            files,
+            [
+                '/foo2/top3.bin'
+            ]
+        )
+
+        files = list(self.fs.walk.files(filter=['*.nope']))
+
+        self.assertEqual(
+            files, []
+        )
+
     def test_walk_info(self):
         walk = []
         for path, info in self.fs.walk.info():
             walk.append((path, info.is_dir, info.name))
-        expected = [(u'/foo1', True, u'foo1'), (u'/foo2', True, u'foo2'), (u'/foo3', True, u'foo3'), (u'/foo1/bar1', True, u'bar1'), (u'/foo1/top1.txt', False, u'top1.txt'), (u'/foo1/top2.txt', False, u'top2.txt'), (u'/foo2/bar2', True, u'bar2'), (u'/foo2/top3.txt', False, u'top3.txt'), (u'/foo2/bar2/bar3', True, u'bar3'), (u'/foo2/bar2/bar3/test.txt', False, u'test.txt')]
+
+        expected = [(u'/foo1', True, u'foo1'), (u'/foo2', True, u'foo2'), (u'/foo3', True, u'foo3'), (u'/foo1/top1.txt', False, u'top1.txt'), (u'/foo1/top2.txt', False, u'top2.txt'), (u'/foo1/bar1', True, u'bar1'), (u'/foo2/bar2', True, u'bar2'), (u'/foo2/top3.bin', False, u'top3.bin'), (u'/foo2/bar2/bar3', True, u'bar3'), (u'/foo2/bar2/bar3/test.txt', False, u'test.txt')]
         self.assertEqual(walk, expected)
 
     def test_broken(self):
@@ -198,7 +250,7 @@ class TestWalk(unittest.TestCase):
     def test_on_error_invalid(self):
         with self.assertRaises(TypeError):
             walk.Walker(on_error='nope')
-    
+
     def test_subdir_uses_same_walker(self):
         class CustomWalker(walk.Walker):