Address review feedback and fix missing commit in upload endpoint

Review fixes:
- Use StaticPool for in-memory SQLite so all sessions share one
  connection, avoiding potential "no such table" errors.
- Fix test_valid_bearer_token to actually test a protected endpoint
  with and without auth headers.
- Assert ordering in trends test instead of building a dict.
- Make health test explicit about the expected unhealthy DB status.
- Add memray status assertion to test_upload_clears_memray_failure.

Bug fix found by the new assertion: the upload endpoint's delete of
memray failures was missing an await db.commit(), so the deletion
was never persisted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: ambv

backend/app/routers/upload.py

@@ -329,6 +329,7 @@ def clean_flag(flag):
                 models.MemrayBuildFailure.environment_id == environment_id
             )
         )
+        await db.commit()
 
         return {
             "message": "Worker run uploaded successfully",

backend/tests/conftest.py

@@ -8,6 +8,7 @@
 import pytest_asyncio
 from httpx import ASGITransport, AsyncClient
 from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine
+from sqlalchemy.pool import StaticPool
 
 from app.database import get_database
 from app.factory import create_app
@@ -28,7 +29,12 @@ def test_settings():
 
 @pytest_asyncio.fixture
 async def db_engine():
-    engine = create_async_engine("sqlite+aiosqlite://", echo=False)
+    engine = create_async_engine(
+        "sqlite+aiosqlite://",
+        echo=False,
+        connect_args={"check_same_thread": False},
+        poolclass=StaticPool,
+    )
     async with engine.begin() as conn:
         await conn.run_sync(Base.metadata.create_all)
     yield engine

backend/tests/test_auth.py

@@ -5,8 +5,22 @@
 
 @pytest.mark.asyncio
 async def test_valid_bearer_token(client, auth_headers, sample_binary, sample_environment):
-    """A valid Bearer token should authenticate successfully."""
-    response = await client.get("/api/binaries")
+    """A valid Bearer token should authenticate successfully on a protected endpoint."""
+    payload = {
+        "commit_sha": "a" * 40,
+        "commit_timestamp": "2025-06-16T10:00:00",
+        "binary_id": "default",
+        "environment_id": "linux-x86_64",
+        "error_message": "test",
+    }
+    # Without auth → rejected
+    response = await client.post("/api/report-memray-failure", json=payload)
+    assert response.status_code in (401, 403)
+
+    # With auth → accepted
+    response = await client.post(
+        "/api/report-memray-failure", json=payload, headers=auth_headers
+    )
     assert response.status_code == 200
 
 

backend/tests/test_health.py

@@ -13,12 +13,16 @@ async def test_health_check(client):
 
 
 @pytest.mark.asyncio
-async def test_health_check_returns_db_status(client):
-    """The health endpoint reports database status when db check is enabled."""
+async def test_health_check_reports_db_status(client):
+    """The health router uses a module-level settings object with
+    enable_health_check_db=True (not overridable via test_settings).
+    It attempts db.execute("SELECT 1") which fails on SQLAlchemy 2.x
+    because raw strings need text(). This is a pre-existing app bug.
+    We verify the endpoint still returns 200 and reports the DB as
+    unhealthy rather than crashing."""
     response = await client.get("/health")
+    assert response.status_code == 200
     data = response.json()
-    # The module-level settings have enable_health_check_db=True,
-    # but db.execute("SELECT 1") uses a raw string which fails on
-    # SQLAlchemy 2.x (needs text()). This is a pre-existing issue
-    # in the app code, not a test problem.
     assert "database" in data
+    assert data["database"] == "unhealthy"
+    assert data["status"] == "unhealthy"

backend/tests/test_production_data.py

@@ -155,10 +155,11 @@ async def test_trends_returns_chronological_data(client, prod_data):
     data = response.json()
     assert len(data) == 2
 
-    # Both data points present with correct memory values
-    hwm_values = {d["sha"][:8]: d["high_watermark_bytes"] for d in data}
-    assert hwm_values["e05182f9"] == 1_557_777
-    assert hwm_values["d3d94e0e"] == 1_721_155
+    # Ordered by timestamp DESC: newer commit first
+    assert data[0]["sha"] == COMMIT_CURR["sha"]
+    assert data[0]["high_watermark_bytes"] == 1_721_155
+    assert data[1]["sha"] == COMMIT_PREV["sha"]
+    assert data[1]["high_watermark_bytes"] == 1_557_777
 
 
 @pytest.mark.asyncio

backend/tests/test_upload.py

@@ -216,10 +216,12 @@ async def test_upload_clears_memray_failure(
         "/api/upload-run", json=UPLOAD_PAYLOAD, headers=auth_headers
     )
     assert resp.status_code == 200
+    assert resp.json()["results_created"] == 1
 
-    # Verify the upload response confirms success
-    data = resp.json()
-    assert data["results_created"] == 1
+    # Verify the failure was cleared
+    status = await client.get("/api/memray-status")
+    assert status.json()["has_failures"] is False
+    assert status.json()["failure_count"] == 0
 
 
 @pytest.mark.asyncio