From 9372d5f87253824eafc4d0d872f3dc5f1c4142b6 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Apr 2015 16:14:42 -0700 Subject: [PATCH 1/7] add coverage --- .coveragerc | 4 ++++ .gitignore | 3 +++ .travis.yml | 4 +++- dev-requirements.txt | 2 ++ jupyterhub/tests/mocking.py | 14 ++++++++++++++ 5 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 .coveragerc diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 00000000..63568422 --- /dev/null +++ b/.coveragerc @@ -0,0 +1,4 @@ +[run] +omit = + jupyterhub/tests/* + jupyterhub/singleuser.py diff --git a/.gitignore b/.gitignore index de3322be..693f6d80 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,6 @@ share/jupyter/hub/static/css/style.min.css share/jupyter/hub/static/css/style.min.css.map *.egg-info MANIFEST +.coverage +htmlcov + diff --git a/.travis.yml b/.travis.yml index 03270c67..7cd48d22 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,4 +12,6 @@ install: - pip install -f travis-wheels/wheelhouse -r dev-requirements.txt . - pip install -f travis-wheels/wheelhouse ipython[notebook] script: - - py.test jupyterhub + - py.test --cov jupyterhub jupyterhub/tests +after_success: + - coveralls diff --git a/dev-requirements.txt b/dev-requirements.txt index 26b77f68..71546ccb 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,2 +1,4 @@ -r requirements.txt +coveralls +pytest-cov pytest diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 39cf9477..923b39d6 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -7,6 +7,8 @@ import threading from unittest import mock +import requests + from tornado import gen from tornado.concurrent import Future from tornado.ioloop import IOLoop @@ -126,3 +128,15 @@ class MockHub(JupyterHub): # ignore the call that will fire in atexit self.cleanup = lambda : None self.db_file.close() + + def login_user(self, name): + r = requests.post(self.proxy.public_server.url + 'hub/login', + data={ + 'username': name, + 'password': name, + }, + allow_redirects=False, + ) + assert r.cookies + return r.cookies + From d0b4e5bc2abf669a2586df0d8ae6550892bf92d8 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Apr 2015 16:47:37 -0700 Subject: [PATCH 2/7] add some basic exercise for HTML pages --- jupyterhub/tests/test_pages.py | 58 ++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 jupyterhub/tests/test_pages.py diff --git a/jupyterhub/tests/test_pages.py b/jupyterhub/tests/test_pages.py new file mode 100644 index 00000000..99291684 --- /dev/null +++ b/jupyterhub/tests/test_pages.py @@ -0,0 +1,58 @@ +"""Tests for HTML pages""" + +import requests + +from ..utils import url_path_join as ujoin +from .. import orm + + +def get_page(path, app, **kw): + base_url = ujoin(app.proxy.public_server.host, app.hub.server.base_url) + print(base_url) + return requests.get(ujoin(base_url, path), **kw) + +def test_root_no_auth(app, io_loop): + print(app.hub.server.is_up()) + routes = io_loop.run_sync(app.proxy.get_routes) + print(routes) + print(app.hub.server) + r = requests.get(app.proxy.public_server.host) + r.raise_for_status() + assert r.url == ujoin(app.proxy.public_server.host, app.hub.server.base_url) + +def test_root_auth(app): + cookies = app.login_user('river') + r = requests.get(app.proxy.public_server.host, cookies=cookies) + r.raise_for_status() + assert r.url == ujoin(app.proxy.public_server.host, '/user/river') + +def test_home_no_auth(app): + r = get_page('home', app, allow_redirects=False) + r.raise_for_status() + assert r.status_code == 302 + assert '/hub/login' in r.headers['Location'] + +def test_home_auth(app): + cookies = app.login_user('river') + r = get_page('home', app, cookies=cookies) + r.raise_for_status() + assert r.url.endswith('home') + +def test_admin_no_auth(app): + r = get_page('admin', app) + assert r.status_code == 403 + +def test_admin_not_admin(app): + cookies = app.login_user('wash') + r = get_page('admin', app, cookies=cookies) + assert r.status_code == 403 + +def test_admin(app): + cookies = app.login_user('river') + u = orm.User.find(app.db, 'river') + u.admin = True + app.db.commit() + r = get_page('admin', app, cookies=cookies) + r.raise_for_status() + assert r.url.endswith('/admin') + From d9fc40652d35bd930a2f4d158ffe2fd73c847c2e Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Apr 2015 17:04:41 -0700 Subject: [PATCH 3/7] test shutdown API handler --- jupyterhub/tests/test_api.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index 45848144..1eb21830 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -1,6 +1,7 @@ """Tests for the REST API""" import json +import time from datetime import timedelta import requests @@ -284,3 +285,18 @@ def test_get_proxy(app, io_loop): r.raise_for_status() reply = r.json() assert list(reply.keys()) == ['/'] + + +def test_shutdown(app): + r = api_request(app, 'shutdown', method='post', data=json.dumps({ + 'servers': True, + 'proxy': True, + })) + r.raise_for_status() + reply = r.json() + for i in range(100): + if app.io_loop._running: + time.sleep(0.1) + else: + break + assert not app.io_loop._running From 04b7056591099ea3b24cda0fa951e8e0458832a5 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Apr 2015 17:36:41 -0700 Subject: [PATCH 4/7] fix group-whitelist checks and test it --- jupyterhub/auth.py | 6 +++--- jupyterhub/tests/test_auth.py | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/jupyterhub/auth.py b/jupyterhub/auth.py index 347c194e..9c235e9b 100644 --- a/jupyterhub/auth.py +++ b/jupyterhub/auth.py @@ -126,11 +126,11 @@ class LocalAuthenticator(Authenticator): def check_group_whitelist(self, username): if not self.group_whitelist: return False - for group in self.group_whitelist: + for grnam in self.group_whitelist: try: - group = getgrnam(self.group_whitelist) + group = getgrnam(grnam) except KeyError: - self.log.error('No such group: [%s]' % self.group_whitelist) + self.log.error('No such group: [%s]' % grnam) continue if username in group.gr_mem: return True diff --git a/jupyterhub/tests/test_auth.py b/jupyterhub/tests/test_auth.py index 8e5103c7..a467f3b9 100644 --- a/jupyterhub/tests/test_auth.py +++ b/jupyterhub/tests/test_auth.py @@ -4,7 +4,9 @@ # Distributed under the terms of the Modified BSD License. from .mocking import MockPAMAuthenticator +from unittest import mock +from jupyterhub import auth def test_pam_auth(io_loop): authenticator = MockPAMAuthenticator() @@ -39,3 +41,40 @@ def test_pam_auth_whitelist(io_loop): 'password': 'mal', })) assert authorized is None + + +class MockGroup: + def __init__(self, *names): + self.gr_mem = names + + +def test_pam_auth_group_whitelist(io_loop): + g = MockGroup('kaylee') + def getgrnam(name): + return g + + authenticator = MockPAMAuthenticator(group_whitelist={'group'}) + + with mock.patch.object(auth, 'getgrnam', getgrnam): + authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + 'username': 'kaylee', + 'password': 'kaylee', + })) + assert authorized == 'kaylee' + + with mock.patch.object(auth, 'getgrnam', getgrnam): + authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + 'username': 'mal', + 'password': 'mal', + })) + assert authorized is None + + +def test_pam_auth_no_such_group(io_loop): + authenticator = MockPAMAuthenticator(group_whitelist={'nosuchcrazygroup'}) + authorized = io_loop.run_sync(lambda : authenticator.authenticate(None, { + 'username': 'kaylee', + 'password': 'kaylee', + })) + assert authorized is None + From 64c4d00756aabeb57a62cf2e700ea8930317986c Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Apr 2015 17:41:00 -0700 Subject: [PATCH 5/7] test add_system_user --- jupyterhub/tests/test_auth.py | 46 +++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/jupyterhub/tests/test_auth.py b/jupyterhub/tests/test_auth.py index a467f3b9..f67929a2 100644 --- a/jupyterhub/tests/test_auth.py +++ b/jupyterhub/tests/test_auth.py @@ -3,10 +3,13 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. -from .mocking import MockPAMAuthenticator +from subprocess import CalledProcessError from unittest import mock -from jupyterhub import auth +import pytest +from .mocking import MockPAMAuthenticator + +from jupyterhub import auth, orm def test_pam_auth(io_loop): authenticator = MockPAMAuthenticator() @@ -78,3 +81,42 @@ def test_pam_auth_no_such_group(io_loop): })) assert authorized is None + +def test_wont_add_system_user(io_loop): + user = orm.User(name='lioness4321') + authenticator = auth.PAMAuthenticator(whitelist={'mal'}) + authenticator.create_system_users = False + with pytest.raises(KeyError): + io_loop.run_sync(lambda : authenticator.add_user(user)) + +def test_cant_add_system_user(io_loop): + user = orm.User(name='lioness4321') + authenticator = auth.PAMAuthenticator(whitelist={'mal'}) + authenticator.create_system_users = True + + def check_output(cmd, *a, **kw): + raise CalledProcessError(1, cmd) + + with mock.patch.object(auth, 'check_output', check_output): + with pytest.raises(RuntimeError): + io_loop.run_sync(lambda : authenticator.add_user(user)) + +def test_add_system_user(io_loop): + user = orm.User(name='lioness4321') + authenticator = auth.PAMAuthenticator(whitelist={'mal'}) + authenticator.create_system_users = True + + def check_output(*a, **kw): + return + + record = {} + def check_call(cmd, *a, **kw): + record['cmd'] = cmd + + with mock.patch.object(auth, 'check_output', check_output), \ + mock.patch.object(auth, 'check_call', check_call): + io_loop.run_sync(lambda : authenticator.add_user(user)) + + assert user.name in record['cmd'] + + \ No newline at end of file From 34386ba3b7de61108f03d56c086b11673e3506a4 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Apr 2015 17:45:39 -0700 Subject: [PATCH 6/7] more authenticator coverage --- jupyterhub/tests/test_auth.py | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/jupyterhub/tests/test_auth.py b/jupyterhub/tests/test_auth.py index f67929a2..d418b585 100644 --- a/jupyterhub/tests/test_auth.py +++ b/jupyterhub/tests/test_auth.py @@ -88,7 +88,8 @@ def test_wont_add_system_user(io_loop): authenticator.create_system_users = False with pytest.raises(KeyError): io_loop.run_sync(lambda : authenticator.add_user(user)) - + + def test_cant_add_system_user(io_loop): user = orm.User(name='lioness4321') authenticator = auth.PAMAuthenticator(whitelist={'mal'}) @@ -101,6 +102,7 @@ def test_cant_add_system_user(io_loop): with pytest.raises(RuntimeError): io_loop.run_sync(lambda : authenticator.add_user(user)) + def test_add_system_user(io_loop): user = orm.User(name='lioness4321') authenticator = auth.PAMAuthenticator(whitelist={'mal'}) @@ -118,5 +120,30 @@ def test_add_system_user(io_loop): io_loop.run_sync(lambda : authenticator.add_user(user)) assert user.name in record['cmd'] + + +def test_delete_user(io_loop): + user = orm.User(name='zoe') + a = MockPAMAuthenticator(whitelist={'mal'}) - \ No newline at end of file + assert 'zoe' not in a.whitelist + a.add_user(user) + assert 'zoe' in a.whitelist + a.delete_user(user) + assert 'zoe' not in a.whitelist + + +def test_urls(): + a = auth.PAMAuthenticator() + logout = a.logout_url('/base/url/') + login = a.login_url('/base/url') + assert logout == '/base/url/logout' + assert login == '/base/url/login' + + +def test_handlers(app): + a = auth.PAMAuthenticator() + handlers = a.get_handlers(app) + assert handlers[0][0] == '/login' + + From 998fc28c3235e5682212f4bdf004fb61f413e778 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 7 Apr 2015 15:48:52 -0700 Subject: [PATCH 7/7] various testing cleanup - Disable signal register during testing. It doesn't work in background threads. - Fix IOLoop instance management. Some instances were being reused across tests. --- jupyterhub/app.py | 8 +++++++- jupyterhub/tests/mocking.py | 15 ++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 094e04e6..9fd9d6b7 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1003,6 +1003,9 @@ class JupyterHub(Application): # register cleanup on both TERM and INT atexit.register(self.atexit) + self.init_signal() + + def init_signal(self): signal.signal(signal.SIGTERM, self.sigterm) def sigterm(self, signum, frame): @@ -1027,7 +1030,10 @@ class JupyterHub(Application): if not self.io_loop: return if self.http_server: - self.io_loop.add_callback(self.http_server.stop) + if self.io_loop._running: + self.io_loop.add_callback(self.http_server.stop) + else: + self.http_server.stop() self.io_loop.add_callback(self.io_loop.stop) @gen.coroutine diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 923b39d6..02a38c1b 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -84,7 +84,7 @@ class MockHub(JupyterHub): """Hub with various mock bits""" db_file = None - + def _ip_default(self): return 'localhost' @@ -97,12 +97,18 @@ class MockHub(JupyterHub): def _admin_users_default(self): return {'admin'} + def init_signal(self): + pass + def start(self, argv=None): self.db_file = NamedTemporaryFile() self.db_url = 'sqlite:///' + self.db_file.name + evt = threading.Event() + @gen.coroutine def _start_co(): + assert self.io_loop._running # put initialize in start for SQLAlchemy threading reasons yield super(MockHub, self).initialize(argv=argv) # add an initial user @@ -110,16 +116,19 @@ class MockHub(JupyterHub): self.db.add(user) self.db.commit() yield super(MockHub, self).start() + yield self.hub.server.wait_up(http=True) self.io_loop.add_callback(evt.set) def _start(): - self.io_loop = IOLoop.current() + self.io_loop = IOLoop() + self.io_loop.make_current() self.io_loop.add_callback(_start_co) self.io_loop.start() self._thread = threading.Thread(target=_start) self._thread.start() - evt.wait(timeout=5) + ready = evt.wait(timeout=10) + assert ready def stop(self): super().stop()