Skip to content

Commit 53e323e

Browse files
committed
Enhance storage fixture management and add dynamic library fixture; streamline cleanup and improve test reliability
1 parent 981573b commit 53e323e

File tree

9 files changed

+43
-33
lines changed

9 files changed

+43
-33
lines changed

python/arcticdb/storage_fixtures/api.py

+10-4
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,14 @@ def slow_cleanup(self, failure_consequence=""):
132132
self.libs_from_factory.clear()
133133

134134
arctic = self.create_arctic()
135-
for lib in self.libs_names_from_arctic[:]:
135+
libs = set(self.libs_names_from_arctic[:])
136+
with handle_cleanup_exception(self, arctic, consequence=failure_consequence):
137+
# There are some tests that add libraries without using the factory directly (e.g. test_move_storage)
138+
# so make sure that we capture all of the libraries
139+
for lib in arctic.list_libraries():
140+
libs.add(lib)
141+
142+
for lib in libs:
136143
with handle_cleanup_exception(self, lib, consequence=failure_consequence):
137144
arctic.delete_library(lib)
138145

@@ -155,7 +162,7 @@ def replace_uri_field(cls, uri: str, field: ArcticUriFields, replacement: str, s
155162
regex = cls._FIELD_REGEX[field]
156163
match = regex.search(uri)
157164
assert match, f"{uri} does not have {field}"
158-
return f"{uri[:match.start(start)]}{replacement}{uri[match.end(end):]}"
165+
return f"{uri[: match.start(start)]}{replacement}{uri[match.end(end) :]}"
159166

160167

161168
class StorageFixtureFactory(_SaferContextManager):
@@ -188,5 +195,4 @@ def enforcing_permissions_context(self, set_to=True):
188195
self.enforcing_permissions = saved
189196

190197
@abstractmethod
191-
def create_fixture(self) -> StorageFixture:
192-
...
198+
def create_fixture(self) -> StorageFixture: ...

python/tests/conftest.py

+13
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,12 @@ def s3_clean_bucket(s3_storage_factory) -> Generator[S3Bucket, None, None]:
212212
yield f
213213

214214

215+
@pytest.fixture(scope="function")
216+
def azurite_clean_bucket(azurite_storage_factory) -> Generator[S3Bucket, None, None]:
217+
with azurite_storage_factory.create_fixture() as f:
218+
yield f
219+
220+
215221
@pytest.fixture(scope="function")
216222
def nfs_clean_bucket(nfs_backed_s3_storage_factory) -> Generator[NfsS3Bucket, None, None]:
217223
with nfs_backed_s3_storage_factory.create_fixture() as f:
@@ -479,6 +485,13 @@ def arctic_library(arctic_client, lib_name) -> Library:
479485
arctic_client.delete_library(lib_name)
480486

481487

488+
@pytest.fixture
489+
def arctic_library_dynamic(arctic_client, lib_name) -> Library:
490+
lib_opts = LibraryOptions(dynamic_schema=True)
491+
yield arctic_client.create_library(lib_name, library_options=lib_opts)
492+
arctic_client.delete_library(lib_name)
493+
494+
482495
@pytest.fixture
483496
def arctic_library_v1(arctic_client_v1, lib_name) -> Library:
484497
yield arctic_client_v1.create_library(lib_name)

python/tests/integration/arcticdb/test_arctic.py

+2-4
Original file line numberDiff line numberDiff line change
@@ -1157,12 +1157,10 @@ def test_segment_slicing(arctic_client, lib_name):
11571157
assert num_data_segments == math.ceil(rows / rows_per_segment) * math.ceil(columns / columns_per_segment)
11581158

11591159

1160-
# skip this test for now
1161-
@pytest.mark.skip(reason="There is a strange problem with this one TODO")
11621160
@pytest.mark.parametrize("fixture", ["s3_storage", pytest.param("azurite_storage", marks=AZURE_TESTS_MARK)])
1163-
def test_reload_symbol_list(fixture, request, lib_name):
1161+
def test_reload_symbol_list(fixture, request):
11641162
storage_fixture: StorageFixture = request.getfixturevalue(fixture)
1165-
lib_name = f"{lib_name}_reload_symbol_list"
1163+
lib_name = "test_reload_symbol_list"
11661164

11671165
def get_symbol_list_keys(lib_name):
11681166
keys = storage_fixture.iter_underlying_object_names()

python/tests/integration/arcticdb/test_arctic_move_storage.py

-4
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,6 @@ def test_move_storage(storage_type, host_attr, request):
4242
# When - we copy the underlying objects without using the Arctic API
4343
source_storage.copy_underlying_objects_to(dest_storage)
4444

45-
# TODO: FIX THIS
46-
ac.delete_library("lib")
47-
4845
# When - we leave the source_storage context, the storage should have been cleaned up
4946
assert not source_storage.create_arctic().list_libraries()
5047

@@ -55,7 +52,6 @@ def test_move_storage(storage_type, host_attr, request):
5552
assert lib.list_symbols() == ["sym"]
5653
assert_frame_equal(df, lib.read("sym").data)
5754
assert dest_host in lib.read("sym").host
58-
new_ac.delete_library("lib")
5955

6056

6157
@pytest.mark.skipif(sys.platform == "darwin", reason="Test doesn't raise an exception on MacOS ARM")

python/tests/integration/arcticdb/version_store/test_basic_version_store.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -2490,7 +2490,7 @@ def test_missing_first_version_key_batch(basic_store):
24902490

24912491
write_times.append(pd.Timestamp(vit.timestamp))
24922492
expected.append(df1)
2493-
# time.sleep(1)
2493+
24942494
df2 = pd.DataFrame(
24952495
{"d": range(x + 1, l + x + 1), "e": range(x + 2, l + x + 2), "f": range(x + 3, l + x + 3)}, index=idx
24962496
)

python/tests/stress/arcticdb/version_store/test_large_df.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import numpy as np
1414
import pandas as pd
1515

16+
from tests.util.mark import SLOW_TESTS_MARK
17+
1618
from arcticdb.util.test import (
1719
assert_frame_equal,
1820
dataframe_for_date,
@@ -63,7 +65,7 @@ def test_write_and_update_large_df_in_chunks(lmdb_version_store_very_big_map):
6365
assert_frame_equal(lib.head(symbol).data, expected_head)
6466

6567

66-
@pytest.mark.skip(reason="Uses too much storage and fails on GitHub runners")
68+
@SLOW_TESTS_MARK
6769
def test_write_large_df_in_chunks(lmdb_version_store_big_map):
6870
symbol = "symbol"
6971
lib = lmdb_version_store_big_map

python/tests/stress/arcticdb/version_store/test_mem_leaks.py

+4-5
Original file line numberDiff line numberDiff line change
@@ -325,11 +325,10 @@ def gen_random_date(start: pd.Timestamp, end: pd.Timestamp):
325325

326326
@SLOW_TESTS_MARK
327327
@SKIP_CONDA_MARK # Conda CI runner doesn't have enough storage to perform these stress tests
328-
# @pytest.mark.skipif(
329-
# WINDOWS, reason="Not enough storage on Windows runners, due to large Win OS footprint and less free mem"
330-
# )
331-
# @pytest.mark.skipif(MACOS, reason="Problem on MacOs most probably similar to WINDOWS")
332-
@pytest.mark.skip(reason="This test is not conclusive for memory leaks and is very flaky")
328+
@pytest.mark.skipif(
329+
WINDOWS, reason="Not enough storage on Windows runners, due to large Win OS footprint and less free mem"
330+
)
331+
@pytest.mark.skipif(MACOS, reason="Problem on MacOs most probably similar to WINDOWS")
333332
def test_mem_leak_read_all_arctic_lib(arctic_library_lmdb_100gb):
334333
lib: adb.Library = arctic_library_lmdb_100gb
335334

python/tests/stress/arcticdb/version_store/test_stress_delete.py

+6-4
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,9 @@ def test_stress_delete(object_store_factory):
5050
print("Delete took {}".format(datetime.now() - start_time))
5151

5252
# Make sure that the symbols are deleted
53-
with pytest.raises(NoDataFoundException) as e:
54-
lib1.batch_read(syms)
53+
for x in range(num_tests):
54+
with pytest.raises(NoDataFoundException) as e:
55+
lib1.read(f"symbol_{x}")
5556

5657
res = lib2.batch_read(syms)
5758
for i, sym in enumerate(syms):
@@ -63,5 +64,6 @@ def test_stress_delete(object_store_factory):
6364
check_no_keys(lib2)
6465

6566
# Make sure that the symbols are deleted
66-
with pytest.raises(NoDataFoundException) as e:
67-
lib2.batch_read(syms)
67+
for x in range(num_tests):
68+
with pytest.raises(NoDataFoundException) as e:
69+
lib2.read(f"symbol_{x}")

python/tests/unit/arcticdb/version_store/test_sort.py

+4-10
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,8 @@ def test_stage_finalize(arctic_library):
4141
pd.testing.assert_frame_equal(result, expected)
4242

4343

44-
def create_lib_dynamic(ac, lib_name):
45-
lib_opts = adb.LibraryOptions(dynamic_schema=True)
46-
return ac.get_library(lib_name, create_if_missing=True, library_options=lib_opts)
47-
48-
49-
def test_stage_finalize_dynamic(arctic_client, lib_name):
50-
arctic_library = create_lib_dynamic(arctic_client, lib_name)
44+
def test_stage_finalize_dynamic(arctic_library_dynamic):
45+
arctic_library = arctic_library_dynamic
5146
symbol = "AAPL"
5247
sort_cols = ["timestamp", "col1"]
5348

@@ -79,7 +74,6 @@ def test_stage_finalize_dynamic(arctic_client, lib_name):
7974

8075
expected = pd.concat([df1, df2]).sort_values(sort_cols)
8176
pd.testing.assert_frame_equal(result, expected)
82-
arctic_client.delete_library(lib_name)
8377

8478

8579
def random_strings(count, max_length):
@@ -124,8 +118,8 @@ def test_stage_finalize_strings(arctic_library):
124118
pd.testing.assert_frame_equal(result, expected)
125119

126120

127-
def test_stage_finalize_strings_dynamic(arctic_client, lib_name):
128-
arctic_library = create_lib_dynamic(arctic_client, lib_name)
121+
def test_stage_finalize_strings_dynamic(arctic_library_dynamic):
122+
arctic_library = arctic_library_dynamic
129123
symbol = "AAPL"
130124
sort_cols = ["timestamp", "col1"]
131125

0 commit comments

Comments
 (0)