From 423018e92aa69059772a0b20342fe9202622e956 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Felix=20Sp=C3=B6ttel?=
 <1682504+fspoettel@users.noreply.github.com>
Date: Thu, 17 Aug 2023 13:46:59 +0200
Subject: [PATCH] fix: missing commit when deleting jobs (#56)

---
 app/tests/conftest.py | 31 +++++++++++++++++--------------
 app/tests/test_api.py | 21 ++++++++++++++++++---
 app/web/main.py       |  1 +
 3 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/app/tests/conftest.py b/app/tests/conftest.py
index 7d4df65..b2b08d9 100644
--- a/app/tests/conftest.py
+++ b/app/tests/conftest.py
@@ -18,31 +18,34 @@ def pytest_unconfigure() -> None:
         drop_database(engine.url)
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture()
 def auth_headers() -> dict[str, str]:
     return {"Authorization": f"Bearer {settings.API_SECRET}"}
 
 
-@pytest.fixture(scope="function", autouse=True)
-def db_session():
+@pytest.fixture()
+def test_db():
     models.Base.metadata.create_all(engine)
     connection = engine.connect()
-
-    with SessionLocal(bind=connection) as session:
-        yield session
-        connection.close()
-
+    yield connection
+    connection.close()
     models.Base.metadata.drop_all(bind=engine)
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture()
+def db_session(test_db):
+    with SessionLocal(bind=test_db) as session:
+        yield session
+
+
+@pytest.fixture()
 def client(db_session):
     app = app_factory(lambda: db_session)
     client = TestClient(app)
     return client
 
 
-@pytest.fixture(scope="function", autouse=False)
+@pytest.fixture()
 def mock_job(db_session):
     job = models.Job(
         url="https://example.com",
@@ -51,21 +54,21 @@ def mock_job(db_session):
         meta={"task_id": "5c790c76-2cc1-4e91-a305-443df55a4a4c"},
     )
     db_session.add(job)
-    db_session.flush()
+    db_session.commit()
     return job
 
 
-@pytest.fixture(scope="function", autouse=False)
+@pytest.fixture()
 def mock_artifact(db_session, mock_job):
     artifact = models.Artifact(
         data=None, job_id=str(mock_job.id), type=models.ArtifactType.raw_transcript
     )
     db_session.add(artifact)
-    db_session.flush()
+    db_session.commit()
     return artifact
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture()
 def sharing_enabled():
     settings.ENABLE_SHARING = True
     yield
diff --git a/app/tests/test_api.py b/app/tests/test_api.py
index 8c950bb..8d86677 100644
--- a/app/tests/test_api.py
+++ b/app/tests/test_api.py
@@ -107,10 +107,25 @@ def test_get_artifacts_not_found(client, auth_headers, mock_job):
 # DELETE /api/v1/jobs
 # ---
 def test_delete_job_pass(client, auth_headers, mock_job, db_session):
-    res = client.delete(
+    res_job = client.get(
         f"/api/v1/jobs/{mock_job.id}",
         headers=auth_headers,
     )
 
-    assert db_session.query(models.Job).count() == 0
-    assert res.status_code == 204
+    assert res_job.status_code == 200
+
+    client.delete(
+        f"/api/v1/jobs/{mock_job.id}",
+        headers=auth_headers,
+    )
+
+    # HACK: this catches a missed .commit().
+    # TODO: clean up pytest database handling.
+    db_session.rollback()
+
+    res_job_missing = client.get(
+        f"/api/v1/jobs/{mock_job.id}",
+        headers=auth_headers,
+    )
+
+    assert res_job_missing.status_code == 404
diff --git a/app/web/main.py b/app/web/main.py
index 888185d..baa7842 100644
--- a/app/web/main.py
+++ b/app/web/main.py
@@ -112,6 +112,7 @@ def app_factory(
     ) -> None:
         """Remove metadata and artifacts for a single job."""
         session.query(models.Job).filter(models.Job.id == str(id)).delete()
+        session.commit()
         return None
 
     class PostJobPayload(BaseModel):