Repository: jupyter/nbgrader

Issue:
Unexpected behaviour of utils.find_all_files
<!--
Thanks for helping to improve nbgrader!

If you are submitting a bug report or looking for support, please use the below
template so we can efficiently solve the problem.

If you are requesting a new feature, feel free to remove irrelevant pieces of
the issue template. 
-->


### Operating system
Ubunto 16.04

### `nbgrader --version`
nbgrader version 0.5.4

### `jupyterhub --version` (if used with JupyterHub)
0.8.1

### `jupyter notebook --version` 
5.4.1

### Expected behavior
By including '.git' or '.git/**' in CourseDirectory.ignore anything under the git directory to be ignored.

### Actual behavior
Anything in subdirectories of '.git' is included.

### Steps to reproduce the behavior
$ mkdir -p foo/bar/qwe
$ touch foo/bar/qwe/file.py
$ /opt/conda/bin/python -c "from nbgrader.utils import find_all_files;print(find_all_files('foo', ['bar']))"
['foo/bar/qwe/file.py']

I'm sorry if this is expected behaviour but I found it surprising.

Discussion:

Patch:
diff --git a/nbgrader/utils.py b/nbgrader/utils.py
index 55824f3f..55f440ab 100644
--- a/nbgrader/utils.py
+++ b/nbgrader/utils.py
@@ -194,8 +194,10 @@ def find_all_files(path, exclude=None):
     """Recursively finds all filenames rooted at `path`, optionally excluding
     some based on filename globs."""
     files = []
+    to_skip = []
     for dirname, dirnames, filenames in os.walk(path):
-        if is_ignored(dirname, exclude):
+        if is_ignored(dirname, exclude) or dirname in to_skip:
+            to_skip.extend([os.path.join(dirname, x) for x in dirnames])
             continue
         for filename in filenames:
             fullpath = os.path.join(dirname, filename)


Test patch:
diff --git a/nbgrader/tests/utils/test_utils.py b/nbgrader/tests/utils/test_utils.py
index 2814ea5c..ca76e83f 100644
--- a/nbgrader/tests/utils/test_utils.py
+++ b/nbgrader/tests/utils/test_utils.py
@@ -272,18 +272,34 @@ def test_is_ignored(temp_cwd):
 
 
 def test_find_all_files(temp_cwd):
-    os.makedirs(join("foo", "bar"))
+    os.makedirs(join("foo", "bar", "quux"))
     with open(join("foo", "baz.txt"), "w") as fh:
         fh.write("baz")
     with open(join("foo", "bar", "baz.txt"), "w") as fh:
         fh.write("baz")
+    with open(join("foo", "bar", "quux", "baz.txt"), "w") as fh:
+        fh.write("baz")
 
-    assert utils.find_all_files("foo") == [join("foo", "baz.txt"), join("foo", "bar", "baz.txt")]
+    assert utils.find_all_files("foo") == [
+        join("foo", "baz.txt"),
+        join("foo", "bar", "baz.txt"),
+        join("foo", "bar", "quux", "baz.txt")]
     assert utils.find_all_files("foo", ["bar"]) == [join("foo", "baz.txt")]
-    assert utils.find_all_files(join("foo", "bar")) == [join("foo", "bar", "baz.txt")]
+    assert utils.find_all_files("foo", ["quux"]) == [
+        join("foo", "baz.txt"),
+        join("foo", "bar", "baz.txt")]
+    assert utils.find_all_files(join("foo", "bar")) == [
+        join("foo", "bar", "baz.txt"),
+        join("foo", "bar", "quux", "baz.txt")]
     assert utils.find_all_files(join("foo", "bar"), ["*.txt"]) == []
-    assert utils.find_all_files(".") == [join(".", "foo", "baz.txt"), join(".", "foo", "bar", "baz.txt")]
+    assert utils.find_all_files(".") == [
+        join(".", "foo", "baz.txt"),
+        join(".", "foo", "bar", "baz.txt"),
+        join(".", "foo", "bar", "quux", "baz.txt")]
     assert utils.find_all_files(".", ["bar"]) == [join(".", "foo", "baz.txt")]
+    assert utils.find_all_files(".", ["quux"]) == [
+        join(".", "foo", "baz.txt"),
+        join(".", "foo", "bar", "baz.txt")]
 
 
 def test_unzip_invalid_ext(temp_cwd):