From 633880c0e6abff2cece94dc845e091f8b70d1b89 Mon Sep 17 00:00:00 2001 From: saltie2193 Date: Sat, 12 Oct 2024 21:03:27 +0200 Subject: [PATCH 1/7] Improve backend test performance by patching password hashing * Patch password hashing during tests --- backend/app/tests/conftest.py | 18 ++++++++++++++++-- backend/app/tests/utils/utils.py | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/backend/app/tests/conftest.py b/backend/app/tests/conftest.py index 90ab39a357..516e6e4e65 100644 --- a/backend/app/tests/conftest.py +++ b/backend/app/tests/conftest.py @@ -9,12 +9,26 @@ from app.main import app from app.models import Item, User from app.tests.utils.user import authentication_token_from_email -from app.tests.utils.utils import get_superuser_token_headers +from app.tests.utils.utils import get_superuser_token_headers, patch_password_hashing + + +@pytest.fixture(scope="session") +def disable_password_hashing() -> Generator[None, None, None]: + with patch_password_hashing("app.core.security"): + yield @pytest.fixture(scope="session", autouse=True) -def db() -> Generator[Session, None, None]: +def db( + disable_password_hashing: Generator[None, None, None], # noqa: ARG001 +) -> Generator[Session, None, None]: with Session(engine) as session: + # cleanup db to prevent interferences with tests + # all existing data will be deleted anyway after the tests run + session.execute(delete(User)) + session.execute(delete(Item)) + session.commit() + init_db(session) yield session statement = delete(Item) diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index 184bac44d9..8363cf590a 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -1,5 +1,8 @@ import random import string +from collections.abc import Generator +from contextlib import contextmanager +from unittest.mock import patch from fastapi.testclient import TestClient @@ -24,3 +27,23 @@ def get_superuser_token_headers(client: TestClient) -> dict[str, str]: a_token = tokens["access_token"] headers = {"Authorization": f"Bearer {a_token}"} return headers + + +@contextmanager +def patch_password_hashing(*modules: str) -> Generator[None, None, None]: + """ + Contextmanager to patch ``pwd_context`` in the given modules. + :param modules: list of modules to patch. + :return: + """ + patchers = [] + for module in modules: + patcher_p = patch(f"{module}.pwd_context.verify", lambda x, y: x == y) + patcher_h = patch(f"{module}.pwd_context.hash", lambda x: x) + patcher_p.start() + patcher_h.start() + + patchers.extend((patcher_p, patcher_h)) + yield + for patcher in patchers: + patcher.stop() From d37b84d46eaf4466cda34fd7c5e3ddbf1735ffdf Mon Sep 17 00:00:00 2001 From: saltie2193 Date: Sat, 12 Oct 2024 21:03:40 +0200 Subject: [PATCH 2/7] Use `contextlib.ExitStack` for managing multiple patchers --- backend/app/tests/utils/utils.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index 8363cf590a..c1ba93fd13 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -1,7 +1,7 @@ import random import string from collections.abc import Generator -from contextlib import contextmanager +from contextlib import ExitStack, contextmanager from unittest.mock import patch from fastapi.testclient import TestClient @@ -36,14 +36,10 @@ def patch_password_hashing(*modules: str) -> Generator[None, None, None]: :param modules: list of modules to patch. :return: """ - patchers = [] - for module in modules: - patcher_p = patch(f"{module}.pwd_context.verify", lambda x, y: x == y) - patcher_h = patch(f"{module}.pwd_context.hash", lambda x: x) - patcher_p.start() - patcher_h.start() - - patchers.extend((patcher_p, patcher_h)) - yield - for patcher in patchers: - patcher.stop() + with ExitStack() as stack: + for module in modules: + stack.enter_context( + patch(f"{module}.pwd_context.verify", lambda x, y: x == y) + ) + stack.enter_context(patch(f"{module}.pwd_context.hash", lambda x: x)) + yield From 5e2bdf0373d0691c49f6b8e40b7768bc9d9d0c57 Mon Sep 17 00:00:00 2001 From: saltie2193 Date: Wed, 17 Sep 2025 14:42:11 +0200 Subject: [PATCH 3/7] Make password hashing re-activatable - Add marker to activate password hashing on module level. - Change scope of `db` and `client` fixtures to `module`. - Add test without patched password hashing --- .../api/routes/test_login_with_hashing.py | 27 +++++++++ backend/app/tests/conftest.py | 56 +++++++++++++------ backend/pyproject.toml | 3 + 3 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 backend/app/tests/api/routes/test_login_with_hashing.py diff --git a/backend/app/tests/api/routes/test_login_with_hashing.py b/backend/app/tests/api/routes/test_login_with_hashing.py new file mode 100644 index 0000000000..9713168619 --- /dev/null +++ b/backend/app/tests/api/routes/test_login_with_hashing.py @@ -0,0 +1,27 @@ +import pytest +from starlette.testclient import TestClient + +from app.core.config import settings + +pytestmark = pytest.mark.enable_password_hashing + + +def test_get_access_token_with_hashing(client: TestClient) -> None: + login_data = { + "username": settings.FIRST_SUPERUSER, + "password": settings.FIRST_SUPERUSER_PASSWORD, + } + r = client.post(f"{settings.API_V1_STR}/login/access-token", data=login_data) + tokens = r.json() + assert r.status_code == 200 + assert "access_token" in tokens + assert tokens["access_token"] + + +def test_get_access_token_incorrect_password(client: TestClient) -> None: + login_data = { + "username": settings.FIRST_SUPERUSER, + "password": "incorrect", + } + r = client.post(f"{settings.API_V1_STR}/login/access-token", data=login_data) + assert r.status_code == 400 diff --git a/backend/app/tests/conftest.py b/backend/app/tests/conftest.py index 516e6e4e65..4875515151 100644 --- a/backend/app/tests/conftest.py +++ b/backend/app/tests/conftest.py @@ -1,6 +1,8 @@ from collections.abc import Generator +from contextlib import nullcontext import pytest +from _pytest.fixtures import FixtureRequest from fastapi.testclient import TestClient from sqlmodel import Session, delete @@ -12,34 +14,52 @@ from app.tests.utils.utils import get_superuser_token_headers, patch_password_hashing -@pytest.fixture(scope="session") -def disable_password_hashing() -> Generator[None, None, None]: - with patch_password_hashing("app.core.security"): +@pytest.fixture(scope="module", autouse=True) +def disable_password_hashing(request: FixtureRequest) -> Generator[None, None, None]: + """Fixture disabling password hashing + + Password hashing can be enabled on module level by marking the module with `pytest.mark.enable_password_hashing`. + """ + + with ( + patch_password_hashing("app.core.security") + if all(m.name != "enable_password_hashing" for m in request.node.iter_markers()) + else nullcontext() + ): yield -@pytest.fixture(scope="session", autouse=True) -def db( - disable_password_hashing: Generator[None, None, None], # noqa: ARG001 -) -> Generator[Session, None, None]: +@pytest.fixture(scope="session") +def db() -> Generator[Session, None, None]: + """ + Module scoped fixture providing a database session initialized with `init_db`. + """ with Session(engine) as session: - # cleanup db to prevent interferences with tests - # all existing data will be deleted anyway after the tests run - session.execute(delete(User)) + yield session + # Cleanup test database session.execute(delete(Item)) + session.execute(delete(User)) session.commit() - init_db(session) - yield session - statement = delete(Item) - session.execute(statement) - statement = delete(User) - session.execute(statement) - session.commit() + +@pytest.fixture(scope="module", autouse=True) +def init_db_fixture(db: Session) -> None: + # note: deleting all users here is required to enable or disable password hashing per test module. + # If we don't delete all users here, the users created during `init_db` will not be re-created and the password will stay (un)hashed, + # leading to possibly failing tests relying on the created user for authentication. + db.execute(delete(Item)) + db.execute(delete(User)) + init_db(db) + db.commit() @pytest.fixture(scope="module") -def client() -> Generator[TestClient, None, None]: +def client(db: Session) -> Generator[TestClient, None, None]: # noqa: ARG001 + """ + Module scoped fixture providing a `TestClient` instance. + + NOTE: This fixture uses the `db` fixture WITHOUT hashing passwords! + """ with TestClient(app) as c: yield c diff --git a/backend/pyproject.toml b/backend/pyproject.toml index d1fbd0641c..180c28d066 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -41,6 +41,9 @@ build-backend = "hatchling.build" strict = true exclude = ["venv", ".venv", "alembic"] +[tool.pytest.ini_options] +markers = ["enable_password_hashing: mark tests to not patch password hashing."] + [tool.ruff] target-version = "py310" exclude = ["alembic"] From 0fccbdefbe8c7902f68ba05ff2add2ce206a536d Mon Sep 17 00:00:00 2001 From: Yurii Motov Date: Thu, 18 Sep 2025 22:22:13 +0200 Subject: [PATCH 4/7] Simplify tests --- .../api/routes/test_login_with_hashing.py | 15 ++---- backend/app/tests/conftest.py | 54 ++++++++----------- backend/app/tests/utils/utils.py | 19 ------- 3 files changed, 26 insertions(+), 62 deletions(-) diff --git a/backend/app/tests/api/routes/test_login_with_hashing.py b/backend/app/tests/api/routes/test_login_with_hashing.py index 9713168619..d4610c06be 100644 --- a/backend/app/tests/api/routes/test_login_with_hashing.py +++ b/backend/app/tests/api/routes/test_login_with_hashing.py @@ -6,7 +6,11 @@ pytestmark = pytest.mark.enable_password_hashing -def test_get_access_token_with_hashing(client: TestClient) -> None: +def test_get_access_token_with_hashing( + client: TestClient, + disable_password_hashing: bool, +) -> None: + assert disable_password_hashing is False login_data = { "username": settings.FIRST_SUPERUSER, "password": settings.FIRST_SUPERUSER_PASSWORD, @@ -16,12 +20,3 @@ def test_get_access_token_with_hashing(client: TestClient) -> None: assert r.status_code == 200 assert "access_token" in tokens assert tokens["access_token"] - - -def test_get_access_token_incorrect_password(client: TestClient) -> None: - login_data = { - "username": settings.FIRST_SUPERUSER, - "password": "incorrect", - } - r = client.post(f"{settings.API_V1_STR}/login/access-token", data=login_data) - assert r.status_code == 400 diff --git a/backend/app/tests/conftest.py b/backend/app/tests/conftest.py index 4875515151..b3c85c498c 100644 --- a/backend/app/tests/conftest.py +++ b/backend/app/tests/conftest.py @@ -1,5 +1,5 @@ from collections.abc import Generator -from contextlib import nullcontext +from unittest.mock import patch import pytest from _pytest.fixtures import FixtureRequest @@ -11,30 +11,34 @@ from app.main import app from app.models import Item, User from app.tests.utils.user import authentication_token_from_email -from app.tests.utils.utils import get_superuser_token_headers, patch_password_hashing +from app.tests.utils.utils import get_superuser_token_headers -@pytest.fixture(scope="module", autouse=True) +@pytest.fixture(scope="module") def disable_password_hashing(request: FixtureRequest) -> Generator[None, None, None]: - """Fixture disabling password hashing - - Password hashing can be enabled on module level by marking the module with `pytest.mark.enable_password_hashing`. + """ + Disable password hashing if no `enable_password_hashing` marker set for module. """ - with ( - patch_password_hashing("app.core.security") - if all(m.name != "enable_password_hashing" for m in request.node.iter_markers()) - else nullcontext() - ): - yield + module = request.node.getparent(pytest.Module) + if not module.get_closest_marker("enable_password_hashing"): + with ( + patch("app.core.security.pwd_context.verify", lambda x, y: x == y), + patch("app.core.security.pwd_context.hash", lambda x: x), + ): + yield True + else: + yield False # Don't patch if `enable_password_hashing` marker is set -@pytest.fixture(scope="session") -def db() -> Generator[Session, None, None]: - """ - Module scoped fixture providing a database session initialized with `init_db`. - """ +@pytest.fixture(scope="module") +def db(disable_password_hashing) -> Generator[Session, None, None]: # noqa: ARG001 with Session(engine) as session: + session.execute( # Recreate user for every module, with\without pwd hashing + delete(User) + ) + init_db(session) + session.commit() yield session # Cleanup test database session.execute(delete(Item)) @@ -42,24 +46,8 @@ def db() -> Generator[Session, None, None]: session.commit() -@pytest.fixture(scope="module", autouse=True) -def init_db_fixture(db: Session) -> None: - # note: deleting all users here is required to enable or disable password hashing per test module. - # If we don't delete all users here, the users created during `init_db` will not be re-created and the password will stay (un)hashed, - # leading to possibly failing tests relying on the created user for authentication. - db.execute(delete(Item)) - db.execute(delete(User)) - init_db(db) - db.commit() - - @pytest.fixture(scope="module") def client(db: Session) -> Generator[TestClient, None, None]: # noqa: ARG001 - """ - Module scoped fixture providing a `TestClient` instance. - - NOTE: This fixture uses the `db` fixture WITHOUT hashing passwords! - """ with TestClient(app) as c: yield c diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index c1ba93fd13..184bac44d9 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -1,8 +1,5 @@ import random import string -from collections.abc import Generator -from contextlib import ExitStack, contextmanager -from unittest.mock import patch from fastapi.testclient import TestClient @@ -27,19 +24,3 @@ def get_superuser_token_headers(client: TestClient) -> dict[str, str]: a_token = tokens["access_token"] headers = {"Authorization": f"Bearer {a_token}"} return headers - - -@contextmanager -def patch_password_hashing(*modules: str) -> Generator[None, None, None]: - """ - Contextmanager to patch ``pwd_context`` in the given modules. - :param modules: list of modules to patch. - :return: - """ - with ExitStack() as stack: - for module in modules: - stack.enter_context( - patch(f"{module}.pwd_context.verify", lambda x, y: x == y) - ) - stack.enter_context(patch(f"{module}.pwd_context.hash", lambda x: x)) - yield From a6b477c96a00f4ca426a222346c97565924849f6 Mon Sep 17 00:00:00 2001 From: Yurii Motov Date: Thu, 18 Sep 2025 22:27:52 +0200 Subject: [PATCH 5/7] Fix lint errors --- backend/app/tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/tests/conftest.py b/backend/app/tests/conftest.py index b3c85c498c..b1444f6e28 100644 --- a/backend/app/tests/conftest.py +++ b/backend/app/tests/conftest.py @@ -15,7 +15,7 @@ @pytest.fixture(scope="module") -def disable_password_hashing(request: FixtureRequest) -> Generator[None, None, None]: +def disable_password_hashing(request: FixtureRequest) -> Generator[bool, None, None]: """ Disable password hashing if no `enable_password_hashing` marker set for module. """ @@ -32,7 +32,7 @@ def disable_password_hashing(request: FixtureRequest) -> Generator[None, None, N @pytest.fixture(scope="module") -def db(disable_password_hashing) -> Generator[Session, None, None]: # noqa: ARG001 +def db(disable_password_hashing: bool) -> Generator[Session, None, None]: # noqa: ARG001 with Session(engine) as session: session.execute( # Recreate user for every module, with\without pwd hashing delete(User) From 96d029618f3346c974bf0d71c01c1110531dc90a Mon Sep 17 00:00:00 2001 From: Yurii Motov Date: Thu, 18 Sep 2025 22:44:37 +0200 Subject: [PATCH 6/7] Return back `autouse=True` for `db` fixture --- backend/app/tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/tests/conftest.py b/backend/app/tests/conftest.py index b1444f6e28..5bdf15bc43 100644 --- a/backend/app/tests/conftest.py +++ b/backend/app/tests/conftest.py @@ -31,7 +31,7 @@ def disable_password_hashing(request: FixtureRequest) -> Generator[bool, None, N yield False # Don't patch if `enable_password_hashing` marker is set -@pytest.fixture(scope="module") +@pytest.fixture(scope="module", autouse=True) def db(disable_password_hashing: bool) -> Generator[Session, None, None]: # noqa: ARG001 with Session(engine) as session: session.execute( # Recreate user for every module, with\without pwd hashing @@ -47,7 +47,7 @@ def db(disable_password_hashing: bool) -> Generator[Session, None, None]: # noq @pytest.fixture(scope="module") -def client(db: Session) -> Generator[TestClient, None, None]: # noqa: ARG001 +def client() -> Generator[TestClient, None, None]: with TestClient(app) as c: yield c From 471685b8a39a5c19d24d1988fb0ebeb48fbd86ba Mon Sep 17 00:00:00 2001 From: saltie2193 Date: Wed, 24 Sep 2025 13:03:49 +0200 Subject: [PATCH 7/7] Revert removal of `patch_password_hashing` utility --- backend/app/tests/conftest.py | 8 ++------ backend/app/tests/utils/utils.py | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/backend/app/tests/conftest.py b/backend/app/tests/conftest.py index 5bdf15bc43..d34b329952 100644 --- a/backend/app/tests/conftest.py +++ b/backend/app/tests/conftest.py @@ -1,5 +1,4 @@ from collections.abc import Generator -from unittest.mock import patch import pytest from _pytest.fixtures import FixtureRequest @@ -11,7 +10,7 @@ from app.main import app from app.models import Item, User from app.tests.utils.user import authentication_token_from_email -from app.tests.utils.utils import get_superuser_token_headers +from app.tests.utils.utils import get_superuser_token_headers, patch_password_hashing @pytest.fixture(scope="module") @@ -22,10 +21,7 @@ def disable_password_hashing(request: FixtureRequest) -> Generator[bool, None, N module = request.node.getparent(pytest.Module) if not module.get_closest_marker("enable_password_hashing"): - with ( - patch("app.core.security.pwd_context.verify", lambda x, y: x == y), - patch("app.core.security.pwd_context.hash", lambda x: x), - ): + with patch_password_hashing("app.core.security"): yield True else: yield False # Don't patch if `enable_password_hashing` marker is set diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index 184bac44d9..c1ba93fd13 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -1,5 +1,8 @@ import random import string +from collections.abc import Generator +from contextlib import ExitStack, contextmanager +from unittest.mock import patch from fastapi.testclient import TestClient @@ -24,3 +27,19 @@ def get_superuser_token_headers(client: TestClient) -> dict[str, str]: a_token = tokens["access_token"] headers = {"Authorization": f"Bearer {a_token}"} return headers + + +@contextmanager +def patch_password_hashing(*modules: str) -> Generator[None, None, None]: + """ + Contextmanager to patch ``pwd_context`` in the given modules. + :param modules: list of modules to patch. + :return: + """ + with ExitStack() as stack: + for module in modules: + stack.enter_context( + patch(f"{module}.pwd_context.verify", lambda x, y: x == y) + ) + stack.enter_context(patch(f"{module}.pwd_context.hash", lambda x: x)) + yield