From b10d2aba9857d70f3abb25dc6e5cb4ef46898500 Mon Sep 17 00:00:00 2001 From: Xander Bil Date: Mon, 11 Mar 2024 14:19:09 +0100 Subject: [PATCH 1/7] Working subject unittests with login and database rollbacks --- backend/src/auth/dependencies.py | 3 +- backend/src/database.py | 4 +- backend/src/subject/dependencies.py | 2 +- backend/src/subject/router.py | 4 +- backend/src/subject/schemas.py | 12 ++--- backend/src/subject/service.py | 5 +- backend/src/user/schemas.py | 7 ++- backend/src/user/service.py | 9 ++++ backend/tests/conftest.py | 50 +++++++++++++++++++ backend/tests/subject/placeholder.py | 0 backend/tests/test_main.py | 10 ---- backend/tests/test_subject.py | 72 ++++++++++++++++++++++++++++ backend/tests/user/placeholder.py | 0 13 files changed, 147 insertions(+), 31 deletions(-) create mode 100644 backend/tests/conftest.py delete mode 100644 backend/tests/subject/placeholder.py delete mode 100644 backend/tests/test_main.py create mode 100644 backend/tests/test_subject.py delete mode 100644 backend/tests/user/placeholder.py diff --git a/backend/src/auth/dependencies.py b/backend/src/auth/dependencies.py index 407aa119..216572f0 100644 --- a/backend/src/auth/dependencies.py +++ b/backend/src/auth/dependencies.py @@ -23,8 +23,7 @@ def verify_jwt_token( credentials.credentials, CONFIG.secret_key, algorithms=[CONFIG.algorithm], - verify_signature=True, - options={"require": ["exp", "sub"]}, + options={"require": ["exp", "sub"], "verify_signature": True}, ) user_id = payload["sub"] return user_id diff --git a/backend/src/database.py b/backend/src/database.py index a24a139f..21bbb78e 100644 --- a/backend/src/database.py +++ b/backend/src/database.py @@ -1,5 +1,5 @@ -from sqlalchemy import create_engine -from sqlalchemy.ext.declarative import declarative_base +from sqlalchemy import MetaData, create_engine +from sqlalchemy.orm import declarative_base from sqlalchemy.orm import sessionmaker from src import config diff --git a/backend/src/subject/dependencies.py b/backend/src/subject/dependencies.py index 219190f7..eacc4f9a 100644 --- a/backend/src/subject/dependencies.py +++ b/backend/src/subject/dependencies.py @@ -32,5 +32,5 @@ async def user_permission_validation( ): if not user.is_admin: teachers = await service.get_teachers(db, subject_id) - if not list(filter(lambda teacher: teacher.id == user.uid, teachers)): + if not list(filter(lambda teacher: teacher.uid == user.uid, teachers)): raise NotAuthorized() diff --git a/backend/src/subject/router.py b/backend/src/subject/router.py index 02cabbf8..4a9a395d 100644 --- a/backend/src/subject/router.py +++ b/backend/src/subject/router.py @@ -9,8 +9,8 @@ from .schemas import Subject, SubjectCreate, SubjectList router = APIRouter( - prefix="/api/subject", - tags=["subject"], + prefix="/api/subjects", + tags=["subjects"], responses={404: {"description": "Not found"}}, ) diff --git a/backend/src/subject/schemas.py b/backend/src/subject/schemas.py index 62bd2313..1dedacb0 100644 --- a/backend/src/subject/schemas.py +++ b/backend/src/subject/schemas.py @@ -1,6 +1,6 @@ from enum import Enum from typing import Sequence -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, ConfigDict class SubjectBase(BaseModel): @@ -12,15 +12,13 @@ class SubjectCreate(SubjectBase): class Subject(SubjectBase): - id: int + model_config = ConfigDict(from_attributes=True) - class Config: - from_attributes = True + id: int class SubjectList(BaseModel): + model_config = ConfigDict(from_attributes=True) + as_teacher: Sequence[Subject] as_student: Sequence[Subject] - - class Config: - from_attributes = True diff --git a/backend/src/subject/service.py b/backend/src/subject/service.py index 421c0392..5ceee884 100644 --- a/backend/src/subject/service.py +++ b/backend/src/subject/service.py @@ -1,7 +1,7 @@ from typing import Sequence from sqlalchemy.orm import Session -from src.user.models import User from . import models, schemas +from src.user.models import User async def get_subject(db: Session, subject_id: int) -> models.Subject: @@ -19,8 +19,7 @@ async def get_subjects(db: Session, user_id: str) -> tuple[Sequence[models.Subje async def get_teachers(db: Session, subject_id: int) -> list[User]: - return db.query(User).join(models.TeacherSubject).filter( - models.TeacherSubject.c.subject_id == subject_id).all() + return db.query(User).join(models.TeacherSubject, models.TeacherSubject.c.subject_id==subject_id).all() async def create_subject(db: Session, subject: schemas.SubjectCreate) -> models.Subject: diff --git a/backend/src/user/schemas.py b/backend/src/user/schemas.py index 732f3641..a3bf365b 100644 --- a/backend/src/user/schemas.py +++ b/backend/src/user/schemas.py @@ -1,4 +1,4 @@ -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, ConfigDict class Userbase(BaseModel): @@ -12,7 +12,6 @@ class UserCreate(Userbase): class User(Userbase): - is_admin: bool = Field(default=False) + model_config = ConfigDict(from_attributes=True) - class Config: - from_attributes = True + is_admin: bool = Field(default=False) diff --git a/backend/src/user/service.py b/backend/src/user/service.py index 469b4f80..b622ddc0 100644 --- a/backend/src/user/service.py +++ b/backend/src/user/service.py @@ -12,3 +12,12 @@ async def create_user(db: Session, user: schemas.UserCreate) -> models.User: db.commit() db.refresh(db_user) return db_user + +async def delete_user(db: Session, user_id: str): + db.query(models.User).filter_by(uid=user_id).delete() + db.commit() + +async def set_admin(db: Session, user_id: str, value: bool): + user = await get_by_id(db,user_id) + user.is_admin = value + db.commit() diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py new file mode 100644 index 00000000..f1a03190 --- /dev/null +++ b/backend/tests/conftest.py @@ -0,0 +1,50 @@ +from httpx import ASGITransport, AsyncClient +from sqlalchemy.orm import Session, sessionmaker +from src.auth.utils import create_jwt_token +from src.database import engine +from src.main import app +from src.dependencies import get_db +import pytest + +from src.user.schemas import UserCreate +from src.user.service import create_user + +connection = engine.connect() +trans = connection.begin() +TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=connection) + +def get_db_override(): + db = TestingSessionLocal() + try: + yield db + finally: + db.close() + +app.dependency_overrides[get_db] = get_db_override + +@pytest.fixture +def anyio_backend(): + return 'asyncio' + +@pytest.fixture +async def db(): + global trans + db = TestingSessionLocal() + try: + yield db + finally: + db.close() + trans.rollback() + trans = connection.begin() + +@pytest.fixture +async def client(db: Session): + token = create_jwt_token("test") + + await create_user(db,UserCreate(uid="test",given_name="tester", mail="test@test.test")) + + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test", headers = {"Authorization": f"Bearer {token.token}"}) as client: + yield client + +def pytest_sessionfinish(): + connection.close() diff --git a/backend/tests/subject/placeholder.py b/backend/tests/subject/placeholder.py deleted file mode 100644 index e69de29b..00000000 diff --git a/backend/tests/test_main.py b/backend/tests/test_main.py deleted file mode 100644 index a8689155..00000000 --- a/backend/tests/test_main.py +++ /dev/null @@ -1,10 +0,0 @@ -from fastapi import FastAPI -from fastapi.testclient import TestClient -from src.main import app -client = TestClient(app) - - -def test_read_main(): - response = client.get("/api") - assert response.status_code == 200 - assert response.json() == {"message": "Hello World"} diff --git a/backend/tests/test_subject.py b/backend/tests/test_subject.py new file mode 100644 index 00000000..a11d2ac7 --- /dev/null +++ b/backend/tests/test_subject.py @@ -0,0 +1,72 @@ +import pytest +from httpx import AsyncClient +from sqlalchemy.orm import Session +from src.user.schemas import UserCreate + +from src.user.service import set_admin +from src.user.service import create_user + +subject = {'name': 'test subject'} + +@pytest.fixture +async def subject_id(client: AsyncClient, db: Session) -> int: + """Create new subject""" + await set_admin(db,"test", True) + response = await client.post("/api/subjects/", json=subject) + return response.json()["id"] + +@pytest.mark.anyio +async def test_create_subject(client: AsyncClient, db: Session): + + await set_admin(db,"test",False) + response = await client.post("/api/subjects/", json=subject) + assert response.status_code == 403 # Forbidden, not admin + + await set_admin(db,"test", True) + response = await client.post("/api/subjects/", json=subject) + assert response.status_code == 201 # Created + assert response.json()["name"] == subject["name"] + + +@pytest.mark.anyio +async def test_get_subject(client: AsyncClient, subject_id: int): + + response = await client.get(f"/api/subjects/{subject_id}") + assert response.status_code == 200 + assert response.json()["name"] == subject["name"] + +@pytest.mark.anyio +async def test_create_teacher(client: AsyncClient, db: Session, subject_id: int): + + await set_admin(db,"test", False) + response2 = await client.post(f"/api/subjects/{subject_id}/teachers", params={'user_id': 'test'}) + assert response2.status_code == 403 # Forbidden + + await set_admin(db,"test", True) + response2 = await client.post(f"/api/subjects/{subject_id}/teachers", params={'user_id': 'test'}) + assert response2.status_code == 201 + + await set_admin(db,"test", False) + await create_user(db,UserCreate(uid="test2",given_name="tester", mail="test@test.test")) + response2 = await client.post(f"/api/subjects/{subject_id}/teachers", params={'user_id': 'test2'}) + assert response2.status_code == 201 # Success because we are teacher now + +@pytest.mark.anyio +async def test_get_subjects(client: AsyncClient, subject_id: int): + await client.post(f"/api/subjects/{subject_id}/teachers", params={'user_id': 'test'}) + response2 = await client.get("/api/subjects/") + assert response2.status_code == 200 + assert len(response2.json()["as_teacher"]) == 1 + + +@pytest.mark.anyio +async def test_delete_subject(client: AsyncClient, db: Session, subject_id: int): + await set_admin(db,"test", False) + response2 = await client.delete(f"/api/subjects/{subject_id}") + assert response2.status_code == 403 #Forbidden + await set_admin(db,"test", True) + response3 = await client.delete(f"/api/subjects/{subject_id}") + assert response3.status_code == 200 + + response4 = await client.get(f"/api/subjects/{subject_id}") + assert response4.status_code == 404 # Not Found diff --git a/backend/tests/user/placeholder.py b/backend/tests/user/placeholder.py deleted file mode 100644 index e69de29b..00000000 From 84261c0f51a0a6cbbb9bc0e677eae099e672bbef Mon Sep 17 00:00:00 2001 From: Xander Bil Date: Mon, 11 Mar 2024 14:24:56 +0100 Subject: [PATCH 2/7] update workflow --- .github/workflows/lint.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index cdac9afd..4894f61a 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -47,6 +47,12 @@ jobs: pytest: name: Unit Testing runs-on: self-hosted + env: + DATABASE_URI: "postgresql://selab@127.0.0.1:2002/selab-test" + FRONTEND_URL: "https://localhost:8080" + CAS_SERVER_URL: "https://login.ugent.be" + SECRET_KEY: "test" + ALGORITHM: "HS256" steps: - uses: actions/checkout@v3 - name: Set up Python 3.12 @@ -65,7 +71,7 @@ jobs: - name: Test with pytest run: | pip install pytest pytest-cov pytest-html pytest-sugar pytest-json-report - DATABASE_URI='postgresql://selab@127.0.0.1:2002/selab' py.test -v --cov --html=reports/pytest/report.html + py.test -v --cov --html=reports/pytest/report.html - name: Archive pytest coverage results uses: actions/upload-artifact@v1 with: From 88c48f992a378187404cdf91e4b49a68eb3894f1 Mon Sep 17 00:00:00 2001 From: Xander Bil Date: Mon, 11 Mar 2024 14:25:47 +0100 Subject: [PATCH 3/7] update requirements --- backend/requirements.txt | 7 +++++++ backend/src/subject/service.py | 2 +- backend/src/user/service.py | 4 +++- backend/tests/conftest.py | 10 ++++++++-- backend/tests/test_subject.py | 34 +++++++++++++++++++--------------- 5 files changed, 38 insertions(+), 19 deletions(-) diff --git a/backend/requirements.txt b/backend/requirements.txt index b6c4a549..959c0992 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -12,12 +12,17 @@ exceptiongroup==1.1.2 fastapi==0.109.2 greenlet==3.0.3 h11==0.14.0 +httpcore==1.0.4 httptools==0.6.1 +httpx==0.27.0 idna==3.6 +iniconfig==2.0.0 itsdangerous==2.1.2 lsprotocol==2023.0.0a2 lxml==5.1.0 nodeenv==1.8.0 +packaging==24.0 +pluggy==1.4.0 psycopg2-binary==2.9.9 pycodestyle==2.11.1 pycparser==2.21 @@ -26,10 +31,12 @@ pydantic_core==2.16.2 pygls==1.0.1 PyJWT==2.8.0 pyright==1.1.352 +pytest==8.1.1 python-cas==1.6.0 python-dotenv==1.0.1 PyYAML==6.0.1 requests==2.31.0 +setuptools==69.1.1 six==1.16.0 sniffio==1.3.0 SQLAlchemy==2.0.27 diff --git a/backend/src/subject/service.py b/backend/src/subject/service.py index 5ceee884..981fe15b 100644 --- a/backend/src/subject/service.py +++ b/backend/src/subject/service.py @@ -19,7 +19,7 @@ async def get_subjects(db: Session, user_id: str) -> tuple[Sequence[models.Subje async def get_teachers(db: Session, subject_id: int) -> list[User]: - return db.query(User).join(models.TeacherSubject, models.TeacherSubject.c.subject_id==subject_id).all() + return db.query(User).join(models.TeacherSubject, models.TeacherSubject.c.subject_id == subject_id).all() async def create_subject(db: Session, subject: schemas.SubjectCreate) -> models.Subject: diff --git a/backend/src/user/service.py b/backend/src/user/service.py index b622ddc0..fc364b53 100644 --- a/backend/src/user/service.py +++ b/backend/src/user/service.py @@ -13,11 +13,13 @@ async def create_user(db: Session, user: schemas.UserCreate) -> models.User: db.refresh(db_user) return db_user + async def delete_user(db: Session, user_id: str): db.query(models.User).filter_by(uid=user_id).delete() db.commit() + async def set_admin(db: Session, user_id: str, value: bool): - user = await get_by_id(db,user_id) + user = await get_by_id(db, user_id) user.is_admin = value db.commit() diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index f1a03190..750cb299 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -13,6 +13,7 @@ trans = connection.begin() TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=connection) + def get_db_override(): db = TestingSessionLocal() try: @@ -20,12 +21,15 @@ def get_db_override(): finally: db.close() + app.dependency_overrides[get_db] = get_db_override + @pytest.fixture def anyio_backend(): return 'asyncio' + @pytest.fixture async def db(): global trans @@ -37,14 +41,16 @@ async def db(): trans.rollback() trans = connection.begin() + @pytest.fixture async def client(db: Session): token = create_jwt_token("test") - await create_user(db,UserCreate(uid="test",given_name="tester", mail="test@test.test")) + await create_user(db, UserCreate(uid="test", given_name="tester", mail="test@test.test")) - async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test", headers = {"Authorization": f"Bearer {token.token}"}) as client: + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test", headers={"Authorization": f"Bearer {token.token}"}) as client: yield client + def pytest_sessionfinish(): connection.close() diff --git a/backend/tests/test_subject.py b/backend/tests/test_subject.py index a11d2ac7..502e5405 100644 --- a/backend/tests/test_subject.py +++ b/backend/tests/test_subject.py @@ -8,23 +8,25 @@ subject = {'name': 'test subject'} + @pytest.fixture async def subject_id(client: AsyncClient, db: Session) -> int: """Create new subject""" - await set_admin(db,"test", True) + await set_admin(db, "test", True) response = await client.post("/api/subjects/", json=subject) return response.json()["id"] + @pytest.mark.anyio async def test_create_subject(client: AsyncClient, db: Session): - await set_admin(db,"test",False) + await set_admin(db, "test", False) response = await client.post("/api/subjects/", json=subject) - assert response.status_code == 403 # Forbidden, not admin + assert response.status_code == 403 # Forbidden, not admin - await set_admin(db,"test", True) + await set_admin(db, "test", True) response = await client.post("/api/subjects/", json=subject) - assert response.status_code == 201 # Created + assert response.status_code == 201 # Created assert response.json()["name"] == subject["name"] @@ -35,21 +37,23 @@ async def test_get_subject(client: AsyncClient, subject_id: int): assert response.status_code == 200 assert response.json()["name"] == subject["name"] + @pytest.mark.anyio async def test_create_teacher(client: AsyncClient, db: Session, subject_id: int): - await set_admin(db,"test", False) + await set_admin(db, "test", False) response2 = await client.post(f"/api/subjects/{subject_id}/teachers", params={'user_id': 'test'}) - assert response2.status_code == 403 # Forbidden + assert response2.status_code == 403 # Forbidden - await set_admin(db,"test", True) + await set_admin(db, "test", True) response2 = await client.post(f"/api/subjects/{subject_id}/teachers", params={'user_id': 'test'}) assert response2.status_code == 201 - await set_admin(db,"test", False) - await create_user(db,UserCreate(uid="test2",given_name="tester", mail="test@test.test")) + await set_admin(db, "test", False) + await create_user(db, UserCreate(uid="test2", given_name="tester", mail="test@test.test")) response2 = await client.post(f"/api/subjects/{subject_id}/teachers", params={'user_id': 'test2'}) - assert response2.status_code == 201 # Success because we are teacher now + assert response2.status_code == 201 # Success because we are teacher now + @pytest.mark.anyio async def test_get_subjects(client: AsyncClient, subject_id: int): @@ -61,12 +65,12 @@ async def test_get_subjects(client: AsyncClient, subject_id: int): @pytest.mark.anyio async def test_delete_subject(client: AsyncClient, db: Session, subject_id: int): - await set_admin(db,"test", False) + await set_admin(db, "test", False) response2 = await client.delete(f"/api/subjects/{subject_id}") - assert response2.status_code == 403 #Forbidden - await set_admin(db,"test", True) + assert response2.status_code == 403 # Forbidden + await set_admin(db, "test", True) response3 = await client.delete(f"/api/subjects/{subject_id}") assert response3.status_code == 200 response4 = await client.get(f"/api/subjects/{subject_id}") - assert response4.status_code == 404 # Not Found + assert response4.status_code == 404 # Not Found From 1e162f3bb18612620f34c1ad52d522876c70d219 Mon Sep 17 00:00:00 2001 From: Xander Bil Date: Mon, 11 Mar 2024 14:32:39 +0100 Subject: [PATCH 4/7] update workflow --- .github/workflows/lint.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 4894f61a..d78d278d 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -48,7 +48,7 @@ jobs: name: Unit Testing runs-on: self-hosted env: - DATABASE_URI: "postgresql://selab@127.0.0.1:2002/selab-test" + DATABASE_URI: "${{ secrets.POSTGRES_CONNECTION }}" FRONTEND_URL: "https://localhost:8080" CAS_SERVER_URL: "https://login.ugent.be" SECRET_KEY: "test" @@ -67,7 +67,7 @@ jobs: - name: Initialize Database working-directory: backend run: | - psql -U selab -p 2002 -d selab < selab_script.sql + psql -U selab -p 2002 -d selabtest < selab_script.sql - name: Test with pytest run: | pip install pytest pytest-cov pytest-html pytest-sugar pytest-json-report From 4115dfa345ef02ede754b015ea13ce92437831ad Mon Sep 17 00:00:00 2001 From: Xander Bil Date: Mon, 11 Mar 2024 14:58:46 +0100 Subject: [PATCH 5/7] Update readme --- backend/README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/backend/README.md b/backend/README.md index 78fa9b53..3f603b19 100644 --- a/backend/README.md +++ b/backend/README.md @@ -60,3 +60,13 @@ authorize each request, add the token in the `Authorization` header. ```sh autopep8 -ri . ``` + +## Testing + +You can add tests by creating `test_*` files and `test_*` functions under `tests` directory. + +### Run the tests (in the virtual environment): + +```sh +pytest -v +``` From de94a495b15965261a79394b1a666403d7ae9664 Mon Sep 17 00:00:00 2001 From: Xander Bil Date: Mon, 11 Mar 2024 20:11:53 +0100 Subject: [PATCH 6/7] fix type error --- backend/tests/conftest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 750cb299..8dc56431 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -48,7 +48,8 @@ async def client(db: Session): await create_user(db, UserCreate(uid="test", given_name="tester", mail="test@test.test")) - async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test", headers={"Authorization": f"Bearer {token.token}"}) as client: + transport = ASGITransport(app=app) # type: ignore + async with AsyncClient(transport=transport, base_url="http://test", headers={"Authorization": f"Bearer {token.token}"}) as client: yield client From 3812fc2c87555b58248a344a560a2455404e97e2 Mon Sep 17 00:00:00 2001 From: Xander Bil Date: Mon, 11 Mar 2024 23:06:34 +0100 Subject: [PATCH 7/7] remove delete user code --- backend/src/user/service.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/backend/src/user/service.py b/backend/src/user/service.py index fc364b53..c69a64fc 100644 --- a/backend/src/user/service.py +++ b/backend/src/user/service.py @@ -14,11 +14,6 @@ async def create_user(db: Session, user: schemas.UserCreate) -> models.User: return db_user -async def delete_user(db: Session, user_id: str): - db.query(models.User).filter_by(uid=user_id).delete() - db.commit() - - async def set_admin(db: Session, user_id: str, value: bool): user = await get_by_id(db, user_id) user.is_admin = value