From 9de9cceda67bea51a0ef136bad3ca6dc45c13065 Mon Sep 17 00:00:00 2001 From: Padraic Shafer <76011594+padraic-shafer@users.noreply.github.com> Date: Fri, 15 Mar 2024 10:26:27 -0700 Subject: [PATCH] Improve resource management in tests (#678) * Use file context managers in tests * File buffer in tests and cleanup are now in a fixture * Tests use resource managed file buffer fixture * Close Zarr array file in test --- tiled/_tests/conftest.py | 8 ++++++++ tiled/_tests/test_access_control.py | 4 +--- tiled/_tests/test_array.py | 6 ++---- tiled/_tests/test_awkward.py | 5 ++--- tiled/_tests/test_container_fields.py | 24 +++++++++--------------- tiled/_tests/test_container_files.py | 12 +++++------- tiled/_tests/test_export.py | 16 +++++----------- tiled/_tests/test_hdf5.py | 20 ++++++++++---------- tiled/_tests/test_writing.py | 4 +--- 9 files changed, 43 insertions(+), 56 deletions(-) diff --git a/tiled/_tests/conftest.py b/tiled/_tests/conftest.py index 921abd0a3..8ef04a21c 100644 --- a/tiled/_tests/conftest.py +++ b/tiled/_tests/conftest.py @@ -1,3 +1,4 @@ +import io import os import sys import tempfile @@ -49,6 +50,13 @@ def set_tiled_cache_dir(): del os.environ["TILED_CACHE_DIR"] +@pytest.fixture(scope="function") +def buffer(): + "Generate a temporary buffer for testing file export + re-import." + with io.BytesIO() as buffer: + yield buffer + + @pytest.fixture def tmp_profiles_dir(): """ diff --git a/tiled/_tests/test_access_control.py b/tiled/_tests/test_access_control.py index 28c67c430..0601289e0 100644 --- a/tiled/_tests/test_access_control.py +++ b/tiled/_tests/test_access_control.py @@ -1,4 +1,3 @@ -import io import json import numpy @@ -180,11 +179,10 @@ def test_access_control_with_api_key_auth(context, enter_password): context.api_key = None -def test_node_export(enter_password, context): +def test_node_export(enter_password, context, buffer): "Exporting a node should include only the children we can see." with enter_password("secret1"): alice_client = from_context(context, username="alice") - buffer = io.BytesIO() alice_client.export(buffer, format="application/json") alice_client.logout() buffer.seek(0) diff --git a/tiled/_tests/test_array.py b/tiled/_tests/test_array.py index e2acd556b..8ac6488a7 100644 --- a/tiled/_tests/test_array.py +++ b/tiled/_tests/test_array.py @@ -122,10 +122,8 @@ def test_nan_infinity_handler(tmpdir, context): def strict_parse_constant(c): raise ValueError(f"{c} is not valid JSON") - open_json = json.load( - open(Path(tmpdir, "testjson", "test.json"), "r"), - parse_constant=strict_parse_constant, - ) + with open(Path(tmpdir, "testjson", "test.json"), "r") as json_file: + open_json = json.load(json_file, parse_constant=strict_parse_constant) expected_list = [0.0, 1.0, None, None, None] assert open_json == expected_list diff --git a/tiled/_tests/test_awkward.py b/tiled/_tests/test_awkward.py index 17c936fa4..3e3d67871 100644 --- a/tiled/_tests/test_awkward.py +++ b/tiled/_tests/test_awkward.py @@ -1,4 +1,3 @@ -import io import json import awkward @@ -70,7 +69,7 @@ def test_slicing(client): assert sliced_response_size < full_response_size -def test_export_json(client): +def test_export_json(client, buffer): # Write data into catalog. It will be stored as directory of buffers # named like 'node0-offsets' and 'node2-data'. array = awkward.Array( @@ -82,7 +81,7 @@ def test_export_json(client): ) aac = client.write_awkward(array, key="test") - file = io.BytesIO() + file = buffer aac.export(file, format="application/json") actual = bytes(file.getbuffer()).decode() assert actual == awkward.to_json(array) diff --git a/tiled/_tests/test_container_fields.py b/tiled/_tests/test_container_fields.py index cd92768ef..5fbe0f36d 100644 --- a/tiled/_tests/test_container_fields.py +++ b/tiled/_tests/test_container_fields.py @@ -1,5 +1,3 @@ -import io - import anyio import h5py import pandas @@ -34,10 +32,9 @@ def example_data_dir(tmpdir_factory): @pytest.mark.parametrize("fields", (None, (), ("a", "b"))) @pytest.mark.parametrize("client", ("example_data_dir",), indirect=True) -def test_directory_fields(client, fields): +def test_directory_fields(client, fields, buffer): "Export selected fields (files) from a directory via /container/full." url_path = client.item["links"]["full"] - buffer = io.BytesIO() with record_history() as history: client.export(buffer, fields=fields, format="application/x-hdf5") @@ -58,11 +55,10 @@ def excel_data_dir(tmpdir_factory): @pytest.mark.parametrize("fields", (None, (), ("Sheet 1", "Sheet 10"))) @pytest.mark.parametrize("client", ("excel_data_dir",), indirect=True) -def test_excel_fields(client, fields): +def test_excel_fields(client, fields, buffer): "Export selected fields (sheets) from an Excel file via /container/full." client = client["spreadsheet"] url_path = client.item["links"]["full"] - buffer = io.BytesIO() with record_history() as history: client.export(buffer, fields=fields, format="application/x-hdf5") # TODO: Enable container to export XLSX if all nodes are tables? @@ -82,19 +78,18 @@ def mark_xfail(value, unsupported="UNSPECIFIED ADAPTER"): def zarr_data_dir(tmpdir_factory): "Generate a temporary Zarr group file with multiple datasets." tmpdir = tmpdir_factory.mktemp("zarr_files") - root = zarr.open(str(tmpdir / "zarr_group.zarr"), "w") - for i, name in enumerate("abcde"): - root.create_dataset(name, data=range(i, i + 3)) + with zarr.open(str(tmpdir / "zarr_group.zarr"), "w") as root: + for i, name in enumerate("abcde"): + root.create_dataset(name, data=range(i, i + 3)) return tmpdir @pytest.mark.parametrize("fields", (None, (), mark_xfail(("b", "d"), "Zarr"))) @pytest.mark.parametrize("client", ("zarr_data_dir",), indirect=True) -def test_zarr_group_fields(client, fields): +def test_zarr_group_fields(client, fields, buffer): "Export selected fields (Datasets) from a Zarr group via /container/full." client = client["zarr_group"] url_path = client.item["links"]["full"] - buffer = io.BytesIO() with record_history() as history: client.export(buffer, fields=fields, format="application/x-hdf5") @@ -117,11 +112,10 @@ def hdf5_data_dir(tmpdir_factory): "fields", (None, (), mark_xfail(("x",), "HDF5"), mark_xfail(("g",), "HDF5")) ) @pytest.mark.parametrize("client", ("hdf5_data_dir",), indirect=True) -def test_hdf5_fields(client, fields): +def test_hdf5_fields(client, fields, buffer): "Export selected fields (array/group) from a HDF5 file via /container/full." client = client["hdf5_example"] url_path = client.item["links"]["full"] - buffer = io.BytesIO() with record_history() as history: client.export(buffer, fields=fields, format="application/x-hdf5") @@ -138,7 +132,7 @@ def assert_single_request_to_url(history, url_path): def assert_requested_fields_fetched(buffer, fields, client): "Only the requested fields were fetched." - file = h5py.File(buffer) - actual_fields = set(file.keys()) + with h5py.File(buffer) as file: + actual_fields = set(file.keys()) expected = set(fields or client.keys()) # By default all fields were fetched assert actual_fields == expected diff --git a/tiled/_tests/test_container_files.py b/tiled/_tests/test_container_files.py index 150e6d370..defa1e9c7 100644 --- a/tiled/_tests/test_container_files.py +++ b/tiled/_tests/test_container_files.py @@ -1,5 +1,3 @@ -import io - import h5py import pandas import pytest @@ -33,13 +31,14 @@ async def test_zarr_array(tmpdir): await register(client, tmpdir) tree(client) client["za"].read() + z.store.close() @pytest.mark.asyncio async def test_zarr_group(tmpdir): - root = zarr.open(str(tmpdir / "zg.zarr"), "w") - root.create_dataset("x", data=[1, 2, 3]) - root.create_dataset("y", data=[4, 5, 6]) + with zarr.open(str(tmpdir / "zg.zarr"), "w") as root: + root.create_dataset("x", data=[1, 2, 3]) + root.create_dataset("y", data=[4, 5, 6]) catalog = in_memory(readable_storage=[tmpdir]) with Context.from_app(build_app(catalog)) as context: client = from_context(context) @@ -51,7 +50,7 @@ async def test_zarr_group(tmpdir): @pytest.mark.asyncio -async def test_hdf5(tmpdir): +async def test_hdf5(tmpdir, buffer): with h5py.File(str(tmpdir / "h.h5"), "w") as file: file["x"] = [1, 2, 3] group = file.create_group("g") @@ -64,5 +63,4 @@ async def test_hdf5(tmpdir): client["h"]["x"].read() client["h"]["g"]["y"].read() - buffer = io.BytesIO() client.export(buffer, format="application/json") diff --git a/tiled/_tests/test_export.py b/tiled/_tests/test_export.py index 22d4c979b..e4d866473 100644 --- a/tiled/_tests/test_export.py +++ b/tiled/_tests/test_export.py @@ -1,4 +1,3 @@ -import io import json from pathlib import Path @@ -87,9 +86,8 @@ def test_export_table(client, filename, tmpdir): client["C"].export(Path(tmpdir, filename)) -def test_streaming_export(client): +def test_streaming_export(client, buffer): "The application/json-seq format is streamed via a generator." - buffer = io.BytesIO() client["C"].export(buffer, format="application/json-seq") # Verify that output is valid newline-delimited JSON. buffer.seek(0) @@ -99,27 +97,24 @@ def test_streaming_export(client): json.loads(line) -def test_streaming_export_empty(client): +def test_streaming_export_empty(client, buffer): "The application/json-seq format is streamed via a generator." - buffer = io.BytesIO() client["empty_table"].export(buffer, format="application/json-seq") buffer.seek(0) assert buffer.read() == b"" -def test_export_weather_data_var(client, tmpdir): - buffer = io.BytesIO() +def test_export_weather_data_var(client, tmpdir, buffer): client["structured_data"]["weather"]["temperature"].export( buffer, slice=(0,), format="text/csv" ) -def test_export_weather_all(client): - buffer = io.BytesIO() +def test_export_weather_all(client, buffer): client["structured_data"]["weather"].export(buffer, format="application/x-hdf5") -def test_serialization_error_hdf5_metadata(client): +def test_serialization_error_hdf5_metadata(client, buffer): tree = MapAdapter( { "good": MapAdapter({}, metadata={"a": 1}), @@ -128,7 +123,6 @@ def test_serialization_error_hdf5_metadata(client): ) with Context.from_app(build_app(tree)) as context: client = from_context(context) - buffer = io.BytesIO() client["good"].export(buffer, format="application/x-hdf5") with pytest.raises(ClientError, match="contains types or structure"): client["bad"].export(buffer, format="application/x-hdf5") diff --git a/tiled/_tests/test_hdf5.py b/tiled/_tests/test_hdf5.py index ff6c0f66c..6067a9f15 100644 --- a/tiled/_tests/test_hdf5.py +++ b/tiled/_tests/test_hdf5.py @@ -19,7 +19,9 @@ def example_file(): b = a.create_group("b") c = b.create_group("c") c.create_dataset("d", data=numpy.ones((3, 3))) - return file + yield file + + file.close() @pytest.fixture @@ -34,10 +36,12 @@ def example_file_with_vlen_str_in_dataset(): dset = c.create_dataset("d", (100,), dtype=dt) assert dset.dtype == "object" dset[0] = b"test" - return file + yield file + + file.close() -def test_from_file(example_file): +def test_from_file(example_file, buffer): """Serve a single HDF5 file at top level.""" h5py = pytest.importorskip("h5py") tree = HDF5Adapter(example_file) @@ -45,13 +49,12 @@ def test_from_file(example_file): client = from_context(context) arr = client["a"]["b"]["c"]["d"].read() assert isinstance(arr, numpy.ndarray) - buffer = io.BytesIO() client.export(buffer, format="application/x-hdf5") file = h5py.File(buffer, "r") file["a"]["b"]["c"]["d"] -def test_from_file_with_vlen_str_dataset(example_file_with_vlen_str_in_dataset): +def test_from_file_with_vlen_str_dataset(example_file_with_vlen_str_in_dataset, buffer): """Serve a single HDF5 file at top level.""" h5py = pytest.importorskip("h5py") tree = HDF5Adapter(example_file_with_vlen_str_in_dataset) @@ -60,14 +63,13 @@ def test_from_file_with_vlen_str_dataset(example_file_with_vlen_str_in_dataset): client = from_context(context) arr = client["a"]["b"]["c"]["d"].read() assert isinstance(arr, numpy.ndarray) - buffer = io.BytesIO() with pytest.warns(UserWarning): client.export(buffer, format="application/x-hdf5") file = h5py.File(buffer, "r") file["a"]["b"]["c"]["d"] -def test_from_group(example_file): +def test_from_group(example_file, buffer): """Serve a Group within an HDF5 file.""" h5py = pytest.importorskip("h5py") tree = HDF5Adapter(example_file["a"]["b"]) @@ -75,13 +77,12 @@ def test_from_group(example_file): client = from_context(context) arr = client["c"]["d"].read() assert isinstance(arr, numpy.ndarray) - buffer = io.BytesIO() client.export(buffer, format="application/x-hdf5") file = h5py.File(buffer, "r") file["c"]["d"] -def test_from_multiple(example_file): +def test_from_multiple(example_file, buffer): """Serve two files within a mapping.""" h5py = pytest.importorskip("h5py") tree = MapAdapter( @@ -96,7 +97,6 @@ def test_from_multiple(example_file): assert isinstance(arr_A, numpy.ndarray) arr_B = client["B"]["a"]["b"]["c"]["d"].read() assert isinstance(arr_B, numpy.ndarray) - buffer = io.BytesIO() client.export(buffer, format="application/x-hdf5") file = h5py.File(buffer, "r") file["A"]["a"]["b"]["c"]["d"] diff --git a/tiled/_tests/test_writing.py b/tiled/_tests/test_writing.py index 83813c757..6c7c37cce 100644 --- a/tiled/_tests/test_writing.py +++ b/tiled/_tests/test_writing.py @@ -4,7 +4,6 @@ Persistent stores are being developed externally to the tiled package. """ import base64 -import io from datetime import datetime import awkward @@ -443,11 +442,10 @@ async def test_bytes_in_metadata(tree): @pytest.mark.asyncio -async def test_container_export(tree): +async def test_container_export(tree, buffer): with Context.from_app(build_app(tree)) as context: client = from_context(context) a = client.create_container("a") a.write_array([1, 2, 3], key="b") - buffer = io.BytesIO() client.export(buffer, format="application/json")