Skip to content

Commit

Permalink
Remove test mark dependency in storage fixtures
Browse files Browse the repository at this point in the history
  • Loading branch information
phoebusm committed Jun 17, 2024
1 parent 9e3a47d commit ae2a0c1
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 26 deletions.
6 changes: 3 additions & 3 deletions python/arcticdb/storage_fixtures/azure.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from .utils import get_ephemeral_port, GracefulProcessUtils, wait_for_server_to_come_up, safer_rmtree, get_ca_cert_for_testing
from arcticc.pb2.storage_pb2 import EnvironmentConfigsMap
from arcticdb.version_store.helper import add_azure_library_to_env
from tests.util.mark import SSL_TEST_ENABLED

# All storage client libraries to be imported on-demand to speed up start-up of ad-hoc test runs
if TYPE_CHECKING:
Expand Down Expand Up @@ -134,18 +133,19 @@ class AzuriteStorageFixtureFactory(StorageFixtureFactory):
enforcing_permissions = False
"""Set to True to create AzureContainer with SAS authentication"""

def __init__(self, port=0, working_dir: Optional[str] = None, use_ssl: bool = True):
def __init__(self, port=0, working_dir: Optional[str] = None, use_ssl: bool = True, ssl_test_support: bool = True):
self.http_protocol = "https" if use_ssl else "http"
self.port = port or get_ephemeral_port(0)
self.endpoint_root = f"{self.http_protocol}://{self.host}:{self.port}"
self.working_dir = str(working_dir) if working_dir else mkdtemp(suffix="AzuriteStorageFixtureFactory")
self.ssl_test_support = ssl_test_support

def __str__(self):
return f"AzuriteStorageFixtureFactory[port={self.port},dir={self.working_dir}]"

def _safe_enter(self):
args = f"{shutil.which('azurite')} --blobPort {self.port} --blobHost {self.host} --queuePort 0 --tablePort 0"
if SSL_TEST_ENABLED:
if self.ssl_test_support:
self.client_cert_dir = self.working_dir
self.ca, self.key_file, self.cert_file, self.client_cert_file = get_ca_cert_for_testing(self.client_cert_dir)
else:
Expand Down
6 changes: 3 additions & 3 deletions python/arcticdb/storage_fixtures/s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
from .utils import get_ephemeral_port, GracefulProcessUtils, wait_for_server_to_come_up, safer_rmtree, get_ca_cert_for_testing
from arcticc.pb2.storage_pb2 import EnvironmentConfigsMap
from arcticdb.version_store.helper import add_s3_library_to_env
from tests.util.mark import SSL_TEST_ENABLED

# All storage client libraries to be imported on-demand to speed up start-up of ad-hoc test runs

Expand Down Expand Up @@ -217,8 +216,9 @@ class MotoS3StorageFixtureFactory(BaseS3StorageFixtureFactory):
_bucket_id = 0
_live_buckets: List[S3Bucket] = []

def __init__(self, use_ssl: bool):
def __init__(self, use_ssl: bool, ssl_test_support: bool):
self.http_protocol = "https" if use_ssl else "http"
self.ssl_test_support = ssl_test_support

@staticmethod
def run_server(port, key_file, cert_file):
Expand Down Expand Up @@ -290,7 +290,7 @@ def _start_server(self):
self._iam_endpoint = f"{self.http_protocol}://localhost:{port}"

