diff --git a/.gitignore b/.gitignore index 5663f62d0..1b2b63e65 100644 --- a/.gitignore +++ b/.gitignore @@ -83,6 +83,7 @@ src/zarr/_version.py data/* src/fixture/ fixture/ +junit.xml .DS_Store tests/.hypothesis diff --git a/changes/2693.bugfix.rst b/changes/2693.bugfix.rst new file mode 100644 index 000000000..14b45a221 --- /dev/null +++ b/changes/2693.bugfix.rst @@ -0,0 +1,13 @@ +Implement open() for LoggingStore +LoggingStore is now a generic class. +Use stdout rather than stderr as the default stream for LoggingStore +Ensure that ZipStore is open before getting or setting any values +Update equality for LoggingStore and WrapperStore such that 'other' must also be a LoggingStore or WrapperStore respectively, rather than only checking the types of the stores they wrap. +Indicate StoreTest's `test_store_repr`, `test_store_supports_writes`, `test_store_supports_partial_writes`, and `test_store_supports_listing` need to be implemented using `@abstractmethod` rather than `NotImplementedError`. +Separate instantiating and opening a store in StoreTests +Test using Store as a context manager in StoreTests +Match the errors raised by read only stores in StoreTests +Test that a ValueError is raise for invalid byte range syntax in StoreTests +Test getsize() and getsize_prefix() in StoreTests +Test the error raised for invalid buffer arguments in StoreTests +Test that data can be written to a store that's not yet open using the store.set method in StoreTests diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py index e6a5518a4..96165f8ba 100644 --- a/src/zarr/abc/store.py +++ b/src/zarr/abc/store.py @@ -176,10 +176,10 @@ async def get( Parameters ---------- key : str + prototype : BufferPrototype + The prototype of the output buffer. Stores may support a default buffer prototype. byte_range : ByteRequest, optional - ByteRequest may be one of the following. If not provided, all data associated with the key is retrieved. - - RangeByteRequest(int, int): Request a specific range of bytes in the form (start, end). The end is exclusive. If the given range is zero-length or starts after the end of the object, an error will be returned. Additionally, if the range ends after the end of the object, the entire remainder of the object will be returned. Otherwise, the exact requested range will be returned. - OffsetByteRequest(int): Request all bytes starting from a given byte offset. This is equivalent to bytes={int}- as an HTTP header. - SuffixByteRequest(int): Request the last int bytes. Note that here, int is the size of the request, not the byte offset. This is equivalent to bytes=-{int} as an HTTP header. @@ -200,6 +200,8 @@ async def get_partial_values( Parameters ---------- + prototype : BufferPrototype + The prototype of the output buffer. Stores may support a default buffer prototype. key_ranges : Iterable[tuple[str, tuple[int | None, int | None]]] Ordered set of key, range pairs, a key may occur multiple times with different ranges diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py index 752d23740..c30c9b601 100644 --- a/src/zarr/storage/_fsspec.py +++ b/src/zarr/storage/_fsspec.py @@ -10,6 +10,7 @@ Store, SuffixByteRequest, ) +from zarr.core.buffer import Buffer from zarr.storage._common import _dereference_path if TYPE_CHECKING: @@ -17,7 +18,7 @@ from fsspec.asyn import AsyncFileSystem - from zarr.core.buffer import Buffer, BufferPrototype + from zarr.core.buffer import BufferPrototype from zarr.core.common import BytesLike @@ -264,6 +265,10 @@ async def set( if not self._is_open: await self._open() self._check_writable() + if not isinstance(value, Buffer): + raise TypeError( + f"FsspecStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead." + ) path = _dereference_path(self.path, key) # write data if byte_range: diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index 5eaa85c59..1defea26b 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -96,7 +96,7 @@ def __init__(self, root: Path | str, *, read_only: bool = False) -> None: root = Path(root) if not isinstance(root, Path): raise TypeError( - f'"root" must be a string or Path instance. Got an object with type {type(root)} instead.' + f"'root' must be a string or Path instance. Got an instance of {type(root)} instead." ) self.root = root @@ -169,7 +169,9 @@ async def _set(self, key: str, value: Buffer, exclusive: bool = False) -> None: self._check_writable() assert isinstance(key, str) if not isinstance(value, Buffer): - raise TypeError("LocalStore.set(): `value` must a Buffer instance") + raise TypeError( + f"LocalStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead." + ) path = self.root / key await asyncio.to_thread(_put, path, value, start=None, exclusive=exclusive) diff --git a/src/zarr/storage/_logging.py b/src/zarr/storage/_logging.py index 5ca716df2..e9d621158 100644 --- a/src/zarr/storage/_logging.py +++ b/src/zarr/storage/_logging.py @@ -2,10 +2,11 @@ import inspect import logging +import sys import time from collections import defaultdict from contextlib import contextmanager -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Self, TypeVar from zarr.abc.store import Store from zarr.storage._wrapper import WrapperStore @@ -18,8 +19,10 @@ counter: defaultdict[str, int] +T_Store = TypeVar("T_Store", bound=Store) -class LoggingStore(WrapperStore[Store]): + +class LoggingStore(WrapperStore[T_Store]): """ Store wrapper that logs all calls to the wrapped store. @@ -42,7 +45,7 @@ class LoggingStore(WrapperStore[Store]): def __init__( self, - store: Store, + store: T_Store, log_level: str = "DEBUG", log_handler: logging.Handler | None = None, ) -> None: @@ -67,7 +70,7 @@ def _configure_logger( def _default_handler(self) -> logging.Handler: """Define a default log handler""" - handler = logging.StreamHandler() + handler = logging.StreamHandler(stream=sys.stdout) handler.setLevel(self.log_level) handler.setFormatter( logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s") @@ -94,6 +97,14 @@ def log(self, hint: Any = "") -> Generator[None, None, None]: end_time = time.time() self.logger.info("Finished %s [%.2f s]", op, end_time - start_time) + @classmethod + async def open(cls: type[Self], store_cls: type[T_Store], *args: Any, **kwargs: Any) -> Self: + log_level = kwargs.pop("log_level", "DEBUG") + log_handler = kwargs.pop("log_handler", None) + store = store_cls(*args, **kwargs) + await store._open() + return cls(store=store, log_level=log_level, log_handler=log_handler) + @property def supports_writes(self) -> bool: with self.log(): @@ -126,8 +137,7 @@ def _is_open(self) -> bool: @_is_open.setter def _is_open(self, value: bool) -> None: - with self.log(value): - self._store._is_open = value + raise NotImplementedError("LoggingStore must be opened via the `_open` method") async def _open(self) -> None: with self.log(): @@ -151,11 +161,11 @@ def __str__(self) -> str: return f"logging-{self._store}" def __repr__(self) -> str: - return f"LoggingStore({repr(self._store)!r})" + return f"LoggingStore({self._store.__class__.__name__}, '{self._store}')" def __eq__(self, other: object) -> bool: with self.log(other): - return self._store == other + return type(self) is type(other) and self._store.__eq__(other._store) # type: ignore[attr-defined] async def get( self, diff --git a/src/zarr/storage/_memory.py b/src/zarr/storage/_memory.py index d35ecbe33..b37fc8d5c 100644 --- a/src/zarr/storage/_memory.py +++ b/src/zarr/storage/_memory.py @@ -111,7 +111,9 @@ async def set(self, key: str, value: Buffer, byte_range: tuple[int, int] | None await self._ensure_open() assert isinstance(key, str) if not isinstance(value, Buffer): - raise TypeError(f"Expected Buffer. Got {type(value)}.") + raise TypeError( + f"MemoryStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead." + ) if byte_range is not None: buf = self._store_dict[key] @@ -231,8 +233,9 @@ async def set(self, key: str, value: Buffer, byte_range: tuple[int, int] | None self._check_writable() assert isinstance(key, str) if not isinstance(value, Buffer): - raise TypeError(f"Expected Buffer. Got {type(value)}.") - + raise TypeError( + f"GpuMemoryStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead." + ) # Convert to gpu.Buffer gpu_value = value if isinstance(value, gpu.Buffer) else gpu.Buffer.from_buffer(value) await super().set(key, gpu_value, byte_range=byte_range) diff --git a/src/zarr/storage/_wrapper.py b/src/zarr/storage/_wrapper.py index 255e96543..349048e49 100644 --- a/src/zarr/storage/_wrapper.py +++ b/src/zarr/storage/_wrapper.py @@ -56,6 +56,14 @@ async def _ensure_open(self) -> None: async def is_empty(self, prefix: str) -> bool: return await self._store.is_empty(prefix) + @property + def _is_open(self) -> bool: + return self._store._is_open + + @_is_open.setter + def _is_open(self, value: bool) -> None: + raise NotImplementedError("WrapperStore must be opened via the `_open` method") + async def clear(self) -> None: return await self._store.clear() @@ -67,7 +75,13 @@ def _check_writable(self) -> None: return self._store._check_writable() def __eq__(self, value: object) -> bool: - return type(self) is type(value) and self._store.__eq__(value) + return type(self) is type(value) and self._store.__eq__(value._store) # type: ignore[attr-defined] + + def __str__(self) -> str: + return f"wrapping-{self._store}" + + def __repr__(self) -> str: + return f"WrapperStore({self._store.__class__.__name__}, '{self._store}')" async def get( self, key: str, prototype: BufferPrototype, byte_range: ByteRequest | None = None diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index 5a8b51196..bf8f9900b 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -149,6 +149,8 @@ def _get( prototype: BufferPrototype, byte_range: ByteRequest | None = None, ) -> Buffer | None: + if not self._is_open: + self._sync_open() # docstring inherited try: with self._zf.open(key) as f: # will raise KeyError @@ -193,6 +195,8 @@ async def get_partial_values( return out def _set(self, key: str, value: Buffer) -> None: + if not self._is_open: + self._sync_open() # generally, this should be called inside a lock keyinfo = zipfile.ZipInfo(filename=key, date_time=time.localtime(time.time())[:6]) keyinfo.compress_type = self.compression @@ -206,9 +210,13 @@ def _set(self, key: str, value: Buffer) -> None: async def set(self, key: str, value: Buffer) -> None: # docstring inherited self._check_writable() + if not self._is_open: + self._sync_open() assert isinstance(key, str) if not isinstance(value, Buffer): - raise TypeError("ZipStore.set(): `value` must a Buffer instance") + raise TypeError( + f"ZipStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead." + ) with self._lock: self._set(key, value) diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py index 7de88a2a8..1fe544d29 100644 --- a/src/zarr/testing/store.py +++ b/src/zarr/testing/store.py @@ -2,6 +2,7 @@ import asyncio import pickle +from abc import abstractmethod from typing import TYPE_CHECKING, Generic, TypeVar from zarr.storage import WrapperStore @@ -37,30 +38,53 @@ class StoreTests(Generic[S, B]): store_cls: type[S] buffer_cls: type[B] + @abstractmethod async def set(self, store: S, key: str, value: Buffer) -> None: """ Insert a value into a storage backend, with a specific key. This should not not use any store methods. Bypassing the store methods allows them to be tested. """ - raise NotImplementedError + ... + @abstractmethod async def get(self, store: S, key: str) -> Buffer: """ Retrieve a value from a storage backend, by key. This should not not use any store methods. Bypassing the store methods allows them to be tested. """ + ... - raise NotImplementedError - + @abstractmethod @pytest.fixture def store_kwargs(self) -> dict[str, Any]: - return {"read_only": False} + """Kwargs for instantiating a store""" + ... + + @abstractmethod + def test_store_repr(self, store: S) -> None: ... + + @abstractmethod + def test_store_supports_writes(self, store: S) -> None: ... + + @abstractmethod + def test_store_supports_partial_writes(self, store: S) -> None: ... + + @abstractmethod + def test_store_supports_listing(self, store: S) -> None: ... @pytest.fixture - async def store(self, store_kwargs: dict[str, Any]) -> Store: - return await self.store_cls.open(**store_kwargs) + def open_kwargs(self, store_kwargs: dict[str, Any]) -> dict[str, Any]: + return store_kwargs + + @pytest.fixture + async def store(self, open_kwargs: dict[str, Any]) -> Store: + return await self.store_cls.open(**open_kwargs) + + @pytest.fixture + async def store_not_open(self, store_kwargs: dict[str, Any]) -> Store: + return self.store_cls(**store_kwargs) def test_store_type(self, store: S) -> None: assert isinstance(store, Store) @@ -87,39 +111,38 @@ def test_store_read_only(self, store: S) -> None: store.read_only = False # type: ignore[misc] @pytest.mark.parametrize("read_only", [True, False]) - async def test_store_open_read_only( - self, store_kwargs: dict[str, Any], read_only: bool - ) -> None: - store_kwargs["read_only"] = read_only - store = await self.store_cls.open(**store_kwargs) + async def test_store_open_read_only(self, open_kwargs: dict[str, Any], read_only: bool) -> None: + open_kwargs["read_only"] = read_only + store = await self.store_cls.open(**open_kwargs) assert store._is_open assert store.read_only == read_only - async def test_read_only_store_raises(self, store_kwargs: dict[str, Any]) -> None: - kwargs = {**store_kwargs, "read_only": True} + async def test_store_context_manager(self, open_kwargs: dict[str, Any]) -> None: + # Test that the context manager closes the store + with await self.store_cls.open(**open_kwargs) as store: + assert store._is_open + # Test trying to open an already open store + with pytest.raises(ValueError, match="store is already open"): + await store._open() + assert not store._is_open + + async def test_read_only_store_raises(self, open_kwargs: dict[str, Any]) -> None: + kwargs = {**open_kwargs, "read_only": True} store = await self.store_cls.open(**kwargs) assert store.read_only # set - with pytest.raises(ValueError): + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): await store.set("foo", self.buffer_cls.from_bytes(b"bar")) # delete - with pytest.raises(ValueError): + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): await store.delete("foo") - def test_store_repr(self, store: S) -> None: - raise NotImplementedError - - def test_store_supports_writes(self, store: S) -> None: - raise NotImplementedError - - def test_store_supports_partial_writes(self, store: S) -> None: - raise NotImplementedError - - def test_store_supports_listing(self, store: S) -> None: - raise NotImplementedError - @pytest.mark.parametrize("key", ["c/0", "foo/c/0.0", "foo/0/0"]) @pytest.mark.parametrize("data", [b"\x01\x02\x03\x04", b""]) @pytest.mark.parametrize( @@ -136,6 +159,26 @@ async def test_get(self, store: S, key: str, data: bytes, byte_range: ByteReques expected = data_buf[start:stop] assert_bytes_equal(observed, expected) + async def test_get_not_open(self, store_not_open: S) -> None: + """ + Ensure that data can be read from the store that isn't yet open using the store.get method. + """ + assert not store_not_open._is_open + data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04") + key = "c/0" + await self.set(store_not_open, key, data_buf) + observed = await store_not_open.get(key, prototype=default_buffer_prototype()) + assert_bytes_equal(observed, data_buf) + + async def test_get_raises(self, store: S) -> None: + """ + Ensure that a ValueError is raise for invalid byte range syntax + """ + data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04") + await self.set(store, "c/0", data_buf) + with pytest.raises((ValueError, TypeError), match=r"Unexpected byte_range, got.*"): + await store.get("c/0", prototype=default_buffer_prototype(), byte_range=(0, 2)) # type: ignore[arg-type] + async def test_get_many(self, store: S) -> None: """ Ensure that multiple keys can be retrieved at once with the _get_many method. @@ -158,6 +201,37 @@ async def test_get_many(self, store: S) -> None: expected_kvs = sorted(((k, b) for k, b in zip(keys, values, strict=False))) assert observed_kvs == expected_kvs + @pytest.mark.parametrize("key", ["c/0", "foo/c/0.0", "foo/0/0"]) + @pytest.mark.parametrize("data", [b"\x01\x02\x03\x04", b""]) + async def test_getsize(self, store: S, key: str, data: bytes) -> None: + """ + Test the result of store.getsize(). + """ + data_buf = self.buffer_cls.from_bytes(data) + expected = len(data_buf) + await self.set(store, key, data_buf) + observed = await store.getsize(key) + assert observed == expected + + async def test_getsize_prefix(self, store: S) -> None: + """ + Test the result of store.getsize_prefix(). + """ + data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04") + keys = ["c/0/0", "c/0/1", "c/1/0", "c/1/1"] + keys_values = [(k, data_buf) for k in keys] + await store._set_many(keys_values) + expected = len(data_buf) * len(keys) + observed = await store.getsize_prefix("c") + assert observed == expected + + async def test_getsize_raises(self, store: S) -> None: + """ + Test that getsize() raise a FileNotFoundError if the key doesn't exist. + """ + with pytest.raises(FileNotFoundError): + await store.getsize("c/1000") + @pytest.mark.parametrize("key", ["zarr.json", "c/0", "foo/c/0.0", "foo/0/0"]) @pytest.mark.parametrize("data", [b"\x01\x02\x03\x04", b""]) async def test_set(self, store: S, key: str, data: bytes) -> None: @@ -170,6 +244,17 @@ async def test_set(self, store: S, key: str, data: bytes) -> None: observed = await self.get(store, key) assert_bytes_equal(observed, data_buf) + async def test_set_not_open(self, store_not_open: S) -> None: + """ + Ensure that data can be written to the store that's not yet open using the store.set method. + """ + assert not store_not_open._is_open + data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04") + key = "c/0" + await store_not_open.set(key, data_buf) + observed = await self.get(store_not_open, key) + assert_bytes_equal(observed, data_buf) + async def test_set_many(self, store: S) -> None: """ Test that a dict of key : value pairs can be inserted into the store via the @@ -182,6 +267,16 @@ async def test_set_many(self, store: S) -> None: for k, v in store_dict.items(): assert (await self.get(store, k)).to_bytes() == v.to_bytes() + async def test_set_invalid_buffer(self, store: S) -> None: + """ + Ensure that set raises a Type or Value Error for invalid buffer arguments. + """ + with pytest.raises( + (ValueError, TypeError), + match=r"\S+\.set\(\): `value` must be a Buffer instance. Got an instance of instead.", + ): + await store.set("c/0", 0) # type: ignore[arg-type] + @pytest.mark.parametrize( "key_ranges", [ diff --git a/tests/test_store/test_core.py b/tests/test_store/test_core.py index 7806f3ece..726da06a5 100644 --- a/tests/test_store/test_core.py +++ b/tests/test_store/test_core.py @@ -4,12 +4,55 @@ import pytest from _pytest.compat import LEGACY_PATH -from zarr.core.common import AccessModeLiteral +from zarr import Group +from zarr.core.common import AccessModeLiteral, ZarrFormat from zarr.storage import FsspecStore, LocalStore, MemoryStore, StoreLike, StorePath -from zarr.storage._common import make_store_path +from zarr.storage._common import contains_array, contains_group, make_store_path from zarr.storage._utils import normalize_path +@pytest.mark.parametrize("path", ["foo", "foo/bar"]) +@pytest.mark.parametrize("write_group", [True, False]) +@pytest.mark.parametrize("zarr_format", [2, 3]) +async def test_contains_group( + local_store, path: str, write_group: bool, zarr_format: ZarrFormat +) -> None: + """ + Test that the contains_group method correctly reports the existence of a group. + """ + root = Group.from_store(store=local_store, zarr_format=zarr_format) + if write_group: + root.create_group(path) + store_path = StorePath(local_store, path=path) + assert await contains_group(store_path, zarr_format=zarr_format) == write_group + + +@pytest.mark.parametrize("path", ["foo", "foo/bar"]) +@pytest.mark.parametrize("write_array", [True, False]) +@pytest.mark.parametrize("zarr_format", [2, 3]) +async def test_contains_array( + local_store, path: str, write_array: bool, zarr_format: ZarrFormat +) -> None: + """ + Test that the contains array method correctly reports the existence of an array. + """ + root = Group.from_store(store=local_store, zarr_format=zarr_format) + if write_array: + root.create_array(path, shape=(100,), chunks=(10,), dtype="i4") + store_path = StorePath(local_store, path=path) + assert await contains_array(store_path, zarr_format=zarr_format) == write_array + + +@pytest.mark.parametrize("func", [contains_array, contains_group]) +async def test_contains_invalid_format_raises(local_store, func: callable) -> None: + """ + Test contains_group and contains_array raise errors for invalid zarr_formats + """ + store_path = StorePath(local_store) + with pytest.raises(ValueError): + assert await func(store_path, zarr_format="3.0") + + @pytest.mark.parametrize("path", [None, "", "bar"]) async def test_make_store_path_none(path: str) -> None: """ @@ -56,10 +99,18 @@ async def test_make_store_path_store_path( assert Path(store_path.store.root) == Path(tmpdir) path_normalized = normalize_path(path) assert store_path.path == (store_like / path_normalized).path - assert store_path.read_only == ro +@pytest.mark.parametrize("modes", [(True, "w"), (False, "x")]) +async def test_store_path_invalid_mode_raises(tmpdir: LEGACY_PATH, modes: tuple) -> None: + """ + Test that ValueErrors are raise for invalid mode. + """ + with pytest.raises(ValueError): + await StorePath.open(LocalStore(str(tmpdir), read_only=modes[0]), path=None, mode=modes[1]) + + async def test_make_store_path_invalid() -> None: """ Test that invalid types raise TypeError diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py index 22597a2c3..d9d941c6f 100644 --- a/tests/test_store/test_local.py +++ b/tests/test_store/test_local.py @@ -8,6 +8,7 @@ from zarr.core.buffer import Buffer, cpu from zarr.storage import LocalStore from zarr.testing.store import StoreTests +from zarr.testing.utils import assert_bytes_equal if TYPE_CHECKING: import pathlib @@ -53,3 +54,23 @@ def test_creates_new_directory(self, tmp_path: pathlib.Path): store = self.store_cls(root=target) zarr.group(store=store) + + def test_invalid_root_raises(self): + """ + Test that a TypeError is raised when a non-str/Path type is used for the `root` argument + """ + with pytest.raises( + TypeError, + match=r"'root' must be a string or Path instance. Got an instance of instead.", + ): + LocalStore(root=0) + + async def test_get_with_prototype_default(self, store: LocalStore): + """ + Ensure that data can be read via ``store.get`` if the prototype keyword argument is unspecified, i.e. set to ``None``. + """ + data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04") + key = "c/0" + await self.set(store, key, data_buf) + observed = await store.get(key, prototype=None) + assert_bytes_equal(observed, data_buf) diff --git a/tests/test_store/test_logging.py b/tests/test_store/test_logging.py index b32a214db..1a89dca87 100644 --- a/tests/test_store/test_logging.py +++ b/tests/test_store/test_logging.py @@ -1,17 +1,87 @@ from __future__ import annotations +import logging from typing import TYPE_CHECKING import pytest import zarr -from zarr.core.buffer import default_buffer_prototype -from zarr.storage import LoggingStore +from zarr.core.buffer import Buffer, cpu, default_buffer_prototype +from zarr.storage import LocalStore, LoggingStore +from zarr.testing.store import StoreTests if TYPE_CHECKING: + from _pytest.compat import LEGACY_PATH + from zarr.abc.store import Store +class TestLoggingStore(StoreTests[LoggingStore, cpu.Buffer]): + store_cls = LoggingStore + buffer_cls = cpu.Buffer + + async def get(self, store: LoggingStore, key: str) -> Buffer: + return self.buffer_cls.from_bytes((store._store.root / key).read_bytes()) + + async def set(self, store: LoggingStore, key: str, value: Buffer) -> None: + parent = (store._store.root / key).parent + if not parent.exists(): + parent.mkdir(parents=True) + (store._store.root / key).write_bytes(value.to_bytes()) + + @pytest.fixture + def store_kwargs(self, tmpdir: LEGACY_PATH) -> dict[str, str]: + return {"store": LocalStore(str(tmpdir)), "log_level": "DEBUG"} + + @pytest.fixture + def open_kwargs(self, tmpdir) -> dict[str, str]: + return {"store_cls": LocalStore, "root": str(tmpdir), "log_level": "DEBUG"} + + @pytest.fixture + def store(self, store_kwargs: str | dict[str, Buffer] | None) -> LoggingStore: + return self.store_cls(**store_kwargs) + + def test_store_supports_writes(self, store: LoggingStore) -> None: + assert store.supports_writes + + def test_store_supports_partial_writes(self, store: LoggingStore) -> None: + assert store.supports_partial_writes + + def test_store_supports_listing(self, store: LoggingStore) -> None: + assert store.supports_listing + + def test_store_repr(self, store: LoggingStore) -> None: + assert f"{store!r}" == f"LoggingStore(LocalStore, 'file://{store._store.root.as_posix()}')" + + def test_store_str(self, store: LoggingStore) -> None: + assert str(store) == f"logging-file://{store._store.root.as_posix()}" + + async def test_default_handler(self, local_store, capsys) -> None: + # Store and then remove existing handlers to enter default handler code path + handlers = logging.getLogger().handlers[:] + for h in handlers: + logging.getLogger().removeHandler(h) + # Test logs are sent to stdout + wrapped = LoggingStore(store=local_store) + buffer = default_buffer_prototype().buffer + res = await wrapped.set("foo/bar/c/0", buffer.from_bytes(b"\x01\x02\x03\x04")) + assert res is None + captured = capsys.readouterr() + assert len(captured) == 2 + assert "Calling LocalStore.set" in captured.out + assert "Finished LocalStore.set" in captured.out + # Restore handlers + for h in handlers: + logging.getLogger().addHandler(h) + + def test_is_open_setter_raises(self, store: LoggingStore) -> None: + "Test that a user cannot change `_is_open` without opening the underlying store." + with pytest.raises( + NotImplementedError, match="LoggingStore must be opened via the `_open` method" + ): + store._is_open = True + + @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=["store"]) async def test_logging_store(store: Store, caplog) -> None: wrapped = LoggingStore(store=store, log_level="DEBUG") diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py index 489bcd5a7..7e933548b 100644 --- a/tests/test_store/test_wrapper.py +++ b/tests/test_store/test_wrapper.py @@ -5,13 +5,73 @@ import pytest from zarr.core.buffer.cpu import Buffer, buffer_prototype -from zarr.storage import WrapperStore +from zarr.storage import LocalStore, WrapperStore +from zarr.testing.store import StoreTests if TYPE_CHECKING: + from _pytest.compat import LEGACY_PATH + from zarr.abc.store import Store from zarr.core.buffer.core import BufferPrototype +class TestWrapperStore(StoreTests[WrapperStore, Buffer]): + store_cls = WrapperStore + buffer_cls = Buffer + + async def get(self, store: WrapperStore, key: str) -> Buffer: + return self.buffer_cls.from_bytes((store._store.root / key).read_bytes()) + + async def set(self, store: WrapperStore, key: str, value: Buffer) -> None: + parent = (store._store.root / key).parent + if not parent.exists(): + parent.mkdir(parents=True) + (store._store.root / key).write_bytes(value.to_bytes()) + + @pytest.fixture + def store_kwargs(self, tmpdir: LEGACY_PATH) -> dict[str, str]: + return {"store": LocalStore(str(tmpdir))} + + @pytest.fixture + def open_kwargs(self, tmpdir) -> dict[str, str]: + return {"store_cls": LocalStore, "root": str(tmpdir)} + + def test_store_supports_writes(self, store: WrapperStore) -> None: + assert store.supports_writes + + def test_store_supports_partial_writes(self, store: WrapperStore) -> None: + assert store.supports_partial_writes + + def test_store_supports_listing(self, store: WrapperStore) -> None: + assert store.supports_listing + + def test_store_repr(self, store: WrapperStore) -> None: + assert f"{store!r}" == f"WrapperStore(LocalStore, 'file://{store._store.root.as_posix()}')" + + def test_store_str(self, store: WrapperStore) -> None: + assert str(store) == f"wrapping-file://{store._store.root.as_posix()}" + + def test_check_writeable(self, store: WrapperStore) -> None: + """ + Test _check_writeable() runs without errors. + """ + store._check_writable() + + def test_close(self, store: WrapperStore) -> None: + "Test store can be closed" + store.close() + assert not store._is_open + + def test_is_open_setter_raises(self, store: WrapperStore) -> None: + """ + Test that a user cannot change `_is_open` without opening the underlying store. + """ + with pytest.raises( + NotImplementedError, match="WrapperStore must be opened via the `_open` method" + ): + store._is_open = True + + @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=True) async def test_wrapped_set(store: Store, capsys: pytest.CaptureFixture[str]) -> None: # define a class that prints when it sets