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

Feature/sqlite repo #71

Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
fb8da82
Checks for correct signature used by the implementation class.
codecakes Aug 12, 2024
dbf8df1
Implements signatures for functional and decoupled GeolocationQuerySe…
codecakes Aug 12, 2024
bfa02f9
adds make test
codecakes Aug 12, 2024
d5ff73a
Merge branch 'main' into feature-location-impl
codecakes Aug 17, 2024
a55413c
ensure signature type checks for interface implementations
codecakes Aug 21, 2024
e288445
refactoring project structure to match a more decoupled logic away fr…
codecakes Aug 21, 2024
d1c733f
added todos command to check all TODOs in project. updated repository…
codecakes Aug 25, 2024
ad6805e
refactored code to adhere to interface requirements and implement mis…
codecakes Aug 25, 2024
b296d89
Merge branch 'main' into feature-location-impl
codecakes Aug 25, 2024
e99d741
update libraries for adding sqlite orm support
codecakes Aug 30, 2024
59fbde7
Merge branch 'main' into feature-location-impl
codecakes Aug 30, 2024
4f24bd9
added support libraries
codecakes Sep 2, 2024
ef85176
implements configure_database_session(services, settings) and on_start
codecakes Sep 2, 2024
3d3ff7c
Merge branch 'main' into feature/sqlite-repo
codecakes Sep 2, 2024
962a1da
Merge branch 'main' into feature/sqlite-repo
codecakes Sep 3, 2024
916defe
refactored unit tests to have dummy functions move to conftest. fix t…
codecakes Sep 7, 2024
22e14e3
refactors dummy tests, removed to conftest. make test works. added te…
codecakes Sep 9, 2024
00d5884
Merge branch 'main' into feature/sqlite-repo
codecakes Sep 9, 2024
90cb943
fix typo
codecakes Sep 9, 2024
a180a72
updated how tests are run to include make test which sets necessary e…
codecakes Sep 9, 2024
a7d58c9
Merge remote-tracking branch 'ssh' into feature/sqlite-repo
codecakes Sep 9, 2024
5d79824
added changes to support api and integration tests
codecakes Sep 9, 2024
c576c43
added sqlite models to support sqlite database. database.py enhances …
codecakes Sep 9, 2024
fca13b0
removed obsolete trigger functions
codecakes Sep 9, 2024
c8f23f9
major rewrite of test services for integration test for Sqlite Repo D…
codecakes Sep 9, 2024
39bbd27
Merge branch 'main' into feature/sqlite-repo
codecakes Sep 9, 2024
5ddf9ad
refactored into setUpTestDatabase class for DRY
codecakes Sep 11, 2024
3d15a1f
WIP test_fetch_facilities integration test
codecakes Sep 11, 2024
87265fd
Feature/sqlite repo: setup for GeoLocationServiceSqlRepoDBTest (#68)
codecakes Sep 9, 2024
fbb2c0a
Merge branch 'main' into feature/sqlite-repo
codecakes Sep 11, 2024
0dd37fb
added docstrings, TODO explainer and specific exception
codecakes Sep 13, 2024
a6f45ed
Merge branch 'main' into feature/sqlite-repo
codecakes Sep 14, 2024
938b62f
Fix: Removed commented out line in pyproject.toml and setup project i…
JiyaGupta-cs Sep 17, 2024
210d0c6
updated promotion product messaging
codecakes Sep 20, 2024
e19185b
adds docker support for integration testing and running containerized…
codecakes Sep 20, 2024
71f5c43
these changes reflect a working spatialite extension to sqlite. the t…
codecakes Sep 22, 2024
b98428a
WIP: Feature/fix infra spatialite (#79)
codecakes Sep 22, 2024
4354017
Merge branch 'main' into feature/sqlite-repo
codecakes Sep 22, 2024
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
67 changes: 57 additions & 10 deletions xcov19/tests/start_server.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,27 @@
from collections.abc import AsyncGenerator
from blacksheep import Application
from contextlib import asynccontextmanager
from contextlib import AsyncExitStack, asynccontextmanager
from rodi import Container, ContainerProtocol
from xcov19.app.database import configure_database_session, setup_database
from xcov19.app.database import (
configure_database_session,
setup_database,
start_db_session,
)
from xcov19.app.settings import load_settings
from sqlalchemy.ext.asyncio import AsyncEngine
from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession
from sqlmodel.ext.asyncio.session import AsyncSession as AsyncSessionWrapper


class InvalidSessionTypeError(RuntimeError):
"""Exception raised when the session is not of the expected type."""

pass


class InvalidDIContainerTypeError(RuntimeError):
"""Exception raised when valid DI container not found."""

pass


@asynccontextmanager
Expand All @@ -18,10 +35,40 @@ async def start_server(app: Application) -> AsyncGenerator[Application, None]:
await app.stop()


async def start_test_database(container: ContainerProtocol) -> None:
"""Database setup for integration tests."""
if not isinstance(container, Container):
raise RuntimeError("container not of type Container.")
configure_database_session(container, load_settings())
engine = container.resolve(AsyncEngine)
await setup_database(engine)
class setUpTestDatabase:
"""Manages the lifecycle of the test database."""

def __init__(self) -> None:
self._stack = AsyncExitStack()
self._session: AsyncSession | AsyncSessionWrapper | None = None
self._container: ContainerProtocol = Container()

async def setup_test_database(self) -> None:
"""Database setup for integration tests."""
if not isinstance(self._container, Container):
raise InvalidDIContainerTypeError("Container not of valid type.")
configure_database_session(self._container, load_settings())
engine = self._container.resolve(AsyncEngine)
await setup_database(engine)

async def start_async_session(self) -> AsyncSession | AsyncSessionWrapper:
if not isinstance(self._container, Container):
raise InvalidDIContainerTypeError("Container not of valid type.")
self._session = await self._stack.enter_async_context(
start_db_session(self._container)
)
if not isinstance(self._session, AsyncSessionWrapper):
raise InvalidSessionTypeError(
f"{self._session} is not a AsyncSessionWrapper value."
)
return self._session

async def aclose(self) -> None:
print("async closing test server db session closing.")
if not isinstance(self._session, AsyncSessionWrapper):
raise InvalidSessionTypeError(
f"{self._session} is not a AsyncSessionWrapper value."
)
await self._session.commit()
await self._stack.aclose()
print("async test server closing.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Good refactoring of test database setup

The new setUpTestDatabase class improves the organization of test database setup and teardown. Consider adding more error handling and logging to improve debugging of test setup issues.

class SetUpTestDatabase:
    """Manages the lifecycle of the test database."""

    def __init__(self) -> None:
        self._stack = AsyncExitStack()
        self._session: AsyncSession | AsyncSessionWrapper | None = None
        self._container: ContainerProtocol = Container()
        self._logger = logging.getLogger(__name__)

    async def setup_test_database(self) -> None:
        try:
            if not isinstance(self._container, Container):
                raise InvalidDIContainerTypeError("Container not of valid type.")
            configure_database_session(self._container, load_settings())
            engine = self._container.resolve(AsyncEngine)
            await setup_database(engine)
        except Exception as e:
            self._logger.error(f"Error setting up test database: {e}")
            raise

2 changes: 1 addition & 1 deletion xcov19/tests/test_geolocation_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from blacksheep import Content, Response


@pytest.mark.integration
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (testing): Changed test marker from integration to api

Ensure that this change in test categorization is intentional and consistent with your testing strategy. Update any test runners or CI/CD pipelines that might rely on these markers.

@pytest.mark.api
@pytest.mark.usefixtures("client")
class TestGeolocationAPI:
async def test_location_query_endpoint(self, client):
Expand Down
55 changes: 29 additions & 26 deletions xcov19/tests/test_services.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
from collections.abc import Callable
from contextlib import AsyncExitStack
from typing import List
import pytest
import unittest
import random


from rodi import Container, ContainerProtocol
from xcov19.app.database import start_db_session
from xcov19.tests.data.seed_db import seed_data
from xcov19.tests.start_server import start_test_database
from xcov19.tests.start_server import setUpTestDatabase
from xcov19.domain.models.provider import (
Contact,
FacilityEstablishment,
Expand All @@ -24,9 +23,6 @@


from xcov19.utils.mixins import InterfaceProtocolCheckMixin

import random

from sqlmodel.ext.asyncio.session import AsyncSession as AsyncSessionWrapper


Expand Down Expand Up @@ -201,37 +197,44 @@ class GeoLocationServiceSqlRepoDBTest(unittest.IsolatedAsyncioTestCase):
3. patient_query_lookup_svc is configured to call sqlite repository.
"""

@pytest.fixture(autouse=True)
def autouse(
self,
dummy_geolocation_query_json: LocationQueryJSON,
dummy_reverse_geo_lookup_svc: Callable[[LocationQueryJSON], dict],
):
self.dummy_geolocation_query_json = dummy_geolocation_query_json
self.dummy_reverse_geo_lookup_svc = dummy_reverse_geo_lookup_svc

async def asyncSetUp(self) -> None:
self._stack = AsyncExitStack()
container: ContainerProtocol = Container()
await start_test_database(container)
self._session = await self._stack.enter_async_context(
start_db_session(container)
)
if not isinstance(self._session, AsyncSessionWrapper):
raise RuntimeError(f"{self._session} is not a AsyncSessionWrapper value.")
self._test_db = setUpTestDatabase()
await self._test_db.setup_test_database()
self._session = await self._test_db.start_async_session()
assert isinstance(self._session, AsyncSessionWrapper)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Improved test database setup

The new setup is more concise and uses a dedicated class for database setup. Consider adding error handling for the case where the session is not an AsyncSessionWrapper.

self._test_db = setUpTestDatabase()
await self._test_db.setup_test_database()
try:
    self._session = await self._test_db.start_async_session()
    if not isinstance(self._session, AsyncSessionWrapper):
        raise TypeError("Session is not an AsyncSessionWrapper")
except Exception as e:
    await self._test_db.teardown_test_database()
    raise RuntimeError(f"Failed to set up test database: {str(e)}")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sourcery-ai It is implemented in this file:

if not isinstance(self._session, AsyncSessionWrapper):

await seed_data(self._session)
await super().asyncSetUp()

async def asyncTearDown(self) -> None:
print("async closing test server db session closing.")
await self._session.commit()
await self._stack.aclose()
print("async test server closing.")
await self._test_db.aclose()
await super().asyncTearDown()

def _patient_query_lookup_svc_using_repo(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (testing): Changed return type of _patient_query_lookup_svc_using_repo

The return type has changed from a Callable to a List[FacilitiesResult]. Ensure that this change is intentional and that it doesn't break any existing tests or functionality.

self, address: Address, query: LocationQueryJSON
) -> Callable[[Address, LocationQueryJSON], List[FacilitiesResult]]: ...
) -> List[FacilitiesResult]:
# TODO: Implement a patient query lookup service
# that returns type List[FacilitiesResult]
...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (testing): Implement the _patient_query_lookup_svc_using_repo method

This method is currently a stub. Implement it to return a List[FacilitiesResult] as indicated in the TODO comment. This is crucial for testing the patient query lookup functionality.


async def test_fetch_facilities(self):
# TODO Implement test_fetch_facilities like this:
# providers = await GeolocationQueryService.fetch_facilities(
# dummy_reverse_geo_lookup_svc,
# dummy_geolocation_query_json,
# self._patient_query_lookup_svc_using_repo
# )
...
providers = await GeolocationQueryService.fetch_facilities(
self.dummy_reverse_geo_lookup_svc,
self.dummy_geolocation_query_json,
self._patient_query_lookup_svc_using_repo,
)
assert providers
self.assertIsInstance(providers, list)
self.assertIs(len(providers), 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Implemented test_fetch_facilities

Good implementation of the TODO. Consider adding more assertions to check the content of the returned providers, not just the length and type.

        providers = await GeolocationQueryService.fetch_facilities(
            self.dummy_reverse_geo_lookup_svc,
            self.dummy_geolocation_query_json,
            self._patient_query_lookup_svc_using_repo,
        )
        self.assertIsInstance(providers, list)
        self.assertEqual(len(providers), 1)
        provider = providers[0]
        self.assertIsInstance(provider, dict)
        self.assertIn('name', provider)
        self.assertIn('address', provider)
        self.assertIn('coordinates', provider)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Good implementation of test_fetch_facilities

The test now properly calls GeolocationQueryService.fetch_facilities and asserts the result. Consider adding more specific assertions about the content of the returned providers.

async def test_fetch_facilities(self):
    providers = await GeolocationQueryService.fetch_facilities(
        self.dummy_reverse_geo_lookup_svc,
        self.dummy_geolocation_query_json,
        self._patient_query_lookup_svc_using_repo,
    )
    assert providers
    self.assertIsInstance(providers, list)
    self.assertEqual(len(providers), 1)
    self.assertIsInstance(providers[0], dict)
    self.assertIn('name', providers[0])
    self.assertIn('address', providers[0])



@pytest.mark.usefixtures("dummy_geolocation_query_json", "dummy_reverse_geo_lookup_svc")
Expand Down
Loading