add a pytest fixture for capturing logging stream (#588)

* add a pytest fixture for capturing logging stream

* Update jupyter_server/pytest_plugin.py

Author: Zsailer

jupyter_server/pytest_plugin.py

@@ -1,6 +1,8 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
+import io
 import json
+import logging
 import os
 import shutil
 import sys
@@ -18,7 +20,6 @@
 from jupyter_server.serverapp import ServerApp
 from jupyter_server.services.contents.filemanager import FileContentsManager
 from jupyter_server.services.contents.largefilemanager import LargeFileManager
-from jupyter_server.utils import run_sync
 from jupyter_server.utils import url_path_join
 
 
@@ -180,6 +181,22 @@ def jp_nbconvert_templates(jp_data_dir):
         shutil.copytree(nbconvert_path, str(nbconvert_target))
 
 
+@pytest.fixture
+def jp_logging_stream():
+    """StringIO stream intended to be used by the core
+    Jupyter ServerApp logger's default StreamHandler. This
+    helps avoid collision with stdout which is hijacked
+    by Pytest.
+    """
+    logging_stream = io.StringIO()
+    yield logging_stream
+    output = logging_stream.getvalue()
+    # If output exists, print it.
+    if output:
+        print(output)
+    return output
+
+
 @pytest.fixture(scope="function")
 def jp_configurable_serverapp(
     jp_nbconvert_templates,  # this fixture must preceed jp_environ
@@ -191,6 +208,7 @@ def jp_configurable_serverapp(
     tmp_path,
     jp_root_dir,
     io_loop,
+    jp_logging_stream,
 ):
     """Starts a Jupyter Server instance based on
     the provided configuration values.
@@ -240,6 +258,11 @@ def _configurable_serverapp(
         app.log.handlers = []
         # Initialize app without httpserver
         app.initialize(argv=argv, new_httpserver=False)
+        # Reroute all logging StreamHandlers away from stdin/stdout since pytest hijacks
+        # these streams and closes them at unfortunate times.
+        stream_handlers = [h for h in app.log.handlers if isinstance(h, logging.StreamHandler)]
+        for handler in stream_handlers:
+            handler.setStream(jp_logging_stream)
         app.log.propagate = True
         app.log.handlers = []
         # Start app without ioloop
@@ -279,7 +302,8 @@ def jp_serverapp(jp_ensure_app_fixture, jp_server_config, jp_argv, jp_configurab
     """Starts a Jupyter Server instance based on the established configuration values."""
     app = jp_configurable_serverapp(config=jp_server_config, argv=jp_argv)
     yield app
-    run_sync(app._cleanup())
+    app.remove_server_info_file()
+    app.remove_browser_open_files()
 
 
 @pytest.fixture
@@ -440,3 +464,22 @@ def inner(nbpath):
 def jp_server_cleanup():
     yield
     ServerApp.clear_instance()
+
+
+@pytest.fixture
+def jp_cleanup_subprocesses(jp_serverapp):
+    """Clean up subprocesses started by a Jupyter Server, i.e. kernels and terminal."""
+
+    async def _():
+        terminal_cleanup = jp_serverapp.web_app.settings["terminal_manager"].terminate_all
+        kernel_cleanup = jp_serverapp.kernel_manager.shutdown_all
+        if asyncio.iscoroutinefunction(terminal_cleanup):
+            await terminal_cleanup()
+        else:
+            terminal_cleanup()
+        if asyncio.iscoroutinefunction(kernel_cleanup):
+            await kernel_cleanup()
+        else:
+            kernel_cleanup()
+
+    return _

jupyter_server/tests/services/kernels/test_api.py

@@ -23,7 +23,7 @@ async def test_no_kernels(jp_fetch):
     assert kernels == []
 
 
-async def test_default_kernels(jp_fetch, jp_base_url):
+async def test_default_kernels(jp_fetch, jp_base_url, jp_cleanup_subprocesses):
     r = await jp_fetch("api", "kernels", method="POST", allow_nonstandard_methods=True)
     kernel = json.loads(r.body.decode())
     assert r.headers["location"] == url_path_join(jp_base_url, "/api/kernels/", kernel["id"])
@@ -35,9 +35,10 @@ async def test_default_kernels(jp_fetch, jp_base_url):
         ["frame-ancestors 'self'", "report-uri " + report_uri, "default-src 'none'"]
     )
     assert r.headers["Content-Security-Policy"] == expected_csp
+    await jp_cleanup_subprocesses()
 
 
-async def test_main_kernel_handler(jp_fetch, jp_base_url):
+async def test_main_kernel_handler(jp_fetch, jp_base_url, jp_cleanup_subprocesses):
     # Start the first kernel
     r = await jp_fetch(
         "api", "kernels", method="POST", body=json.dumps({"name": NATIVE_KERNEL_NAME})
@@ -98,9 +99,10 @@ async def test_main_kernel_handler(jp_fetch, jp_base_url):
     )
     kernel3 = json.loads(r.body.decode())
     assert isinstance(kernel3, dict)
+    await jp_cleanup_subprocesses()
 
 
-async def test_kernel_handler(jp_fetch):
+async def test_kernel_handler(jp_fetch, jp_cleanup_subprocesses):
     # Create a kernel
     r = await jp_fetch(
         "api", "kernels", method="POST", body=json.dumps({"name": NATIVE_KERNEL_NAME})
@@ -138,10 +140,12 @@ async def test_kernel_handler(jp_fetch):
     with pytest.raises(tornado.httpclient.HTTPClientError) as e:
         await jp_fetch("api", "kernels", bad_id, method="DELETE")
     assert expected_http_error(e, 404, "Kernel does not exist: " + bad_id)
+    await jp_cleanup_subprocesses()
 
 
-async def test_connection(jp_fetch, jp_ws_fetch, jp_http_port, jp_auth_header):
-    print("hello")
+async def test_connection(
+    jp_fetch, jp_ws_fetch, jp_http_port, jp_auth_header, jp_cleanup_subprocesses
+):
     # Create kernel
     r = await jp_fetch(
         "api", "kernels", method="POST", body=json.dumps({"name": NATIVE_KERNEL_NAME})
@@ -175,3 +179,4 @@ async def test_connection(jp_fetch, jp_ws_fetch, jp_http_port, jp_auth_header):
     r = await jp_fetch("api", "kernels", kid, method="GET")
     model = json.loads(r.body.decode())
     assert model["connections"] == 0
+    await jp_cleanup_subprocesses()

jupyter_server/tests/services/kernels/test_cull.py

@@ -34,7 +34,7 @@ def jp_server_config():
     )
 
 
-async def test_culling(jp_fetch, jp_ws_fetch):
+async def test_culling(jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses):
     r = await jp_fetch("api", "kernels", method="POST", allow_nonstandard_methods=True)
     kernel = json.loads(r.body.decode())
     kid = kernel["id"]
@@ -50,6 +50,7 @@ async def test_culling(jp_fetch, jp_ws_fetch):
     ws.close()
     culled = await get_cull_status(kid, jp_fetch)  # not connected, should be culled
     assert culled
+    await jp_cleanup_subprocesses()
 
 
 async def get_cull_status(kid, jp_fetch):

jupyter_server/tests/services/sessions/test_api.py

@@ -151,7 +151,7 @@ def assert_session_equality(actual, expected):
     assert_kernel_equality(actual["kernel"], expected["kernel"])
 
 
-async def test_create(session_client, jp_base_url):
+async def test_create(session_client, jp_base_url, jp_cleanup_subprocesses):
     # Make sure no sessions exist.
     resp = await session_client.list()
     sessions = j(resp)
@@ -182,28 +182,31 @@ async def test_create(session_client, jp_base_url):
 
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_create_file_session(session_client):
+async def test_create_file_session(session_client, jp_cleanup_subprocesses):
     resp = await session_client.create("foo/nb1.py", type="file")
     assert resp.code == 201
     newsession = j(resp)
     assert newsession["path"] == "foo/nb1.py"
     assert newsession["type"] == "file"
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_create_console_session(session_client):
+async def test_create_console_session(session_client, jp_cleanup_subprocesses):
     resp = await session_client.create("foo/abc123", type="console")
     assert resp.code == 201
     newsession = j(resp)
     assert newsession["path"] == "foo/abc123"
     assert newsession["type"] == "console"
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_create_deprecated(session_client):
+async def test_create_deprecated(session_client, jp_cleanup_subprocesses):
     resp = await session_client.create_deprecated("foo/nb1.ipynb")
     assert resp.code == 201
     newsession = j(resp)
@@ -212,9 +215,12 @@ async def test_create_deprecated(session_client):
     assert newsession["notebook"]["path"] == "foo/nb1.ipynb"
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_create_with_kernel_id(session_client, jp_fetch, jp_base_url):
+async def test_create_with_kernel_id(
+    session_client, jp_fetch, jp_base_url, jp_cleanup_subprocesses
+):
     # create a new kernel
     resp = await jp_fetch("api/kernels", method="POST", allow_nonstandard_methods=True)
     kernel = j(resp)
@@ -241,9 +247,10 @@ async def test_create_with_kernel_id(session_client, jp_fetch, jp_base_url):
     assert_session_equality(got, new_session)
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_delete(session_client):
+async def test_delete(session_client, jp_cleanup_subprocesses):
     resp = await session_client.create("foo/nb1.ipynb")
     newsession = j(resp)
     sid = newsession["id"]
@@ -260,9 +267,10 @@ async def test_delete(session_client):
     assert expected_http_error(e, 404)
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_modify_path(session_client):
+async def test_modify_path(session_client, jp_cleanup_subprocesses):
     resp = await session_client.create("foo/nb1.ipynb")
     newsession = j(resp)
     sid = newsession["id"]
@@ -273,9 +281,10 @@ async def test_modify_path(session_client):
     assert changed["path"] == "nb2.ipynb"
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_modify_path_deprecated(session_client):
+async def test_modify_path_deprecated(session_client, jp_cleanup_subprocesses):
     resp = await session_client.create("foo/nb1.ipynb")
     newsession = j(resp)
     sid = newsession["id"]
@@ -286,9 +295,10 @@ async def test_modify_path_deprecated(session_client):
     assert changed["notebook"]["path"] == "nb2.ipynb"
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_modify_type(session_client):
+async def test_modify_type(session_client, jp_cleanup_subprocesses):
     resp = await session_client.create("foo/nb1.ipynb")
     newsession = j(resp)
     sid = newsession["id"]
@@ -299,9 +309,10 @@ async def test_modify_type(session_client):
     assert changed["type"] == "console"
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_modify_kernel_name(session_client, jp_fetch):
+async def test_modify_kernel_name(session_client, jp_fetch, jp_cleanup_subprocesses):
     resp = await session_client.create("foo/nb1.ipynb")
     before = j(resp)
     sid = before["id"]
@@ -321,9 +332,10 @@ async def test_modify_kernel_name(session_client, jp_fetch):
     assert kernel_list == [after["kernel"]]
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_modify_kernel_id(session_client, jp_fetch):
+async def test_modify_kernel_id(session_client, jp_fetch, jp_cleanup_subprocesses):
     resp = await session_client.create("foo/nb1.ipynb")
     before = j(resp)
     sid = before["id"]
@@ -351,9 +363,12 @@ async def test_modify_kernel_id(session_client, jp_fetch):
 
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()
 
 
-async def test_restart_kernel(session_client, jp_base_url, jp_fetch, jp_ws_fetch):
+async def test_restart_kernel(
+    session_client, jp_base_url, jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses
+):
 
     # Create a session.
     resp = await session_client.create("foo/nb1.ipynb")
@@ -412,3 +427,4 @@ async def test_restart_kernel(session_client, jp_base_url, jp_fetch, jp_ws_fetch
 
     # Need to find a better solution to this.
     await session_client.cleanup()
+    await jp_cleanup_subprocesses()

jupyter_server/tests/test_terminal.py

@@ -8,16 +8,6 @@
 from traitlets.config import Config
 
 
-# Kill all running terminals after each test to avoid cross-test issues
-# with still running terminals.
-@pytest.fixture
-def kill_all(jp_serverapp):
-    async def _():
-        await jp_serverapp.web_app.settings["terminal_manager"].kill_all()
-
-    return _
-
-
 @pytest.fixture
 def terminal_path(tmp_path):
     subdir = tmp_path.joinpath("terminal_path")
@@ -59,7 +49,7 @@ async def test_no_terminals(jp_fetch):
     assert len(data) == 0
 
 
-async def test_terminal_create(jp_fetch, kill_all):
+async def test_terminal_create(jp_fetch, jp_cleanup_subprocesses):
     resp = await jp_fetch(
         "api",
         "terminals",
@@ -80,10 +70,12 @@ async def test_terminal_create(jp_fetch, kill_all):
 
     assert len(data) == 1
     assert data[0] == term
-    await kill_all()
+    await jp_cleanup_subprocesses()
 
 
-async def test_terminal_create_with_kwargs(jp_fetch, jp_ws_fetch, terminal_path, kill_all):
+async def test_terminal_create_with_kwargs(
+    jp_fetch, jp_ws_fetch, terminal_path, jp_cleanup_subprocesses
+):
     resp_create = await jp_fetch(
         "api",
         "terminals",
@@ -106,10 +98,12 @@ async def test_terminal_create_with_kwargs(jp_fetch, jp_ws_fetch, terminal_path,
     data = json.loads(resp_get.body.decode())
 
     assert data["name"] == term_name
-    await kill_all()
+    await jp_cleanup_subprocesses()
 
 
-async def test_terminal_create_with_cwd(jp_fetch, jp_ws_fetch, terminal_path):
+async def test_terminal_create_with_cwd(
+    jp_fetch, jp_ws_fetch, terminal_path, jp_cleanup_subprocesses
+):
     resp = await jp_fetch(
         "api",
         "terminals",
@@ -140,6 +134,7 @@ async def test_terminal_create_with_cwd(jp_fetch, jp_ws_fetch, terminal_path):
     ws.close()
 
     assert os.path.basename(terminal_path) in message_stdout
+    await jp_cleanup_subprocesses()
 
 
 async def test_culling_config(jp_server_config, jp_configurable_serverapp):
@@ -151,7 +146,7 @@ async def test_culling_config(jp_server_config, jp_configurable_serverapp):
     assert terminal_mgr_settings.cull_interval == CULL_INTERVAL
 
 
-async def test_culling(jp_server_config, jp_fetch):
+async def test_culling(jp_server_config, jp_fetch, jp_cleanup_subprocesses):
     # POST request
     resp = await jp_fetch(
         "api",
@@ -181,3 +176,4 @@ async def test_culling(jp_server_config, jp_fetch):
             await asyncio.sleep(1)
 
     assert culled
+    await jp_cleanup_subprocesses()