From 1c6759c5f84e2b83b99cc8ff32fee639504b7c11 Mon Sep 17 00:00:00 2001 From: podmar Date: Thu, 11 Jun 2026 22:02:41 +0200 Subject: [PATCH 01/18] feat: add integration test infrastructure --- .gitignore | 3 +- backend/.env.example | 1 + backend/app/config.py | 2 +- backend/pyproject.toml | 2 ++ backend/tests/__init__.py | 0 backend/tests/conftest.py | 71 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 backend/tests/__init__.py create mode 100644 backend/tests/conftest.py diff --git a/.gitignore b/.gitignore index c508e42..9907fa4 100644 --- a/.gitignore +++ b/.gitignore @@ -232,4 +232,5 @@ Thumbs.db docs/context.md docs/pr-description.md docs/pr-comments.md -docs/lint-report.md \ No newline at end of file +docs/lint-report.md +docs/test-report.txt \ No newline at end of file diff --git a/backend/.env.example b/backend/.env.example index e8b9a80..4113983 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -3,6 +3,7 @@ # Use the asyncpg driver (required for async FastAPI) # Use ssl=require (not sslmode=require) for asyncpg compatibility with Neon DATABASE_URL=postgresql+asyncpg://user:password@host/dbname?ssl=require +TEST_DATABASE_URL=postgresql+asyncpg://user:password@host/test_dbname?ssl=require # ── Auth (FastAPI Users) ───────────────────────────────────────────── # Generate with: python -c "import secrets; print(secrets.token_hex(32))" diff --git a/backend/app/config.py b/backend/app/config.py index f4e7ae4..002c5bc 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -5,7 +5,7 @@ class Settings(BaseSettings): - model_config = SettingsConfigDict(env_file=".env") + model_config = SettingsConfigDict(env_file=".env", extra="ignore") # no default value for database_url, it must be provided via environment variable or .env file database_url: SecretStr = Field( description="Database URL in the format: postgresql+asyncpg://user:password@host:port/database", diff --git a/backend/pyproject.toml b/backend/pyproject.toml index b6bf3da..e556de8 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -77,4 +77,6 @@ testpaths = ["tests"] [dependency-groups] dev = [ "pyright>=1.1.410", + "pytest>=9.0.3", + "pytest-asyncio>=1.4.0", ] diff --git a/backend/tests/__init__.py b/backend/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py new file mode 100644 index 0000000..7406d3f --- /dev/null +++ b/backend/tests/conftest.py @@ -0,0 +1,71 @@ +from collections.abc import AsyncGenerator + +import pytest +from httpx import ASGITransport, AsyncClient +from pydantic import SecretStr +from pydantic_settings import BaseSettings, SettingsConfigDict +from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine +from sqlalchemy.pool import NullPool +from sqlmodel import SQLModel +from sqlmodel.ext.asyncio.session import AsyncSession + +import app.database as _db + + +class _Env(BaseSettings): + model_config = SettingsConfigDict(env_file=".env", extra="ignore") + test_database_url: SecretStr + + +_env = _Env() # type: ignore[call-arg] + +# Replace the app's engine and session factory with test-DB equivalents before +# app.main is imported. All subsequent imports see the patched module globals, +# so every request in tests hits the test DB — not production. +# NullPool creates a fresh connection per operation and never retains one between +# uses — required for async SQLAlchemy tests where each test runs in its own +# event loop. A retained connection would be bound to the loop that created it +# and fail with "Future attached to a different loop" in subsequent tests. +_test_engine = create_async_engine( + _env.test_database_url.get_secret_value(), + poolclass=NullPool, +) +_db.engine = _test_engine +_db._session_factory = async_sessionmaker(_test_engine, class_=AsyncSession, expire_on_commit=False) + +from app.main import app # noqa: E402 — must follow the patch above + + +@pytest.fixture(autouse=True) +async def clean_db() -> None: + # Drop and recreate all tables so the schema always reflects the current models, + # including any constraints added after the test DB was first provisioned. + # With NullPool each connection is fresh, so this is safe across event loops. + async with _test_engine.begin() as conn: + await conn.run_sync(SQLModel.metadata.drop_all) + await conn.run_sync(SQLModel.metadata.create_all) + + +@pytest.fixture +async def client() -> AsyncGenerator[AsyncClient]: + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac: + yield ac + + +@pytest.fixture +async def auth(client: AsyncClient) -> dict[str, str]: + await client.post("/auth/register", json={"email": "alice@test.com", "password": "testpass123"}) + resp = await client.post( + "/auth/jwt/login", data={"username": "alice@test.com", "password": "testpass123"} + ) + return {"Authorization": f"Bearer {resp.json()['access_token']}"} + + +@pytest.fixture +async def other_auth(client: AsyncClient) -> dict[str, str]: + """Second user in a separate household — for isolation tests.""" + await client.post("/auth/register", json={"email": "bob@test.com", "password": "testpass123"}) + resp = await client.post( + "/auth/jwt/login", data={"username": "bob@test.com", "password": "testpass123"} + ) + return {"Authorization": f"Bearer {resp.json()['access_token']}"} From 77a4a50859a154fa96e36c8981a0ac1d780f48e8 Mon Sep 17 00:00:00 2001 From: podmar Date: Fri, 12 Jun 2026 13:49:34 +0200 Subject: [PATCH 02/18] test: locations CRUD, isolation, and delete-with-move --- backend/tests/test_locations.py | 108 ++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 backend/tests/test_locations.py diff --git a/backend/tests/test_locations.py b/backend/tests/test_locations.py new file mode 100644 index 0000000..5f63c21 --- /dev/null +++ b/backend/tests/test_locations.py @@ -0,0 +1,108 @@ +import pytest +from httpx import AsyncClient + + +@pytest.fixture +async def location_id(client, auth): + """A second location — Pantry is always seeded; tests that delete need two.""" + resp = await client.post("/locations", json={"name": "Fridge"}, headers=auth) + return resp.json()["id"] + + +@pytest.fixture +async def item_id(client, auth): + resp = await client.post("/items", json={"name": "Milk", "unit": "litre"}, headers=auth) + return resp.json()["id"] + + +async def test_list_returns_seeded_pantry(client, auth): + resp = await client.get("/locations", headers=auth) + assert resp.status_code == 200 + assert any(loc["name"] == "Pantry" for loc in resp.json()) + + +async def test_create_location(client, auth): + resp = await client.post("/locations", json={"name": "Fridge"}, headers=auth) + assert resp.status_code == 201 + assert resp.json()["name"] == "Fridge" + + +async def test_create_duplicate_name_returns_409(client, auth): + await client.post("/locations", json={"name": "Fridge"}, headers=auth) + resp = await client.post("/locations", json={"name": "Fridge"}, headers=auth) + assert resp.status_code == 409 + + +async def test_update_location(client, auth, location_id): + resp = await client.patch(f"/locations/{location_id}", json={"name": "Deep Freeze"}, headers=auth) + assert resp.status_code == 200 + assert resp.json()["name"] == "Deep Freeze" + + +async def test_delete_location(client, auth, location_id): + resp = await client.delete(f"/locations/{location_id}", headers=auth) + assert resp.status_code == 204 + names = [loc["name"] for loc in (await client.get("/locations", headers=auth)).json()] + assert "Fridge" not in names + + +async def test_cannot_delete_last_location(client, auth): + pantry_id = (await client.get("/locations", headers=auth)).json()[0]["id"] + resp = await client.delete(f"/locations/{pantry_id}", headers=auth) + assert resp.status_code == 409 + + +async def test_delete_location_with_batches_requires_move_to(client: AsyncClient, auth, location_id, item_id): + await client.post(f"/items/{item_id}/batches", json={"quantity": 2, "location_id": location_id}, headers=auth) + resp = await client.delete(f"/locations/{location_id}", headers=auth) + assert resp.status_code == 409 + + +async def test_delete_location_moves_batches(client: AsyncClient, auth, location_id, item_id): + pantry_id = next( + loc["id"] for loc in (await client.get("/locations", headers=auth)).json() + if loc["name"] == "Pantry" + ) + await client.post(f"/items/{item_id}/batches", json={"quantity": 2, "location_id": location_id}, headers=auth) + + resp = await client.delete(f"/locations/{location_id}?move_to={pantry_id}", headers=auth) + assert resp.status_code == 204 + + batches = (await client.get(f"/items/{item_id}/batches", headers=auth)).json() + assert len(batches) == 1 + assert batches[0]["location_id"] == pantry_id + assert batches[0]["quantity"] == 2 + + +async def test_delete_location_merges_batches_on_collision(client: AsyncClient, auth, location_id, item_id): + pantry_id = next( + loc["id"] for loc in (await client.get("/locations", headers=auth)).json() + if loc["name"] == "Pantry" + ) + # Same item, same default expiry → moving Fridge batch to Pantry should merge. + await client.post(f"/items/{item_id}/batches", json={"quantity": 3, "location_id": pantry_id}, headers=auth) + await client.post(f"/items/{item_id}/batches", json={"quantity": 2, "location_id": location_id}, headers=auth) + + resp = await client.delete(f"/locations/{location_id}?move_to={pantry_id}", headers=auth) + assert resp.status_code == 204 + + batches = (await client.get(f"/items/{item_id}/batches", headers=auth)).json() + assert len(batches) == 1 + assert batches[0]["quantity"] == 5 + + +async def test_delete_location_move_to_same_location_returns_400(client: AsyncClient, auth, location_id, item_id): + await client.post(f"/items/{item_id}/batches", json={"quantity": 1, "location_id": location_id}, headers=auth) + resp = await client.delete(f"/locations/{location_id}?move_to={location_id}", headers=auth) + assert resp.status_code == 400 + + +async def test_isolation_cannot_update_other_household_location(client: AsyncClient, auth, other_auth): + pantry_id = (await client.get("/locations", headers=auth)).json()[0]["id"] + resp = await client.patch(f"/locations/{pantry_id}", json={"name": "Stolen"}, headers=other_auth) + assert resp.status_code == 404 + + +async def test_isolation_cannot_delete_other_household_location(client: AsyncClient, auth, other_auth, location_id): + resp = await client.delete(f"/locations/{location_id}", headers=other_auth) + assert resp.status_code == 404 From 43a2f1b061f505f73cca3b968600351611ee9ad4 Mon Sep 17 00:00:00 2001 From: podmar Date: Mon, 15 Jun 2026 13:57:29 +0200 Subject: [PATCH 03/18] test: idems CRUD, isolation, barcode upsert and cascade delete --- backend/tests/test_items.py | 130 ++++++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 backend/tests/test_items.py diff --git a/backend/tests/test_items.py b/backend/tests/test_items.py new file mode 100644 index 0000000..a8cff59 --- /dev/null +++ b/backend/tests/test_items.py @@ -0,0 +1,130 @@ +import pytest +from httpx import AsyncClient + + +@pytest.fixture +async def item_id(client, auth): + resp = await client.post( + "/items", json={"name": "Milk", "unit": "litre"}, headers=auth + ) + return resp.json()["id"] + + +async def test_create_item(client, auth): + resp = await client.post( + "/items", json={"name": "Beans", "unit": "can"}, headers=auth + ) + assert resp.status_code == 201 + assert resp.json()["name"] == "Beans" + + +async def test_list_items(client, auth, item_id): + resp = await client.get("/items", headers=auth) + assert resp.status_code == 200 + assert any(i["id"] == item_id for i in resp.json()) + + +async def test_list_items_filter_by_name(client, auth): + await client.post( + "/items", json={"name": "Whole Milk", "unit": "litre"}, headers=auth + ) + await client.post( + "/items", json={"name": "Oat Milk", "unit": "litre"}, headers=auth + ) + await client.post("/items", json={"name": "Butter", "unit": "g"}, headers=auth) + resp = await client.get("/items?name=milk", headers=auth) + names = [i["name"] for i in resp.json()] + assert "Whole Milk" in names + assert "Oat Milk" in names + assert "Butter" not in names + + +async def test_get_item(client, auth, item_id): + resp = await client.get(f"/items/{item_id}", headers=auth) + assert resp.status_code == 200 + assert resp.json()["id"] == item_id + + +async def test_update_item(client, auth, item_id): + resp = await client.patch( + f"/items/{item_id}", json={"name": "Skimmed Milk"}, headers=auth + ) + assert resp.status_code == 200 + assert resp.json()["name"] == "Skimmed Milk" + + +async def test_duplicate_barcode_returns_existing_with_200(client: AsyncClient, auth): + first = await client.post( + "/items", + json={"name": "Milk", "barcode": "1234567890123", "unit": "litre"}, + headers=auth, + ) + assert first.status_code == 201 + + second = await client.post( + "/items", + json={"name": "Milk", "barcode": "1234567890123", "unit": "litre"}, + headers=auth, + ) + assert second.status_code == 200 + assert second.json()["id"] == first.json()["id"] + + +async def test_delete_item_cascades_batches(client: AsyncClient, auth, item_id): + await client.post(f"/items/{item_id}/batches", json={"quantity": 3}, headers=auth) + await client.delete(f"/items/{item_id}", headers=auth) + # Item is gone + assert (await client.get(f"/items/{item_id}", headers=auth)).status_code == 404 + # Batches are gone too + assert ( + await client.get(f"/items/{item_id}/batches", headers=auth) + ).status_code == 404 + + +async def test_list_items_filter_by_location(client: AsyncClient, auth): + fridge_id = ( + await client.post("/locations", json={"name": "Fridge"}, headers=auth) + ).json()["id"] + pantry_id = next( + loc["id"] + for loc in (await client.get("/locations", headers=auth)).json() + if loc["name"] == "Pantry" + ) + milk_id = ( + await client.post( + "/items", json={"name": "Milk", "unit": "litre"}, headers=auth + ) + ).json()["id"] + butter_id = ( + await client.post("/items", json={"name": "Butter", "unit": "g"}, headers=auth) + ).json()["id"] + + await client.post( + f"/items/{milk_id}/batches", + json={"quantity": 1, "location_id": fridge_id}, + headers=auth, + ) + await client.post( + f"/items/{butter_id}/batches", + json={"quantity": 1, "location_id": pantry_id}, + headers=auth, + ) + + resp = await client.get(f"/items?location_id={fridge_id}", headers=auth) + names = [i["name"] for i in resp.json()] + assert "Milk" in names + assert "Butter" not in names + + +async def test_isolation_cannot_read_other_household_item( + client: AsyncClient, auth, other_auth, item_id +): + resp = await client.get(f"/items/{item_id}", headers=other_auth) + assert resp.status_code == 404 + + +async def test_isolation_cannot_delete_other_household_item( + client: AsyncClient, auth, other_auth, item_id +): + resp = await client.delete(f"/items/{item_id}", headers=other_auth) + assert resp.status_code == 404 From 9cee6654be3e6e4c7538d00b0faba5753a0dfca3 Mon Sep 17 00:00:00 2001 From: podmar Date: Mon, 15 Jun 2026 14:02:01 +0200 Subject: [PATCH 04/18] test: batches CRUD, merge-on-collision, adjust, expiry, and isolation --- backend/tests/test_batches.py | 128 ++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 backend/tests/test_batches.py diff --git a/backend/tests/test_batches.py b/backend/tests/test_batches.py new file mode 100644 index 0000000..ac3dc40 --- /dev/null +++ b/backend/tests/test_batches.py @@ -0,0 +1,128 @@ +import pytest +from httpx import AsyncClient + + +@pytest.fixture +async def item_id(client, auth): + resp = await client.post("/items", json={"name": "Milk", "unit": "litre"}, headers=auth) + return resp.json()["id"] + + +@pytest.fixture +async def batch_id(client, auth, item_id): + resp = await client.post(f"/items/{item_id}/batches", json={"quantity": 5}, headers=auth) + return resp.json()["id"] + + +async def test_create_batch(client, auth, item_id): + resp = await client.post(f"/items/{item_id}/batches", json={"quantity": 3}, headers=auth) + assert resp.status_code == 201 + assert resp.json()["quantity"] == 3 + + +async def test_create_batch_merges_on_collision(client: AsyncClient, auth, item_id): + # Two POSTs with no explicit location or expiry resolve to the same defaults, + # so the second should merge into the first rather than creating a duplicate. + first = await client.post(f"/items/{item_id}/batches", json={"quantity": 3}, headers=auth) + assert first.status_code == 201 + + second = await client.post(f"/items/{item_id}/batches", json={"quantity": 2}, headers=auth) + assert second.status_code == 200 + assert second.json()["id"] == first.json()["id"] + assert second.json()["quantity"] == 5 + + +async def test_list_batches(client, auth, item_id, batch_id): + resp = await client.get(f"/items/{item_id}/batches", headers=auth) + assert resp.status_code == 200 + assert any(b["id"] == batch_id for b in resp.json()) + + +async def test_update_batch_quantity(client, auth, batch_id): + resp = await client.patch(f"/batches/{batch_id}", json={"quantity": 10}, headers=auth) + assert resp.status_code == 200 + assert resp.json()["quantity"] == 10 + + +async def test_update_batch_merges_on_collision(client: AsyncClient, auth, item_id): + loc_resp = await client.post("/locations", json={"name": "Fridge"}, headers=auth) + fridge_id = loc_resp.json()["id"] + pantry_id = (await client.get("/locations", headers=auth)).json() + pantry_id = next(loc["id"] for loc in pantry_id if loc["name"] == "Pantry") + + # Batch A in Pantry, batch B in Fridge — same item, same default expiry. + batch_a = (await client.post(f"/items/{item_id}/batches", json={"quantity": 3, "location_id": pantry_id}, headers=auth)).json() + batch_b = (await client.post(f"/items/{item_id}/batches", json={"quantity": 2, "location_id": fridge_id}, headers=auth)).json() + + # Moving batch B to Pantry collides with batch A — should merge. + resp = await client.patch(f"/batches/{batch_b['id']}", json={"location_id": pantry_id}, headers=auth) + assert resp.status_code == 200 + assert resp.json()["id"] == batch_a["id"] + assert resp.json()["quantity"] == 5 + + +async def test_delete_batch(client, auth, item_id, batch_id): + resp = await client.delete(f"/batches/{batch_id}", headers=auth) + assert resp.status_code == 204 + batches = (await client.get(f"/items/{item_id}/batches", headers=auth)).json() + assert not any(b["id"] == batch_id for b in batches) + + +async def test_adjust_increases_quantity(client, auth, batch_id): + resp = await client.post(f"/batches/{batch_id}/adjust", json={"delta": 3}, headers=auth) + assert resp.status_code == 200 + assert resp.json()["quantity"] == 8 + + +async def test_adjust_decreases_quantity(client, auth, batch_id): + resp = await client.post(f"/batches/{batch_id}/adjust", json={"delta": -2}, headers=auth) + assert resp.status_code == 200 + assert resp.json()["quantity"] == 3 + + +async def test_adjust_to_zero_deletes_batch(client: AsyncClient, auth, item_id, batch_id): + resp = await client.post(f"/batches/{batch_id}/adjust", json={"delta": -5}, headers=auth) + assert resp.status_code == 204 + batches = (await client.get(f"/items/{item_id}/batches", headers=auth)).json() + assert batches == [] + + +async def test_adjust_below_zero_returns_422(client, auth, batch_id): + resp = await client.post(f"/batches/{batch_id}/adjust", json={"delta": -99}, headers=auth) + assert resp.status_code == 422 + + +async def test_expiring(client: AsyncClient, auth, item_id): + from datetime import date, timedelta + + expiring_soon = str(date.today() + timedelta(days=7)) + not_expiring = str(date.today() + timedelta(days=60)) + + await client.post(f"/items/{item_id}/batches", json={"quantity": 1, "expiry_date": expiring_soon}, headers=auth) + await client.post(f"/items/{item_id}/batches", json={"quantity": 1, "expiry_date": not_expiring}, headers=auth) + + resp = await client.get("/expiring?days=30", headers=auth) + assert resp.status_code == 200 + expiry_dates = [b["expiry_date"] for b in resp.json()] + assert expiring_soon in expiry_dates + assert not_expiring not in expiry_dates + + +async def test_expiring_isolation(client: AsyncClient, auth, other_auth): + from datetime import date, timedelta + + expiring_soon = str(date.today() + timedelta(days=7)) + + # Create a batch for user A expiring soon. + item_id = (await client.post("/items", json={"name": "Milk", "unit": "litre"}, headers=auth)).json()["id"] + await client.post(f"/items/{item_id}/batches", json={"quantity": 1, "expiry_date": expiring_soon}, headers=auth) + + # User B's expiring list should be empty — different household. + resp = await client.get("/expiring?days=30", headers=other_auth) + assert resp.status_code == 200 + assert resp.json() == [] + + +async def test_isolation_cannot_adjust_other_household_batch(client: AsyncClient, auth, other_auth, batch_id): + resp = await client.post(f"/batches/{batch_id}/adjust", json={"delta": -1}, headers=other_auth) + assert resp.status_code == 404 From ac4ee676fb1f723a5f5d35e9550cf9dacaa5858e Mon Sep 17 00:00:00 2001 From: podmar Date: Mon, 15 Jun 2026 14:10:02 +0200 Subject: [PATCH 05/18] test: categories CRUD and isolation --- backend/tests/test_categories.py | 56 ++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 backend/tests/test_categories.py diff --git a/backend/tests/test_categories.py b/backend/tests/test_categories.py new file mode 100644 index 0000000..15dc110 --- /dev/null +++ b/backend/tests/test_categories.py @@ -0,0 +1,56 @@ +import pytest +from httpx import AsyncClient + + +@pytest.fixture +async def category_id(client, auth): + """A second category — Food is always seeded.""" + resp = await client.post("/categories", json={"name": "Cleaning"}, headers=auth) + return resp.json()["id"] + + +async def test_list_returns_seeded_food(client, auth): + resp = await client.get("/categories", headers=auth) + assert resp.status_code == 200 + assert any(cat["name"] == "Food" for cat in resp.json()) + + +async def test_create_category(client, auth): + resp = await client.post("/categories", json={"name": "Cleaning"}, headers=auth) + assert resp.status_code == 201 + assert resp.json()["name"] == "Cleaning" + + +async def test_create_duplicate_name_returns_409(client, auth): + await client.post("/categories", json={"name": "Cleaning"}, headers=auth) + resp = await client.post("/categories", json={"name": "Cleaning"}, headers=auth) + assert resp.status_code == 409 + + +async def test_update_category(client, auth, category_id): + resp = await client.patch(f"/categories/{category_id}", json={"name": "Toiletries"}, headers=auth) + assert resp.status_code == 200 + assert resp.json()["name"] == "Toiletries" + + +async def test_delete_category(client, auth, category_id): + resp = await client.delete(f"/categories/{category_id}", headers=auth) + assert resp.status_code == 204 + names = [cat["name"] for cat in (await client.get("/categories", headers=auth)).json()] + assert "Cleaning" not in names + + +async def test_cannot_delete_category_with_items(client: AsyncClient, auth, category_id): + await client.post("/items", json={"name": "Soap", "unit": "bar", "category_id": category_id}, headers=auth) + resp = await client.delete(f"/categories/{category_id}", headers=auth) + assert resp.status_code == 409 + + +async def test_isolation_cannot_update_other_household_category(client: AsyncClient, auth, other_auth, category_id): + resp = await client.patch(f"/categories/{category_id}", json={"name": "Stolen"}, headers=other_auth) + assert resp.status_code == 404 + + +async def test_isolation_cannot_delete_other_household_category(client: AsyncClient, auth, other_auth, category_id): + resp = await client.delete(f"/categories/{category_id}", headers=other_auth) + assert resp.status_code == 404 From a7b0b7fb8813d5f05a8d398628afae7a4d78199d Mon Sep 17 00:00:00 2001 From: podmar Date: Mon, 15 Jun 2026 14:23:28 +0200 Subject: [PATCH 06/18] fix: check batch collision before setattr to avoid autoflush on PATCH --- backend/app/routers/batches.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/backend/app/routers/batches.py b/backend/app/routers/batches.py index 9650f82..24a4470 100644 --- a/backend/app/routers/batches.py +++ b/backend/app/routers/batches.py @@ -168,19 +168,19 @@ async def update_batch( if location is None or location.household_id != user.household_id: raise HTTPException(status_code=404, detail="Location not found") - for field, value in updates.items(): - setattr(batch, field, value) + # Resolve what the new (location, expiry) would be after the update, then check + # for a collision BEFORE modifying the batch. Modifying first would mark the ORM + # object dirty; SQLAlchemy's autoflush would then emit the UPDATE before our SELECT, + # hitting the unique constraint instead of letting us handle the merge gracefully. + new_location_id = updates.get("location_id", batch.location_id) + new_expiry_date = updates.get("expiry_date", batch.expiry_date) - # Check whether the updated (item, location, expiry) would collide with an existing batch. - # If so, merge quantities into the surviving batch and delete this one — same behaviour - # as the location-delete move flow. Returns the surviving batch so the client sees the - # correct quantity at the destination without needing a separate fetch. existing = ( await session.exec( select(Batch).where( Batch.item_id == batch.item_id, - Batch.location_id == batch.location_id, - Batch.expiry_date == batch.expiry_date, + Batch.location_id == new_location_id, + Batch.expiry_date == new_expiry_date, Batch.household_id == user.household_id, Batch.id != batch_id, ) @@ -195,6 +195,9 @@ async def update_batch( await session.refresh(existing) return BatchRead.model_validate(existing) + for field, value in updates.items(): + setattr(batch, field, value) + session.add(batch) await session.commit() await session.refresh(batch) From 15735d9cf7d722ed5f1dd9afdab142233a9f56dc Mon Sep 17 00:00:00 2001 From: podmar Date: Mon, 15 Jun 2026 22:46:57 +0200 Subject: [PATCH 07/18] feat: add run-tests script, fix-test skill, and skill template guidance --- .claude/commands/fix-test.md | 29 +++++++++++++++++++++++ .claude/prompts/skill-template.md | 2 ++ scripts/run-tests.sh | 38 +++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 .claude/commands/fix-test.md create mode 100755 scripts/run-tests.sh diff --git a/.claude/commands/fix-test.md b/.claude/commands/fix-test.md new file mode 100644 index 0000000..ec38cc9 --- /dev/null +++ b/.claude/commands/fix-test.md @@ -0,0 +1,29 @@ +Read `docs/test-report.txt`. If the file doesn't exist, tell the user to run `./scripts/run-tests.sh` first and stop. If all tests passed, say so and stop. + +Count the failures and tell the user how many there are before starting. Work through them one at a time in the order they appear in the report. + +For each failure: + +Show the test name and the full error message and traceback — do not truncate it. + +Read the failing test function from its test file. Also read the router or source function it exercises — find it by tracing the HTTP path the test calls (e.g. `POST /items` → `app/routers/items.py`). + +Diagnose which of these four failure modes applies: +- **Source bug**: the source code does not implement what the test expects. Fix the source. +- **Test bug**: the test asserts something wrong (wrong status code, wrong field name, wrong logic). Fix the test. +- **Fixture bug**: a shared fixture in `conftest.py` produces unexpected state. Fix the fixture. +- **Structural error**: import failure, missing dependency, schema mismatch. Name the root cause explicitly before proposing anything. + +If the diagnosis is ambiguous between two modes, say which two and ask the user before touching anything. + +For source bugs and test bugs: propose the exact change and state why you're confident in the diagnosis. Wait for the user to confirm before writing. + +For fixture bugs: do the same, but name every test that uses the fixture so the user knows the blast radius. + +For structural errors: do not propose a fix until the root cause is certain. Describe what you'd need to verify and ask whether to proceed. + +After each fix, ask the user to re-run `./scripts/run-tests.sh` and confirm before continuing. + +Do not commit. The user handles all commits. + +When all failures are done, list every file changed and what was changed in each. List any failure you skipped and why. diff --git a/.claude/prompts/skill-template.md b/.claude/prompts/skill-template.md index 2b5c97d..c38f555 100644 --- a/.claude/prompts/skill-template.md +++ b/.claude/prompts/skill-template.md @@ -5,3 +5,5 @@ Paste into the conversation before describing the skill you want: --- Write this as direct instructions to Claude, not a document. No headers. Imperative voice throughout ("do X", "skip if Y"). For each thing that could go wrong, name the failure mode explicitly rather than stating a virtue. Include: where to get inputs, confirm-before-write if it writes to a file, "if nothing qualifies say so and stop" if relevant, and pacing (iterative: wait after each item / single-output: gather → draft → confirm → write). + +Do not add a self-review step for quality or correctness — Claude is anchored to what it just produced and will tend to confirm rather than critique. For those, end the skill by telling the user they can send a follow-up message (e.g. "check this for X" or "/code-review"). A self-check step is fine for completeness against a concrete checklist (e.g. "confirm every failure was either fixed or explicitly skipped"). diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh new file mode 100755 index 0000000..77dd296 --- /dev/null +++ b/scripts/run-tests.sh @@ -0,0 +1,38 @@ +#!/bin/bash +# Runs the pytest suite against the backend and writes the full output to +# docs/test-report.txt. The file is plain text so Claude Code can read it +# directly with /fix-test. +# +# Assumptions: +# - Run from the project root (not from backend/) +# - `uv` is installed and on PATH (https://docs.astral.sh/uv/) +# - TEST_DATABASE_URL is set in backend/.env +# +# Usage: chmod +x scripts/run-tests.sh && ./scripts/run-tests.sh +# Shortcut: alias run-tests='./scripts/run-tests.sh' +# +# Output: docs/test-report.txt + +set -uo pipefail # No -e: test failures are non-zero exits we want to capture + +OUTPUT="docs/test-report.txt" +BACKEND_DIR="backend" +TIMESTAMP=$(date "+%Y-%m-%d %H:%M:%S %Z") + +mkdir -p "$(dirname "$OUTPUT")" + +if ! command -v uv >/dev/null 2>&1; then + echo "Error: uv not found on PATH. Install from https://docs.astral.sh/uv/" + exit 1 +fi + +if [ ! -d "$BACKEND_DIR" ]; then + echo "Error: $BACKEND_DIR/ not found. Run this script from the project root." + exit 1 +fi + +echo "Test run: $TIMESTAMP" > "$OUTPUT" +(cd "$BACKEND_DIR" && uv run pytest -v 2>&1 | tee -a "../$OUTPUT") || true + +echo "" +echo "Done — written to $OUTPUT" From 26fc123db7e8c54556aab203efff049c1f33405a Mon Sep 17 00:00:00 2001 From: podmar Date: Mon, 15 Jun 2026 23:07:29 +0200 Subject: [PATCH 08/18] fix: improve speed by truncating rather than dropping all tables after every test --- backend/tests/conftest.py | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 7406d3f..bf9d0a4 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -1,9 +1,11 @@ +import asyncio from collections.abc import AsyncGenerator import pytest from httpx import ASGITransport, AsyncClient from pydantic import SecretStr from pydantic_settings import BaseSettings, SettingsConfigDict +from sqlalchemy import text from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from sqlalchemy.pool import NullPool from sqlmodel import SQLModel @@ -36,14 +38,26 @@ class _Env(BaseSettings): from app.main import app # noqa: E402 — must follow the patch above +@pytest.fixture(scope="session", autouse=True) +def create_schema() -> None: + # Run DDL once per session rather than per test. asyncio.run() opens a fresh + # event loop here so the session-scoped fixture doesn't conflict with the + # function-scoped loops used by individual tests (required with NullPool). + async def _setup() -> None: + async with _test_engine.begin() as conn: + await conn.run_sync(SQLModel.metadata.drop_all) + await conn.run_sync(SQLModel.metadata.create_all) + + asyncio.run(_setup()) + + @pytest.fixture(autouse=True) -async def clean_db() -> None: - # Drop and recreate all tables so the schema always reflects the current models, - # including any constraints added after the test DB was first provisioned. - # With NullPool each connection is fresh, so this is safe across event loops. +async def truncate_tables() -> None: + # TRUNCATE is DML, not DDL — much faster than drop/create per test. + # CASCADE handles FK ordering; RESTART IDENTITY resets sequences between tests. + table_names = ", ".join(f'"{t.name}"' for t in SQLModel.metadata.sorted_tables) async with _test_engine.begin() as conn: - await conn.run_sync(SQLModel.metadata.drop_all) - await conn.run_sync(SQLModel.metadata.create_all) + await conn.execute(text(f"TRUNCATE {table_names} RESTART IDENTITY CASCADE")) @pytest.fixture From 917d9fd427c592fd372837689da70a06012cd171 Mon Sep 17 00:00:00 2001 From: podmar Date: Tue, 16 Jun 2026 11:45:48 +0200 Subject: [PATCH 09/18] fix: guard against TEST_DATABASE_URL pointing at production --- backend/tests/conftest.py | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index bf9d0a4..91f2801 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -16,11 +16,17 @@ class _Env(BaseSettings): model_config = SettingsConfigDict(env_file=".env", extra="ignore") + database_url: SecretStr test_database_url: SecretStr _env = _Env() # type: ignore[call-arg] +if _env.database_url.get_secret_value() == _env.test_database_url.get_secret_value(): + raise RuntimeError( + "TEST_DATABASE_URL must differ from DATABASE_URL — tests TRUNCATE all tables." + ) + # Replace the app's engine and session factory with test-DB equivalents before # app.main is imported. All subsequent imports see the patched module globals, # so every request in tests hits the test DB — not production. @@ -33,7 +39,9 @@ class _Env(BaseSettings): poolclass=NullPool, ) _db.engine = _test_engine -_db._session_factory = async_sessionmaker(_test_engine, class_=AsyncSession, expire_on_commit=False) +_db._session_factory = async_sessionmaker( + _test_engine, class_=AsyncSession, expire_on_commit=False +) from app.main import app # noqa: E402 — must follow the patch above @@ -62,15 +70,20 @@ async def truncate_tables() -> None: @pytest.fixture async def client() -> AsyncGenerator[AsyncClient]: - async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac: + async with AsyncClient( + transport=ASGITransport(app=app), base_url="http://test" + ) as ac: yield ac @pytest.fixture async def auth(client: AsyncClient) -> dict[str, str]: - await client.post("/auth/register", json={"email": "alice@test.com", "password": "testpass123"}) + await client.post( + "/auth/register", json={"email": "alice@test.com", "password": "testpass123"} + ) resp = await client.post( - "/auth/jwt/login", data={"username": "alice@test.com", "password": "testpass123"} + "/auth/jwt/login", + data={"username": "alice@test.com", "password": "testpass123"}, ) return {"Authorization": f"Bearer {resp.json()['access_token']}"} @@ -78,7 +91,9 @@ async def auth(client: AsyncClient) -> dict[str, str]: @pytest.fixture async def other_auth(client: AsyncClient) -> dict[str, str]: """Second user in a separate household — for isolation tests.""" - await client.post("/auth/register", json={"email": "bob@test.com", "password": "testpass123"}) + await client.post( + "/auth/register", json={"email": "bob@test.com", "password": "testpass123"} + ) resp = await client.post( "/auth/jwt/login", data={"username": "bob@test.com", "password": "testpass123"} ) From 1adec6c478d9f84bf49f39ebb38c9a623664d837 Mon Sep 17 00:00:00 2001 From: podmar Date: Tue, 16 Jun 2026 12:04:02 +0200 Subject: [PATCH 10/18] test: make batch merge collision test explicit about location and expiry --- backend/tests/test_batches.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/backend/tests/test_batches.py b/backend/tests/test_batches.py index ac3dc40..0ebe49d 100644 --- a/backend/tests/test_batches.py +++ b/backend/tests/test_batches.py @@ -21,12 +21,26 @@ async def test_create_batch(client, auth, item_id): async def test_create_batch_merges_on_collision(client: AsyncClient, auth, item_id): - # Two POSTs with no explicit location or expiry resolve to the same defaults, - # so the second should merge into the first rather than creating a duplicate. - first = await client.post(f"/items/{item_id}/batches", json={"quantity": 3}, headers=auth) + from datetime import date, timedelta + + pantry_id = next( + loc["id"] for loc in (await client.get("/locations", headers=auth)).json() + if loc["name"] == "Pantry" + ) + expiry = str(date.today() + timedelta(days=365)) + + first = await client.post( + f"/items/{item_id}/batches", + json={"quantity": 3, "location_id": pantry_id, "expiry_date": expiry}, + headers=auth, + ) assert first.status_code == 201 - second = await client.post(f"/items/{item_id}/batches", json={"quantity": 2}, headers=auth) + second = await client.post( + f"/items/{item_id}/batches", + json={"quantity": 2, "location_id": pantry_id, "expiry_date": expiry}, + headers=auth, + ) assert second.status_code == 200 assert second.json()["id"] == first.json()["id"] assert second.json()["quantity"] == 5 From 8a759e9662d0ac166482d092c67dbf7b769e3dac Mon Sep 17 00:00:00 2001 From: podmar Date: Tue, 16 Jun 2026 12:08:25 +0200 Subject: [PATCH 11/18] test: add expiry-date collision test for PATCH batch --- backend/tests/test_batches.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/backend/tests/test_batches.py b/backend/tests/test_batches.py index 0ebe49d..a9b1116 100644 --- a/backend/tests/test_batches.py +++ b/backend/tests/test_batches.py @@ -75,6 +75,36 @@ async def test_update_batch_merges_on_collision(client: AsyncClient, auth, item_ assert resp.json()["quantity"] == 5 +async def test_update_batch_expiry_merges_on_collision(client: AsyncClient, auth, item_id): + from datetime import date, timedelta + + pantry_id = next( + loc["id"] for loc in (await client.get("/locations", headers=auth)).json() + if loc["name"] == "Pantry" + ) + expiry_a = str(date.today() + timedelta(days=30)) + expiry_b = str(date.today() + timedelta(days=60)) + + batch_a = (await client.post( + f"/items/{item_id}/batches", + json={"quantity": 3, "location_id": pantry_id, "expiry_date": expiry_a}, + headers=auth, + )).json() + batch_b = (await client.post( + f"/items/{item_id}/batches", + json={"quantity": 2, "location_id": pantry_id, "expiry_date": expiry_b}, + headers=auth, + )).json() + + # Changing batch B's expiry to match batch A → should merge. + resp = await client.patch( + f"/batches/{batch_b['id']}", json={"expiry_date": expiry_a}, headers=auth + ) + assert resp.status_code == 200 + assert resp.json()["id"] == batch_a["id"] + assert resp.json()["quantity"] == 5 + + async def test_delete_batch(client, auth, item_id, batch_id): resp = await client.delete(f"/batches/{batch_id}", headers=auth) assert resp.status_code == 204 From 68bc28fae4060abfd23914890688963489ef7a0f Mon Sep 17 00:00:00 2001 From: podmar Date: Tue, 16 Jun 2026 12:14:37 +0200 Subject: [PATCH 12/18] chore: clarify why NullPool is required in test setup --- 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 91f2801..ebdb14a 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -33,7 +33,8 @@ class _Env(BaseSettings): # NullPool creates a fresh connection per operation and never retains one between # uses — required for async SQLAlchemy tests where each test runs in its own # event loop. A retained connection would be bound to the loop that created it -# and fail with "Future attached to a different loop" in subsequent tests. +# and fail with "Future attached to a different loop" in subsequent tests, +# because pytest-asyncio creates a new event loop per test function by default. _test_engine = create_async_engine( _env.test_database_url.get_secret_value(), poolclass=NullPool, From 7ed3f1924d479985eaab5fb56d8252513a401c00 Mon Sep 17 00:00:00 2001 From: podmar Date: Tue, 16 Jun 2026 12:19:06 +0200 Subject: [PATCH 13/18] test: verify barcode uniqueness is scoped to household --- backend/tests/test_items.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/backend/tests/test_items.py b/backend/tests/test_items.py index a8cff59..d34d15c 100644 --- a/backend/tests/test_items.py +++ b/backend/tests/test_items.py @@ -70,6 +70,26 @@ async def test_duplicate_barcode_returns_existing_with_200(client: AsyncClient, assert second.json()["id"] == first.json()["id"] +async def test_duplicate_barcode_different_households_allowed( + client: AsyncClient, auth, other_auth +): + first = await client.post( + "/items", + json={"name": "Milk", "barcode": "1234567890123", "unit": "litre"}, + headers=auth, + ) + assert first.status_code == 201 + + # Same barcode, different household — should create a separate item. + second = await client.post( + "/items", + json={"name": "Milk", "barcode": "1234567890123", "unit": "litre"}, + headers=other_auth, + ) + assert second.status_code == 201 + assert second.json()["id"] != first.json()["id"] + + async def test_delete_item_cascades_batches(client: AsyncClient, auth, item_id): await client.post(f"/items/{item_id}/batches", json={"quantity": 3}, headers=auth) await client.delete(f"/items/{item_id}", headers=auth) From f548c1eb47956f4b86d9270d311a5b45361cad10 Mon Sep 17 00:00:00 2001 From: podmar Date: Tue, 16 Jun 2026 12:20:44 +0200 Subject: [PATCH 14/18] test: add isolation test for PATCH items --- backend/tests/test_items.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/backend/tests/test_items.py b/backend/tests/test_items.py index d34d15c..7bd2dd4 100644 --- a/backend/tests/test_items.py +++ b/backend/tests/test_items.py @@ -148,3 +148,12 @@ async def test_isolation_cannot_delete_other_household_item( ): resp = await client.delete(f"/items/{item_id}", headers=other_auth) assert resp.status_code == 404 + + +async def test_isolation_cannot_update_other_household_item( + client: AsyncClient, auth, other_auth, item_id +): + resp = await client.patch( + f"/items/{item_id}", json={"name": "Stolen"}, headers=other_auth + ) + assert resp.status_code == 404 From 0b350da737e9f99b646aedda1cc13c278c997942 Mon Sep 17 00:00:00 2001 From: podmar Date: Tue, 16 Jun 2026 12:22:35 +0200 Subject: [PATCH 15/18] test: add isolation tests for PATCH, DELETE, and GET batches --- backend/tests/test_batches.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/backend/tests/test_batches.py b/backend/tests/test_batches.py index a9b1116..f1ef860 100644 --- a/backend/tests/test_batches.py +++ b/backend/tests/test_batches.py @@ -170,3 +170,26 @@ async def test_expiring_isolation(client: AsyncClient, auth, other_auth): async def test_isolation_cannot_adjust_other_household_batch(client: AsyncClient, auth, other_auth, batch_id): resp = await client.post(f"/batches/{batch_id}/adjust", json={"delta": -1}, headers=other_auth) assert resp.status_code == 404 + + +async def test_isolation_cannot_update_other_household_batch( + client: AsyncClient, auth, other_auth, batch_id +): + resp = await client.patch( + f"/batches/{batch_id}", json={"quantity": 99}, headers=other_auth + ) + assert resp.status_code == 404 + + +async def test_isolation_cannot_delete_other_household_batch( + client: AsyncClient, auth, other_auth, batch_id +): + resp = await client.delete(f"/batches/{batch_id}", headers=other_auth) + assert resp.status_code == 404 + + +async def test_isolation_cannot_list_other_household_batches( + client: AsyncClient, auth, other_auth, item_id, batch_id +): + resp = await client.get(f"/items/{item_id}/batches", headers=other_auth) + assert resp.status_code == 404 From 77088239f32e4cda4ba59630f10e51a6f2e0ddaa Mon Sep 17 00:00:00 2001 From: podmar Date: Tue, 16 Jun 2026 12:46:08 +0200 Subject: [PATCH 16/18] chore: include NullPool, tee, TRUNCATE into til --- docs/til.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/til.md b/docs/til.md index c667cf8..3a4982c 100644 --- a/docs/til.md +++ b/docs/til.md @@ -71,6 +71,7 @@ - **`uv run` via a subshell, not a direct binary path.** Running `(cd backend && uv run ruff check .)` ensures `uv` resolves the correct project virtualenv without hardcoding `.venv/bin/ruff` or assuming anything about PATH. Works identically on any machine with `uv` installed. - **`gh pr edit --body "$(cat docs/pr-description.md)"`** updates the PR description from a local file. Combine with a shell function (not an alias) so the `$()` expands at call time, not at definition time: `update-pr() { gh pr edit --body "$(cat docs/pr-description.md)"; }` in `~/.zshrc`. - **Shell functions vs aliases for subshell expansion.** A plain `alias` expands `$()` when the alias is defined, capturing the value at shell startup. A function re-evaluates `$()` every time it's called — necessary whenever the substitution reads a file that changes between calls. +- **`tee -a` writes to a file and prints to the terminal at the same time.** `command | tee file` takes the output of `command`, writes it to `file`, and also prints it live to the terminal. Think of it as a T-junction in a pipe: one stream in, two streams out. The `-a` flag means append — without it, `tee` overwrites from the beginning, erasing anything written before the pipe (e.g. a timestamp header). --- ## Claude Code Skill Design @@ -82,11 +83,38 @@ - **Explicit "nothing qualifies" instruction prevents invented entries.** Without it, Claude will pad output to justify running. One line eliminates the whole failure mode. - **Pacing depends on task shape.** Iterative skills (fix-lint, review-comments) pause after each item. Single-output skills (til, update-spec, pr-description) follow gather → draft → confirm → write. Match the pattern to the task. - **CLAUDE.md is ambient; `.claude/prompts/` is on-demand.** CLAUDE.md loads every session — use it for things needed most sessions. For occasional tasks (e.g. writing a new skill), a prompt template in `.claude/prompts/` costs nothing when not in use and the user pastes it when needed. Frequency of use is the deciding factor. +- **Self-review in the same pass is unreliable for quality judgements.** Claude is anchored to what it just produced and confirms rather than critiques. For quality/correctness, end the skill by prompting the user to send a follow-up message. A self-check step is valid for completeness against a concrete checklist (e.g. "confirm every failure was addressed") — verification, not judgement. + +--- + +## Testing + +- **`NullPool` prevents "Future attached to a different loop" with per-test event loops.** A connection to Postgres is a TCP connection — opening one involves a handshake and auth, so SQLAlchemy keeps a pool of open connections ready to reuse. Each retained connection holds internal `Future` objects (promises of async results) bound to the event loop that created them. An event loop is Python's async scheduler — the thing that drives `await`. pytest-asyncio creates a fresh event loop for each `async def test_` function. When the next test tries to reuse a pooled connection from the previous loop, Python sees the connection's Futures belong to a different loop and raises `"Future attached to a different loop"`. `NullPool` disables pooling entirely: every query opens a fresh TCP connection, uses it, closes it. Nothing is retained, nothing is bound to a stale loop. The cost is a new connection per operation — which is why tests against a remote DB like Neon are slow. This is a tool mismatch: SQLAlchemy's pool assumes one long-lived loop (designed for servers); pytest-asyncio creates a new loop per test (designed for isolation). Each default is sensible in its own domain — they just didn't anticipate each other. `NullPool` opts out of the SQLAlchemy assumption to make both work together. + +- **`asyncio.run()` is the right pattern for session-scoped async setup with NullPool.** A session-scoped async pytest fixture needs a shared event loop, but per-test NullPool means there isn't one. Wrapping the setup in `asyncio.run()` creates and immediately closes its own isolated loop — no connections retained, compatible with the per-test loop pattern. + +- **TRUNCATE is orders of magnitude faster than DROP/CREATE for per-test isolation.** DDL (`drop_all`/`create_all`) makes Postgres rebuild schema structure on every call. `TRUNCATE ... RESTART IDENTITY CASCADE` just deletes rows — DML, not DDL. Move DDL to a session-scoped fixture (runs once), TRUNCATE per test. + +- **Guard `TEST_DATABASE_URL != DATABASE_URL` at import time.** A per-test TRUNCATE against production is catastrophic. Read both URLs via the settings class and raise `RuntimeError` at module import — fails before any connection is made. --- ## Session Log +### 2026-06-16 — Integration test suite + test tooling (PR: feat/test) + +Built: 43 integration tests across all resources, `run-tests.sh`, `/fix-test` skill, batch PATCH autoflush bug fix. + +Key learnings: +- `NullPool` required with pytest-asyncio: per-test event loops make pooled connections invalid across tests +- `asyncio.run()` for session-scoped DDL: creates/closes its own loop, compatible with per-test NullPool pattern +- TRUNCATE per test vs DROP/CREATE: DML vs DDL — same isolation, dramatically faster +- Safety guard: `TEST_DATABASE_URL == DATABASE_URL` check at import time prevents catastrophic truncation +- Self-review in same pass is anchored — use follow-up message for quality review, checklist step only for completeness +- `tee -a`: live output + file write simultaneously without overwriting prior lines + +--- + ### 2026-06-11 — TIL/spec boundary + skill design (PR: chore/til-spec-split) Built: moved API Design Decisions from TIL to spec, created `/update-spec` skill, rewrote `/til` and `/update-spec` for better performance. From f15c1fb6625a3d5f4ae7b459cb47faa793d547cb Mon Sep 17 00:00:00 2001 From: podmar Date: Tue, 16 Jun 2026 12:53:34 +0200 Subject: [PATCH 17/18] chore: restrict the uniqe barcode contraint to household in tech spec --- docs/spec.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/spec.md b/docs/spec.md index 5b82bf2..4c908d7 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -305,6 +305,7 @@ Registration forks into two paths depending on whether an invite token is presen | Expiry date source | Manual entry in v1, OCR in v2 | Standard consumer barcodes (EAN-13, UPC-A) encode only the product ID — no expiry date. The date must be read from the packaging. Manual entry with a +1 year default covers v1; OCR via camera is the v2 solution. | | Past expiry dates | Allowed | No validation rejecting past `expiry_date` on batch creation — needed for initial inventory setup where users scan items already in the house. These batches appear immediately in the expiring-soon view. | | Scan-out (deduction) flow | `POST /batches/{id}/adjust` | A dedicated adjust endpoint with a signed delta is simpler for the frontend than requiring it to calculate new quantity and branch between PATCH and DELETE. Auto-deletes the batch when quantity reaches zero. | +| Barcode uniqueness scope | `UNIQUE(household_id, barcode)` not global | Global uniqueness would prevent two households from stocking the same product | ### API Contract Decisions @@ -315,6 +316,7 @@ Registration forks into two paths depending on whether an invite token is presen - **409 for FK-protected deletes.** Deleting a location that has batches, or a category that has items, returns 409. Data is not orphaned; the client gets a clear rejection signal. - **`DELETE /locations/{id}?move_to={id}` — query param for cascading deletes.** One endpoint covers all cases: no batches → clean delete; has batches + no `move_to` → 409 with instructions; has batches + `move_to` → move then delete. - **Cannot delete the last location.** Deleting the only location in a household is blocked with 409 — batches would have nowhere to move. +- **Barcode constraint is household-scoped, not global.** `UNIQUE(household_id, barcode)` — two households can each have a separate item with the same barcode. A global unique constraint would silently break multi-tenant use. - **FE categorises inventory, not the backend.** `GET /items` returns all items; the frontend sorts into stocked / expiring soon / expired / out of stock from batch data it already has. No `expiry_before` filter on `GET /items` — don't add backend filters until a real screen proves it's needed. `GET /expiring` exists for the dedicated flat-list sorted by date. --- From 4c3e1fbd221d75a47fe02f36b6c4aee320667457 Mon Sep 17 00:00:00 2001 From: podmar Date: Tue, 16 Jun 2026 12:59:19 +0200 Subject: [PATCH 18/18] chore: include info on running tests and testing tooling in docs --- CLAUDE.md | 2 ++ README.md | 12 +++++++----- backend/README.md | 9 ++++++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3f920d7..b6d17f5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -126,8 +126,10 @@ Import `SQLAlchemyUserDatabase` directly from `fastapi_users_db_sqlalchemy`, not - `./scripts/create-pr.sh` — opens a PR from the current branch - `./scripts/fetch-pr-comments.sh` — fetches open PR review comments into `docs/pr-comments.md` +- `./scripts/run-tests.sh` — runs the test suite and saves output to `docs/test-report.txt` - `/pr-description` — generates a PR description and saves to `docs/pr-description.md` - `/review-comments` — works through `docs/pr-comments.md` one comment at a time +- `/fix-test` — works through `docs/test-report.txt` one failure at a time - `/til` — updates `docs/til.md` with learnings from the current session - `/update-spec` — updates `docs/spec.md` with project-specific API and data model decisions diff --git a/README.md b/README.md index 9c4631a..2d9123a 100644 --- a/README.md +++ b/README.md @@ -22,11 +22,13 @@ The loop looks like this: 1. `./scripts/run-lint.sh` — runs Ruff and Pyright, writes results to `docs/lint-report.md` 2. `/fix-lint` (Claude Code skill) — works through lint issues one file at a time, auto-fixing clear errors and confirming design decisions -3. `/pr-description` (Claude Code skill) — drafts the PR description -4. `./scripts/create-pr.sh` — opens a PR from the current branch -5. CI runs an automated Claude review (`.github/workflows/claude-review.yml`) -6. `./scripts/fetch-pr-comments.sh` — pulls review comments into `docs/pr-comments.md` -7. `/review-comments` (Claude Code skill) — works through each comment interactively, one at a time +3. `./scripts/run-tests.sh` — runs the test suite, writes results to `docs/test-report.txt` +4. `/fix-test` (Claude Code skill) — works through test failures one at a time, diagnosing root cause before applying any fix +5. `/pr-description` (Claude Code skill) — drafts the PR description +6. `./scripts/create-pr.sh` — opens a PR from the current branch +7. CI runs an automated Claude review (`.github/workflows/claude-review.yml`) +8. `./scripts/fetch-pr-comments.sh` — pulls review comments into `docs/pr-comments.md` +9. `/review-comments` (Claude Code skill) — works through each comment interactively, one at a time ## Stack diff --git a/backend/README.md b/backend/README.md index 03c25b8..d7fe915 100644 --- a/backend/README.md +++ b/backend/README.md @@ -27,6 +27,7 @@ uv run fastapi dev app/main.py | Variable | Description | |-----------------------------|------------------------------------| | `DATABASE_URL` | Neon PostgreSQL connection string | +| `TEST_DATABASE_URL` | Separate Neon database for tests — must differ from `DATABASE_URL` (tests truncate all tables) | | `SECRET_KEY` | JWT signing secret | | `ALGORITHM` | JWT algorithm (default: HS256) | | `ACCESS_TOKEN_EXPIRE_MINUTES` | Token lifetime (default: 30) | @@ -50,5 +51,11 @@ uv run pyright ## Tests ```bash +# Run tests (from project root — writes output to docs/test-report.txt) +./scripts/run-tests.sh + +# Or run directly from backend/ uv run pytest -``` \ No newline at end of file +``` + +Use `/fix-test` in Claude Code to work through any failures. \ No newline at end of file