Add 100% coverage (#73)

Author: davidbrochart

.github/workflows/test.yml

@@ -8,6 +8,10 @@ on:
     branches:
     - main
 
+defaults:
+  run:
+    shell: bash
+
 jobs:
   test:
     name: ${{ matrix.os }} python-${{ matrix.python-version }}
@@ -34,10 +38,16 @@ jobs:
         create-args: python=${{ matrix.python-version }}
 
     - name: Build and install pycrdt
-      run: pip install .[test]
+      run: pip install -e ".[test]"
 
     - name: Check types
       run: mypy python
 
     - name: Run tests
       run: pytest --color=yes -v tests
+
+    - name: Run code coverage
+      if: ${{ (matrix.python-version == '3.12') && (matrix.os == 'ubuntu-latest') }}
+      run: |
+        coverage run -m pytest tests
+        coverage report --show-missing --fail-under=100

README.md

@@ -1,4 +1,5 @@
 [![Build Status](https://github.com/jupyter-server/pycrdt/actions/workflows/test.yml/badge.svg?query=branch%3Amain++)](https://github.com/jupyter-server/pycrdt/actions/workflows/test.yml/badge.svg?query=branch%3Amain++)
+[![Code Coverage](https://img.shields.io/badge/coverage-100%25-green)](https://img.shields.io/badge/coverage-100%25-green)
 [![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black)
 
 ⚠️ This project is still in an **incubating** phase (i.e. it's not ready for production yet) ⚠️

pyproject.toml

@@ -35,6 +35,7 @@ test = [
     "y-py >=0.7.0a1,<0.8",
     "pydantic >=2.5.2,<3",
     "mypy",
+    "coverage[toml] >=7",
 ]
 docs = [ "mkdocs", "mkdocs-material" ]
 
@@ -49,3 +50,9 @@ module-name = "pycrdt._pycrdt"
 [tool.ruff]
 line-length = 100
 select = ["F", "E", "W", "I001"]
+
+[tool.coverage.run]
+source = ["python", "tests"]
+
+[tool.coverage.report]
+show_missing = true

python/pycrdt/array.py

@@ -7,7 +7,7 @@
 from ._pycrdt import ArrayEvent as _ArrayEvent
 from .base import BaseDoc, BaseEvent, BaseType, base_types, event_types
 
-if TYPE_CHECKING:
+if TYPE_CHECKING:  # pragma: no cover
     from .doc import Doc
 
 
@@ -106,16 +106,16 @@ def __setitem__(self, key: int | slice, value: Any | list[Any]) -> None:
                     raise RuntimeError("Step not supported")
                 if key.start != key.stop:
                     raise RuntimeError("Start and stop must be equal")
-                if len(self) <= key.start < 0:
+                if key.start > len(self) or key.start < 0:
                     raise RuntimeError("Index out of range")
                 for i, v in enumerate(value):
                     self._set(i + key.start, v)
             else:
-                raise RuntimeError(f"Index not supported: {key}")
+                raise RuntimeError("Index must be of type integer")
 
     def _check_index(self, idx: int) -> int:
         if not isinstance(idx, int):
-            raise RuntimeError("Index must be of type int")
+            raise RuntimeError("Index must be of type integer")
         length = len(self)
         if idx < 0:
             idx += length
@@ -146,7 +146,9 @@ def __delitem__(self, key: int | slice) -> None:
                     n = key.stop - i
                 self.integrated.remove_range(txn._txn, i, n)
             else:
-                raise RuntimeError(f"Index not supported: {key}")
+                raise TypeError(
+                    f"array indices must be integers or slices, not {type(key).__name__}"
+                )
 
     def __getitem__(self, key: int) -> BaseType:
         with self.doc.transaction() as txn:
@@ -163,7 +165,7 @@ def __iter__(self):
         return ArrayIterator(self)
 
     def __contains__(self, item: Any) -> bool:
-        return item in iter(self)
+        return item in list(self)
 
     def __str__(self) -> str:
         with self.doc.transaction() as txn:

python/pycrdt/base.py

@@ -7,7 +7,7 @@
 from ._pycrdt import Transaction as _Transaction
 from .transaction import ReadTransaction, Transaction
 
-if TYPE_CHECKING:
+if TYPE_CHECKING:  # pragma: no cover
     from .doc import Doc
 
 
@@ -77,14 +77,6 @@ def _forbid_read_transaction(self, txn: Transaction):
                 "Read-only transaction cannot be used to modify document structure"
             )
 
-    def _current_transaction(self) -> Transaction:
-        if self._doc is None:
-            raise RuntimeError("Not associated with a document")
-        if self._doc._txn is None:
-            raise RuntimeError("No current transaction")
-        res = cast(Transaction, self._doc._txn)
-        return res
-
     def _integrate(self, doc: Doc, integrated: Any) -> Any:
         prelim = self._prelim
         self._doc = doc
@@ -95,8 +87,6 @@ def _integrate(self, doc: Doc, integrated: Any) -> Any:
     def _do_and_integrate(
         self, action: str, value: BaseType, txn: _Transaction, *args
     ) -> None:
-        if value.is_integrated:
-            raise RuntimeError("Already integrated")
         method = getattr(self._integrated, f"{action}_{value.type_name}_prelim")
         integrated = method(txn, *args)
         assert self._doc is not None

python/pycrdt/doc.py

@@ -71,7 +71,7 @@ def __getitem__(self, key: str) -> BaseType:
         return self._roots[key]
 
     def __iter__(self):
-        return self.keys()
+        return iter(self.keys())
 
     def keys(self):
         return self._roots.keys()

python/pycrdt/map.py

@@ -7,7 +7,7 @@
 from ._pycrdt import MapEvent as _MapEvent
 from .base import BaseDoc, BaseEvent, BaseType, base_types, event_types
 
-if TYPE_CHECKING:
+if TYPE_CHECKING:  # pragma: no cover
     from .doc import Doc
 
 
@@ -116,7 +116,7 @@ def _check_key(self, key: str):
         if not isinstance(key, str):
             raise RuntimeError("Key must be of type string")
         if key not in self.keys():
-            raise KeyError(f"KeyError: {key}")
+            raise KeyError(key)
 
     def keys(self):
         with self.doc.transaction() as txn:

python/pycrdt/text.py

@@ -7,7 +7,7 @@
 from ._pycrdt import TextEvent as _TextEvent
 from .base import BaseEvent, BaseType, base_types, event_types
 
-if TYPE_CHECKING:
+if TYPE_CHECKING:  # pragma: no cover
     from .doc import Doc
 
 
@@ -92,6 +92,10 @@ def __delitem__(self, key: int | slice) -> None:
             else:
                 raise RuntimeError(f"Index not supported: {key}")
 
+    def __getitem__(self, key: int | slice) -> str:
+        value = str(self)
+        return value[key]
+
     def __setitem__(self, key: int | slice, value: str) -> None:
         with self.doc.transaction() as txn:
             self._forbid_read_transaction(txn)

python/pycrdt/transaction.py

@@ -5,7 +5,7 @@
 
 from ._pycrdt import Transaction as _Transaction
 
-if TYPE_CHECKING:
+if TYPE_CHECKING:  # pragma: no cover
     from .doc import Doc
 
 

tests/test_array.py

@@ -1,6 +1,7 @@
 import json
 from functools import partial
 
+import pytest
 from pycrdt import Array, Doc, Map, Text
 
 
@@ -47,7 +48,7 @@ def test_array():
     doc["array"] = array
     events = []
 
-    array.observe(partial(callback, events))
+    idx = array.observe(partial(callback, events))
     ref = [
         -1,
         -2,
@@ -62,6 +63,7 @@ def test_array():
         -3,
         -4,
         -6,
+        -7,
     ]
     with doc.transaction():
         array.append("foo")
@@ -79,6 +81,7 @@ def test_array():
         array = array + [-3, -4]
         array += [-5]
         array[-1] = -6
+        array.extend([-7])
 
     assert json.loads(str(array)) == ref
     assert len(array) == len(ref)
@@ -92,24 +95,45 @@ def test_array():
         }
     ]
 
+    array.clear()
+    assert array.to_py() == []
+
+    events.clear()
+    array.unobserve(idx)
+    array.append("foo")
+    assert events == []
+
 
 def test_observe():
     doc = Doc()
     array = Array()
     doc["array"] = array
 
-    def callback(e):
-        pass
-
-    sid0 = array.observe(callback)
-    sid1 = array.observe(callback)
-    sid2 = array.observe_deep(callback)
-    sid3 = array.observe_deep(callback)
+    sid0 = array.observe(lambda x: x)
+    sid1 = array.observe(lambda x: x)
+    sid2 = array.observe_deep(lambda x: x)
+    sid3 = array.observe_deep(lambda x: x)
     assert sid0 == "o_0"
     assert sid1 == "o_1"
     assert sid2 == "od0"
     assert sid3 == "od1"
 
+    deep_events = []
+
+    def cb(events):
+        deep_events.append(events)
+
+    sid4 = array.observe_deep(cb)
+    array.append("bar")
+    assert (
+        str(deep_events[0][0])
+        == """{target: ["bar"], delta: [{'insert': ['bar']}], path: []}"""
+    )
+    deep_events.clear()
+    array.unobserve(sid4)
+    array.append("baz")
+    assert deep_events == []
+
 
 def test_api():
     # pop
@@ -129,6 +153,50 @@ def test_api():
     array.insert(1, 4)
     assert str(array) == "[1.0,4.0,2.0,3.0]"
 
+    # slices
+    doc = Doc()
+    array = Array([i for i in range(10)])
+    doc["array"] = array
+    with pytest.raises(RuntimeError) as excinfo:
+        array[::2] = 1
+    assert str(excinfo.value) == "Step not supported"
+    with pytest.raises(RuntimeError) as excinfo:
+        array[1:2] = 1
+    assert str(excinfo.value) == "Start and stop must be equal"
+    with pytest.raises(RuntimeError) as excinfo:
+        array[-1:-1] = 1
+    assert str(excinfo.value) == "Index out of range"
+    with pytest.raises(RuntimeError) as excinfo:
+        array["a"] = 1
+    assert str(excinfo.value) == "Index must be of type integer"
+    with pytest.raises(RuntimeError) as excinfo:
+        array.pop("a")
+    assert str(excinfo.value) == "Index must be of type integer"
+    with pytest.raises(IndexError) as excinfo:
+        array.pop(len(array))
+    assert str(excinfo.value) == "Array index out of range"
+    with pytest.raises(RuntimeError) as excinfo:
+        del array[::2]
+    assert str(excinfo.value) == "Step not supported"
+    with pytest.raises(RuntimeError) as excinfo:
+        del array[-1:]
+    assert str(excinfo.value) == "Negative start not supported"
+    with pytest.raises(RuntimeError) as excinfo:
+        del array[:-1]
+    assert str(excinfo.value) == "Negative stop not supported"
+    with pytest.raises(TypeError) as excinfo:
+        del array["a"]
+    assert str(excinfo.value) == "array indices must be integers or slices, not str"
+
+    assert [value for value in array] == [value for value in range(10)]
+    assert 1 in array
+
+    array = Array([0, 1, 2])
+    assert array.to_py() == [0, 1, 2]
+
+    array = Array()
+    assert array.to_py() is None
+
 
 def test_move():
     doc = Doc()

tests/test_doc.py

@@ -1,5 +1,6 @@
 from functools import partial
 
+import pytest
 from pycrdt import Array, Doc, Map, Text
 
 
@@ -18,6 +19,25 @@ def encode_client_id(client_id_bytes):
     return bytes(b)
 
 
+def test_api():
+    doc = Doc()
+
+    with pytest.raises(RuntimeError) as excinfo:
+        doc[0] = Array()
+    assert str(excinfo.value) == "Key must be of type string"
+
+    doc["a0"] = a0 = Array()
+    doc["m0"] = m0 = Map()
+    doc["t0"] = t0 = Text()
+    assert set((key for key in doc)) == set(("a0", "m0", "t0"))
+    assert set([type(value) for value in doc.values()]) == set(
+        [type(value) for value in (a0, m0, t0)]
+    )
+    assert set([(key, type(value)) for key, value in doc.items()]) == set(
+        [(key, type(value)) for key, value in (("a0", a0), ("m0", m0), ("t0", t0))]
+    )
+
+
 def test_subdoc():
     doc0 = Doc()
     map0 = Map()
@@ -77,6 +97,15 @@ def test_subdoc():
     assert event.loaded == []
 
 
+def test_doc_in_event():
+    doc = Doc()
+    doc["array"] = array = Array()
+    events = []
+    array.observe(partial(callback, events))
+    array.append(Doc())
+    assert isinstance(events[0].delta[0]["insert"][0], Doc)
+
+
 def test_transaction_event():
     doc = Doc()
     events = []