Fix crash when handling a stream after non-stream output (#48)

* Add a failing test case

* Move forked to `build.yaml`

* Fix test to run with a real `ycell`

* Fix the exception

* Add everything to addopts to streamline DevExp, add min cov

Author: krassowski

.github/workflows/build.yml

@@ -32,7 +32,7 @@ jobs:
         jlpm
         jlpm run lint:check
 
-    - name: Test the extension
+    - name: Frontend test
       run: |
         set -eux
         jlpm run test
@@ -41,15 +41,19 @@ jobs:
       run: |
         set -eux
         python -m pip install .[test]
-
-        pytest -vv -r ap --cov jupyter_server_nbmodel
         jupyter server extension list
         jupyter server extension list 2>&1 | grep -ie "jupyter_server_nbmodel.*OK"
 
         jupyter labextension list
         jupyter labextension list 2>&1 | grep -ie "jupyter-server-nbmodel.*OK"
+
         python -m jupyterlab.browser_check
 
+    - name: Python test
+      run: |
+        set -eux
+        pytest
+
     - name: Package the extension
       run: |
         set -eux

README.md

@@ -230,7 +230,7 @@ jupyter labextension develop . --overwrite
 To execute them, run:
 
 ```sh
-pytest -vv -r ap --cov jupyter_server_nbmodel
+pytest
 ```
 
 #### Frontend tests

jupyter_server_nbmodel/actions.py

@@ -107,7 +107,7 @@ def _output_hook(outputs: list[NotebookNode], ycell: y.Map | None, msg: dict) ->
                     # FIXME Logic is quite complex at https://github.com/jupyterlab/jupyterlab/blob/7ae2d436fc410b0cff51042a3350ba71f54f4445/packages/outputarea/src/model.ts#L518
                     if text.endswith((os.linesep, "\n")):
                         text = text[:-1]
-                    if (not cell_outputs) or (cell_outputs[-1]["name"] != output["name"]):
+                    if (not cell_outputs) or (cell_outputs[-1].get("name", None) != output["name"]):
                         output["text"] = [text]
                         cell_outputs.append(output)
                     else:

jupyter_server_nbmodel/tests/test_handlers.py

@@ -74,6 +74,23 @@ async def _(kernel_id, ready=None):
     return _
 
 
+@pytest.fixture()
+def rtc_test_notebook(jp_serverapp, rtc_create_notebook):
+    async def _(notebook, path="test.ipynb"):
+        nb_content = nbformat.writes(notebook, version=4)
+        returned_path, _ = await rtc_create_notebook(path, nb_content, store=True)
+        assert path == returned_path
+        document_id = get_document_id(jp_serverapp, "test.ipynb")
+        return document_id
+    return _
+
+
+def get_document_id(jp_serverapp, notebook_name):
+    fim = jp_serverapp.web_app.settings["file_id_manager"]
+    document_id = f'json:notebook:{fim.get_id(notebook_name)}'
+    return document_id
+
+
 @pytest.mark.timeout(TEST_TIMEOUT)
 @pytest.mark.parametrize(
     "snippet,output",
@@ -82,29 +99,89 @@ async def _(kernel_id, ready=None):
             "print('hello buddy')",
             '{"output_type": "stream", "name": "stdout", "text": "hello buddy\\n"}',
         ),
+    ),
+)
+async def test_post_execute_no_ycell(jp_fetch, pending_kernel_is_ready, snippet, output):
+    r = await jp_fetch(
+        "api", "kernels", method="POST", body=json.dumps({"name": NATIVE_KERNEL_NAME})
+    )
+    kernel = json.loads(r.body.decode())
+    await pending_kernel_is_ready(kernel["id"])
+
+    response = await wait_for_request(
+        jp_fetch,
+        "api",
+        "kernels",
+        kernel["id"],
+        "execute",
+        method="POST",
+        body=json.dumps({"code": snippet}),
+    )
+
+    assert response.code == 200
+    payload = json.loads(response.body)
+    assert payload == {
+        "status": "ok",
+        "execution_count": 1,
+        "outputs": f"[{output}]",
+    }
+
+    response2 = await jp_fetch("api", "kernels", kernel["id"], method="DELETE")
+    assert response2.code == 204
+
+    await asyncio.sleep(1)
+
+
+@pytest.mark.timeout(2 * TEST_TIMEOUT)
+@pytest.mark.parametrize(
+    "snippet,output",
+    (
+        (
+            "print('hello buddy')",
+            '{"output_type": "stream", "name": "stdout", "text": ["hello buddy"]}',
+        ),
         ("a = 1", ""),
         (
             """from IPython.display import HTML
 HTML('<p><b>Jupyter</b> rocks.</p>')""",
             '{"output_type": "execute_result", "metadata": {}, "data": {"text/plain": "<IPython.core.display.HTML object>", "text/html": "<p><b>Jupyter</b> rocks.</p>"}, "execution_count": 1}',  # noqa: E501
         ),
+        (
+          "display('a'); print('b')",
+          (
+            '{"output_type": "display_data", "metadata": {}, "data": {"text/plain": "\'a\'"}}'
+            ', {"output_type": "stream", "name": "stdout", "text": ["b"]}'
+          )
+        )
     ),
 )
-async def test_post_execute(jp_fetch, pending_kernel_is_ready, snippet, output):
+async def test_post_execute_wiht_ycell(jp_fetch, pending_kernel_is_ready, snippet, output, rtc_test_notebook):
     r = await jp_fetch(
         "api", "kernels", method="POST", body=json.dumps({"name": NATIVE_KERNEL_NAME})
     )
     kernel = json.loads(r.body.decode())
     await pending_kernel_is_ready(kernel["id"])
 
+    nb = nbformat.v4.new_notebook(
+        cells=[nbformat.v4.new_code_cell(source=snippet)]
+    )
+    document_id = await rtc_test_notebook(nb)
+    cell_id = nb["cells"][0]["id"]
+
     response = await wait_for_request(
         jp_fetch,
         "api",
         "kernels",
         kernel["id"],
         "execute",
         method="POST",
-        body=json.dumps({"code": snippet}),
+        body=json.dumps({
+            "code": snippet,
+            "metadata": {
+                "cell_id": cell_id,
+                "document_id": document_id
+            }
+        }),
     )
 
     assert response.code == 200
@@ -223,17 +300,15 @@ async def fake_execute(client, ydoc, snippet, metadata, stdin_hook):
 
 
 @pytest.mark.timeout(TEST_TIMEOUT)
-async def test_execution_timing_metadata(jp_root_dir, jp_fetch, pending_kernel_is_ready, rtc_create_notebook, jp_serverapp):
+async def test_execution_timing_metadata(jp_root_dir, jp_fetch, pending_kernel_is_ready, rtc_test_notebook, jp_serverapp):
     snippet = "a = 1"
     nb = nbformat.v4.new_notebook(
         cells=[nbformat.v4.new_code_cell(source=snippet, execution_count=1)]
     )
-    nb_content = nbformat.writes(nb, version=4)
-    path, _ = await rtc_create_notebook("test.ipynb", nb_content, store=True)
+    path = "test.ipynb"
+    document_id = await rtc_test_notebook(nb, path=path)
     collaboration = jp_serverapp.web_app.settings["jupyter_server_ydoc"]
-    fim = jp_serverapp.web_app.settings["file_id_manager"]
-    document_id = f'json:notebook:{fim.get_id("test.ipynb")}'
-    cell_id = nb["cells"][0].get("id")
+    cell_id = nb["cells"][0]["id"]
 
     r = await jp_fetch(
         "api", "kernels", method="POST", body=json.dumps({"name": NATIVE_KERNEL_NAME})

pyproject.toml

@@ -91,7 +91,13 @@ source_dir = "src"
 build_dir = "jupyter_server_nbmodel/labextension"
 
 [tool.pytest.ini_options]
-addopts = "-vv --forked"
+addopts = [
+    "-vv",
+    "-r ap",
+    "--forked",
+    "--cov=jupyter_server_nbmodel",
+    "--cov-fail-under=80",
+]
 filterwarnings = [
   "error",
   "ignore:Unclosed context <zmq.asyncio.Context:ResourceWarning",