self.ssl = self.http_protocol == "https" # In real world, using https protocol doesn't necessarily mean ssl will be verified
if SSL_TEST_ENABLED:
if self.ssl_test_support:
self.ca, self.key_file, self.cert_file, self.client_cert_file = get_ca_cert_for_testing(self.working_dir)
else:
self.ca = ""
Expand Down
13 changes: 6 additions & 7 deletions python/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,11 @@
from arcticdb.storage_fixtures.in_memory import InMemoryStorageFixture
from arcticdb.version_store._normalization import MsgPackNormalizer
from arcticdb.util.test import create_df
from tests.util.mark import (
from .util.mark import (
AZURE_TESTS_MARK,
MONGO_TESTS_MARK,
REAL_S3_TESTS_MARK,
SSL_TEST_ENABLED,
ARCTICDB_USING_CONDA
SSL_TEST_SUPPORTED,
)

# region =================================== Misc. Constants & Setup ====================================
Expand Down Expand Up @@ -114,13 +113,13 @@ def lmdb_library(lmdb_storage, lib_name):
# ssl is enabled by default to maximize test coverage as ssl is enabled most of the times in real world
@pytest.fixture(scope="session")
def s3_storage_factory():
with MotoS3StorageFixtureFactory(use_ssl=SSL_TEST_ENABLED) as f:
with MotoS3StorageFixtureFactory(use_ssl=SSL_TEST_SUPPORTED, ssl_test_support=SSL_TEST_SUPPORTED) as f:
yield f


@pytest.fixture(scope="session")
def s3_no_ssl_storage_factory():
with MotoS3StorageFixtureFactory(use_ssl=False) as f:
with MotoS3StorageFixtureFactory(use_ssl=False, ssl_test_support=SSL_TEST_SUPPORTED) as f:
yield f


Expand Down Expand Up @@ -170,7 +169,7 @@ def real_s3_storage(real_s3_storage_factory):
# ssl cannot be ON by default due to azurite performance constraints https://github.com/man-group/ArcticDB/issues/1539
@pytest.fixture(scope="session")
def azurite_storage_factory():
with AzuriteStorageFixtureFactory(use_ssl=False) as f:
with AzuriteStorageFixtureFactory(use_ssl=False, ssl_test_support=SSL_TEST_SUPPORTED) as f:
yield f


Expand All @@ -182,7 +181,7 @@ def azurite_storage(azurite_storage_factory: AzuriteStorageFixtureFactory):

@pytest.fixture(scope="session")
def azurite_ssl_storage_factory():
with AzuriteStorageFixtureFactory(use_ssl=True) as f:
with AzuriteStorageFixtureFactory(use_ssl=True, ssl_test_support=SSL_TEST_SUPPORTED) as f:
yield f


Expand Down
12 changes: 6 additions & 6 deletions python/tests/integration/arcticdb/test_arctic.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
ArcticInvalidApiUsageException,
)

from tests.util.mark import AZURE_TESTS_MARK, MONGO_TESTS_MARK, REAL_S3_TESTS_MARK, SSL_TESTS_MARK, SSL_TEST_ENABLED
from ...util.mark import AZURE_TESTS_MARK, MONGO_TESTS_MARK, REAL_S3_TESTS_MARK, SSL_TESTS_MARK, SSL_TEST_SUPPORTED

class ParameterDisplayStatus(Enum):
NOT_SHOW = 1
Expand All @@ -52,7 +52,7 @@ def __init__(self, factory):

