-
-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
No tests for notebooks, fixture does not work when store = True
#274
Comments
It looks like the jupyter-collaboration/tests/conftest.py Lines 152 to 168 in 7898b25
Removing
def observe(self, callback: Callable[[str, Any], None]) -> None:
"""
Subscribes to document changes.
:param callback: Callback that will be called when the document changes.
:type callback: Callable[[str, Any], None]
"""
self.unobserve()
self._subscriptions[self._ystate] = self._ystate.observe(partial(callback, "state"))
self._subscriptions[self._ymeta] = self._ymeta.observe_deep(partial(callback, "meta"))
self._subscriptions[self._ycells] = self._ycells.observe_deep(partial(callback, "cells")) I am not familiar with the lower-level codebase enough to debug this further easily. Any thoughts? |
I didn't write these fixtures, so I'm also not very familiar with them. |
This shows that observing notebook changes works as expected: from jupyter_ydoc import YNotebook
ynb = YNotebook()
def callback(target, events):
print(f"{target=}")
for event in events:
print(f"{str(event)=}")
ynb.observe(callback)
ynb.set(
{
"cells": [],
}
)
# target='cells'
# str(event)='{target: [{"execution_count":null,"metadata":{"trusted":true},"source":"","id":"c6506805-eec8-496d-9ec1-28d7239bf0b8","cell_type":"code","outputs":[]}], delta: [{\'insert\': [<pycrdt._map.Map object at 0x7f9a0c5d84a0>]}], path: []}'
# target='meta'
# str(event)='{target: {"metadata":{"language_info":{"name":""},"kernelspec":{"name":"","display_name":""}},"nbformat":4.0,"nbformat_minor":5.0}, keys: {\'metadata\': {\'action\': \'add\', \'newValue\': <pycrdt._map.Map object at 0x7f9a0c5d92b0>}, \'nbformat_minor\': {\'action\': \'add\', \'newValue\': 5.0}, \'nbformat\': {\'action\': \'add\', \'newValue\': 4.0}}, path: []}' |
I think the problem is not that observing does not work when a change is made via |
It looks like the problem is possibly not with the fixture, but with the logic itself. I see that the content is loaded for the notebook:
In code it hits this path: jupyter-collaboration/projects/jupyter-server-ydoc/jupyter_server_ydoc/rooms.py Lines 155 to 165 in bcc36e1
I am a bit confused as to why I think this might be the error - the state is not set with |
It looks like this logic traces back to #2. It looks like in ytstore implementation this method does apply the update: Anyways, that looks like a wrong avenue to pursue. The bug is in the different between |
I think I got it. There is no We can trigger the same issue with a text file by passing an empty string (equal to default content): async def test_get_document_file2(rtc_create_file, jp_serverapp):
path, content = await rtc_create_file("test.txt", "", store=True) # will timeout
collaboration = jp_serverapp.web_app.settings["jupyter_server_ydoc"]
document = await collaboration.get_document(
path=path, content_type="file", file_format="text"
)
assert document.get() == content == ""
await collaboration.stop_extension() |
In addition, the |
Changing the fixture to: @pytest.fixture
def rtc_add_doc_to_store(rtc_connect_doc_client):
event = Event()
async def _inner(format: str, type: str, path: str) -> None:
def _on_document_change(target: str, e: Any) -> None:
expected_target = "cells" if type == "notebook" else "source"
if target == expected_target:
event.set()
if type == "notebook":
doc = YNotebook()
else:
doc = YUnicode()
doc.observe(_on_document_change)
async with await rtc_connect_doc_client(
format, type, path
) as ws, WebsocketProvider(doc.ydoc, ws):
await event.wait()
await sleep(0.1)
return _inner Appears sufficient. |
Description
Heads up to anyone trying to improve test coverage:
I tried to write tests for #270 using
rtc_create_notebook
andrtc_create_file
fixtures. While I was successful withrtc_create_file
, tests withrtc_create_notebook
appear to stall if usingstore=True
.I also noticed that this fixture is not used, not even once in the tests. It looks like it needs some attention.
jupyter-collaboration/tests/conftest.py
Lines 78 to 110 in 7898b25
It stalls on line 106:
The text was updated successfully, but these errors were encountered: