From 58f072e5af90fc0dd220f67b0b18ad7bdfd7136b Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 26 Jul 2017 17:40:37 +0200 Subject: [PATCH 1/9] start MockHub without threads everything's going to run with gen_test need our own io_loop fixture that's module_scoped to go with our app fixture --- jupyterhub/tests/conftest.py | 32 ++++++++++++++++++++----------- jupyterhub/tests/mocking.py | 37 ++++++++---------------------------- 2 files changed, 29 insertions(+), 40 deletions(-) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 5a2d8ac7..1724f7a5 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -36,25 +36,35 @@ def db(): return _db -@fixture -def io_loop(): - """Get the current IOLoop""" - loop = ioloop.IOLoop() - loop.make_current() - return loop +@fixture(scope='module') +def io_loop(request): + """Same as pytest-tornado.gen""" + print("my io_loop fixture") + io_loop = ioloop.IOLoop() + io_loop.make_current() + def _close(): + io_loop.clear_current() + if (not ioloop.IOLoop.initialized() or + io_loop is not ioloop.IOLoop.instance()): + io_loop.close(all_fds=True) + + request.addfinalizer(_close) + return io_loop @fixture(scope='module') -def app(request): +def app(request, io_loop): """Mock a jupyterhub app for testing""" mocked_app = MockHub.instance(log_level=logging.DEBUG) - mocked_app.start([]) - + @gen.coroutine + def make_app(): + yield mocked_app.initialize([]) + yield mocked_app.start() + io_loop.run_sync(make_app) def fin(): # disconnect logging during cleanup because pytest closes captured FDs prematurely mocked_app.log.handlers = [] - MockHub.clear_instance() mocked_app.stop() request.addfinalizer(fin) @@ -68,7 +78,7 @@ class MockServiceSpawner(jupyterhub.services.service._ServiceSpawner): _mock_service_counter = 0 -def _mockservice(request, app, url=False): +def _mockservice(request, app, io_loop, url=False): global _mock_service_counter _mock_service_counter += 1 name = 'mock-service-%i' % _mock_service_counter diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index d31bc1de..8b773890 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -159,41 +159,20 @@ class MockHub(JupyterHub): def load_config_file(self, *args, **kwargs): pass - - def start(self, argv=None): + + @gen.coroutine + def initialize(self, argv=None): self.db_file = NamedTemporaryFile() self.pid_file = NamedTemporaryFile(delete=False).name self.db_url = self.db_file.name + yield super().initialize([]) - 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 - user = orm.User(name='user') - self.db.add(user) - self.db.commit() - yield super(MockHub, self).start() - yield self.hub.wait_up(http=True) - self.io_loop.add_callback(evt.set) - - def _start(): - 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() - ready = evt.wait(timeout=10) - assert ready - + user = orm.User(name='user') + self.db.add(user) + self.db.commit() + def stop(self): super().stop() - self._thread.join() IOLoop().run_sync(self.cleanup) # ignore the call that will fire in atexit self.cleanup = lambda : None From f05aecf5f977b577466bf5392037da908be5823d Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 27 Jul 2017 10:34:54 +0200 Subject: [PATCH 2/9] test_api passes without threads just put requests in a thread via `utils.async_requests` eliminates db threads issue --- jupyterhub/tests/conftest.py | 14 +- jupyterhub/tests/mocking.py | 4 +- jupyterhub/tests/test_api.py | 251 ++++++++++++++++++--------------- jupyterhub/tests/test_utils.py | 0 jupyterhub/tests/utils.py | 18 +++ 5 files changed, 165 insertions(+), 122 deletions(-) delete mode 100644 jupyterhub/tests/test_utils.py create mode 100644 jupyterhub/tests/utils.py diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 1724f7a5..33e96351 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -38,8 +38,7 @@ def db(): @fixture(scope='module') def io_loop(request): - """Same as pytest-tornado.gen""" - print("my io_loop fixture") + """Same as pytest-tornado.io_loop, but re-scoped to module-level""" io_loop = ioloop.IOLoop() io_loop.make_current() @@ -78,7 +77,7 @@ class MockServiceSpawner(jupyterhub.services.service._ServiceSpawner): _mock_service_counter = 0 -def _mockservice(request, app, io_loop, url=False): +def _mockservice(request, app, url=False): global _mock_service_counter _mock_service_counter += 1 name = 'mock-service-%i' % _mock_service_counter @@ -90,6 +89,8 @@ def _mockservice(request, app, io_loop, url=False): if url: spec['url'] = 'http://127.0.0.1:%i' % random_port() + io_loop = app.io_loop + with mock.patch.object(jupyterhub.services.service, '_ServiceSpawner', MockServiceSpawner): app.services = [spec] app.init_services() @@ -100,20 +101,17 @@ def _mockservice(request, app, io_loop, url=False): # wait for proxy to be updated before starting the service yield app.proxy.add_all_services(app._service_map) service.start() - app.io_loop.add_callback(start) + io_loop.run_sync(start) def cleanup(): service.stop() app.services[:] = [] app._service_map.clear() request.addfinalizer(cleanup) - for i in range(20): - if not getattr(service, 'proc', False): - time.sleep(0.2) # ensure process finishes starting with raises(TimeoutExpired): service.proc.wait(1) if url: - ioloop.IOLoop().run_sync(service.server.wait_up) + io_loop.run_sync(service.server.wait_up) return service diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 8b773890..11d6472c 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -22,6 +22,7 @@ from ..objects import Server from ..spawner import LocalProcessSpawner from ..singleuser import SingleUserNotebookApp from ..utils import random_port, url_path_join +from .utils import async_requests from pamela import PAMError @@ -178,10 +179,11 @@ class MockHub(JupyterHub): self.cleanup = lambda : None self.db_file.close() + @gen.coroutine def login_user(self, name): """Login a user by name, returning her cookies.""" base_url = public_url(self) - r = requests.post(base_url + 'hub/login', + r = yield async_requests.post(base_url + 'hub/login', data={ 'username': name, 'password': name, diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index a0dfc720..a3e3e843 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -2,12 +2,11 @@ import json import time -from queue import Queue import sys from unittest import mock from urllib.parse import urlparse, quote -from pytest import mark, yield_fixture +from pytest import mark import requests from tornado import gen @@ -18,6 +17,7 @@ from ..user import User from ..utils import url_path_join as ujoin from . import mocking from .mocking import public_host, public_url +from .utils import async_requests def check_db_locks(func): @@ -81,6 +81,7 @@ def auth_header(db, name): @check_db_locks +@gen.coroutine def api_request(app, *api_path, **kwargs): """Make an API request""" base_url = app.hub.url @@ -91,8 +92,8 @@ def api_request(app, *api_path, **kwargs): url = ujoin(base_url, 'api', *api_path) method = kwargs.pop('method', 'get') - f = getattr(requests, method) - resp = f(url, **kwargs) + f = getattr(async_requests, method) + resp = yield f(url, **kwargs) assert "frame-ancestors 'self'" in resp.headers['Content-Security-Policy'] assert ujoin(app.hub.base_url, "security/csp-report") in resp.headers['Content-Security-Policy'] assert 'http' not in resp.headers['Content-Security-Policy'] @@ -104,9 +105,10 @@ def api_request(app, *api_path, **kwargs): # -------------------- +@mark.gen_test def test_auth_api(app): db = app.db - r = api_request(app, 'authorizations', 'gobbledygook') + r = yield api_request(app, 'authorizations', 'gobbledygook') assert r.status_code == 404 # make a new cookie token @@ -114,36 +116,37 @@ def test_auth_api(app): api_token = user.new_api_token() # check success: - r = api_request(app, 'authorizations/token', api_token) + r = yield api_request(app, 'authorizations/token', api_token) assert r.status_code == 200 reply = r.json() assert reply['name'] == user.name # check fail - r = api_request(app, 'authorizations/token', api_token, + r = yield api_request(app, 'authorizations/token', api_token, headers={'Authorization': 'no sir'}, ) assert r.status_code == 403 - r = api_request(app, 'authorizations/token', api_token, + r = yield api_request(app, 'authorizations/token', api_token, headers={'Authorization': 'token: %s' % user.cookie_id}, ) assert r.status_code == 403 +@mark.gen_test def test_referer_check(app, io_loop): url = ujoin(public_host(app), app.hub.base_url) host = urlparse(url).netloc user = find_user(app.db, 'admin') if user is None: user = add_user(app.db, name='admin', admin=True) - cookies = app.login_user('admin') - app_user = get_app_user(app, 'admin') + cookies = yield app.login_user('admin') + app_user = app.users[user] # stop the admin's server so we don't mess up future tests - io_loop.run_sync(lambda: app.proxy.delete_user(app_user)) - io_loop.run_sync(app_user.stop) + yield app.proxy.delete_user(app_user) + yield app_user.stop() - r = api_request(app, 'users', + r = yield api_request(app, 'users', headers={ 'Authorization': '', 'Referer': 'null', @@ -151,7 +154,7 @@ def test_referer_check(app, io_loop): ) assert r.status_code == 403 - r = api_request(app, 'users', + r = yield api_request(app, 'users', headers={ 'Authorization': '', 'Referer': 'http://attack.com/csrf/vulnerability', @@ -159,7 +162,7 @@ def test_referer_check(app, io_loop): ) assert r.status_code == 403 - r = api_request(app, 'users', + r = yield api_request(app, 'users', headers={ 'Authorization': '', 'Referer': url, @@ -168,7 +171,7 @@ def test_referer_check(app, io_loop): ) assert r.status_code == 200 - r = api_request(app, 'users', + r = yield api_request(app, 'users', headers={ 'Authorization': '', 'Referer': ujoin(url, 'foo/bar/baz/bat'), @@ -184,9 +187,10 @@ def test_referer_check(app, io_loop): @mark.user +@mark.gen_test def test_get_users(app): db = app.db - r = api_request(app, 'users') + r = yield api_request(app, 'users') assert r.status_code == 200 users = sorted(r.json(), key=lambda d: d['name']) @@ -211,17 +215,18 @@ def test_get_users(app): } ] - r = api_request(app, 'users', + r = yield api_request(app, 'users', headers=auth_header(db, 'user'), ) assert r.status_code == 403 @mark.user +@mark.gen_test def test_add_user(app): db = app.db name = 'newuser' - r = api_request(app, 'users', name, method='post') + r = yield api_request(app, 'users', name, method='post') assert r.status_code == 201 user = find_user(db, name) assert user is not None @@ -230,9 +235,10 @@ def test_add_user(app): @mark.user +@mark.gen_test def test_get_user(app): name = 'user' - r = api_request(app, 'users', name) + r = yield api_request(app, 'users', name) assert r.status_code == 200 user = r.json() user.pop('last_activity') @@ -247,19 +253,21 @@ def test_get_user(app): @mark.user +@mark.gen_test def test_add_multi_user_bad(app): - r = api_request(app, 'users', method='post') + r = yield api_request(app, 'users', method='post') assert r.status_code == 400 - r = api_request(app, 'users', method='post', data='{}') + r = yield api_request(app, 'users', method='post', data='{}') assert r.status_code == 400 - r = api_request(app, 'users', method='post', data='[]') + r = yield api_request(app, 'users', method='post', data='[]') assert r.status_code == 400 @mark.user +@mark.gen_test def test_add_multi_user_invalid(app): app.authenticator.username_pattern = r'w.*' - r = api_request(app, 'users', method='post', + r = yield api_request(app, 'users', method='post', data=json.dumps({'usernames': ['Willow', 'Andrew', 'Tara']}) ) app.authenticator.username_pattern = '' @@ -268,10 +276,11 @@ def test_add_multi_user_invalid(app): @mark.user +@mark.gen_test def test_add_multi_user(app): db = app.db names = ['a', 'b'] - r = api_request(app, 'users', method='post', + r = yield api_request(app, 'users', method='post', data=json.dumps({'usernames': names}), ) assert r.status_code == 201 @@ -286,7 +295,7 @@ def test_add_multi_user(app): assert not user.admin # try to create the same users again - r = api_request(app, 'users', method='post', + r = yield api_request(app, 'users', method='post', data=json.dumps({'usernames': names}), ) assert r.status_code == 400 @@ -294,7 +303,7 @@ def test_add_multi_user(app): names = ['a', 'b', 'ab'] # try to create the same users again - r = api_request(app, 'users', method='post', + r = yield api_request(app, 'users', method='post', data=json.dumps({'usernames': names}), ) assert r.status_code == 201 @@ -304,10 +313,11 @@ def test_add_multi_user(app): @mark.user +@mark.gen_test def test_add_multi_user_admin(app): db = app.db names = ['c', 'd'] - r = api_request(app, 'users', method='post', + r = yield api_request(app, 'users', method='post', data=json.dumps({'usernames': names, 'admin': True}), ) assert r.status_code == 201 @@ -323,20 +333,22 @@ def test_add_multi_user_admin(app): @mark.user +@mark.gen_test def test_add_user_bad(app): db = app.db name = 'dne_newuser' - r = api_request(app, 'users', name, method='post') + r = yield api_request(app, 'users', name, method='post') assert r.status_code == 400 user = find_user(db, name) assert user is None @mark.user +@mark.gen_test def test_add_admin(app): db = app.db name = 'newadmin' - r = api_request(app, 'users', name, method='post', + r = yield api_request(app, 'users', name, method='post', data=json.dumps({'admin': True}), ) assert r.status_code == 201 @@ -347,25 +359,27 @@ def test_add_admin(app): @mark.user +@mark.gen_test def test_delete_user(app): db = app.db mal = add_user(db, name='mal') - r = api_request(app, 'users', 'mal', method='delete') + r = yield api_request(app, 'users', 'mal', method='delete') assert r.status_code == 204 @mark.user +@mark.gen_test def test_make_admin(app): db = app.db name = 'admin2' - r = api_request(app, 'users', name, method='post') + r = yield api_request(app, 'users', name, method='post') assert r.status_code == 201 user = find_user(db, name) assert user is not None assert user.name == name assert not user.admin - r = api_request(app, 'users', name, method='patch', + r = yield api_request(app, 'users', name, method='patch', data=json.dumps({'admin': True}) ) assert r.status_code == 200 @@ -375,23 +389,7 @@ def test_make_admin(app): assert user.admin -def get_app_user(app, name): - """Helper to get the User object from the main thread. - - Needed for access to the Spawner during testing. - - No ORM methods should be called on the result. - """ - q = Queue() - - def get_user_id(): - user = find_user(app.db, name) - q.put(user.id) - app.io_loop.add_callback(get_user_id) - user_id = q.get(timeout=2) - return app.users[user_id] - - +@mark.gen_test def test_spawn(app, io_loop): db = app.db name = 'wash' @@ -401,41 +399,41 @@ def test_spawn(app, io_loop): 'i': 5, } before_servers = sorted(db.query(orm.Server), key=lambda s: s.url) - r = api_request(app, 'users', name, 'server', method='post', + r = yield api_request(app, 'users', name, 'server', method='post', data=json.dumps(options), ) assert r.status_code == 201 assert 'pid' in user.orm_spawners[''].state - app_user = get_app_user(app, name) + app_user = app.users[name] assert app_user.spawner is not None spawner = app_user.spawner assert app_user.spawner.user_options == options assert not app_user.spawner._spawn_pending - status = io_loop.run_sync(app_user.spawner.poll) + status = yield app_user.spawner.poll() assert status is None assert spawner.server.base_url == ujoin(app.base_url, 'user/%s' % name) + '/' url = public_url(app, user) - r = requests.get(url) + r = yield async_requests.get(url) assert r.status_code == 200 assert r.text == spawner.server.base_url - r = requests.get(ujoin(url, 'args')) + r = yield async_requests.get(ujoin(url, 'args')) assert r.status_code == 200 argv = r.json() assert '--port' in ' '.join(argv) - r = requests.get(ujoin(url, 'env')) + r = yield async_requests.get(ujoin(url, 'env')) env = r.json() for expected in ['JUPYTERHUB_USER', 'JUPYTERHUB_BASE_URL', 'JUPYTERHUB_API_TOKEN']: assert expected in env if app.subdomain_host: assert env['JUPYTERHUB_HOST'] == app.subdomain_host - r = api_request(app, 'users', name, 'server', method='delete') + r = yield api_request(app, 'users', name, 'server', method='delete') assert r.status_code == 204 assert 'pid' not in user.orm_spawners[''].state - status = io_loop.run_sync(app_user.spawner.poll) + status = yield app_user.spawner.poll() assert status == 0 # check that we cleaned up after ourselves @@ -446,6 +444,7 @@ def test_spawn(app, io_loop): assert tokens == [] +@mark.gen_test def test_slow_spawn(app, io_loop, no_patience, request): patch = mock.patch.dict(app.tornado_settings, {'spawner_class': mocking.SlowSpawner}) patch.start() @@ -453,10 +452,10 @@ def test_slow_spawn(app, io_loop, no_patience, request): db = app.db name = 'zoe' user = add_user(db, app=app, name=name) - r = api_request(app, 'users', name, 'server', method='post') + r = yield api_request(app, 'users', name, 'server', method='post') r.raise_for_status() assert r.status_code == 202 - app_user = get_app_user(app, name) + app_user = app.users[name] assert app_user.spawner is not None assert app_user.spawner._spawn_pending assert not app_user.spawner._stop_pending @@ -466,9 +465,9 @@ def test_slow_spawn(app, io_loop, no_patience, request): while app_user.spawner._spawn_pending: yield gen.sleep(0.1) - io_loop.run_sync(wait_spawn) + yield wait_spawn() assert not app_user.spawner._spawn_pending - status = io_loop.run_sync(app_user.spawner.poll) + status = yield app_user.spawner.poll() assert status is None @gen.coroutine @@ -476,25 +475,26 @@ def test_slow_spawn(app, io_loop, no_patience, request): while app_user.spawner._stop_pending: yield gen.sleep(0.1) - r = api_request(app, 'users', name, 'server', method='delete') + r = yield api_request(app, 'users', name, 'server', method='delete') r.raise_for_status() assert r.status_code == 202 assert app_user.spawner is not None assert app_user.spawner._stop_pending - r = api_request(app, 'users', name, 'server', method='delete') + r = yield api_request(app, 'users', name, 'server', method='delete') r.raise_for_status() assert r.status_code == 202 assert app_user.spawner is not None assert app_user.spawner._stop_pending - io_loop.run_sync(wait_stop) + yield wait_stop() assert not app_user.spawner._stop_pending assert app_user.spawner is not None - r = api_request(app, 'users', name, 'server', method='delete') + r = yield api_request(app, 'users', name, 'server', method='delete') assert r.status_code == 400 +@mark.gen_test def test_never_spawn(app, io_loop, no_patience, request): patch = mock.patch.dict(app.tornado_settings, {'spawner_class': mocking.NeverSpawner}) patch.start() @@ -503,8 +503,8 @@ def test_never_spawn(app, io_loop, no_patience, request): db = app.db name = 'badger' user = add_user(db, app=app, name=name) - r = api_request(app, 'users', name, 'server', method='post') - app_user = get_app_user(app, name) + r = yield api_request(app, 'users', name, 'server', method='post') + app_user = app.users[name] assert app_user.spawner is not None assert app_user.spawner._spawn_pending @@ -513,38 +513,40 @@ def test_never_spawn(app, io_loop, no_patience, request): while app_user.spawner._spawn_pending: yield gen.sleep(0.1) - io_loop.run_sync(wait_pending) + yield wait_pending() assert not app_user.spawner._spawn_pending - status = io_loop.run_sync(app_user.spawner.poll) + status = yield app_user.spawner.poll() assert status is not None +@mark.gen_test def test_get_proxy(app, io_loop): - r = api_request(app, 'proxy') + r = yield api_request(app, 'proxy') r.raise_for_status() reply = r.json() assert list(reply.keys()) == ['/'] +@mark.gen_test def test_cookie(app): db = app.db name = 'patience' user = add_user(db, app=app, name=name) - r = api_request(app, 'users', name, 'server', method='post') + r = yield api_request(app, 'users', name, 'server', method='post') assert r.status_code == 201 assert 'pid' in user.orm_spawners[''].state - app_user = get_app_user(app, name) + app_user = app.users[name] - cookies = app.login_user(name) + cookies = yield app.login_user(name) cookie_name = app.hub.cookie_name # cookie jar gives '"cookie-value"', we want 'cookie-value' cookie = cookies[cookie_name][1:-1] - r = api_request(app, 'authorizations/cookie', + r = yield api_request(app, 'authorizations/cookie', cookie_name, "nothintoseehere", ) assert r.status_code == 404 - r = api_request(app, 'authorizations/cookie', + r = yield api_request(app, 'authorizations/cookie', cookie_name, quote(cookie, safe=''), ) r.raise_for_status() @@ -552,7 +554,7 @@ def test_cookie(app): assert reply['name'] == name # deprecated cookie in body: - r = api_request(app, 'authorizations/cookie', + r = yield api_request(app, 'authorizations/cookie', cookie_name, data=cookie, ) r.raise_for_status() @@ -560,18 +562,20 @@ def test_cookie(app): assert reply['name'] == name +@mark.gen_test def test_token(app): name = 'book' user = add_user(app.db, app=app, name=name) token = user.new_api_token() - r = api_request(app, 'authorizations/token', token) + r = yield api_request(app, 'authorizations/token', token) r.raise_for_status() user_model = r.json() assert user_model['name'] == name - r = api_request(app, 'authorizations/token', 'notauthorized') + r = yield api_request(app, 'authorizations/token', 'notauthorized') assert r.status_code == 404 +@mark.gen_test @mark.parametrize("headers, data, status", [ ({}, None, 200), ({'Authorization': ''}, None, 403), @@ -581,13 +585,13 @@ def test_get_new_token(app, headers, data, status): if data: data = json.dumps(data) # request a new token - r = api_request(app, 'authorizations', 'token', method='post', data=data, headers=headers) + r = yield api_request(app, 'authorizations', 'token', method='post', data=data, headers=headers) assert r.status_code == status if status != 200: return reply = r.json() assert 'token' in reply - r = api_request(app, 'authorizations', 'token', reply['token']) + r = yield api_request(app, 'authorizations', 'token', reply['token']) r.raise_for_status() assert 'name' in r.json() @@ -598,8 +602,9 @@ def test_get_new_token(app, headers, data, status): @mark.group +@mark.gen_test def test_groups_list(app): - r = api_request(app, 'groups') + r = yield api_request(app, 'groups') r.raise_for_status() reply = r.json() assert reply == [] @@ -609,7 +614,7 @@ def test_groups_list(app): app.db.add(group) app.db.commit() - r = api_request(app, 'groups') + r = yield api_request(app, 'groups') r.raise_for_status() reply = r.json() assert reply == [{ @@ -620,16 +625,17 @@ def test_groups_list(app): @mark.group +@mark.gen_test def test_group_get(app): group = orm.Group.find(app.db, name='alphaflight') user = add_user(app.db, app=app, name='sasquatch') group.users.append(user) app.db.commit() - r = api_request(app, 'groups/runaways') + r = yield api_request(app, 'groups/runaways') assert r.status_code == 404 - r = api_request(app, 'groups/alphaflight') + r = yield api_request(app, 'groups/alphaflight') r.raise_for_status() reply = r.json() assert reply == { @@ -640,18 +646,19 @@ def test_group_get(app): @mark.group +@mark.gen_test def test_group_create_delete(app): db = app.db - r = api_request(app, 'groups/runaways', method='delete') + r = yield api_request(app, 'groups/runaways', method='delete') assert r.status_code == 404 - r = api_request(app, 'groups/new', method='post', + r = yield api_request(app, 'groups/new', method='post', data=json.dumps({'users': ['doesntexist']}), ) assert r.status_code == 400 assert orm.Group.find(db, name='new') is None - r = api_request(app, 'groups/omegaflight', method='post', + r = yield api_request(app, 'groups/omegaflight', method='post', data=json.dumps({'users': ['sasquatch']}), ) r.raise_for_status() @@ -662,29 +669,30 @@ def test_group_create_delete(app): assert sasquatch in omegaflight.users # create duplicate raises 400 - r = api_request(app, 'groups/omegaflight', method='post') + r = yield api_request(app, 'groups/omegaflight', method='post') assert r.status_code == 400 - r = api_request(app, 'groups/omegaflight', method='delete') + r = yield api_request(app, 'groups/omegaflight', method='delete') assert r.status_code == 204 assert omegaflight not in sasquatch.groups assert orm.Group.find(db, name='omegaflight') is None # delete nonexistent gives 404 - r = api_request(app, 'groups/omegaflight', method='delete') + r = yield api_request(app, 'groups/omegaflight', method='delete') assert r.status_code == 404 @mark.group +@mark.gen_test def test_group_add_users(app): db = app.db # must specify users - r = api_request(app, 'groups/alphaflight/users', method='post', data='{}') + r = yield api_request(app, 'groups/alphaflight/users', method='post', data='{}') assert r.status_code == 400 names = ['aurora', 'guardian', 'northstar', 'sasquatch', 'shaman', 'snowbird'] users = [ find_user(db, name=name) or add_user(db, app=app, name=name) for name in names ] - r = api_request(app, 'groups/alphaflight/users', method='post', data=json.dumps({ + r = yield api_request(app, 'groups/alphaflight/users', method='post', data=json.dumps({ 'users': names, })) r.raise_for_status() @@ -698,15 +706,16 @@ def test_group_add_users(app): @mark.group +@mark.gen_test def test_group_delete_users(app): db = app.db # must specify users - r = api_request(app, 'groups/alphaflight/users', method='delete', data='{}') + r = yield api_request(app, 'groups/alphaflight/users', method='delete', data='{}') assert r.status_code == 400 names = ['aurora', 'guardian', 'northstar', 'sasquatch', 'shaman', 'snowbird'] users = [ find_user(db, name=name) for name in names ] - r = api_request(app, 'groups/alphaflight/users', method='delete', data=json.dumps({ + r = yield api_request(app, 'groups/alphaflight/users', method='delete', data=json.dumps({ 'users': names[:2], })) r.raise_for_status() @@ -726,10 +735,11 @@ def test_group_delete_users(app): @mark.services +@mark.gen_test def test_get_services(app, mockservice_url): mockservice = mockservice_url db = app.db - r = api_request(app, 'services') + r = yield api_request(app, 'services') r.raise_for_status() assert r.status_code == 200 @@ -745,17 +755,18 @@ def test_get_services(app, mockservice_url): } } - r = api_request(app, 'services', + r = yield api_request(app, 'services', headers=auth_header(db, 'user'), ) assert r.status_code == 403 @mark.services +@mark.gen_test def test_get_service(app, mockservice_url): mockservice = mockservice_url db = app.db - r = api_request(app, 'services/%s' % mockservice.name) + r = yield api_request(app, 'services/%s' % mockservice.name) r.raise_for_status() assert r.status_code == 200 @@ -769,22 +780,23 @@ def test_get_service(app, mockservice_url): 'url': mockservice.url, } - r = api_request(app, 'services/%s' % mockservice.name, + r = yield api_request(app, 'services/%s' % mockservice.name, headers={ 'Authorization': 'token %s' % mockservice.api_token } ) r.raise_for_status() - r = api_request(app, 'services/%s' % mockservice.name, + r = yield api_request(app, 'services/%s' % mockservice.name, headers=auth_header(db, 'user'), ) assert r.status_code == 403 +@mark.gen_test def test_root_api(app): base_url = app.hub.url url = ujoin(base_url, 'api') - r = requests.get(url) + r = yield async_requests.get(url) r.raise_for_status() expected = { 'version': jupyterhub.__version__ @@ -792,8 +804,9 @@ def test_root_api(app): assert r.json() == expected +@mark.gen_test def test_info(app): - r = api_request(app, 'info') + r = yield api_request(app, 'info') r.raise_for_status() data = r.json() assert data['version'] == jupyterhub.__version__ @@ -821,14 +834,16 @@ def test_info(app): # ----------------- +@mark.gen_test def test_options(app): - r = api_request(app, 'users', method='options') + r = yield api_request(app, 'users', method='options') r.raise_for_status() assert 'Access-Control-Allow-Headers' in r.headers +@mark.gen_test def test_bad_json_body(app): - r = api_request(app, 'users', method='post', data='notjson') + r = yield api_request(app, 'users', method='post', data='notjson') assert r.status_code == 400 @@ -838,14 +853,24 @@ def test_bad_json_body(app): def test_shutdown(app): - r = api_request(app, 'shutdown', method='post', - data=json.dumps({'servers': True, 'proxy': True,}), - ) + loop = app.io_loop + + # have to do things a little funky since we are going to stop the loop, + # which makes gen_test unhappy. So we run the loop ourselves. + + @gen.coroutine + def shutdown(): + r = yield api_request(app, 'shutdown', method='post', + data=json.dumps({'servers': True, 'proxy': True,}), + ) + return r + + real_stop = loop.stop + def stop(): + stop.called = True + loop.call_later(1, real_stop) + with mock.patch.object(loop, 'stop', stop): + r = loop.run_sync(shutdown, timeout=5) 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 + assert stop.called diff --git a/jupyterhub/tests/test_utils.py b/jupyterhub/tests/test_utils.py deleted file mode 100644 index e69de29b..00000000 diff --git a/jupyterhub/tests/utils.py b/jupyterhub/tests/utils.py new file mode 100644 index 00000000..58d9222a --- /dev/null +++ b/jupyterhub/tests/utils.py @@ -0,0 +1,18 @@ +from concurrent.futures import ThreadPoolExecutor +import requests + +class _AsyncRequests: + """Wrapper around requests to return a Future from request methods + + A single thread is allocated to avoid blocking the IOLoop thread. + """ + def __init__(self): + self.executor = ThreadPoolExecutor(1) + + def __getattr__(self, name): + requests_method = getattr(requests, name) + return lambda *args, **kwargs: self.executor.submit(requests_method, *args, **kwargs) + +# async_requests.get = requests.get returning a Future, etc. +async_requests = _AsyncRequests() + From d559cad0429f1da1f94b50d3cb5473012c585552 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 27 Jul 2017 10:38:03 +0200 Subject: [PATCH 3/9] test_pages without threads --- jupyterhub/tests/test_pages.py | 158 +++++++++++++++++++-------------- 1 file changed, 93 insertions(+), 65 deletions(-) diff --git a/jupyterhub/tests/test_pages.py b/jupyterhub/tests/test_pages.py index 243ee495..6f415d30 100644 --- a/jupyterhub/tests/test_pages.py +++ b/jupyterhub/tests/test_pages.py @@ -2,7 +2,6 @@ from urllib.parse import urlencode, urlparse -import requests from tornado import gen from ..handlers import BaseHandler @@ -11,8 +10,11 @@ from .. import orm from ..auth import Authenticator import mock +import pytest + from .mocking import FormSpawner, public_url, public_host from .test_api import api_request +from .utils import async_requests def get_page(path, app, hub=True, **kw): if hub: @@ -20,113 +22,128 @@ def get_page(path, app, hub=True, **kw): else: prefix = app.base_url base_url = ujoin(public_host(app), prefix) - print(base_url) - return requests.get(ujoin(base_url, path), **kw) + return async_requests.get(ujoin(base_url, path), **kw) -def test_root_no_auth(app, io_loop): - print(app.hub.is_up()) - routes = io_loop.run_sync(app.proxy.get_all_routes) - print(routes) - print(app.hub) + +@pytest.mark.gen_test +def test_root_no_auth(app): url = ujoin(public_host(app), app.hub.base_url) - print(url) - r = requests.get(url) + r = yield async_requests.get(url) r.raise_for_status() assert r.url == ujoin(url, 'login') + +@pytest.mark.gen_test def test_root_auth(app): - cookies = app.login_user('river') - r = requests.get(public_url(app), cookies=cookies) + cookies = yield app.login_user('river') + r = yield async_requests.get(public_url(app), cookies=cookies) r.raise_for_status() assert r.url == public_url(app, app.users['river']) + +@pytest.mark.gen_test def test_root_redirect(app): name = 'wash' - cookies = app.login_user(name) + cookies = yield app.login_user(name) next_url = ujoin(app.base_url, 'user/other/test.ipynb') url = '/?' + urlencode({'next': next_url}) - r = get_page(url, app, cookies=cookies) + r = yield get_page(url, app, cookies=cookies) r.raise_for_status() path = urlparse(r.url).path assert path == ujoin(app.base_url, 'user/%s/test.ipynb' % name) + +@pytest.mark.gen_test def test_home_no_auth(app): - r = get_page('home', app, allow_redirects=False) + r = yield get_page('home', app, allow_redirects=False) r.raise_for_status() assert r.status_code == 302 assert '/hub/login' in r.headers['Location'] + +@pytest.mark.gen_test def test_home_auth(app): - cookies = app.login_user('river') - r = get_page('home', app, cookies=cookies) + cookies = yield app.login_user('river') + r = yield get_page('home', app, cookies=cookies) r.raise_for_status() assert r.url.endswith('home') + +@pytest.mark.gen_test def test_admin_no_auth(app): - r = get_page('admin', app) + r = yield get_page('admin', app) assert r.status_code == 403 + +@pytest.mark.gen_test def test_admin_not_admin(app): - cookies = app.login_user('wash') - r = get_page('admin', app, cookies=cookies) + cookies = yield app.login_user('wash') + r = yield get_page('admin', app, cookies=cookies) assert r.status_code == 403 + +@pytest.mark.gen_test def test_admin(app): - cookies = app.login_user('admin') - r = get_page('admin', app, cookies=cookies) + cookies = yield app.login_user('admin') + r = yield get_page('admin', app, cookies=cookies) r.raise_for_status() assert r.url.endswith('/admin') -def test_spawn_redirect(app, io_loop): +@pytest.mark.gen_test +def test_spawn_redirect(app): name = 'wash' - cookies = app.login_user(name) + cookies = yield app.login_user(name) u = app.users[orm.User.find(app.db, name)] # ensure wash's server isn't running: - r = api_request(app, 'users', name, 'server', method='delete', cookies=cookies) + r = yield api_request(app, 'users', name, 'server', method='delete', cookies=cookies) r.raise_for_status() - status = io_loop.run_sync(u.spawner.poll) + status = yield u.spawner.poll() assert status is not None # test spawn page when no server is running - r = get_page('spawn', app, cookies=cookies) + r = yield get_page('spawn', app, cookies=cookies) r.raise_for_status() print(urlparse(r.url)) path = urlparse(r.url).path assert path == ujoin(app.base_url, 'user/%s/' % name) # should have started server - status = io_loop.run_sync(u.spawner.poll) + status = yield u.spawner.poll() assert status is None # test spawn page when server is already running (just redirect) - r = get_page('spawn', app, cookies=cookies) + r = yield get_page('spawn', app, cookies=cookies) r.raise_for_status() print(urlparse(r.url)) path = urlparse(r.url).path assert path == ujoin(app.base_url, '/user/%s/' % name) + +@pytest.mark.gen_test def test_spawn_page(app): with mock.patch.dict(app.users.settings, {'spawner_class': FormSpawner}): - cookies = app.login_user('jones') - r = get_page('spawn', app, cookies=cookies) + cookies = yield app.login_user('jones') + r = yield get_page('spawn', app, cookies=cookies) assert r.url.endswith('/spawn') assert FormSpawner.options_form in r.text - r = get_page('spawn?next=foo', app, cookies=cookies) + r = yield get_page('spawn?next=foo', app, cookies=cookies) assert r.url.endswith('/spawn?next=foo') assert FormSpawner.options_form in r.text -def test_spawn_form(app, io_loop): + +@pytest.mark.gen_test +def test_spawn_form(app): with mock.patch.dict(app.users.settings, {'spawner_class': FormSpawner}): base_url = ujoin(public_host(app), app.hub.base_url) - cookies = app.login_user('jones') + cookies = yield app.login_user('jones') orm_u = orm.User.find(app.db, 'jones') u = app.users[orm_u] - io_loop.run_sync(u.stop) + yield u.stop() - r = requests.post(ujoin(base_url, 'spawn?next=/user/jones/tree'), cookies=cookies, data={ + r = yield async_requests.post(ujoin(base_url, 'spawn?next=/user/jones/tree'), cookies=cookies, data={ 'bounds': ['-1', '1'], 'energy': '511keV', }) @@ -140,15 +157,17 @@ def test_spawn_form(app, io_loop): 'notspecified': 5, } -def test_spawn_form_with_file(app, io_loop): + +@pytest.mark.gen_test +def test_spawn_form_with_file(app): with mock.patch.dict(app.users.settings, {'spawner_class': FormSpawner}): base_url = ujoin(public_host(app), app.hub.base_url) - cookies = app.login_user('jones') + cookies = yield app.login_user('jones') orm_u = orm.User.find(app.db, 'jones') u = app.users[orm_u] - io_loop.run_sync(u.stop) + yield u.stop() - r = requests.post(ujoin(base_url, 'spawn'), + r = yield async_requests.post(ujoin(base_url, 'spawn'), cookies=cookies, data={ 'bounds': ['-1', '1'], @@ -167,11 +186,12 @@ def test_spawn_form_with_file(app, io_loop): } +@pytest.mark.gen_test def test_user_redirect(app): name = 'wash' - cookies = app.login_user(name) + cookies = yield app.login_user(name) - r = get_page('/user-redirect/tree/top/', app) + r = yield get_page('/user-redirect/tree/top/', app) r.raise_for_status() print(urlparse(r.url)) path = urlparse(r.url).path @@ -181,31 +201,32 @@ def test_user_redirect(app): 'next': ujoin(app.hub.base_url, '/user-redirect/tree/top/') }) - r = get_page('/user-redirect/notebooks/test.ipynb', app, cookies=cookies) + r = yield get_page('/user-redirect/notebooks/test.ipynb', app, cookies=cookies) r.raise_for_status() print(urlparse(r.url)) path = urlparse(r.url).path assert path == ujoin(app.base_url, '/user/%s/notebooks/test.ipynb' % name) +@pytest.mark.gen_test def test_user_redirect_deprecated(app): """redirecting from /user/someonelse/ URLs (deprecated)""" name = 'wash' - cookies = app.login_user(name) + cookies = yield app.login_user(name) - r = get_page('/user/baduser', app, cookies=cookies, hub=False) + r = yield get_page('/user/baduser', app, cookies=cookies, hub=False) r.raise_for_status() print(urlparse(r.url)) path = urlparse(r.url).path assert path == ujoin(app.base_url, '/user/%s' % name) - r = get_page('/user/baduser/test.ipynb', app, cookies=cookies, hub=False) + r = yield get_page('/user/baduser/test.ipynb', app, cookies=cookies, hub=False) r.raise_for_status() print(urlparse(r.url)) path = urlparse(r.url).path assert path == ujoin(app.base_url, '/user/%s/test.ipynb' % name) - r = get_page('/user/baduser/test.ipynb', app, hub=False) + r = yield get_page('/user/baduser/test.ipynb', app, hub=False) r.raise_for_status() print(urlparse(r.url)) path = urlparse(r.url).path @@ -216,10 +237,11 @@ def test_user_redirect_deprecated(app): }) +@pytest.mark.gen_test def test_login_fail(app): name = 'wash' base_url = public_url(app) - r = requests.post(base_url + 'hub/login', + r = yield async_requests.post(base_url + 'hub/login', data={ 'username': name, 'password': 'wrong', @@ -229,6 +251,7 @@ def test_login_fail(app): assert not r.cookies +@pytest.mark.gen_test def test_login_strip(app): """Test that login form doesn't strip whitespace from passwords""" form_data = { @@ -242,7 +265,7 @@ def test_login_strip(app): called_with.append(data) with mock.patch.object(app.authenticator, 'authenticate', mock_authenticate): - r = requests.post(base_url + 'hub/login', + yield async_requests.post(base_url + 'hub/login', data=form_data, allow_redirects=False, ) @@ -250,31 +273,33 @@ def test_login_strip(app): assert called_with == [form_data] -def test_login_redirect(app, io_loop): - cookies = app.login_user('river') +@pytest.mark.gen_test +def test_login_redirect(app): + cookies = yield app.login_user('river') user = app.users['river'] # no next_url, server running - io_loop.run_sync(user.spawn) - r = get_page('login', app, cookies=cookies, allow_redirects=False) + yield user.spawn() + r = yield get_page('login', app, cookies=cookies, allow_redirects=False) r.raise_for_status() assert r.status_code == 302 assert '/user/river' in r.headers['Location'] # no next_url, server not running - io_loop.run_sync(user.stop) - r = get_page('login', app, cookies=cookies, allow_redirects=False) + yield user.stop() + r = yield get_page('login', app, cookies=cookies, allow_redirects=False) r.raise_for_status() assert r.status_code == 302 assert '/hub/' in r.headers['Location'] # next URL given, use it - r = get_page('login?next=/hub/admin', app, cookies=cookies, allow_redirects=False) + r = yield get_page('login?next=/hub/admin', app, cookies=cookies, allow_redirects=False) r.raise_for_status() assert r.status_code == 302 assert r.headers['Location'].endswith('/hub/admin') -def test_auto_login(app, io_loop, request): +@pytest.mark.gen_test +def test_auto_login(app, request): class DummyLoginHandler(BaseHandler): def get(self): self.write('ok!') @@ -283,7 +308,7 @@ def test_auto_login(app, io_loop, request): (ujoin(app.hub.base_url, 'dummy'), DummyLoginHandler), ]) # no auto_login: end up at /hub/login - r = requests.get(base_url) + r = yield async_requests.get(base_url) assert r.url == public_url(app, path='hub/login') # enable auto_login: redirect from /hub/login to /hub/dummy authenticator = Authenticator(auto_login=True) @@ -292,38 +317,41 @@ def test_auto_login(app, io_loop, request): with mock.patch.dict(app.tornado_application.settings, { 'authenticator': authenticator, }): - r = requests.get(base_url) + r = yield async_requests.get(base_url) assert r.url == public_url(app, path='hub/dummy') +@pytest.mark.gen_test def test_logout(app): name = 'wash' - cookies = app.login_user(name) - r = requests.get(public_host(app) + app.tornado_settings['logout_url'], cookies=cookies) + cookies = yield app.login_user(name) + r = yield async_requests.get(public_host(app) + app.tornado_settings['logout_url'], cookies=cookies) r.raise_for_status() login_url = public_host(app) + app.tornado_settings['login_url'] assert r.url == login_url assert r.cookies == {} +@pytest.mark.gen_test def test_login_no_whitelist_adds_user(app): auth = app.authenticator mock_add_user = mock.Mock() with mock.patch.object(auth, 'add_user', mock_add_user): - cookies = app.login_user('jubal') + cookies = yield app.login_user('jubal') user = app.users['jubal'] assert mock_add_user.mock_calls == [mock.call(user)] +@pytest.mark.gen_test def test_static_files(app): base_url = ujoin(public_host(app), app.hub.base_url) - r = requests.get(ujoin(base_url, 'logo')) + r = yield async_requests.get(ujoin(base_url, 'logo')) r.raise_for_status() assert r.headers['content-type'] == 'image/png' - r = requests.get(ujoin(base_url, 'static', 'images', 'jupyter.png')) + r = yield async_requests.get(ujoin(base_url, 'static', 'images', 'jupyter.png')) r.raise_for_status() assert r.headers['content-type'] == 'image/png' - r = requests.get(ujoin(base_url, 'static', 'css', 'style.min.css')) + r = yield async_requests.get(ujoin(base_url, 'static', 'css', 'style.min.css')) r.raise_for_status() assert r.headers['content-type'] == 'text/css' From 91d042f6f3eae778007eac91bc438d4a4da80858 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 27 Jul 2017 11:10:03 +0200 Subject: [PATCH 4/9] get the rest of tests running without threads all tests pass! --- jupyterhub/tests/test_named_servers.py | 23 +++++---- jupyterhub/tests/test_proxy.py | 69 ++++++++++++++------------ jupyterhub/tests/test_services.py | 23 ++++----- jupyterhub/tests/test_services_auth.py | 34 ++++++++----- jupyterhub/tests/test_singleuser.py | 34 +++++++------ jupyterhub/tests/test_spawner.py | 4 +- 6 files changed, 102 insertions(+), 85 deletions(-) diff --git a/jupyterhub/tests/test_named_servers.py b/jupyterhub/tests/test_named_servers.py index 22f156b8..5115766d 100644 --- a/jupyterhub/tests/test_named_servers.py +++ b/jupyterhub/tests/test_named_servers.py @@ -1,11 +1,11 @@ """Tests for named servers""" import pytest -import requests from ..utils import url_path_join -from .test_api import api_request, add_user, find_user +from .test_api import api_request, add_user from .mocking import public_url +from .utils import async_requests @pytest.fixture def named_servers(app): @@ -17,19 +17,20 @@ def named_servers(app): app.tornado_application.settings[key] = app.tornado_settings[key] = False +@pytest.mark.gen_test def test_create_named_server(app, named_servers): username = 'walnut' user = add_user(app.db, app, name=username) # assert user.allow_named_servers == True - cookies = app.login_user(username) + cookies = yield app.login_user(username) servername = 'trevor' - r = api_request(app, 'users', username, 'servers', servername, method='post') + r = yield api_request(app, 'users', username, 'servers', servername, method='post') r.raise_for_status() assert r.status_code == 201 assert r.text == '' url = url_path_join(public_url(app, user), servername, 'env') - r = requests.get(url, cookies=cookies) + r = yield async_requests.get(url, cookies=cookies) r.raise_for_status() assert r.url == url env = r.json() @@ -38,21 +39,22 @@ def test_create_named_server(app, named_servers): assert prefix.endswith('/user/%s/%s/' % (username, servername)) +@pytest.mark.gen_test def test_delete_named_server(app, named_servers): username = 'donaar' user = add_user(app.db, app, name=username) assert user.allow_named_servers cookies = app.login_user(username) servername = 'splugoth' - r = api_request(app, 'users', username, 'servers', servername, method='post') + r = yield api_request(app, 'users', username, 'servers', servername, method='post') r.raise_for_status() assert r.status_code == 201 - r = api_request(app, 'users', username, 'servers', servername, method='delete') + r = yield api_request(app, 'users', username, 'servers', servername, method='delete') r.raise_for_status() assert r.status_code == 204 - r = api_request(app, 'users', username) + r = yield api_request(app, 'users', username) r.raise_for_status() user_model = r.json() @@ -73,10 +75,11 @@ def test_delete_named_server(app, named_servers): }, } +@pytest.mark.gen_test def test_named_server_disabled(app): username = 'user' servername = 'okay' - r = api_request(app, 'users', username, 'servers', servername, method='post') + r = yield api_request(app, 'users', username, 'servers', servername, method='post') assert r.status_code == 400 - r = api_request(app, 'users', username, 'servers', servername, method='delete') + r = yield api_request(app, 'users', username, 'servers', servername, method='delete') assert r.status_code == 400 diff --git a/jupyterhub/tests/test_proxy.py b/jupyterhub/tests/test_proxy.py index 0d72e02b..0f6334db 100644 --- a/jupyterhub/tests/test_proxy.py +++ b/jupyterhub/tests/test_proxy.py @@ -16,7 +16,8 @@ from .test_api import api_request from ..utils import wait_for_http_server, url_path_join as ujoin -def test_external_proxy(request, io_loop): +@pytest.mark.gen_test +def test_external_proxy(request): auth_token = 'secret!' proxy_ip = '127.0.0.1' @@ -30,7 +31,7 @@ def test_external_proxy(request, io_loop): def fin(): MockHub.clear_instance() - app.stop() + app.http_server.stop() request.addfinalizer(fin) @@ -52,28 +53,32 @@ def test_external_proxy(request, io_loop): def _cleanup_proxy(): if proxy.poll() is None: + print("Terminating proxy") proxy.terminate() + else: + print("Not stopping proxy", proxy) request.addfinalizer(_cleanup_proxy) def wait_for_proxy(): - io_loop.run_sync(lambda: wait_for_http_server('http://%s:%i' % (proxy_ip, proxy_port))) - wait_for_proxy() + return wait_for_http_server('http://%s:%i' % (proxy_ip, proxy_port)) + yield wait_for_proxy() - app.start([]) + yield app.initialize([]) + yield app.start() assert app.proxy.proxy_process is None # test if api service has a root route '/' - routes = io_loop.run_sync(app.proxy.get_all_routes) + routes = yield app.proxy.get_all_routes() assert list(routes.keys()) == ['/'] # add user to the db and start a single user server name = 'river' - r = api_request(app, 'users', name, method='post') + r = yield api_request(app, 'users', name, method='post') r.raise_for_status() - r = api_request(app, 'users', name, 'server', method='post') + r = yield api_request(app, 'users', name, 'server', method='post') r.raise_for_status() - routes = io_loop.run_sync(app.proxy.get_all_routes) + routes = yield app.proxy.get_all_routes() # sets the desired path result user_path = ujoin(app.base_url, 'user/river') + '/' print(app.base_url, user_path) @@ -86,18 +91,18 @@ def test_external_proxy(request, io_loop): # teardown the proxy and start a new one in the same place proxy.terminate() proxy = Popen(cmd, env=env) - wait_for_proxy() + yield wait_for_proxy() - routes = io_loop.run_sync(app.proxy.get_all_routes) + routes = yield app.proxy.get_all_routes() assert list(routes.keys()) == [] # poke the server to update the proxy - r = api_request(app, 'proxy', method='post') + r = yield api_request(app, 'proxy', method='post') r.raise_for_status() # check that the routes are correct - routes = io_loop.run_sync(app.proxy.get_all_routes) + routes = yield app.proxy.get_all_routes() assert sorted(routes.keys()) == ['/', user_spec] # teardown the proxy, and start a new one with different auth and port @@ -115,41 +120,35 @@ def test_external_proxy(request, io_loop): if app.subdomain_host: cmd.append('--host-routing') proxy = Popen(cmd, env=env) - wait_for_proxy() + yield wait_for_proxy() # tell the hub where the new proxy is new_api_url = 'http://{}:{}'.format(proxy_ip, proxy_port) - r = api_request(app, 'proxy', method='patch', data=json.dumps({ + r = yield api_request(app, 'proxy', method='patch', data=json.dumps({ 'api_url': new_api_url, 'auth_token': new_auth_token, })) r.raise_for_status() assert app.proxy.api_url == new_api_url - # get updated auth token from main thread - def get_app_proxy_token(): - q = Queue() - app.io_loop.add_callback(lambda: q.put(app.proxy.auth_token)) - return q.get(timeout=2) - - assert get_app_proxy_token() == new_auth_token - app.proxy.auth_token = new_auth_token + assert app.proxy.auth_token == new_auth_token # check that the routes are correct - routes = io_loop.run_sync(app.proxy.get_all_routes) + routes = yield app.proxy.get_all_routes() assert sorted(routes.keys()) == ['/', user_spec] +@pytest.mark.gen_test @pytest.mark.parametrize("username, endpoints", [ ('zoe', ['users/zoe', 'users/zoe/server']), ('50fia', ['users/50fia', 'users/50fia/server']), ('秀樹', ['users/秀樹', 'users/秀樹/server']), ]) -def test_check_routes(app, io_loop, username, endpoints): +def test_check_routes(app, username, endpoints): proxy = app.proxy for endpoint in endpoints: - r = api_request(app, endpoint, method='post') + r = yield api_request(app, endpoint, method='post') r.raise_for_status() test_user = orm.User.find(app.db, username) @@ -157,18 +156,21 @@ def test_check_routes(app, io_loop, username, endpoints): # check a valid route exists for user test_user = app.users[username] - before = sorted(io_loop.run_sync(app.proxy.get_all_routes)) + routes = yield app.proxy.get_all_routes() + before = sorted(routes) assert test_user.proxy_spec() in before # check if a route is removed when user deleted - io_loop.run_sync(lambda: app.proxy.check_routes(app.users, app._service_map)) - io_loop.run_sync(lambda: proxy.delete_user(test_user)) - during = sorted(io_loop.run_sync(app.proxy.get_all_routes)) + yield app.proxy.check_routes(app.users, app._service_map) + yield proxy.delete_user(test_user) + routes = yield app.proxy.get_all_routes() + during = sorted(routes) assert test_user.proxy_spec() not in during # check if a route exists for user - io_loop.run_sync(lambda: app.proxy.check_routes(app.users, app._service_map)) - after = sorted(io_loop.run_sync(app.proxy.get_all_routes)) + yield app.proxy.check_routes(app.users, app._service_map) + routes = yield app.proxy.get_all_routes() + after = sorted(routes) assert test_user.proxy_spec() in after # check that before and after state are the same @@ -222,7 +224,8 @@ def test_add_get_delete(app, routespec): assert route is None +@pytest.mark.gen_test @pytest.mark.parametrize("test_data", [None, 'notjson', json.dumps([])]) def test_proxy_patch_bad_request_data(app, test_data): - r = api_request(app, 'proxy', method='patch', data=test_data) + r = yield api_request(app, 'proxy', method='patch', data=test_data) assert r.status_code == 400 diff --git a/jupyterhub/tests/test_services.py b/jupyterhub/tests/test_services.py index f039c0df..5e56977f 100644 --- a/jupyterhub/tests/test_services.py +++ b/jupyterhub/tests/test_services.py @@ -8,12 +8,14 @@ import sys from threading import Event import time +import pytest import requests from tornado import gen from tornado.ioloop import IOLoop from .mocking import public_url from ..utils import url_path_join, wait_for_http_server, random_port +from .utils import async_requests mockservice_path = os.path.dirname(os.path.abspath(__file__)) mockservice_py = os.path.join(mockservice_path, 'mockservice.py') @@ -38,6 +40,7 @@ def external_service(app, name='mockservice'): proc.terminate() +@pytest.mark.gen_test def test_managed_service(mockservice): service = mockservice proc = service.proc @@ -55,18 +58,19 @@ def test_managed_service(mockservice): if service.proc is not proc: break else: - time.sleep(0.2) + yield gen.sleep(0.2) assert service.proc.pid != first_pid assert service.proc.poll() is None -def test_proxy_service(app, mockservice_url, io_loop): +@pytest.mark.gen_test +def test_proxy_service(app, mockservice_url): service = mockservice_url name = service.name - io_loop.run_sync(app.proxy.get_all_routes) + yield app.proxy.get_all_routes() url = public_url(app, service) + '/foo' - r = requests.get(url, allow_redirects=False) + r = yield async_requests.get(url, allow_redirects=False) path = '/services/{}/foo'.format(name) r.raise_for_status() @@ -74,6 +78,7 @@ def test_proxy_service(app, mockservice_url, io_loop): assert r.text.endswith(path) +@pytest.mark.gen_test def test_external_service(app): name = 'external' with external_service(app, name=name) as env: @@ -85,17 +90,11 @@ def test_external_service(app): }] app.init_services() app.init_api_tokens() - evt = Event() + yield app.proxy.add_all_services(app._service_map) - @gen.coroutine - def add_services(): - yield app.proxy.add_all_services(app._service_map) - evt.set() - app.io_loop.add_callback(add_services) - assert evt.wait(10) service = app._service_map[name] url = public_url(app, service) + '/api/users' - r = requests.get(url, allow_redirects=False) + r = yield async_requests.get(url, allow_redirects=False) r.raise_for_status() assert r.status_code == 200 resp = r.json() diff --git a/jupyterhub/tests/test_services_auth.py b/jupyterhub/tests/test_services_auth.py index da5c62e6..4d759aa8 100644 --- a/jupyterhub/tests/test_services_auth.py +++ b/jupyterhub/tests/test_services_auth.py @@ -8,6 +8,7 @@ import time from unittest import mock from urllib.parse import urlparse +import pytest from pytest import raises import requests import requests_mock @@ -20,6 +21,7 @@ from ..services.auth import _ExpiringDict, HubAuth, HubAuthenticated from ..utils import url_path_join from .mocking import public_url, public_host from .test_api import add_user +from .utils import async_requests # mock for sending monotonic counter way into the future monotonic_future = mock.patch('time.monotonic', lambda : sys.maxsize) @@ -215,10 +217,11 @@ def test_hub_authenticated(request): assert r.status_code == 403 +@pytest.mark.gen_test def test_hubauth_cookie(app, mockservice_url): """Test HubAuthenticated service with user cookies""" - cookies = app.login_user('badger') - r = requests.get(public_url(app, mockservice_url) + '/whoami/', cookies=cookies) + cookies = yield app.login_user('badger') + r = yield async_requests.get(public_url(app, mockservice_url) + '/whoami/', cookies=cookies) r.raise_for_status() print(r.text) reply = r.json() @@ -229,6 +232,7 @@ def test_hubauth_cookie(app, mockservice_url): } +@pytest.mark.gen_test def test_hubauth_token(app, mockservice_url): """Test HubAuthenticated service with user API tokens""" u = add_user(app.db, name='river') @@ -236,7 +240,7 @@ def test_hubauth_token(app, mockservice_url): app.db.commit() # token in Authorization header - r = requests.get(public_url(app, mockservice_url) + '/whoami/', + r = yield async_requests.get(public_url(app, mockservice_url) + '/whoami/', headers={ 'Authorization': 'token %s' % token, }) @@ -248,7 +252,7 @@ def test_hubauth_token(app, mockservice_url): } # token in ?token parameter - r = requests.get(public_url(app, mockservice_url) + '/whoami/?token=%s' % token) + r = yield async_requests.get(public_url(app, mockservice_url) + '/whoami/?token=%s' % token) r.raise_for_status() reply = r.json() sub_reply = { key: reply.get(key, 'missing') for key in ['name', 'admin']} @@ -257,7 +261,7 @@ def test_hubauth_token(app, mockservice_url): 'admin': False, } - r = requests.get(public_url(app, mockservice_url) + '/whoami/?token=no-such-token', + r = yield async_requests.get(public_url(app, mockservice_url) + '/whoami/?token=no-such-token', allow_redirects=False, ) assert r.status_code == 302 @@ -267,16 +271,17 @@ def test_hubauth_token(app, mockservice_url): assert path.endswith('/hub/login') -def test_hubauth_service_token(app, mockservice_url, io_loop): +@pytest.mark.gen_test +def test_hubauth_service_token(app, mockservice_url): """Test HubAuthenticated service with service API tokens""" token = hexlify(os.urandom(5)).decode('utf8') name = 'test-api-service' app.service_tokens[token] = name - io_loop.run_sync(app.init_api_tokens) + yield app.init_api_tokens() # token in Authorization header - r = requests.get(public_url(app, mockservice_url) + '/whoami/', + r = yield async_requests.get(public_url(app, mockservice_url) + '/whoami/', headers={ 'Authorization': 'token %s' % token, }) @@ -289,7 +294,7 @@ def test_hubauth_service_token(app, mockservice_url, io_loop): } # token in ?token parameter - r = requests.get(public_url(app, mockservice_url) + '/whoami/?token=%s' % token) + r = yield async_requests.get(public_url(app, mockservice_url) + '/whoami/?token=%s' % token) r.raise_for_status() reply = r.json() assert reply == { @@ -298,7 +303,7 @@ def test_hubauth_service_token(app, mockservice_url, io_loop): 'admin': False, } - r = requests.get(public_url(app, mockservice_url) + '/whoami/?token=no-such-token', + r = yield async_requests.get(public_url(app, mockservice_url) + '/whoami/?token=no-such-token', allow_redirects=False, ) assert r.status_code == 302 @@ -308,16 +313,19 @@ def test_hubauth_service_token(app, mockservice_url, io_loop): assert path.endswith('/hub/login') +@pytest.mark.gen_test def test_oauth_service(app, mockservice_url): url = url_path_join(public_url(app, mockservice_url) + 'owhoami/') # first request is only going to set login cookie # FIXME: redirect to originating URL (OAuth loses this info) s = requests.Session() - s.cookies = app.login_user('link') - r = s.get(url) + s.cookies = yield app.login_user('link') + # run session.get in async_requests thread + s_get = lambda *args, **kwargs: async_requests.executor.submit(s.get, *args, **kwargs) + r = yield s_get(url) r.raise_for_status() # second request should be authenticated - r = s.get(url, allow_redirects=False) + r = yield s_get(url, allow_redirects=False) r.raise_for_status() assert r.status_code == 200 reply = r.json() diff --git a/jupyterhub/tests/test_singleuser.py b/jupyterhub/tests/test_singleuser.py index 3edc55d9..a983b741 100644 --- a/jupyterhub/tests/test_singleuser.py +++ b/jupyterhub/tests/test_singleuser.py @@ -3,68 +3,72 @@ from subprocess import check_output import sys -import requests +import pytest import jupyterhub from .mocking import StubSingleUserSpawner, public_url from ..utils import url_path_join +from .utils import async_requests -def test_singleuser_auth(app, io_loop): + +@pytest.mark.gen_test +def test_singleuser_auth(app): # use StubSingleUserSpawner to launch a single-user app in a thread app.spawner_class = StubSingleUserSpawner app.tornado_settings['spawner_class'] = StubSingleUserSpawner # login, start the server - cookies = app.login_user('nandy') + cookies = yield app.login_user('nandy') user = app.users['nandy'] if not user.running(''): - io_loop.run_sync(user.spawn) + yield user.spawn() url = public_url(app, user) # no cookies, redirects to login page - r = requests.get(url) + r = yield async_requests.get(url) r.raise_for_status() assert '/hub/login' in r.url # with cookies, login successful - r = requests.get(url, cookies=cookies) + r = yield async_requests.get(url, cookies=cookies) r.raise_for_status() assert r.url.rstrip('/').endswith('/user/nandy/tree') assert r.status_code == 200 # logout - r = requests.get(url_path_join(url, 'logout'), cookies=cookies) + r = yield async_requests.get(url_path_join(url, 'logout'), cookies=cookies) assert len(r.cookies) == 0 # another user accessing should get 403, not redirect to login - cookies = app.login_user('burgess') - r = requests.get(url, cookies=cookies) + cookies = yield app.login_user('burgess') + r = yield async_requests.get(url, cookies=cookies) assert r.status_code == 403 assert 'burgess' in r.text -def test_disable_user_config(app, io_loop): +@pytest.mark.gen_test +def test_disable_user_config(app): # use StubSingleUserSpawner to launch a single-user app in a thread app.spawner_class = StubSingleUserSpawner app.tornado_settings['spawner_class'] = StubSingleUserSpawner # login, start the server - cookies = app.login_user('nandy') + cookies = yield app.login_user('nandy') user = app.users['nandy'] # stop spawner, if running: if user.running(''): print("stopping") - io_loop.run_sync(user.stop) + yield user.stop() # start with new config: user.spawner.debug = True user.spawner.disable_user_config = True - io_loop.run_sync(user.spawn) - io_loop.run_sync(lambda : app.proxy.add_user(user)) + yield user.spawn() + yield app.proxy.add_user(user) url = public_url(app, user) # with cookies, login successful - r = requests.get(url, cookies=cookies) + r = yield async_requests.get(url, cookies=cookies) r.raise_for_status() assert r.url.rstrip('/').endswith('/user/nandy/tree') assert r.status_code == 200 diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index 17794ca6..6b546741 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -13,7 +13,6 @@ import time from unittest import mock import pytest -import requests from tornado import gen from ..user import User @@ -21,6 +20,7 @@ from ..objects import Hub from .. import spawner as spawnermod from ..spawner import LocalProcessSpawner from .. import orm +from .utils import async_requests _echo_sleep = """ import sys, time @@ -239,7 +239,7 @@ def test_shell_cmd(db, tmpdir, request): s.server.port = port db.commit() yield wait_for_spawner(s) - r = requests.get('http://%s:%i/env' % (ip, port)) + r = yield async_requests.get('http://%s:%i/env' % (ip, port)) r.raise_for_status() env = r.json() assert env['TESTVAR'] == 'foo' From 9e8b6503a0e9a5d957e878a8ac80f9562b382d41 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 27 Jul 2017 11:28:03 +0200 Subject: [PATCH 5/9] use gen_test in place of IOLoop.run_sync even where not strictly required for consistency, now that we are using gen_test on the main app tests --- jupyterhub/tests/test_api.py | 10 ++-- jupyterhub/tests/test_app.py | 20 +++---- jupyterhub/tests/test_auth.py | 97 ++++++++++++++++++-------------- jupyterhub/tests/test_db.py | 7 ++- jupyterhub/tests/test_orm.py | 5 +- jupyterhub/tests/test_spawner.py | 53 +++++++++-------- 6 files changed, 106 insertions(+), 86 deletions(-) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index a3e3e843..c5d83092 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -134,7 +134,7 @@ def test_auth_api(app): @mark.gen_test -def test_referer_check(app, io_loop): +def test_referer_check(app): url = ujoin(public_host(app), app.hub.base_url) host = urlparse(url).netloc user = find_user(app.db, 'admin') @@ -390,7 +390,7 @@ def test_make_admin(app): @mark.gen_test -def test_spawn(app, io_loop): +def test_spawn(app): db = app.db name = 'wash' user = add_user(db, app=app, name=name) @@ -445,7 +445,7 @@ def test_spawn(app, io_loop): @mark.gen_test -def test_slow_spawn(app, io_loop, no_patience, request): +def test_slow_spawn(app, no_patience, request): patch = mock.patch.dict(app.tornado_settings, {'spawner_class': mocking.SlowSpawner}) patch.start() request.addfinalizer(patch.stop) @@ -495,7 +495,7 @@ def test_slow_spawn(app, io_loop, no_patience, request): @mark.gen_test -def test_never_spawn(app, io_loop, no_patience, request): +def test_never_spawn(app, no_patience, request): patch = mock.patch.dict(app.tornado_settings, {'spawner_class': mocking.NeverSpawner}) patch.start() request.addfinalizer(patch.stop) @@ -520,7 +520,7 @@ def test_never_spawn(app, io_loop, no_patience, request): @mark.gen_test -def test_get_proxy(app, io_loop): +def test_get_proxy(app): r = yield api_request(app, 'proxy') r.raise_for_status() reply = r.json() diff --git a/jupyterhub/tests/test_app.py b/jupyterhub/tests/test_app.py index e879efeb..2f9af395 100644 --- a/jupyterhub/tests/test_app.py +++ b/jupyterhub/tests/test_app.py @@ -56,7 +56,9 @@ def test_generate_config(): assert 'Spawner.cmd' in cfg_text assert 'Authenticator.whitelist' in cfg_text -def test_init_tokens(io_loop): + +@pytest.mark.gen_test +def test_init_tokens(): with TemporaryDirectory() as td: db_file = os.path.join(td, 'jupyterhub.sqlite') tokens = { @@ -65,7 +67,7 @@ def test_init_tokens(io_loop): 'boagasdfasdf': 'chell', } app = MockHub(db_url=db_file, api_tokens=tokens) - io_loop.run_sync(lambda : app.initialize([])) + yield app.initialize([]) db = app.db for token, username in tokens.items(): api_token = orm.APIToken.find(db, token) @@ -75,7 +77,7 @@ def test_init_tokens(io_loop): # simulate second startup, reloading same tokens: app = MockHub(db_url=db_file, api_tokens=tokens) - io_loop.run_sync(lambda : app.initialize([])) + yield app.initialize([]) db = app.db for token, username in tokens.items(): api_token = orm.APIToken.find(db, token) @@ -87,7 +89,7 @@ def test_init_tokens(io_loop): tokens['short'] = 'gman' app = MockHub(db_url=db_file, api_tokens=tokens) with pytest.raises(ValueError): - io_loop.run_sync(lambda : app.initialize([])) + yield app.initialize([]) assert orm.User.find(app.db, 'gman') is None @@ -141,15 +143,16 @@ def test_cookie_secret_env(tmpdir): assert not os.path.exists(hub.cookie_secret_file) -def test_load_groups(io_loop): +@pytest.mark.gen_test +def test_load_groups(): to_load = { 'blue': ['cyclops', 'rogue', 'wolverine'], 'gold': ['storm', 'jean-grey', 'colossus'], } hub = MockHub(load_groups=to_load) hub.init_db() - io_loop.run_sync(hub.init_users) - hub.init_groups() + yield hub.init_users() + yield hub.init_groups() db = hub.db blue = orm.Group.find(db, name='blue') assert blue is not None @@ -158,6 +161,3 @@ def test_load_groups(io_loop): assert gold is not None assert sorted([ u.name for u in gold.users ]) == sorted(to_load['gold']) -def test_version(): - if sys.version_info[:2] < (3, 3): - assertRaises(ValueError) diff --git a/jupyterhub/tests/test_auth.py b/jupyterhub/tests/test_auth.py index 9e63c017..680536be 100644 --- a/jupyterhub/tests/test_auth.py +++ b/jupyterhub/tests/test_auth.py @@ -10,38 +10,42 @@ from .mocking import MockPAMAuthenticator from jupyterhub import auth, orm -def test_pam_auth(io_loop): + +@pytest.mark.gen_test +def test_pam_auth(): authenticator = MockPAMAuthenticator() - authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { + authorized = yield authenticator.get_authenticated_user(None, { 'username': 'match', 'password': 'match', - })) + }) assert authorized['name'] == 'match' - authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { + authorized = yield authenticator.get_authenticated_user(None, { 'username': 'match', 'password': 'nomatch', - })) + }) assert authorized is None -def test_pam_auth_whitelist(io_loop): + +@pytest.mark.gen_test +def test_pam_auth_whitelist(): authenticator = MockPAMAuthenticator(whitelist={'wash', 'kaylee'}) - authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { + authorized = yield authenticator.get_authenticated_user(None, { 'username': 'kaylee', 'password': 'kaylee', - })) + }) assert authorized['name'] == 'kaylee' - authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { + authorized = yield authenticator.get_authenticated_user(None, { 'username': 'wash', 'password': 'nomatch', - })) + }) assert authorized is None - authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { + authorized = yield authenticator.get_authenticated_user(None, { 'username': 'mal', 'password': 'mal', - })) + }) assert authorized is None @@ -50,7 +54,8 @@ class MockGroup: self.gr_mem = names -def test_pam_auth_group_whitelist(io_loop): +@pytest.mark.gen_test +def test_pam_auth_group_whitelist(): g = MockGroup('kaylee') def getgrnam(name): return g @@ -58,38 +63,41 @@ def test_pam_auth_group_whitelist(io_loop): authenticator = MockPAMAuthenticator(group_whitelist={'group'}) with mock.patch.object(auth, 'getgrnam', getgrnam): - authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { + authorized = yield authenticator.get_authenticated_user(None, { 'username': 'kaylee', 'password': 'kaylee', - })) + }) assert authorized['name'] == 'kaylee' with mock.patch.object(auth, 'getgrnam', getgrnam): - authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { + authorized = yield authenticator.get_authenticated_user(None, { 'username': 'mal', 'password': 'mal', - })) + }) assert authorized is None -def test_pam_auth_no_such_group(io_loop): +@pytest.mark.gen_test +def test_pam_auth_no_such_group(): authenticator = MockPAMAuthenticator(group_whitelist={'nosuchcrazygroup'}) - authorized = io_loop.run_sync(lambda : authenticator.get_authenticated_user(None, { + authorized = yield authenticator.get_authenticated_user(None, { 'username': 'kaylee', 'password': 'kaylee', - })) + }) assert authorized is None -def test_wont_add_system_user(io_loop): +@pytest.mark.gen_test +def test_wont_add_system_user(): 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)) + yield authenticator.add_user(user) -def test_cant_add_system_user(io_loop): +@pytest.mark.gen_test +def test_cant_add_system_user(): user = orm.User(name='lioness4321') authenticator = auth.PAMAuthenticator(whitelist={'mal'}) authenticator.add_user_cmd = ['jupyterhub-fake-command'] @@ -111,11 +119,12 @@ def test_cant_add_system_user(io_loop): with mock.patch.object(auth, 'Popen', DummyPopen): with pytest.raises(RuntimeError) as exc: - io_loop.run_sync(lambda : authenticator.add_user(user)) + yield authenticator.add_user(user) assert str(exc.value) == 'Failed to create system user lioness4321: dummy error' -def test_add_system_user(io_loop): +@pytest.mark.gen_test +def test_add_system_user(): user = orm.User(name='lioness4321') authenticator = auth.PAMAuthenticator(whitelist={'mal'}) authenticator.create_system_users = True @@ -131,11 +140,12 @@ def test_add_system_user(io_loop): return with mock.patch.object(auth, 'Popen', DummyPopen): - io_loop.run_sync(lambda : authenticator.add_user(user)) + yield authenticator.add_user(user) assert record['cmd'] == ['echo', '/home/lioness4321', 'lioness4321'] -def test_delete_user(io_loop): +@pytest.mark.gen_test +def test_delete_user(): user = orm.User(name='zoe') a = MockPAMAuthenticator(whitelist={'mal'}) @@ -160,49 +170,52 @@ def test_handlers(app): assert handlers[0][0] == '/login' -def test_normalize_names(io_loop): +@pytest.mark.gen_test +def test_normalize_names(): a = MockPAMAuthenticator() - authorized = io_loop.run_sync(lambda : a.get_authenticated_user(None, { + authorized = yield a.get_authenticated_user(None, { 'username': 'ZOE', 'password': 'ZOE', - })) + }) assert authorized['name'] == 'zoe' - authorized = io_loop.run_sync(lambda: a.get_authenticated_user(None, { + authorized = yield a.get_authenticated_user(None, { 'username': 'Glenn', 'password': 'Glenn', - })) + }) assert authorized['name'] == 'glenn' - authorized = io_loop.run_sync(lambda: a.get_authenticated_user(None, { + authorized = yield a.get_authenticated_user(None, { 'username': 'hExi', 'password': 'hExi', - })) + }) assert authorized['name'] == 'hexi' - authorized = io_loop.run_sync(lambda: a.get_authenticated_user(None, { + authorized = yield a.get_authenticated_user(None, { 'username': 'Test', 'password': 'Test', - })) + }) assert authorized['name'] == 'test' -def test_username_map(io_loop): + +@pytest.mark.gen_test +def test_username_map(): a = MockPAMAuthenticator(username_map={'wash': 'alpha'}) - authorized = io_loop.run_sync(lambda : a.get_authenticated_user(None, { + authorized = yield a.get_authenticated_user(None, { 'username': 'WASH', 'password': 'WASH', - })) + }) assert authorized['name'] == 'alpha' - authorized = io_loop.run_sync(lambda : a.get_authenticated_user(None, { + authorized = yield a.get_authenticated_user(None, { 'username': 'Inara', 'password': 'Inara', - })) + }) assert authorized['name'] == 'inara' -def test_validate_names(io_loop): +def test_validate_names(): a = auth.PAMAuthenticator() assert a.validate_username('willow') assert a.validate_username('giles') diff --git a/jupyterhub/tests/test_db.py b/jupyterhub/tests/test_db.py index 45f857d7..8950fd2f 100644 --- a/jupyterhub/tests/test_db.py +++ b/jupyterhub/tests/test_db.py @@ -3,6 +3,8 @@ import os import shutil from sqlalchemy.exc import OperationalError + +import pytest from pytest import raises from ..dbutil import upgrade @@ -24,7 +26,8 @@ def test_upgrade(tmpdir): print(db_url) upgrade(db_url) -def test_upgrade_entrypoint(tmpdir, io_loop): +@pytest.mark.gen_test +def test_upgrade_entrypoint(tmpdir): generate_old_db(str(tmpdir)) tmpdir.chdir() tokenapp = NewToken() @@ -36,7 +39,7 @@ def test_upgrade_entrypoint(tmpdir, io_loop): assert len(sqlite_files) == 1 upgradeapp = UpgradeDB() - io_loop.run_sync(lambda : upgradeapp.initialize([])) + yield upgradeapp.initialize([]) upgradeapp.start() # check that backup was created: diff --git a/jupyterhub/tests/test_orm.py b/jupyterhub/tests/test_orm.py index 4fb8f598..7b6bafc3 100644 --- a/jupyterhub/tests/test_orm.py +++ b/jupyterhub/tests/test_orm.py @@ -140,7 +140,8 @@ def test_token_find(db): assert found is None -def test_spawn_fails(db, io_loop): +@pytest.mark.gen_test +def test_spawn_fails(db): orm_user = orm.User(name='aeofel') db.add(orm_user) db.commit() @@ -156,7 +157,7 @@ def test_spawn_fails(db, io_loop): }) with pytest.raises(RuntimeError) as exc: - io_loop.run_sync(user.spawn) + yield user.spawn() assert user.spawners[''].server is None assert not user.running('') diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index 6b546741..e2e01eb7 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -96,7 +96,7 @@ def wait_for_spawner(spawner, timeout=10): yield wait() -@pytest.mark.gen_test(run_sync=False) +@pytest.mark.gen_test def test_single_user_spawner(app, request): user = next(iter(app.users.values()), None) spawner = user.spawner @@ -112,42 +112,45 @@ def test_single_user_spawner(app, request): assert status == 0 -def test_stop_spawner_sigint_fails(db, io_loop): +@pytest.mark.gen_test +def test_stop_spawner_sigint_fails(db): spawner = new_spawner(db, cmd=[sys.executable, '-c', _uninterruptible]) - io_loop.run_sync(spawner.start) - + yield spawner.start() + # wait for the process to get to the while True: loop - time.sleep(1) - - status = io_loop.run_sync(spawner.poll) + yield gen.sleep(1) + + status = yield spawner.poll() assert status is None - io_loop.run_sync(spawner.stop) - status = io_loop.run_sync(spawner.poll) + yield spawner.stop() + status = yield spawner.poll() assert status == -signal.SIGTERM -def test_stop_spawner_stop_now(db, io_loop): +@pytest.mark.gen_test +def test_stop_spawner_stop_now(db): spawner = new_spawner(db) - io_loop.run_sync(spawner.start) - + yield spawner.start() + # wait for the process to get to the while True: loop - time.sleep(1) - - status = io_loop.run_sync(spawner.poll) + yield gen.sleep(1) + + status = yield spawner.poll() assert status is None - io_loop.run_sync(lambda : spawner.stop(now=True)) - status = io_loop.run_sync(spawner.poll) + yield spawner.stop(now=True) + status = yield spawner.poll() assert status == -signal.SIGTERM -def test_spawner_poll(db, io_loop): +@pytest.mark.gen_test +def test_spawner_poll(db): first_spawner = new_spawner(db) user = first_spawner.user - io_loop.run_sync(first_spawner.start) + yield first_spawner.start() proc = first_spawner.proc - status = io_loop.run_sync(first_spawner.poll) + status = yield first_spawner.poll() assert status is None if user.state is None: user.state = {} @@ -159,21 +162,21 @@ def test_spawner_poll(db, io_loop): spawner.start_polling() # wait for the process to get to the while True: loop - io_loop.run_sync(lambda : gen.sleep(1)) - status = io_loop.run_sync(spawner.poll) + yield gen.sleep(1) + status = yield spawner.poll() assert status is None # kill the process proc.terminate() for i in range(10): if proc.poll() is None: - time.sleep(1) + yield gen.sleep(1) else: break assert proc.poll() is not None - io_loop.run_sync(lambda : gen.sleep(2)) - status = io_loop.run_sync(spawner.poll) + yield gen.sleep(2) + status = yield spawner.poll() assert status is not None From 8f1115a257640135287cb4e70c52d257f78f977a Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 27 Jul 2017 11:37:19 +0200 Subject: [PATCH 6/9] remove handling of changing db sessions this was purely for accessing the db from multiple threads in tests --- jupyterhub/app.py | 13 +------------ jupyterhub/user.py | 20 -------------------- 2 files changed, 1 insertion(+), 32 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index edf34901..6bb62a1c 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -16,7 +16,6 @@ import shutil import signal import sys from textwrap import dedent -import threading from urllib.parse import urlparse if sys.version_info[:2] < (3, 3): @@ -423,7 +422,6 @@ class JupyterHub(Application): @observe('base_url') def _update_hub_prefix(self, change): """add base URL to hub prefix""" - base_url = change['new'] self.hub_prefix = self._hub_prefix_default() cookie_secret = Bytes( @@ -813,15 +811,6 @@ class JupyterHub(Application): # store the loaded trait value self.cookie_secret = secret - # thread-local storage of db objects - _local = Instance(threading.local, ()) - - @property - def db(self): - if not hasattr(self._local, 'db'): - self._local.db = scoped_session(self.session_factory)() - return self._local.db - def init_db(self): """Create the database connection""" self.log.debug("Connecting to db: %s", self.db_url) @@ -833,7 +822,7 @@ class JupyterHub(Application): **self.db_kwargs ) # trigger constructing thread local db property - _ = self.db + self.db = scoped_session(self.session_factory)() except OperationalError as e: self.log.error("Failed to connect to db: %s", self.db_url) self.log.debug("Database error was:", exc_info=True) diff --git a/jupyterhub/user.py b/jupyterhub/user.py index 720c0897..7d04185d 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -98,27 +98,7 @@ class User(HasTraits): if self.orm_user: return inspect(self.orm_user).session - @observe('db') - def _db_changed(self, change): - """Changing db session reacquires ORM User object""" - # db session changed, re-get orm User - db = change.new - if self._user_id is not None: - # fetch our orm.User from the new db session - self.orm_user = db.query(orm.User).filter(orm.User.id == self._user_id).first() - # update our spawners' ORM objects with the new session, - # which can be found on our new orm_user. - for name, spawner in self.spawners.items(): - spawner.orm_spawner = self.orm_user.orm_spawners[name] - - _user_id = None orm_user = Any(allow_none=True) - @observe('orm_user') - def _orm_user_changed(self, change): - if change.new: - self._user_id = change.new.id - else: - self._user_id = None @property def authenticator(self): From 638f9802810fefc815266ddc2392080306ef61cc Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 27 Jul 2017 11:45:53 +0200 Subject: [PATCH 7/9] fix race waiting for slow spawners wait for `.running` instead of `._spawn_pending`, since we now have `._proxy_pending` as well to wait for. --- jupyterhub/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index c5d83092..46a6d4f8 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -462,7 +462,7 @@ def test_slow_spawn(app, no_patience, request): @gen.coroutine def wait_spawn(): - while app_user.spawner._spawn_pending: + while not app_user.running(''): yield gen.sleep(0.1) yield wait_spawn() From afc968146d2d043eed45bee475cc1287ef9fdd5c Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 27 Jul 2017 11:52:45 +0200 Subject: [PATCH 8/9] fix race in test_proxy prevent `.check_routes` from firing while we wait for a new proxy to come up We check explicitly that it comes up with no routes, so makes sure check_routes hasn't restored its state, which is causing intermittent failures --- jupyterhub/tests/test_proxy.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jupyterhub/tests/test_proxy.py b/jupyterhub/tests/test_proxy.py index 0f6334db..134434b8 100644 --- a/jupyterhub/tests/test_proxy.py +++ b/jupyterhub/tests/test_proxy.py @@ -28,6 +28,9 @@ def test_external_proxy(request): cfg.ConfigurableHTTPProxy.should_start = False app = MockHub.instance(config=cfg) + # disable last_activity polling to avoid check_routes being called during the test, + # which races with some of our test conditions + app.last_activity_interval = 0 def fin(): MockHub.clear_instance() @@ -53,10 +56,7 @@ def test_external_proxy(request): def _cleanup_proxy(): if proxy.poll() is None: - print("Terminating proxy") proxy.terminate() - else: - print("Not stopping proxy", proxy) request.addfinalizer(_cleanup_proxy) def wait_for_proxy(): From e7fe6d25b6644c6be7b7eeafc79a8b829006148a Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 27 Jul 2017 12:48:46 +0200 Subject: [PATCH 9/9] set ASYNC_TEST_TIMEOUT=15 on Travis Travis is super slow and default timeout is 5 seconds, which is too low sometimes. --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index 986820b8..669a96eb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,9 @@ python: - 3.6 - 3.5 - 3.4 +env: + global: + - ASYNC_TEST_TIMEOUT=15 # installing dependencies before_install: