Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More fixture robustness improvements #1132

Merged
merged 2 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,14 @@ jobs:
include:
- python_deps_ids: [""]
matrix_override: ${{fromJson(needs.common_config.outputs.linux_matrix)}}
pytest_xdist_mode: "--dist worksteal"
- python3: 6
python_deps_ids: ["", -compat36]
matrix_override:
- ${{fromJson(needs.common_config.outputs.linux_matrix)[0]}}
- python_deps_id: -compat36
python_deps: requirements-compatibility-py36.txt
pytest_xdist_mode: "" # worksteal Not supported on Python 3.6
- python3: 8
python_deps_ids: ["", -compat38]
matrix_override:
Expand All @@ -191,6 +193,7 @@ jobs:
python_deps_ids: ${{toJson(matrix.python_deps_ids)}}
persistent_storage: ${{inputs.persistent_storage}}
compile-override: compile-on-ec2
pytest_xdist_mode: ${{matrix.pytest_xdist_mode}}

cpp-test-windows:
needs: [common_config]
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/build_steps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ on:
python_deps_ids: {default: '[""]', type: string, description: build-python-wheels test matrix parameter. JSON string.}
python3: {default: -1, type: number, description: Python 3 minor version}
persistent_storage: {default: "false", type: string, description: Specifies whether the python tests should tests against real storages e.g. AWS S3 }
pytest_xdist_mode: {default: "", type: string, description: additional argument to pass for pytest-xdist}
jobs:
start_ec2_runner:
if: inputs.compile-override == 'compile-on-ec2'
Expand Down Expand Up @@ -314,6 +315,7 @@ jobs:
# Use the Mongo created in the service container above to test against
CI_MONGO_HOST: mongodb
HYPOTHESIS_PROFILE: ci_${{matrix.os}}
PYTEST_XDIST_MODE: ${{inputs.pytest_xdist_mode}}

- name: Collect crash dumps (Windows)
if: matrix.os == 'windows' && failure()
Expand Down
2 changes: 1 addition & 1 deletion build_tooling/parallel_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ catch=`{ which catchsegv 2>/dev/null || echo ; } | tail -n 1`

export ARCTICDB_RAND_SEED=$RANDOM

$catch python -m pytest -n $splits -v --log-file="$TEST_OUTPUT_DIR/pytest-logger.$group.log" \
$catch python -m pytest -n $splits $PYTEST_XDIST_MODE -v --log-file="$TEST_OUTPUT_DIR/pytest-logger.$group.log" \
--junitxml="$TEST_OUTPUT_DIR/pytest.$group.xml" \
--basetemp="$PARALLEL_TEST_ROOT/temp-pytest-output" \
"$@" 2>&1 | sed -ur "s#^(tests/.*/([^/]+\.py))?#\2#"
4 changes: 4 additions & 0 deletions cpp/arcticdb/version/python_bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,10 @@ void register_bindings(py::module &version, py::exception<arcticdb::ArcticExcept
Allocator::instance()->trim();
},
py::call_guard<SingleThreadMutexHolder>(), "Call trim on the native store's underlining memory allocator")
.def_static("reuse_storage_for_testing",
[](PythonVersionStore& from, PythonVersionStore& to) {
to._test_set_store(from._test_get_store());
})
;

py::class_<ManualClockVersionStore, PythonVersionStore>(version, "ManualClockVersionStore")
Expand Down
5 changes: 4 additions & 1 deletion python/arcticdb/storage_fixtures/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ def _factory_impl(
lib.version.symbol_list = True
apply_lib_cfg(lib, kwargs)
out = ArcticMemoryConfig(cfg, Defaults.ENV)[name]
libs_from_factory[name] = out
suffix = 0
while f"{name}{suffix or ''}" in libs_from_factory:
suffix += 1
libs_from_factory[f"{name}{suffix or ''}"] = out
return out

def create_version_store_factory(self, default_lib_name: str, **defaults) -> Callable[..., NativeVersionStore]:
Expand Down
12 changes: 12 additions & 0 deletions python/arcticdb/storage_fixtures/in_memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from .api import *
from arcticdb.version_store.helper import add_memory_library_to_env
from arcticdb.adapters.in_memory_library_adapter import InMemoryLibraryAdapter
from arcticdb_ext.version_store import PythonVersionStore


class InMemoryStorageFixture(StorageFixture):
Expand All @@ -23,3 +24,14 @@ def create_test_cfg(self, lib_name: str) -> EnvironmentConfigsMap:
cfg = EnvironmentConfigsMap()
add_memory_library_to_env(cfg, lib_name=lib_name, env_name=Defaults.ENV)
return cfg

def _factory_impl(self, *args, **kwargs):
if kwargs.get("reuse_name", False):
default_name = args[2]
name = kwargs.get("name", default_name)
existing = self.libs_from_factory[name]
out = super()._factory_impl(*args, **kwargs)
PythonVersionStore.reuse_storage_for_testing(existing.version_store, out.version_store)
else:
out = super()._factory_impl(*args, **kwargs)
return out
7 changes: 5 additions & 2 deletions python/arcticdb/storage_fixtures/lmdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@ def create_test_cfg(
return cfg

def create_version_store_factory(self, default_lib_name: str, **defaults):
default_lmdb_config = MappingProxyType(defaults.pop("lmdb_config", _DEFAULT_CONFIG))

def create_version_store(
col_per_group: Optional[int] = None, # Legacy option names
row_per_segment: Optional[int] = None,
lmdb_config: Dict[str, Any] = {}, # Mainly to allow setting map_size
lmdb_config: Dict[str, Any] = default_lmdb_config, # Mainly to allow setting map_size
**kwargs,
) -> NativeVersionStore:
if col_per_group is not None and "column_group_size" not in kwargs:
Expand All @@ -72,7 +74,8 @@ def create_version_store(
cfg_factory = self.create_test_cfg
if lmdb_config:
cfg_factory = functools.partial(cfg_factory, lmdb_config=lmdb_config)
return self._factory_impl(self.libs_from_factory, cfg_factory, default_lib_name, **defaults, **kwargs)
combined_args = {**defaults, **kwargs} if defaults else kwargs
return self._factory_impl(self.libs_from_factory, cfg_factory, default_lib_name, **combined_args)

return create_version_store

Expand Down
3 changes: 3 additions & 0 deletions python/arcticdb/storage_fixtures/mongo.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ def __exit__(self, exc_type, exc_value, traceback):

with handle_cleanup_exception(self, "prefix_mongo_database"):
self.client.drop_database("arcticc_" + self.prefix[:-1])
with handle_cleanup_exception(self, "pymongo client", consequence="The test process may never exit"):
self.client.close()
self.client = None

# With Mongo, the LibraryManager configuration database is global/reused across fixtures, so must delete the
# library definitions
Expand Down
13 changes: 12 additions & 1 deletion python/arcticdb/storage_fixtures/s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import json
import os
import re
import sys

import requests
from typing import NamedTuple, Optional, Any, Type

Expand Down Expand Up @@ -204,7 +206,7 @@ def __call__(self, environ, start_response):
ssl_context=None,
)

def _safe_enter(self):
def _start_server(self):
port = self.port = get_ephemeral_port(2)
self.endpoint = f"http://{self.host}:{port}"
self._iam_endpoint = f"http://127.0.0.1:{port}"
Expand All @@ -213,6 +215,15 @@ def _safe_enter(self):
p.start()
wait_for_server_to_come_up(self.endpoint, "moto", p)

def _safe_enter(self):
for attempt in range(3): # For unknown reason, Moto, when running in pytest-xdist, will randomly fail to start
try:
self._start_server()
break
except AssertionError as e: # Thrown by wait_for_server_to_come_up
sys.stderr.write(repr(e))
GracefulProcessUtils.terminate(self._p)

self._s3_admin = self._boto("s3", self.default_key)
return self

Expand Down
2 changes: 0 additions & 2 deletions python/arcticdb/storage_fixtures/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
import os
import platform
import sys
from types import TracebackType

import requests
import signal
import socketserver
Expand Down
9 changes: 9 additions & 0 deletions python/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import pytest
import numpy as np
import pandas as pd
import platform
import random
import re
from datetime import datetime
Expand Down Expand Up @@ -42,6 +43,14 @@
# Use a smaller memory mapped limit for all tests
MsgPackNormalizer.MMAP_DEFAULT_SIZE = 20 * (1 << 20)

if platform.system() == "Linux":
try:
from ctypes import cdll

cdll.LoadLibrary("libSegFault.so")
except:
pass


@pytest.fixture()
def sym(request):
Expand Down
Loading