🐛 fix(bdd): revert PR #26 schema isolation + add cache flush + sequential CI tests #28

Merged
arcodange merged 1 commits from fix/bdd-cache-flush-and-revert-schema-isolation into main 2026-05-03 16:28:58 +02:00
6 changed files with 50 additions and 6 deletions

View File

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

View File

@@ -115,6 +115,15 @@ func InitializeTestSuite(ctx *godog.TestSuiteContext) {
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)
if !isSchemaIsolationEnabled() {
if cleanupErr := sharedServer.CleanupDatabase(); cleanupErr != nil {

View File

@@ -15,6 +15,7 @@ import (
"sync"
"time"
"dance-lessons-coach/pkg/cache"
"dance-lessons-coach/pkg/config"
"dance-lessons-coach/pkg/server"
"dance-lessons-coach/pkg/user"
@@ -48,6 +49,7 @@ type Server struct {
baseURL string
db *sql.DB
authService user.AuthService // Reference to auth service for cleanup
cacheService cache.Service // Reference to cache service for cleanup
schemaMutex sync.Mutex // Protects schema operations
currentSchema string // Current schema being used
originalSearchPath string // Original search_path to restore
@@ -153,6 +155,9 @@ func (s *Server) Start() error {
// Store auth service for cleanup
s.authService = realServer.GetAuthService()
// Store cache service for cleanup
s.cacheService = realServer.GetCacheService()
// Initialize database connection for cleanup
if err := s.initDBConnection(); err != nil {
return fmt.Errorf("failed to initialize database connection: %w", err)
@@ -409,6 +414,23 @@ func (s *Server) ResetJWTSecrets() error {
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
// 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
@@ -555,7 +577,7 @@ func (s *Server) SetupScenarioSchema(feature, scenario string) error {
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)
if _, err := s.db.Exec(searchPathSQL); err != nil {
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)
}
// TraceStateCacheOperation logs a cache operation
func TraceStateCacheOperation(feature, scenario, operation, details string) {
writeTraceLine(feature, scenario, "CACHE_"+operation, details)
}
// TraceStateSchemaIsolation logs a schema isolation operation
func TraceStateSchemaIsolation(feature, scenario, operation, details string) {
writeTraceLine(feature, scenario, "SCHEMA_"+operation, details)

View File

@@ -118,6 +118,12 @@ func (s *Server) GetAuthService() user.AuthService {
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
func initializeUserServices(cfg *config.Config) (user.UserRepository, user.UserService, error) {
// 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
# We use GODOG_TAGS env var which is read by the test framework
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]}
fi