From 8776bbdda682256aa02eafd021a00312a6cc14be Mon Sep 17 00:00:00 2001
From: Samir Boulahtit <samir.boulahtit@wizard.lu>
Date: Fri, 5 Dec 2025 21:43:11 +0100
Subject: [PATCH] docs: add SQLAlchemy fixture best practices to testing guide
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add comprehensive section on SQLAlchemy fixture best practices:
- Explain why db.expunge() is an anti-pattern in test fixtures
- Document how test isolation works via the db fixture
- Provide examples of correct vs incorrect fixture patterns
- Show how to use db.refresh() for fresh data instead of expunge

This documents the lessons learned from the fixture refactoring
and helps prevent future issues with DetachedInstanceError.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 docs/testing/testing-guide.md | 73 ++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/docs/testing/testing-guide.md b/docs/testing/testing-guide.md
index cbd3a484..bcc125f1 100644
--- a/docs/testing/testing-guide.md
+++ b/docs/testing/testing-guide.md
@@ -893,7 +893,78 @@ def test_delete_user(db):
     delete_user(db, user_id)  # Depends on previous test
 ```
 
-### 2. Arrange-Act-Assert Pattern
+### 2. SQLAlchemy Fixture Best Practices
+
+**IMPORTANT**: Follow these rules when working with SQLAlchemy fixtures to avoid common pitfalls:
+
+#### ❌ Never Use `db.expunge()` in Fixtures
+
+```python
+# ❌ BAD - Causes lazy loading errors
+@pytest.fixture
+def test_user(db, auth_manager):
+    user = User(email="test@example.com", ...)
+    db.add(user)
+    db.commit()
+    db.refresh(user)
+    db.expunge(user)  # ❌ NEVER DO THIS!
+    return user
+
+# When you later try to access relationships:
+# user.company.name  # ❌ DetachedInstanceError!
+```
+
+```python
+# ✅ GOOD - Keep objects attached to session
+@pytest.fixture
+def test_user(db, auth_manager):
+    user = User(email="test@example.com", ...)
+    db.add(user)
+    db.commit()
+    db.refresh(user)
+    return user  # ✅ Object stays attached to session
+```
+
+#### Why `db.expunge()` Is an Anti-Pattern
+
+1. **Breaks lazy loading**: Detached objects cannot access relationships like `user.company` or `product.marketplace_product`
+2. **Causes DetachedInstanceError**: SQLAlchemy throws errors when accessing unloaded relationships on detached objects
+3. **Test isolation is already handled**: The `db` fixture drops and recreates all tables after each test
+
+#### How Test Isolation Works
+
+The `db` fixture in `tests/conftest.py` provides isolation by:
+- Creating fresh tables before each test
+- Dropping all tables after each test completes
+- Using `expire_on_commit=False` to prevent objects from expiring after commits
+
+```python
+@pytest.fixture(scope="function")
+def db(engine, testing_session_local):
+    Base.metadata.create_all(bind=engine)  # Fresh tables
+    db_session = testing_session_local()
+
+    try:
+        yield db_session
+    finally:
+        db_session.close()
+        Base.metadata.drop_all(bind=engine)    # Clean up
+        Base.metadata.create_all(bind=engine)  # Ready for next test
+```
+
+#### If You Need Fresh Data
+
+Use `db.refresh(obj)` instead of expunge/re-query patterns:
+
+```python
+# ✅ GOOD - Refresh for fresh data
+def test_update_user(db, test_user):
+    update_user_email(db, test_user.id, "new@example.com")
+    db.refresh(test_user)  # Get latest data from database
+    assert test_user.email == "new@example.com"
+```
+
+### 3. Arrange-Act-Assert Pattern
 
 Structure tests clearly using AAA pattern: