Merge branch 'client_test_fixture_rework' into 'master'

fix client_test by explicitly passing parameters to each client's factory

See merge request osdu/platform/domain-data-mgmt-services/wellbore/wellbore-domain-services!466

NOTICE

@@ -276,7 +276,7 @@ The following software have components provided under the terms of this license:
 - pydantic (from https://github.com/samuelcolvin/pydantic)
 - pyparsing (from http://pyparsing.wikispaces.com/)
 - pyrsistent (from http://github.com/tobgu/pyrsistent/)
-- pytest (from https://docs.pytest.org/en/latest/)
+- pytest (from http://pytest.org, https://docs.pytest.org/en/latest/)
 - pytest-cov (from https://github.com/pytest-dev/pytest-cov)
 - pytest-httpx (from https://colin-b.github.io/pytest_httpx/)
 - pytest-mock (from https://github.com/pytest-dev/pytest-mock/)

app/clients/__init__.py

@@ -17,7 +17,6 @@ import odes_search
 import odes_storage
 from odes_search.api_client import AsyncSearchApi
 from odes_storage.api_client import AsyncRecordsApi
-from app.conf import Config
 from dataclasses import dataclass
 from typing import Optional
 
@@ -40,26 +39,26 @@ class Limits:
     keepalive_expiry: Optional[float] = 5.0
 
 
-def make_search_client(host) -> SearchServiceClient:
+def make_search_client(*, host, timeout, max_connections=None, max_keepalive_connections=None) -> SearchServiceClient:
     search_client = odes_search.ApiClient(
         host=host,
-        timeout=Config.de_client_config_timeout.value,
+        timeout=timeout,
         limits=Limits(
-            max_connections=Config.de_client_config_max_connection.value or None,
-            max_keepalive_connections=Config.de_client_config_max_keepalive.value or None)
+            max_connections=max_connections,
+            max_keepalive_connections=max_keepalive_connections)
     )
     search_client.add_middleware(middleware=client_middleware)
     search_client.add_middleware(middleware=backoff_middleware)
     return odes_search.AsyncApis(search_client).search_api
 
 
-def make_storage_record_client(host) -> StorageRecordServiceClient:
+def make_storage_record_client(*, host, timeout, max_connections=None, max_keepalive_connections=None) -> StorageRecordServiceClient:
     storage_client = odes_storage.ApiClient(
         host=host,
-        timeout=Config.de_client_config_timeout.value,
+        timeout=timeout,
         limits=Limits(
-            max_connections=Config.de_client_config_max_connection.value or None,
-            max_keepalive_connections=Config.de_client_config_max_keepalive.value or None)
+            max_connections=max_connections,
+            max_keepalive_connections=max_keepalive_connections)
     )
     storage_client.add_middleware(middleware=client_middleware)
     storage_client.add_middleware(middleware=backoff_middleware)

app/injector/main_injector.py

@@ -15,7 +15,7 @@
 from osdu.core.api.storage.blob_storage_base import BlobStorageBase
 from osdu.core.api.storage.blob_storage_local_fs import LocalFSBlobStorage
 
-from app.conf import *
+from app.conf import Config
 from app.helper.logger import get_logger
 
 from .app_injector import AppInjector, AppInjectorModule, WithLifeTime
@@ -131,8 +131,24 @@ class MainInjector(AppInjectorModule):
 
     @staticmethod
     async def build_storage_service_client(host=None, *args, **kwargs) -> StorageRecordServiceClient:
-        return make_storage_record_client(host or Config.service_host_storage.value)
+        if host is None:
+            host = Config.service_host_storage.value
+
+        return make_storage_record_client(
+            host=host,
+            timeout=Config.de_client_config_timeout.value,
+            max_connections=Config.de_client_config_max_connection.value,
+            max_keepalive_connections=Config.de_client_config_max_keepalive.value
+        )
 
     @staticmethod
     async def build_search_service_client(host=None, *args, **kwargs) -> SearchServiceClient:
-        return make_search_client(host or Config.service_host_search.value)
+        if host is None:
+            host = Config.service_host_search.value
+
+        return make_search_client(
+            host=host,
+            timeout=Config.de_client_config_timeout.value,
+            max_connections=Config.de_client_config_max_connection.value,
+            max_keepalive_connections=Config.de_client_config_max_keepalive.value
+        )

tests/unit/clients/client_factory_test.py

@@ -36,13 +36,13 @@ from app.conf import Config
 from tests.unit.test_utils import ctx_fixture
 
 @pytest.mark.asyncio
-async def test_make_storage_client(httpx_mock: HTTPXMock, ctx_fixture):
-    host = 'http://my_host:81234'
-    async with make_storage_record_client(host) as client:
+async def test_make_storage_client(local_dev_config, httpx_mock: HTTPXMock, ctx_fixture):
+    async with make_storage_record_client(host=local_dev_config.service_host_storage.value,
+                                          timeout=local_dev_config.de_client_config_timeout.value) as client:
         assert isinstance(client, StorageRecordServiceClient)
 
         # ensure host
-        assert client.api_client.host == host
+        assert client.api_client.host == local_dev_config.service_host_storage.value
         # using literal here to make config change visible
         assert client.api_client._async_client.timeout == httpx.Timeout(timeout=10)
 
@@ -54,13 +54,13 @@ async def test_make_storage_client(httpx_mock: HTTPXMock, ctx_fixture):
 
 
 @pytest.mark.asyncio
-async def test_make_search_client(httpx_mock: HTTPXMock, ctx_fixture):
-    host = 'http://my_host:81234'
-    async with make_search_client(host) as client:
+async def test_make_search_client(local_dev_config, httpx_mock: HTTPXMock, ctx_fixture):
+    async with make_search_client(host=local_dev_config.service_host_search.value,
+                                  timeout=local_dev_config.de_client_config_timeout.value) as client:
         assert isinstance(client, SearchServiceClient)
 
         # ensure host
-        assert client.api_client.host == host
+        assert client.api_client.host == local_dev_config.service_host_search.value
         assert client.api_client._async_client.timeout == httpx.Timeout(timeout=10)
         get_or_create_ctx()
 

tests/unit/clients/clients_middleware_test.py

@@ -20,8 +20,7 @@ from app.context import Context, get_or_create_ctx
 from tests.unit.test_utils import ctx_fixture
 
 @pytest.mark.asyncio
-async def test_fwd_correlation_id_to_outgoing_request_to_storage(ctx_fixture: Context, httpx_mock: HTTPXMock):
-    storage_url = "http://example.com"  # well formed url required
+async def test_fwd_correlation_id_to_outgoing_request_to_storage(local_dev_config, ctx_fixture: Context, httpx_mock: HTTPXMock):
     expected_correlation_id = 'some-correlation-id'
 
     ctx = ctx_fixture.with_correlation_id(expected_correlation_id).with_auth("foobar")
@@ -30,7 +29,8 @@ async def test_fwd_correlation_id_to_outgoing_request_to_storage(ctx_fixture: Co
     # safety: make sure no methods on tracer have been called yet
     assert ctx.tracer.method_calls == []
 
-    async with make_storage_record_client(storage_url) as storage_client:
+    async with make_storage_record_client(host=local_dev_config.service_host_search.value,
+                                          timeout=local_dev_config.de_client_config_timeout.value) as storage_client:
         httpx_mock.add_response(match_headers={'correlation-id': expected_correlation_id})
         # force to use endpoint which does not return a response to skip model validation
         response = await storage_client.delete_record(id="123", data_partition_id="test")
@@ -43,8 +43,7 @@ async def test_fwd_correlation_id_to_outgoing_request_to_storage(ctx_fixture: Co
     )
 
 @pytest.mark.asyncio
-async def test_fwd_correlation_id_to_outgoing_request_to_search(ctx_fixture: Context, httpx_mock: HTTPXMock):
-    storage_url = "http://example.com"  # well formed url required
+async def test_fwd_correlation_id_to_outgoing_request_to_search(local_dev_config, ctx_fixture: Context, httpx_mock: HTTPXMock):
     expected_correlation_id = 'some-correlation-id'
 
     ctx = ctx_fixture.with_correlation_id(expected_correlation_id).with_auth("foobar")
@@ -53,7 +52,8 @@ async def test_fwd_correlation_id_to_outgoing_request_to_search(ctx_fixture: Con
     # safety: make sure no methods on tracer have been called yet
     assert ctx.tracer.method_calls == []
 
-    async with make_search_client(storage_url) as search_client:
+    async with make_search_client(host=local_dev_config.service_host_search.value,
+                                  timeout=local_dev_config.de_client_config_timeout.value) as search_client:
         httpx_mock.add_response(match_headers={'correlation-id': expected_correlation_id})
         # force to use endpoint which does not return a response to skip model validation
         response = await search_client.delete_index(kind="kind", data_partition_id="test")

tests/unit/fixtures.py

@@ -50,13 +50,14 @@ def mock_storage_client_holding_data(local_dev_config, nope_logger_fixture):
      For usage examples, see fixtures_test.py in this directory
 
      We depend on :
-     - local_dev_config to ha ve a valid configuration, but also avoid doing unexpected network requests
+     - local_dev_config to have a valid configuration, but also avoid doing unexpected network requests
      - nope_logger_fixture because configuring this will mount middlewares, and they need a logger
     """
 
     def setup_data_for_mock(data):
         template_client = make_storage_record_client(
-            local_dev_config.service_host_storage
+            host=local_dev_config.service_host_storage.value,
+            timeout=local_dev_config.de_client_config_timeout.value
         )
 
         # Note: we want to be able to modify the mock to handle get_record and get_record_version specifically

tests/unit/middleware/client_test.py

@@ -13,11 +13,11 @@ from app.injector.main_injector import MainInjector
 
 
 @pytest.mark.asyncio
-async def test_fwd_correlation_id_to_outgoing_request_to_storage(httpx_mock: HTTPXMock, ctx_fixture):
-    storage_url = "http://example.com"  # well formed url required
+async def test_fwd_correlation_id_to_outgoing_request_to_storage(local_dev_config, httpx_mock: HTTPXMock, ctx_fixture):
     expected_correlation_id = 'some-correlation-id'
 
-    async with make_storage_record_client(storage_url) as storage_client:
+    async with make_storage_record_client(host=local_dev_config.service_host_storage.value,
+                                          timeout=local_dev_config.de_client_config_timeout.value) as storage_client:
         httpx_mock.add_response(match_headers={'correlation-id': expected_correlation_id})
 
         ctx_fixture.set_current_with_value(correlation_id=expected_correlation_id)
@@ -28,11 +28,11 @@ async def test_fwd_correlation_id_to_outgoing_request_to_storage(httpx_mock: HTT
 
 
 @pytest.mark.asyncio
-async def test_fwd_correlation_id_to_outgoing_request_to_search(httpx_mock: HTTPXMock, ctx_fixture):
-    storage_url = "http://example.com"  # well formed url required
+async def test_fwd_correlation_id_to_outgoing_request_to_search(local_dev_config, httpx_mock: HTTPXMock, ctx_fixture):
     expected_correlation_id = 'some-correlation-id'
 
-    async with make_search_client(storage_url) as search_client:
+    async with make_search_client(host=local_dev_config.service_host_search.value,
+                                  timeout=local_dev_config.de_client_config_timeout.value) as search_client:
         httpx_mock.add_response(match_headers={'correlation-id': expected_correlation_id})
 
         ctx_fixture.set_current_with_value(correlation_id=expected_correlation_id)
@@ -42,36 +42,14 @@ async def test_fwd_correlation_id_to_outgoing_request_to_search(httpx_mock: HTTP
         assert response is not None
 
 
-@pytest.fixture()
-async def wdms_app_mocked(nope_logger_fixture):
+def test_outgoing_tracing_headers_with_incoming_headers(local_dev_config, app_configurable_with_testclient, httpx_mock):
 
-    from fastapi.testclient import TestClient
-    from app.wdms_app import wdms_app, app_injector
-    from app.clients import StorageRecordServiceClient
-
-    # we do not want dev mode, so that we are able to actually send http requests
-    conf.Config.dev_mode.value = False
-    conf.Config.service_host_search.value = "http://localhost:8888"
-    conf.Config.service_host_storage.value = "http://localhost:9999"
-
-    wdms_app.dependency_overrides[require_opendes_authorized_user] = lambda: True
-    wdms_app.dependency_overrides[require_data_partition_id] = lambda: True
-    wdms_app.trace_exporter = traces.CombinedExporter(service_name='tested-ddms')
-    client = TestClient(wdms_app)
-
-    MainInjector().configure(app_injector)
-
-    yield client
-
-    wdms_app.dependency_overrides = {}
-
-    # explicit close client in teardown
-    storage_client = await app_injector.get(StorageRecordServiceClient)
-    if storage_client is not None:
-        await storage_client.api_client.close()
-
-
-def test_outgoing_tracing_headers_with_incoming_headers(wdms_app_mocked, httpx_mock):
+    app, client = app_configurable_with_testclient(
+        storage_client_mock=make_storage_record_client(host=local_dev_config.service_host_storage.value,
+                                                       timeout=local_dev_config.de_client_config_timeout.value),
+        fake_opendes_authorized_user=True,
+        fake_data_partition_id=True
+    )
 
     version = '00'
     trace_id = '80f22fa582f64d2584e76b4aac231f12'
@@ -96,11 +74,18 @@ def test_outgoing_tracing_headers_with_incoming_headers(wdms_app_mocked, httpx_m
 
     httpx_mock.add_callback(custom_response)
 
-    response = wdms_app_mocked.delete(f'/ddms/v2/logs/123456', headers=input_headers)
+    response = client.delete(f'/ddms/v2/logs/123456', headers=input_headers)
     assert response.status_code == 204
 
 
-def test_outgoing_tracing_headers_without_headers(wdms_app_mocked, httpx_mock):
+def test_outgoing_tracing_headers_without_headers(local_dev_config, app_configurable_with_testclient, httpx_mock):
+
+    app, client = app_configurable_with_testclient(
+        storage_client_mock=make_storage_record_client(host=local_dev_config.service_host_storage.value,
+                                                       timeout=local_dev_config.de_client_config_timeout.value),
+        fake_opendes_authorized_user=True,
+        fake_data_partition_id=True
+    )
 
     def custom_response(request: httpx.Request, *args, **kwargs):
         assert request.headers['traceparent'], "check if tracing header is present"
@@ -116,5 +101,5 @@ def test_outgoing_tracing_headers_without_headers(wdms_app_mocked, httpx_mock):
 
     httpx_mock.add_callback(custom_response)
 
-    response = wdms_app_mocked.delete('/ddms/v2/logs/123456')
+    response = client.delete('/ddms/v2/logs/123456')
     assert response.status_code == 204