diff --git a/.github/workflows/hypothesis.yaml b/.github/workflows/hypothesis.yaml index 1029063ef4..0a320de00b 100644 --- a/.github/workflows/hypothesis.yaml +++ b/.github/workflows/hypothesis.yaml @@ -69,6 +69,12 @@ jobs: path: .hypothesis/ key: cache-hypothesis-${{ runner.os }}-${{ github.run_id }} + - name: Upload coverage + uses: codecov/codecov-action@v5 + with: + token: ${{ secrets.CODECOV_TOKEN }} + verbose: true # optional (default = false) + - name: Generate and publish the report if: | failure() diff --git a/changes/2804.feature.rst b/changes/2804.feature.rst new file mode 100644 index 0000000000..5a707752a0 --- /dev/null +++ b/changes/2804.feature.rst @@ -0,0 +1 @@ +:py:class:`LocalStore` learned to ``delete_dir``. This makes array and group deletes more efficient. diff --git a/pyproject.toml b/pyproject.toml index 6fbcd1991e..ab285ff7ff 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -161,7 +161,7 @@ run = "run-coverage --no-cov" run-pytest = "run" run-verbose = "run-coverage --verbose" run-mypy = "mypy src" -run-hypothesis = "pytest --hypothesis-profile ci tests/test_properties.py tests/test_store/test_stateful*" +run-hypothesis = "run-coverage --hypothesis-profile ci --run-slow-hypothesis tests/test_properties.py tests/test_store/test_stateful*" list-env = "pip list" [tool.hatch.envs.doctest] @@ -398,7 +398,8 @@ filterwarnings = [ "ignore:.*is currently not part in the Zarr format 3 specification.*:UserWarning", ] markers = [ - "gpu: mark a test as requiring CuPy and GPU" + "gpu: mark a test as requiring CuPy and GPU", + "slow_hypothesis: slow hypothesis tests", ] [tool.repo-review] diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index 1defea26b4..b20a601bed 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -208,6 +208,19 @@ async def delete(self, key: str) -> None: else: await asyncio.to_thread(path.unlink, True) # Q: we may want to raise if path is missing + async def delete_dir(self, prefix: str) -> None: + # docstring inherited + self._check_writable() + path = self.root / prefix + if path.is_dir(): + shutil.rmtree(path) + elif path.is_file(): + raise ValueError(f"delete_dir was passed a {prefix=!r} that is a file") + else: + # Non-existent directory + # This path is tested by test_group:test_create_creates_parents for one + pass + async def exists(self, key: str) -> bool: # docstring inherited path = self.root / key diff --git a/src/zarr/testing/stateful.py b/src/zarr/testing/stateful.py index 1a1ef0e3a3..3e8dbcdf04 100644 --- a/src/zarr/testing/stateful.py +++ b/src/zarr/testing/stateful.py @@ -68,6 +68,9 @@ def can_add(self, path: str) -> bool: # -------------------- store operations ----------------------- @rule(name=node_names, data=st.data()) def add_group(self, name: str, data: DataObject) -> None: + # Handle possible case-insensitive file systems (e.g. MacOS) + if isinstance(self.store, LocalStore): + name = name.lower() if self.all_groups: parent = data.draw(st.sampled_from(sorted(self.all_groups)), label="Group parent") else: @@ -90,6 +93,9 @@ def add_array( name: str, array_and_chunks: tuple[np.ndarray[Any, Any], tuple[int, ...]], ) -> None: + # Handle possible case-insensitive file systems (e.g. MacOS) + if isinstance(self.store, LocalStore): + name = name.lower() array, chunks = array_and_chunks fill_value = data.draw(npst.from_dtype(array.dtype)) if self.all_groups: @@ -135,6 +141,7 @@ def add_array( # self.model.rename(from_group, new_path) # self.repo.store.rename(from_group, new_path) + @precondition(lambda self: self.store.supports_deletes) @precondition(lambda self: len(self.all_arrays) >= 1) @rule(data=st.data()) def delete_array_using_del(self, data: DataObject) -> None: @@ -149,6 +156,7 @@ def delete_array_using_del(self, data: DataObject) -> None: del group[array_name] self.all_arrays.remove(array_path) + @precondition(lambda self: self.store.supports_deletes) @precondition(lambda self: len(self.all_groups) >= 2) # fixme don't delete root @rule(data=st.data()) def delete_group_using_del(self, data: DataObject) -> None: @@ -284,6 +292,10 @@ def supports_partial_writes(self) -> bool: def supports_writes(self) -> bool: return self.store.supports_writes + @property + def supports_deletes(self) -> bool: + return self.store.supports_deletes + class ZarrStoreStateMachine(RuleBasedStateMachine): """ " @@ -366,6 +378,7 @@ def get_partial_values(self, data: DataObject) -> None: model_vals_ls, ) + @precondition(lambda self: self.store.supports_deletes) @precondition(lambda self: len(self.model.keys()) > 0) @rule(data=st.data()) def delete(self, data: DataObject) -> None: diff --git a/src/zarr/testing/strategies.py b/src/zarr/testing/strategies.py index 81bb482b06..5722f3c99e 100644 --- a/src/zarr/testing/strategies.py +++ b/src/zarr/testing/strategies.py @@ -1,3 +1,4 @@ +import sys from typing import Any import hypothesis.extra.numpy as npst @@ -209,7 +210,7 @@ def basic_indices(draw: st.DrawFn, *, shape: tuple[int], **kwargs: Any) -> Any: def key_ranges( - keys: SearchStrategy = node_names, max_size: int | None = None + keys: SearchStrategy = node_names, max_size: int = sys.maxsize ) -> SearchStrategy[list[int]]: """ Function to generate key_ranges strategy for get_partial_values() @@ -218,10 +219,14 @@ def key_ranges( [(key, (range_start, range_end)), (key, (range_start, range_end)),...] """ + + def make_request(start: int, length: int) -> RangeByteRequest: + return RangeByteRequest(start, end=min(start + length, max_size)) + byte_ranges = st.builds( - RangeByteRequest, + make_request, start=st.integers(min_value=0, max_value=max_size), - end=st.integers(min_value=0, max_value=max_size), + length=st.integers(min_value=0, max_value=max_size), ) key_tuple = st.tuples(keys, byte_ranges) return st.lists(key_tuple, min_size=1, max_size=10) diff --git a/tests/conftest.py b/tests/conftest.py index e9cd2b8120..9be675cb20 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -147,6 +147,24 @@ def zarr_format(request: pytest.FixtureRequest) -> ZarrFormat: raise ValueError(msg) +def pytest_addoption(parser: Any) -> None: + parser.addoption( + "--run-slow-hypothesis", + action="store_true", + default=False, + help="run slow hypothesis tests", + ) + + +def pytest_collection_modifyitems(config: Any, items: Any) -> None: + if config.getoption("--run-slow-hypothesis"): + return + skip_slow_hyp = pytest.mark.skip(reason="need --run-slow-hypothesis option to run") + for item in items: + if "slow_hypothesis" in item.keywords: + item.add_marker(skip_slow_hyp) + + settings.register_profile( "ci", max_examples=1000, diff --git a/tests/test_store/test_stateful.py b/tests/test_store/test_stateful.py index 637eea882d..63df814ac9 100644 --- a/tests/test_store/test_stateful.py +++ b/tests/test_store/test_stateful.py @@ -1,14 +1,15 @@ # Stateful tests for arbitrary Zarr stores. import pytest from hypothesis.stateful import ( - Settings, run_state_machine_as_test, ) from zarr.abc.store import Store -from zarr.storage import LocalStore, MemoryStore, ZipStore +from zarr.storage import LocalStore, ZipStore from zarr.testing.stateful import ZarrHierarchyStateMachine, ZarrStoreStateMachine +pytestmark = pytest.mark.slow_hypothesis + def test_zarr_hierarchy(sync_store: Store): def mk_test_instance_sync() -> ZarrHierarchyStateMachine: @@ -16,10 +17,8 @@ def mk_test_instance_sync() -> ZarrHierarchyStateMachine: if isinstance(sync_store, ZipStore): pytest.skip(reason="ZipStore does not support delete") - if isinstance(sync_store, MemoryStore): - run_state_machine_as_test( - mk_test_instance_sync, settings=Settings(report_multiple_bugs=False, max_examples=50) - ) + + run_state_machine_as_test(mk_test_instance_sync) def test_zarr_store(sync_store: Store) -> None: @@ -28,11 +27,10 @@ def mk_test_instance_sync() -> None: if isinstance(sync_store, ZipStore): pytest.skip(reason="ZipStore does not support delete") - elif isinstance(sync_store, LocalStore): - pytest.skip(reason="This test has errors") - elif isinstance(sync_store, MemoryStore): - run_state_machine_as_test(mk_test_instance_sync, settings=Settings(max_examples=50)) - else: - run_state_machine_as_test( - mk_test_instance_sync, settings=Settings(report_multiple_bugs=True) - ) + + if isinstance(sync_store, LocalStore): + # This test uses arbitrary keys, which are passed to `set` and `delete`. + # It assumes that `set` and `delete` are the only two operations that modify state. + # But LocalStore, directories can hang around even after a key is delete-d. + pytest.skip(reason="Test isn't suitable for LocalStore.") + run_state_machine_as_test(mk_test_instance_sync)