diff --git a/backend/poetry.lock b/backend/poetry.lock index 7decc004..652a4195 100644 --- a/backend/poetry.lock +++ b/backend/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.4.2 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.4.1 and should not be changed by hand. [[package]] name = "alembic" @@ -853,24 +853,6 @@ tomli = {version = ">=1.0.0", markers = "python_version < \"3.11\""} [package.extras] testing = ["argcomplete", "attrs (>=19.2.0)", "hypothesis (>=3.56)", "mock", "nose", "pygments (>=2.7.2)", "requests", "xmlschema"] -[[package]] -name = "pytest-mock" -version = "3.10.0" -description = "Thin-wrapper around the mock package for easier use with pytest" -category = "dev" -optional = false -python-versions = ">=3.7" -files = [ - {file = "pytest-mock-3.10.0.tar.gz", hash = "sha256:fbbdb085ef7c252a326fd8cdcac0aa3b1333d8811f131bdcc701002e1be7ed4f"}, - {file = "pytest_mock-3.10.0-py3-none-any.whl", hash = "sha256:f4c973eeae0282963eb293eb173ce91b091a79c1334455acfac9ddee8a1c784b"}, -] - -[package.dependencies] -pytest = ">=5.0" - -[package.extras] -dev = ["pre-commit", "pytest-asyncio", "tox"] - [[package]] name = "python-dateutil" version = "2.8.2" @@ -1499,4 +1481,4 @@ files = [ [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "cff65644cf63d04091d03894ae93749b5762dff4913b51dd75b231c32c246a2c" +content-hash = "191f55cfa2b0fa59843737982748f5f65c520807a7e949a87a3b01d4ac7589d2" diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 3ae4bde8..64dfc4d6 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -21,7 +21,6 @@ fallback_version = "0.0.0" [tool.poetry.group.test.dependencies] pytest = "^7.3.1" -pytest-mock = "^3.10.0" requests-mock = "^1.10.0" python-multipart = "^0.0.6" httpx = "^0.24.0" diff --git a/backend/src/main.py b/backend/src/main.py index 4192a24c..2b86b555 100644 --- a/backend/src/main.py +++ b/backend/src/main.py @@ -73,10 +73,8 @@ async def get_version(): @app.put("/snapmanager") def snap_manager(db: Session = Depends(get_db)): try: - session = sessionmaker(autocommit=False, autoflush=False, bind=engine) - with session() as sess: - processed_artefacts = snap_manager_controller(sess) - logger.info("INFO: Processed artefacts %s", processed_artefacts) + processed_artefacts = snap_manager_controller(db) + logger.info("INFO: Processed artefacts %s", processed_artefacts) if False in processed_artefacts.values(): return JSONResponse( status_code=500, diff --git a/backend/src/repository.py b/backend/src/repository.py index 4c1b167b..95f89955 100644 --- a/backend/src/repository.py +++ b/backend/src/repository.py @@ -22,26 +22,6 @@ from .data_access.models import Family, Stage, Artefact -def get_stages_by_family_name(session: Session, family_name: str) -> list | None: - """ - Fetch stages objects related to specific family - - :session: DB session - :family_name: name of the family - :return: list of stages - """ - family = session.query(Family).filter(Family.name == family_name).first() - if family is None: - return [] - stages = ( - session.query(Stage) - .filter(Stage.family_id == family.id) - .options(joinedload(Stage.artefacts)) - .all() - ) - return stages - - def get_stage_by_name(session: Session, stage_name: str, family: Family) -> Stage: """ Get the stage object by its name @@ -59,21 +39,9 @@ def get_stage_by_name(session: Session, stage_name: str, family: Family) -> Stag return stage -def get_family_by_name(session: Session, family_name: str): - """ - Get the family object by its name - - :session: DB session - :family_name: Name of the family - :return: Family - """ - family = session.query(Family).filter(Family.name == family_name).first() - return family - - def get_artefacts_by_family_name( session: Session, family_name: str, is_archived: bool = None -): +) -> list[Artefact]: """ Get all the artefacts in a family diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 80e7514d..3e3a3ccd 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -16,103 +16,58 @@ # # Written by: # Nadzeya Hutsko +# Omar Selo """Fixtures for testing""" import pytest -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker, Session -from sqlalchemy_utils import database_exists, create_database, drop_database +from alembic import command +from alembic.config import Config from fastapi.testclient import TestClient -from src.main import app +from sqlalchemy import Engine, create_engine +from sqlalchemy.orm import Session, sessionmaker +from sqlalchemy_utils import create_database, database_exists, drop_database from src.data_access import Base -from src.data_access.models import Family, Stage, Artefact +from src.data_access.models import Artefact, Stage +from src.main import app, get_db -# Setup Test Database -SQLALCHEMY_DATABASE_URL = ( - "postgresql+pg8000://postgres:password@test-observer-db:5432/test" -) -engine = create_engine(SQLALCHEMY_DATABASE_URL) -TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) +@pytest.fixture(scope="session") +def db_engine(): + db_uri = "postgresql+pg8000://postgres:password@test-observer-db:5432/test" + if not database_exists(db_uri): + create_database(db_uri) -@pytest.fixture -def seed_db(db_session: Session): - """Populate database with fake data""" - # Snap family - family = Family(name="snap") - db_session.add(family) - # Edge stage - stage = Stage(name="edge", family=family, position=10) - db_session.add(stage) - artefact = Artefact( - name="core20", stage=stage, version="1.1.1", source={}, artefact_group=None - ) - db_session.add(artefact) - artefact = Artefact( - name="docker", - stage=stage, - version="1.1.1", - source={}, - artefact_group=None, - is_archived=True, - ) - db_session.add(artefact) - # Beta stage - stage = Stage(name="beta", family=family, position=20) - db_session.add(stage) - artefact = Artefact( - name="core22", stage=stage, version="1.1.0", source={}, artefact_group=None - ) - db_session.add(artefact) + engine = create_engine(db_uri) - # Deb family - family = Family(name="deb") - db_session.add(family) - # Proposed stage - stage = Stage(name="proposed", family=family, position=10) - db_session.add(stage) - artefact = Artefact( - name="jammy", stage=stage, version="2.1.1", source={}, artefact_group=None - ) - db_session.add(artefact) - # Updates stage - stage = Stage(name="updates", family=family, position=10) - db_session.add(stage) - artefact = Artefact( - name="raspi", stage=stage, version="2.1.0", source={}, artefact_group=None - ) - db_session.add(artefact) - db_session.commit() + alembic_config = Config("alembic.ini") + alembic_config.set_main_option("sqlalchemy.url", db_uri) + command.upgrade(alembic_config, "head") - yield + yield engine - # Cleanup - db_session.query(Artefact).delete() - db_session.query(Stage).delete() - db_session.query(Family).delete() - db_session.commit() + Base.metadata.drop_all(engine) + engine.dispose() + drop_database(db_uri) -@pytest.fixture(scope="session") -def db_session(): - """Set up and tear down the test database""" - if not database_exists(SQLALCHEMY_DATABASE_URL): - create_database(SQLALCHEMY_DATABASE_URL) +@pytest.fixture(scope="function") +def db_session(db_engine: Engine): + connection = db_engine.connect() + # Start transaction and not commit it to rollback automatically + transaction = connection.begin() + session = sessionmaker(autocommit=False, autoflush=False, bind=connection)() - Base.metadata.create_all(bind=engine) - session = TestingSessionLocal() yield session - # Cleanup session.close() - Base.metadata.drop_all(bind=engine) - drop_database(SQLALCHEMY_DATABASE_URL) + transaction.close() + connection.close() -@pytest.fixture(scope="session") -def test_app(): - """Create a pytest fixture for the app""" - client = TestClient(app) - yield client +@pytest.fixture(scope="function") +def test_client(db_session: Session) -> TestClient: + """Create a test http client""" + app.dependency_overrides[get_db] = lambda: db_session + return TestClient(app) diff --git a/backend/tests/helpers.py b/backend/tests/helpers.py new file mode 100644 index 00000000..34b5632f --- /dev/null +++ b/backend/tests/helpers.py @@ -0,0 +1,18 @@ +from sqlalchemy.orm import Session +from src.data_access.models import Artefact, Stage + + +def create_artefact(db_session: Session, stage_name: str, **kwargs): + """Create a dummy artefact""" + stage = db_session.query(Stage).filter(Stage.name == stage_name).first() + artefact = Artefact( + name=kwargs.get("name", ""), + stage=stage, + version=kwargs.get("version", "1.1.1"), + source=kwargs.get("source", {}), + artefact_group=None, + is_archived=kwargs.get("is_archived", False), + ) + db_session.add(artefact) + db_session.commit() + return artefact diff --git a/backend/tests/test_repository.py b/backend/tests/test_repository.py index e06e8231..264ee2e6 100644 --- a/backend/tests/test_repository.py +++ b/backend/tests/test_repository.py @@ -16,49 +16,21 @@ # # Written by: # Nadzeya Hutsko +# Omar Selo """Test services functions""" -import pytest +from sqlalchemy.orm import Session +from src.data_access.models import Family +from src.repository import get_artefacts_by_family_name, get_stage_by_name -from src.repository import ( - get_stages_by_family_name, - get_stage_by_name, - get_family_by_name, - get_artefacts_by_family_name, -) +from .helpers import create_artefact -def test_get_stages_by_family_name(db_session, seed_db): - """The function should select correct stages for the specified family name""" - # Arrange - family_name = "snap" - expected_stage_names = ["edge", "beta"] - - # Act - stages = get_stages_by_family_name(db_session, family_name) - - # Assert - assert len(stages) == len(expected_stage_names) - assert all(stage.name in expected_stage_names for stage in stages) - - -def test_get_stages_by_family_name_no_such_family(seed_db, db_session): - """The function should return empty list""" - # Arrange - family_name = "fake" - - # Act - stages = get_stages_by_family_name(db_session, family_name) - - # Assert - assert stages == [] - - -def test_get_stage_by_name(seed_db, db_session): +def test_get_stage_by_name(db_session: Session): """The function should select the correct stage by its name""" # Arrange - family = get_family_by_name(db_session, "deb") + family = db_session.query(Family).filter(Family.name == "deb").first() stage_name = "proposed" # Act @@ -68,10 +40,10 @@ def test_get_stage_by_name(seed_db, db_session): assert stage.name == stage_name -def test_get_stage_by_name_no_such_stage(seed_db, db_session): +def test_get_stage_by_name_no_such_stage(db_session: Session): """The function should return None""" # Arrange - family = get_family_by_name(db_session, "deb") + family = db_session.query(Family).filter(Family.name == "deb").first() stage_name = "fakestage" # Act @@ -81,11 +53,11 @@ def test_get_stage_by_name_no_such_stage(seed_db, db_session): assert stage is None -def test_get_stage_by_name_no_such_family(seed_db, db_session): +def test_get_stage_by_name_no_such_family(db_session: Session): """The function should return None""" # Arrange - family = get_family_by_name(db_session, "deb") - stage_name = "fakestage" + family = db_session.query(Family).filter(Family.name == "fakefamily").first() + stage_name = "proposed" # Act stage = get_stage_by_name(db_session, stage_name, family) @@ -94,36 +66,43 @@ def test_get_stage_by_name_no_such_family(seed_db, db_session): assert stage is None -def test_get_artefacts_by_family_name(seed_db, db_session): +def test_get_artefacts_by_family_name(db_session: Session): """We should get a valid list of artefacts""" # Arrange - family_name = "snap" - expected_artefact_names = ["core20", "core22", "docker"] + artefact_name_stage_pair = { + ("core20", "edge"), + ("core22", "beta"), + ("docker", "candidate"), + } + + for name, stage in artefact_name_stage_pair: + create_artefact(db_session, stage, name=name) # Act - artefacts = get_artefacts_by_family_name(db_session, family_name) + artefacts = get_artefacts_by_family_name(db_session, "snap") # Assert - assert len(artefacts) == len(expected_artefact_names) - assert all(artefact.name in expected_artefact_names for artefact in artefacts) + assert len(artefacts) == len(artefact_name_stage_pair) + assert { + (artefact.name, artefact.stage.name) for artefact in artefacts + } == artefact_name_stage_pair -def test_get_artefacts_by_family_name_filter_archived(seed_db, db_session): +def test_get_artefacts_by_family_name_filter_archived(db_session: Session): """We should get a list of archived artefacts""" # Arrange - family_name = "snap" - expected_artefact_names = ["docker"] + create_artefact(db_session, "beta", name="docker", is_archived=True) # Act - artefacts = get_artefacts_by_family_name(db_session, family_name, is_archived=True) + artefacts = get_artefacts_by_family_name(db_session, "snap", is_archived=True) # Assert - assert len(artefacts) == len(expected_artefact_names) - assert all(artefact.name in expected_artefact_names for artefact in artefacts) - assert all(artefact.is_archived for artefact in artefacts) + assert len(artefacts) == 1 + assert artefacts[0].name == "docker" + assert artefacts[0].is_archived -def test_get_artefacts_by_family_name_no_such_family(seed_db, db_session): +def test_get_artefacts_by_family_name_no_such_family(db_session: Session): """We should get an empty list when there's no such family""" # Arrange family_name = "fakename" diff --git a/backend/tests/test_snap_manager.py b/backend/tests/test_snap_manager.py index ad030b9f..abe93f98 100644 --- a/backend/tests/test_snap_manager.py +++ b/backend/tests/test_snap_manager.py @@ -16,15 +16,20 @@ # # Written by: # Nadzeya Hutsko +# Omar Selo """Test snap manager API""" +from fastapi.testclient import TestClient +from requests_mock import Mocker +from sqlalchemy.orm import Session from src.data_access.models import Artefact -from .conftest import engine + +from .helpers import create_artefact def test_run_for_different_revisions( - test_app, db_session, requests_mock, mocker, seed_db + test_client: TestClient, db_session: Session, requests_mock: Mocker ): """ If revision in snapcraft response is different from the revision in @@ -57,23 +62,27 @@ def test_run_for_different_revisions( ] }, ) - core20 = db_session.query(Artefact).filter(Artefact.name == "core20").first() - original_stage = core20.stage - core20.source = {"revision": 1823, "architecture": "amd64", "store": "ubuntu"} - core20.version = "1.1.1" - core20.is_archived = False - db_session.commit() - mocker.patch("src.main.engine", wraps=engine) + artefact = create_artefact( + db_session, + "edge", + name="core20", + version="1.1.1", + source={"revision": 1823, "architecture": "amd64", "store": "ubuntu"}, + ) # Act - test_app.put("/snapmanager") + test_client.put("/snapmanager") + + db_session.refresh(artefact) # Assert - assert core20.stage == original_stage # The artefact should not be moved - assert core20.is_archived + assert artefact.stage.name == "edge" # The artefact should not be moved + assert artefact.is_archived -def test_run_to_move_artefact(db_session, test_app, requests_mock, mocker, seed_db): +def test_run_to_move_artefact( + db_session: Session, test_client: TestClient, requests_mock: Mocker +): """ If card's current list name is different to its list name in snapcraft, the card is moved to the next list @@ -105,16 +114,20 @@ def test_run_to_move_artefact(db_session, test_app, requests_mock, mocker, seed_ ] }, ) - core20 = db_session.query(Artefact).filter(Artefact.name == "core20").first() - core20.source = {"revision": 1883, "architecture": "amd64", "store": "ubuntu"} - core20.version = "1.1.1" - core20.is_archived = False - db_session.commit() - mocker.patch("src.main.engine", wraps=engine) + + artefact = create_artefact( + db_session, + "edge", + name="core20", + version="1.1.1", + source={"revision": 1883, "architecture": "amd64", "store": "ubuntu"}, + ) # Act - test_app.put("/snapmanager") + test_client.put("/snapmanager") + + db_session.refresh(artefact) # Assert - assert core20.stage.name == "beta" - assert not core20.is_archived # The artefact should not be archived + assert artefact.stage.name == "beta" + assert not artefact.is_archived # The artefact should not be archived