🐛 fix(bdd): revert PR #26 schema isolation + cache flush + sequential CI tests (#28)
Some checks failed
CI/CD Pipeline / Build Docker Cache (push) Successful in 11s
CI/CD Pipeline / Trigger Docker Push (push) Has been cancelled
CI/CD Pipeline / CI Pipeline (push) Has been cancelled

Reverts PR #26 (BDD_SCHEMA_ISOLATION caused empty schemas with no tables, 500 errors). Adds sequential package execution (-p 1) + cache flush AfterScenario. AuthBDD goes from 0/5 PASS to 5/5 PASS deterministically. Parallel BDD deferred as architectural follow-up (requires per-schema migrations + dedicated connection pools).

Co-authored-by: Gabriel Radureau <arcodange@gmail.com>
Co-committed-by: Gabriel Radureau <arcodange@gmail.com>
This commit was merged in pull request #28.
This commit is contained in:
2026-05-03 16:28:57 +02:00
committed by arcodange
parent 11fefe3bd9
commit 93bd384ca8
6 changed files with 50 additions and 6 deletions

View File

@@ -219,10 +219,10 @@ jobs:
export DLC_DATABASE_PASSWORD=postgres export DLC_DATABASE_PASSWORD=postgres
export DLC_DATABASE_NAME=dance_lessons_coach_bdd_test export DLC_DATABASE_NAME=dance_lessons_coach_bdd_test
export DLC_DATABASE_SSL_MODE=disable export DLC_DATABASE_SSL_MODE=disable
# Enable per-scenario schema isolation (ADR-0025) to prevent flaky AuthBDD failures. # NOTE: BDD_SCHEMA_ISOLATION was tried (PR #26) but creates empty per-scenario schemas
# Without this, scenarios share the public schema and pollute each other's state. # without table migrations, causing 500 errors on registration. Reverted in PR #28.
# Observed flakiness: same code passes in #605, fails in #606 on TestAuthBDD/*. # The default mode (CleanupDatabase truncates between scenarios) works fine when tests
export BDD_SCHEMA_ISOLATION=true # run sequentially (Go BDD doesn't use t.Parallel by default).
./scripts/run-bdd-tests.sh ./scripts/run-bdd-tests.sh
# Generate BDD coverage report # Generate BDD coverage report

View File

