diff --git a/Makefile b/Makefile index 3b1426a..6a08a80 100644 --- a/Makefile +++ b/Makefile @@ -38,13 +38,18 @@ db-ready: @until docker compose exec -T db pg_isready -U postgres > /dev/null 2>&1; do \ sleep 1; \ done + @docker compose exec -T db psql -U postgres -tc "SELECT 1 FROM pg_database WHERE datname = 'arbret'" | grep -q 1 || \ + docker compose exec -T db psql -U postgres -c "CREATE DATABASE arbret" @docker compose exec -T db psql -U postgres -tc "SELECT 1 FROM pg_database WHERE datname = 'arbret_test'" | grep -q 1 || \ docker compose exec -T db psql -U postgres -c "CREATE DATABASE arbret_test" - @# Create worker-specific databases for parallel test execution (pytest-xdist) + @# Create worker-specific databases for parallel backend test execution (pytest-xdist) @for i in 0 1 2 3 4 5 6 7; do \ docker compose exec -T db psql -U postgres -tc "SELECT 1 FROM pg_database WHERE datname = 'arbret_test_gw$$i'" | grep -q 1 || \ docker compose exec -T db psql -U postgres -c "CREATE DATABASE arbret_test_gw$$i"; \ done + @# Create separate database for e2e tests + @docker compose exec -T db psql -U postgres -tc "SELECT 1 FROM pg_database WHERE datname = 'arbret_e2e'" | grep -q 1 || \ + docker compose exec -T db psql -U postgres -c "CREATE DATABASE arbret_e2e" @echo "PostgreSQL is ready" db-seed: db-ready @@ -63,15 +68,27 @@ dev: # E2E: TEST="auth" (file pattern matching e2e/*.spec.ts) TEST ?= -test-backend: db-clean db-ready +test-backend: db-ready test-backend-clean-dbs cd backend && uv run pytest -v -n 8 $(TEST) +# Clean only backend test databases (not e2e or main db) +test-backend-clean-dbs: + @for db in arbret_test arbret_test_gw0 arbret_test_gw1 arbret_test_gw2 arbret_test_gw3 arbret_test_gw4 arbret_test_gw5 arbret_test_gw6 arbret_test_gw7; do \ + docker compose exec -T db psql -U postgres -c "DROP DATABASE IF EXISTS $$db" 2>/dev/null || true; \ + docker compose exec -T db psql -U postgres -c "CREATE DATABASE $$db"; \ + done + test-frontend: cd frontend && npm run test $(if $(TEST),-- $(TEST),) -test-e2e: db-clean db-ready +test-e2e: db-ready test-e2e-clean-db ./scripts/e2e.sh $(TEST) +# Clean only e2e database (not backend test dbs or main db) +test-e2e-clean-db: + @docker compose exec -T db psql -U postgres -c "DROP DATABASE IF EXISTS arbret_e2e" 2>/dev/null || true + @docker compose exec -T db psql -U postgres -c "CREATE DATABASE arbret_e2e" + test: check-constants check-types-fresh test-backend test-frontend test-e2e typecheck: generate-types-standalone diff --git a/backend/tests/OPTIMIZATION_PLAN.md b/backend/tests/OPTIMIZATION_PLAN.md deleted file mode 100644 index ed3d786..0000000 --- a/backend/tests/OPTIMIZATION_PLAN.md +++ /dev/null @@ -1,206 +0,0 @@ -# Backend Test Optimization Plan - -## Overview -This plan implements three optimizations to speed up backend test execution: -1. **Session-scoped role setup** (#4) -2. **Session-scoped schema + transaction rollback** (#1) -3. **Parallel test execution** (#2) - -Current baseline: 236 tests in ~110 seconds (~0.46s per test) - -## Implementation Steps - -### Step 1: Session-Scoped Role Setup (#4) - -**Goal**: Create roles once per test session instead of 236 times. - -**Changes**: -- Create `@pytest.fixture(scope="session")` for engine -- Create `@pytest.fixture(scope="session")` for roles setup -- Modify `client_factory` to use pre-created roles instead of calling `setup_roles()` each time - -**Benefits**: -- Eliminates 236 role creation operations -- Roles are static data, safe to share across tests - -**Risks**: Low - roles are read-only after creation - ---- - -### Step 2: Session-Scoped Schema Creation (#1) - -**Goal**: Create database schema once per session instead of dropping/recreating 236 times. - -**Changes**: -- Move schema creation (`drop_all` + `create_all`) to session-scoped `engine` fixture -- Schema created once at session start, cleaned up at session end -- Each test still gets a fresh database state via transaction rollback - -**Benefits**: -- Eliminates 236 schema drop/create operations (major bottleneck) -- Expected 40-60% speed improvement - -**Risks**: Medium - need to ensure proper cleanup and isolation - ---- - -### Step 3: Transaction Rollback Pattern (#1) - -**Goal**: Use database transactions to isolate tests instead of dropping tables. - -**Approach**: -- Each test runs inside a transaction -- After test completes, rollback the transaction (not commit) -- Next test starts with clean state automatically - -**Implementation Strategy**: -1. Create a session-scoped connection pool -2. For each test: - - Start a transaction (or use a savepoint) - - Run test with all DB operations in this transaction - - Rollback transaction after test -3. Override `get_db()` to yield sessions within the transaction context - -**Key Challenge**: FastAPI's `get_db` dependency needs to work with transaction boundaries. - -**Solution Options**: -- **Option A**: Use nested transactions (savepoints) - more complex but better isolation -- **Option B**: Use connection-level transactions - simpler, rollback entire connection state - -**Recommended**: Option B (simpler, sufficient for test isolation) - -**Changes**: -- Modify `client_factory` to use transaction-scoped sessions -- Update `get_db_session()` to work within transaction context -- Ensure all test DB operations happen within transaction - -**Benefits**: -- Fast test isolation (rollback is much faster than drop/create) -- Maintains test independence - -**Risks**: Medium - need to ensure: -- No commits happen during tests (or they're rolled back) -- Transaction boundaries are properly managed -- Async context managers work correctly - ---- - -### Step 4: Update Fixtures for New Architecture - -**Changes**: -- Update `client_factory` to depend on session-scoped `engine` and `roles` -- Update `get_db_session()` to work with transaction rollback -- Ensure user fixtures (`regular_user`, `admin_user`, etc.) work with new pattern -- Update `override_get_db()` to yield sessions within transaction context - -**Testing**: Run a subset of tests to verify fixtures work correctly - ---- - -### Step 5: Add pytest-xdist for Parallel Execution (#2) - -**Goal**: Run tests in parallel across CPU cores. - -**Changes**: -1. Add `pytest-xdist` to `pyproject.toml` dev dependencies -2. Update `Makefile` to use `pytest -n auto` for parallel execution -3. Ensure test isolation is maintained (transaction rollback ensures this) - -**Configuration**: -- Use `-n auto` to auto-detect CPU cores -- Can override with `-n 4` for specific core count -- Add `pytest-xdist` to dependency groups - -**Benefits**: -- 2-4x speed improvement (depending on CPU cores) -- Works well with transaction isolation - -**Risks**: Low - transaction rollback ensures tests don't interfere - -**Note**: May need to adjust if tests have shared state (but transaction rollback should prevent this) - ---- - -### Step 6: Testing and Validation - -**Verification Steps**: -1. Run full test suite: `make test-backend` -2. Verify all 236 tests pass -3. Measure execution time improvement -4. Check for any flaky tests (shouldn't happen with proper isolation) -5. Test parallel execution with `pytest -n auto` - -**Success Criteria**: -- All tests pass -- Significant speed improvement (target: 50-70% faster) -- No test flakiness introduced -- Parallel execution works correctly - ---- - -## Implementation Order - -1. ✅ **Step 1**: Session-scoped role setup (easiest, low risk) -2. ✅ **Step 2**: Session-scoped schema creation (foundation for #3) -3. ✅ **Step 3**: Transaction rollback pattern (core optimization) -4. ✅ **Step 4**: Update all fixtures (required for #3 to work) -5. ✅ **Step 5**: Add pytest-xdist (quick win, independent) -6. ✅ **Step 6**: Test and validate - ---- - -## Technical Details - -### Transaction Rollback Pattern - -```python -# Pseudo-code for transaction pattern -@pytest.fixture(scope="function") -async def db_transaction(engine): - async with engine.connect() as conn: - trans = await conn.begin() - try: - # Create session factory that uses this connection - session_factory = async_sessionmaker(bind=conn, ...) - yield session_factory - finally: - await trans.rollback() # Always rollback, never commit -``` - -### Session-Scoped Engine - -```python -@pytest.fixture(scope="session") -async def engine(): - engine = create_async_engine(TEST_DATABASE_URL) - # Create schema once - async with engine.begin() as conn: - await conn.run_sync(Base.metadata.drop_all) - await conn.run_sync(Base.metadata.create_all) - yield engine - await engine.dispose() -``` - -### Role Setup - -```python -@pytest.fixture(scope="session") -async def roles(engine): - session_factory = async_sessionmaker(engine) - async with session_factory() as db: - roles = await setup_roles(db) - await db.commit() # Commit roles once - return roles -``` - ---- - -## Rollback Plan - -If issues arise: -1. Revert `conftest.py` changes -2. Remove `pytest-xdist` dependency -3. Restore original fixture structure - -All changes are isolated to test files, no production code affected. - diff --git a/frontend/e2e/admin-invites.spec.ts b/frontend/e2e/admin-invites.spec.ts index b15482f..c137195 100644 --- a/frontend/e2e/admin-invites.spec.ts +++ b/frontend/e2e/admin-invites.spec.ts @@ -82,11 +82,18 @@ test.describe("Admin Invites Page", () => { // Create an invite first const godfatherSelect = page.locator("select").first(); await godfatherSelect.selectOption({ label: REGULAR_USER_EMAIL }); + + // Wait for create invite response + const createPromise = page.waitForResponse( + (resp) => resp.url().includes("/api/admin/invites") && resp.request().method() === "POST" + ); await page.click('button:has-text("Create Invite")'); + await createPromise; + + // Wait for table to update with new invite await expect(page.locator("table")).toContainText("ready"); // Wait for the new invite to appear and capture its code - // The new invite should be the first row with godfather = REGULAR_USER_EMAIL and status = ready const newInviteRow = page .locator("tr") .filter({ hasText: REGULAR_USER_EMAIL }) @@ -97,23 +104,33 @@ test.describe("Admin Invites Page", () => { // Get the invite code from this row (first cell) const inviteCode = await newInviteRow.locator("td").first().textContent(); - // Click revoke on this specific row + // Click revoke and wait for the response + // The revoke endpoint is POST /api/admin/invites/{invite_id}/revoke + const revokePromise = page.waitForResponse( + (resp) => + resp.url().includes("/api/admin/invites") && + resp.url().includes("/revoke") && + resp.request().method() === "POST" + ); await newInviteRow.locator('button:has-text("Revoke")').click(); + await revokePromise; - // Verify this specific invite now shows "revoked" + // Wait for table to refresh and verify this specific invite now shows "revoked" const revokedRow = page.locator("tr").filter({ hasText: inviteCode! }); - await expect(revokedRow).toContainText("revoked"); + await expect(revokedRow).toContainText("revoked", { timeout: 5000 }); // Test status filter - filter by "revoked" status const statusFilter = page.locator("select").nth(1); // Second select is the status filter await statusFilter.selectOption("revoked"); - // Wait for the filter to apply + // Wait for the filter to apply and verify revoked invite is visible await page.waitForResponse((resp) => resp.url().includes("status=revoked")); + await expect(revokedRow).toBeVisible({ timeout: 5000 }); - // Filter by "ready" status - should show our invite (if we create another one) + // Filter by "ready" status - should not show our revoked invite await statusFilter.selectOption("ready"); await page.waitForResponse((resp) => resp.url().includes("status=ready")); + await expect(revokedRow).not.toBeVisible({ timeout: 5000 }); }); }); diff --git a/frontend/e2e/auth.spec.ts b/frontend/e2e/auth.spec.ts index b417087..cef4365 100644 --- a/frontend/e2e/auth.spec.ts +++ b/frontend/e2e/auth.spec.ts @@ -15,7 +15,7 @@ const ADMIN_EMAIL = "admin@example.com"; const ADMIN_PASSWORD = "admin123"; // Helper to create an invite via the API -const API_BASE = "http://localhost:8000"; +const API_BASE = process.env.NEXT_PUBLIC_API_URL || "http://localhost:8000"; async function createInvite(request: APIRequestContext): Promise { // Login as admin diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 8635f52..b1d1b90 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -3,6 +3,10 @@ set -e cd "$(dirname "$0")/.." +# E2E tests use a separate database and port to allow parallel execution with backend tests +E2E_PORT=${E2E_PORT:-8001} +E2E_DATABASE_URL="postgresql+asyncpg://postgres:postgres@localhost:5432/arbret_e2e" + # Cleanup function to kill background processes cleanup() { kill $BACKEND_PID 2>/dev/null || true @@ -18,34 +22,35 @@ if [ -f .env ]; then set +a fi -# Kill any existing backend -pkill -f "uvicorn main:app" 2>/dev/null || true +# Kill any existing e2e backend (on our specific port) +pkill -f "uvicorn main:app --port $E2E_PORT" 2>/dev/null || true sleep 1 -# Seed the database with roles and test users +# Seed the e2e database with roles and test users cd backend -echo "Seeding database..." -uv run python seed.py +echo "Seeding e2e database..." +DATABASE_URL="$E2E_DATABASE_URL" uv run python seed.py cd .. -# Start backend (SECRET_KEY should be set via .envrc or environment) +# Start backend for e2e tests (uses e2e database and separate port) cd backend -uv run uvicorn main:app --port 8000 --log-level warning & +DATABASE_URL="$E2E_DATABASE_URL" uv run uvicorn main:app --port $E2E_PORT --log-level warning & BACKEND_PID=$! cd .. # Wait for backend sleep 2 -# Generate API types from OpenAPI schema -echo "Generating API types..." +# Generate API types from OpenAPI schema (using e2e backend) +echo "Generating API types from e2e backend..." cd frontend -npm run generate-api-types +npx openapi-typescript "http://localhost:$E2E_PORT/openapi.json" -o app/generated/api.ts cd .. -# Run tests (suppress Node.js color warnings) -# If TEST argument is provided, use it as a file pattern +# Run tests with e2e-specific backend URL +# The frontend will connect to our e2e backend on $E2E_PORT cd frontend +export NEXT_PUBLIC_API_URL="http://localhost:$E2E_PORT" if [ -n "$1" ]; then NODE_NO_WARNINGS=1 npx playwright test "$1" else