def edit_connection_string(uri, delimiter, storage, ssl_setting, client_cert_file, client_cert_dir):
# Clear default setting in the uri
if SSL_TEST_ENABLED:
if SSL_TEST_SUPPORTED:
uri = storage.replace_uri_field(uri, ArcticUriFields.CA_PATH, "", start=1, end=3).rstrip(delimiter)
if isinstance(storage, S3Bucket) and "&ssl=" in uri:
uri = storage.replace_uri_field(uri, ArcticUriFields.SSL, "", start=1, end=3).rstrip(delimiter)
Expand All @@ -73,10 +73,10 @@ def edit_connection_string(uri, delimiter, storage, ssl_setting, client_cert_fil
uri += f"{delimiter}CA_cert_dir={storage.factory.client_cert_dir}"
return uri

# s3_storage will become non-ssl if SSL_TEST_ENABLED is False
@pytest.mark.parametrize('client_cert_file', parameter_display_status if SSL_TEST_ENABLED else no_ssl_parameter_display_status)
@pytest.mark.parametrize('client_cert_dir', parameter_display_status if SSL_TEST_ENABLED else no_ssl_parameter_display_status)
@pytest.mark.parametrize('ssl_setting', parameter_display_status if SSL_TEST_ENABLED else no_ssl_parameter_display_status)
# s3_storage will become non-ssl if SSL_TEST_SUPPORTED is False
@pytest.mark.parametrize('client_cert_file', parameter_display_status if SSL_TEST_SUPPORTED else no_ssl_parameter_display_status)
@pytest.mark.parametrize('client_cert_dir', parameter_display_status if SSL_TEST_SUPPORTED else no_ssl_parameter_display_status)
@pytest.mark.parametrize('ssl_setting', parameter_display_status if SSL_TEST_SUPPORTED else no_ssl_parameter_display_status)
def test_s3_verification(monkeypatch, s3_storage, client_cert_file, client_cert_dir, ssl_setting):
storage = s3_storage
# Leaving ca file and ca dir unset will fallback to using os default setting,
Expand Down
1 change: 0 additions & 1 deletion python/tests/integration/arcticdb/test_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from arcticdb import Arctic
from arcticdb_ext.exceptions import StorageException
from arcticdb_ext import set_config_string
from arcticdb.storage_fixtures.s3 import MotoS3StorageFixtureFactory


def test_s3_storage_failures(mock_s3_store_with_error_simulation):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
from arcticdb_ext import set_config_int, unset_config_int
from arcticdb_ext.storage import KeyType, OpenMode
from arcticdb_ext.tools import CompactionId, CompactionLockName
from arcticdb.storage_fixtures.s3 import MotoS3StorageFixtureFactory
from arcticdb_ext.exceptions import InternalException

from multiprocessing import Pool
Expand Down
15 changes: 15 additions & 0 deletions python/tests/integration/storage_fixtures/test_fixture_import.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from pathlib import Path
import subprocess

# Assuming arcticdb wheel has been pre-installed, which is the case in the pipeline
def test_fixture_import(monkeypatch):
script = """
import sys
sys.path = [path for path in sys.path[1:]]
import arcticdb.storage_fixtures.s3
import arcticdb.storage_fixtures.azure
import arcticdb.storage_fixtures.mongo
import arcticdb.storage_fixtures.lmdb
"""
p = subprocess.Popen(["python", "-c", script], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
assert p.wait() == 0, "Failed to import storage_fixtures modules"
6 changes: 3 additions & 3 deletions python/tests/scripts/test_update_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from arcticdb.storage_fixtures.azure import AzureContainer
from arcticdb.storage_fixtures.s3 import S3Bucket
from arcticdb.adapters.s3_library_adapter import USE_AWS_CRED_PROVIDERS_TOKEN
from tests.util.mark import AZURE_TESTS_MARK, SSL_TEST_ENABLED
from ..util.mark import AZURE_TESTS_MARK, SSL_TEST_SUPPORTED

LIB_NAME = "test_lib"

Expand Down Expand Up @@ -105,7 +105,7 @@ def test_upgrade_script_s3_rbac_ok(s3_storage: S3Bucket, monkeypatch):
@AZURE_TESTS_MARK
def test_upgrade_script_dryrun_azure(azurite_storage: AzureContainer):
# Given
if SSL_TEST_ENABLED:
if SSL_TEST_SUPPORTED:
# azurite factory doesn't set client_cert_dir by default
azurite_storage.arctic_uri += f";CA_cert_dir={azurite_storage.factory.client_cert_dir}"
ac = azurite_storage.create_arctic()
Expand All @@ -129,7 +129,7 @@ def test_upgrade_script_dryrun_azure(azurite_storage: AzureContainer):
@AZURE_TESTS_MARK
def test_upgrade_script_azure(azurite_storage: AzureContainer):
# Given
if SSL_TEST_ENABLED:
if SSL_TEST_SUPPORTED:
# azurite factory doesn't set client_cert_dir by default
azurite_storage.arctic_uri += f";CA_cert_dir={azurite_storage.factory.client_cert_dir}"
ac = azurite_storage.create_arctic()
Expand Down
4 changes: 2 additions & 2 deletions python/tests/util/mark.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@

"""Windows and MacOS have different handling of self-signed CA cert for test.
TODO: https://github.com/man-group/ArcticDB/issues/1394"""
SSL_TEST_ENABLED = sys.platform == "linux"
SSL_TEST_SUPPORTED = sys.platform == "linux"


SSL_TESTS_MARK = pytest.mark.skipif(
not SSL_TEST_ENABLED,
not SSL_TEST_SUPPORTED,
reason="When SSL tests are enabled",
)

Expand Down

0 comments on commit ae2a0c1

Please sign in to comment.