@@ -115,6 +115,15 @@ func InitializeTestSuite(ctx *godog.TestSuiteContext) {
testserver.TraceStateJWTSecretOperation(feature, scenarioKey, "RESET", "ok") testserver.TraceStateJWTSecretOperation(feature, scenarioKey, "RESET", "ok")
} }
// Flush cache after every scenario to prevent cache pollution
if flushErr := sharedServer.FlushCache(); flushErr != nil {
if isCleanupLoggingEnabled() {
log.Warn().Err(flushErr).Msg("CLEANUP: Failed to flush cache after scenario")
}
} else {
testserver.TraceStateCacheOperation(feature, scenarioKey, "FLUSH", "ok")
}
// Clean database after every scenario (only if schema isolation is disabled) // Clean database after every scenario (only if schema isolation is disabled)
if !isSchemaIsolationEnabled() { if !isSchemaIsolationEnabled() {
if cleanupErr := sharedServer.CleanupDatabase(); cleanupErr != nil { if cleanupErr := sharedServer.CleanupDatabase(); cleanupErr != nil {

View File

@@ -15,6 +15,7 @@ import (
"sync" "sync"
"time" "time"
"dance-lessons-coach/pkg/cache"
"dance-lessons-coach/pkg/config" "dance-lessons-coach/pkg/config"
"dance-lessons-coach/pkg/server" "dance-lessons-coach/pkg/server"
"dance-lessons-coach/pkg/user" "dance-lessons-coach/pkg/user"
@@ -48,6 +49,7 @@ type Server struct {
baseURL string baseURL string
db *sql.DB db *sql.DB
authService user.AuthService // Reference to auth service for cleanup authService user.AuthService // Reference to auth service for cleanup
cacheService cache.Service // Reference to cache service for cleanup
schemaMutex sync.Mutex // Protects schema operations schemaMutex sync.Mutex // Protects schema operations
currentSchema string // Current schema being used currentSchema string // Current schema being used
originalSearchPath string // Original search_path to restore originalSearchPath string // Original search_path to restore
@@ -153,6 +155,9 @@ func (s *Server) Start() error {
// Store auth service for cleanup // Store auth service for cleanup
s.authService = realServer.GetAuthService() s.authService = realServer.GetAuthService()
// Store cache service for cleanup
s.cacheService = realServer.GetCacheService()
// Initialize database connection for cleanup // Initialize database connection for cleanup
if err := s.initDBConnection(); err != nil { if err := s.initDBConnection(); err != nil {
return fmt.Errorf("failed to initialize database connection: %w", err) return fmt.Errorf("failed to initialize database connection: %w", err)
@@ -409,6 +414,23 @@ func (s *Server) ResetJWTSecrets() error {
return nil return nil
} }
// FlushCache clears all cached data to prevent cache pollution between scenarios
// This prevents cached responses from affecting subsequent test scenarios
func (s *Server) FlushCache() error {
if s.cacheService == nil {
if isCleanupLoggingEnabled() {
log.Info().Msg("CLEANUP: No cache service available, skipping cache flush")
}
return nil
}
s.cacheService.Flush()
if isCleanupLoggingEnabled() {
log.Info().Msg("CLEANUP: Cache flushed successfully")
}
return nil
}
// CleanupDatabase deletes all test data from all tables // CleanupDatabase deletes all test data from all tables
// This uses raw SQL to avoid dependency on repositories and handles foreign keys properly // This uses raw SQL to avoid dependency on repositories and handles foreign keys properly
// Uses SET CONSTRAINTS ALL DEFERRED to temporarily disable foreign key checks // Uses SET CONSTRAINTS ALL DEFERRED to temporarily disable foreign key checks
@@ -555,7 +577,7 @@ func (s *Server) SetupScenarioSchema(feature, scenario string) error {
return fmt.Errorf("failed to create schema %s: %w", schemaName, err) return fmt.Errorf("failed to create schema %s: %w", schemaName, err)
} }
// Set search path to use the new schema // Set search path to use the new schema (testserver's own connection)
searchPathSQL := fmt.Sprintf("SET search_path = %s, %s", schemaName, s.originalSearchPath) searchPathSQL := fmt.Sprintf("SET search_path = %s, %s", schemaName, s.originalSearchPath)
if _, err := s.db.Exec(searchPathSQL); err != nil { if _, err := s.db.Exec(searchPathSQL); err != nil {
return fmt.Errorf("failed to set search_path: %w", err) return fmt.Errorf("failed to set search_path: %w", err)

View File

@@ -31,6 +31,11 @@ func TraceStateJWTSecretOperation(feature, scenario, operation, details string)
writeTraceLine(feature, scenario, "JWT_"+operation, details) writeTraceLine(feature, scenario, "JWT_"+operation, details)
} }
// TraceStateCacheOperation logs a cache operation
func TraceStateCacheOperation(feature, scenario, operation, details string) {
writeTraceLine(feature, scenario, "CACHE_"+operation, details)
}
// TraceStateSchemaIsolation logs a schema isolation operation // TraceStateSchemaIsolation logs a schema isolation operation
func TraceStateSchemaIsolation(feature, scenario, operation, details string) { func TraceStateSchemaIsolation(feature, scenario, operation, details string) {
writeTraceLine(feature, scenario, "SCHEMA_"+operation, details) writeTraceLine(feature, scenario, "SCHEMA_"+operation, details)

View File

@@ -118,6 +118,12 @@ func (s *Server) GetAuthService() user.AuthService {
return s.userService return s.userService
} }
// GetCacheService returns the cache service for test cleanup
// This allows test suites to flush cache between tests
func (s *Server) GetCacheService() cache.Service {
return s.cacheService
}
// initializeUserServices initializes the user repository and unified user service // initializeUserServices initializes the user repository and unified user service
func initializeUserServices(cfg *config.Config) (user.UserRepository, user.UserService, error) { func initializeUserServices(cfg *config.Config) (user.UserRepository, user.UserService, error) {
// Create user repository using PostgreSQL // Create user repository using PostgreSQL

View File

@@ -144,7 +144,9 @@ run_tests_with_tags() {
# Note: -tags flag in go test is for Go build tags, NOT Godog feature tags # Note: -tags flag in go test is for Go build tags, NOT Godog feature tags
# We use GODOG_TAGS env var which is read by the test framework # We use GODOG_TAGS env var which is read by the test framework
echo "🚀 Running: GODOG_TAGS=\"${DEFAULT_TAGS}\" go test ./features/..." echo "🚀 Running: GODOG_TAGS=\"${DEFAULT_TAGS}\" go test ./features/..."
GODOG_TAGS="$DEFAULT_TAGS" go test ./features/... -v -cover -coverpkg=./... -coverprofile=coverage.out 2>&1 | tee /tmp/bdd_test_output.txt && test_output=$(cat /tmp/bdd_test_output.txt) && rm -f /tmp/bdd_test_output.txt || test_output=$(cat /tmp/bdd_test_output.txt 2>/dev/null || echo "") # -p 1 forces sequential package execution to avoid DB-state contention between feature packages
# (different packages would otherwise spawn concurrent test servers sharing the same Postgres DB).
GODOG_TAGS="$DEFAULT_TAGS" go test ./features/... -v -p 1 -cover -coverpkg=./... -coverprofile=coverage.out 2>&1 | tee /tmp/bdd_test_output.txt && test_output=$(cat /tmp/bdd_test_output.txt) && rm -f /tmp/bdd_test_output.txt || test_output=$(cat /tmp/bdd_test_output.txt 2>/dev/null || echo "")
test_exit_code=${PIPESTATUS[0]} test_exit_code=${PIPESTATUS[0]}
fi fi