Merge pull request #1272 from jhamrick/improve-coverage

Improve test coverage in auth folder

Author: jhamrick

nbgrader/auth/base.py

@@ -4,7 +4,7 @@
 
 class BaseAuthPlugin(LoggingConfigurable):
 
-    def get_student_courses(self, student_id):
+    def get_student_courses(self, student_id):  # pragma: no cover
         """Gets the list of courses that the student is enrolled in.
 
         Arguments
@@ -22,7 +22,7 @@ def get_student_courses(self, student_id):
         """
         raise NotImplementedError
 
-    def add_student_to_course(self, student_id, course_id):
+    def add_student_to_course(self, student_id, course_id):  # pragma: no cover
         """Grants a student access to a given course.
 
         Arguments
@@ -35,7 +35,7 @@ def add_student_to_course(self, student_id, course_id):
         """
         raise NotImplementedError
 
-    def remove_student_from_course(self, student_id, course_id):
+    def remove_student_from_course(self, student_id, course_id):  # pragma: no cover
         """Removes a student's access to a given course.
 
         Arguments

nbgrader/auth/jupyterhub.py

@@ -19,10 +19,6 @@ def get_jupyterhub_user():
         raise JupyterhubEnvironmentError("JUPYTERHUB_USER env is required to run the exchange features of nbgrader.")
 
 
-def get_jupyterhub_url():
-    return os.environ.get('JUPYTERHUB_BASE_URL') or 'http://127.0.0.1:8000'
-
-
 def get_jupyterhub_api_url():
     return os.environ.get('JUPYTERHUB_API_URL') or 'http://127.0.0.1:8081/hub/api'
 
@@ -89,13 +85,11 @@ def get_student_courses(self, student_id):
             self.log.error("Make sure you start your service with a valid admin_user 'api_token' in your Jupyterhub config")
             raise
         courses = set()
-        try:
-            for group in response['groups']:
-                if group.startswith('nbgrader-') or group.startswith('formgrade-'):
-                    courses.add(group.split('-', 1)[1])
-        except KeyError:
-            print("KeyError: See Jupyterhub API: " + str(response))
-            self.log.error("KeyError: See Jupyterhub API: " + str(response))
+        for group in response['groups']:
+            if group.startswith('nbgrader-') or group.startswith('formgrade-'):
+                course = group.split('-', 1)[1]
+                if course:
+                    courses.add(course)
         return list(courses)
 
     def add_student_to_course(self, student_id, course_id):

nbgrader/server_extensions/course_list/handlers.py

@@ -13,13 +13,12 @@
 
 from notebook.utils import url_path_join as ujoin
 from notebook.base.handlers import IPythonHandler
-from traitlets.config import Config
 from jupyter_core.paths import jupyter_config_path
 
 from ...apps import NbGrader
 from ...auth import Authenticator
 from ...auth.jupyterhub import (JupyterhubEnvironmentError, get_jupyterhub_api_url,
-    get_jupyterhub_authorization, get_jupyterhub_url, get_jupyterhub_user)
+                                get_jupyterhub_authorization, get_jupyterhub_user)
 from ...coursedir import CourseDirectory
 from ... import __version__ as nbgrader_version
 

nbgrader/tests/test_auth.py

@@ -52,28 +52,54 @@ def test_default_authenticator():
 
 
 def test_jupyterhub_get_student_courses(env, jupyterhub_auth):
-    # this will fail, because the api token hasn't been set
+    # this will fail, because the user hasn't been set
+    env['JUPYTERHUB_API_TOKEN'] = 'abcd1234'
+    env['JUPYTERHUB_USER'] = ''
     with pytest.raises(JupyterhubEnvironmentError):
         jupyterhub_auth.get_student_courses('foo')
 
-    # should still fail, because the user hasn't been set
-    env['JUPYTERHUB_API_TOKEN'] = 'abcd1234'
+    # this will fail, because the api token hasn't been set
+    env['JUPYTERHUB_API_TOKEN'] = ''
+    env['JUPYTERHUB_USER'] = 'foo'
     with pytest.raises(JupyterhubEnvironmentError):
         jupyterhub_auth.get_student_courses('foo')
 
     # should fail, because the server returns a forbidden response
+    env['JUPYTERHUB_API_TOKEN'] = 'abcd1234'
     env['JUPYTERHUB_USER'] = 'foo'
     with requests_mock.Mocker() as m:
         _mock_api_call(m.get, '/users/foo', status_code=403)
         with pytest.raises(JupyterhubApiError):
             jupyterhub_auth.get_student_courses('foo')
 
-    # should succeed
+    # should succeed, but give no courses, because the group name is invalid
     with requests_mock.Mocker() as m:
+        _mock_api_call(m.get, '/users/foo', json={'groups': ['nbgrader-']})
+        assert jupyterhub_auth.get_student_courses('foo') == []
+
+        _mock_api_call(m.get, '/users/foo', json={'groups': ['course101']})
+        assert jupyterhub_auth.get_student_courses('foo') == []
+
         _mock_api_call(
             m.get, '/users/foo', json={'groups': ['nbgrader-course123']})
         assert jupyterhub_auth.get_student_courses('foo') == ['course123']
 
+    # should succeed
+    with requests_mock.Mocker() as m:
+        _mock_api_call(
+            m.get, '/users/foo', json={'groups': ['nbgrader-course123']})
+        assert jupyterhub_auth.get_student_courses('*') == ['course123']
+
+
+def test_jupyterhub_has_access(env, jupyterhub_auth):
+    env['JUPYTERHUB_API_TOKEN'] = 'abcd1234'
+    env['JUPYTERHUB_USER'] = 'foo'
+    with requests_mock.Mocker() as m:
+        _mock_api_call(
+            m.get, '/users/foo', json={'groups': ['nbgrader-course123']})
+        assert jupyterhub_auth.has_access('foo', 'course123')
+        assert not jupyterhub_auth.has_access('foo', 'courseABC')
+
 
 def test_jupyterhub_add_student_to_course_no_token(jupyterhub_auth):
     # this will fail, because the api token hasn't been set
@@ -98,6 +124,16 @@ def test_jupyterhub_add_student_to_course_forbidden(env, jupyterhub_auth, caplog
         assert 'ERROR' in [rec.levelname for rec in caplog.records]
 
 
+def test_jupyterhub_add_student_to_course_no_courseid(env, jupyterhub_auth, caplog):
+    # test case where something goes wrong
+    env['JUPYTERHUB_API_TOKEN'] = 'abcd1234'
+    env['JUPYTERHUB_USER'] = 'foo'
+    with requests_mock.Mocker() as m:
+        _mock_api_call(m.get, '/groups', status_code=403)
+        jupyterhub_auth.add_student_to_course('foo', None)
+        assert 'ERROR' in [rec.levelname for rec in caplog.records]
+
+
 def test_jupyterhub_add_student_to_course_success(env, jupyterhub_auth, caplog):
     env['JUPYTERHUB_API_TOKEN'] = 'abcd1234'
     env['JUPYTERHUB_USER'] = 'foo'
@@ -108,6 +144,12 @@ def test_jupyterhub_add_student_to_course_success(env, jupyterhub_auth, caplog):
         jupyterhub_auth.add_student_to_course('foo', 'course123')
         assert 'ERROR' not in [rec.levelname for rec in caplog.records]
 
+        # Check that it also works if the group already exists
+        _mock_api_call(m.get, '/groups', json=[{'name': 'nbgrader-course123'}])
+        _mock_api_call(m.post, '/groups/nbgrader-course123/users')
+        jupyterhub_auth.add_student_to_course('foo', 'course123')
+        assert 'ERROR' not in [rec.levelname for rec in caplog.records]
+
 
 def test_jupyterhub_remove_student_from_course_no_token(jupyterhub_auth):
     # this will fail, because the api token hasn't been set
@@ -132,6 +174,16 @@ def test_jupyterhub_remove_student_from_course_forbidden(env, jupyterhub_auth, c
         assert 'ERROR' in [rec.levelname for rec in caplog.records]
 
 
+def test_jupyterhub_remove_student_from_course_no_courseid(env, jupyterhub_auth, caplog):
+    # test case where something goes wrong
+    env['JUPYTERHUB_API_TOKEN'] = 'abcd1234'
+    env['JUPYTERHUB_USER'] = 'foo'
+    with requests_mock.Mocker() as m:
+        _mock_api_call(m.delete, '/groups/nbgrader-course123/users')
+        jupyterhub_auth.remove_student_from_course('foo', None)
+        assert 'ERROR' in [rec.levelname for rec in caplog.records]
+
+
 def test_jupyterhub_remove_student_from_course_success(env, jupyterhub_auth, caplog):
     env['JUPYTERHUB_API_TOKEN'] = 'abcd1234'
     env['JUPYTERHUB_USER'] = 'foo'