✨ feat(bdd): parallel-safe schema-per-package isolation (T12 stage 2/2) — 2.85x speedup (#35)
Per-package isolated Postgres schema with migrations. Local benchmark: 12.87s sequential → 4.51s parallel = 2.85x. ADR-0025 status to Implemented. CI uses BDD_SCHEMA_ISOLATION=true. Co-authored-by: Gabriel Radureau <arcodange@gmail.com> Co-committed-by: Gabriel Radureau <arcodange@gmail.com>
This commit was merged in pull request #35.
This commit is contained in:
@@ -219,10 +219,12 @@ jobs:
|
||||
export DLC_DATABASE_PASSWORD=postgres
|
||||
export DLC_DATABASE_NAME=dance_lessons_coach_bdd_test
|
||||
export DLC_DATABASE_SSL_MODE=disable
|
||||
# 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).
|
||||
# T12: per-package isolated Postgres schema with migrations (re-enables what
|
||||
# PR #26 attempted but couldn't deliver because the empty schemas had no tables).
|
||||
# The fix: testserver Start() now builds a per-package isolated repo via
|
||||
# user.NewPostgresRepositoryFromDSN which DOES run AutoMigrate against the new
|
||||
# schema. Packages then run in parallel (~2.85x speedup observed locally).
|
||||
export BDD_SCHEMA_ISOLATION=true
|
||||
./scripts/run-bdd-tests.sh
|
||||
|
||||
# Generate BDD coverage report
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# ADR 0025: BDD Scenario Isolation Strategies
|
||||
|
||||
**Status:** Partially Implemented
|
||||
**Status:** Implemented (per-package schema isolation since T12 stage 2/2 - 2026-05-03)
|
||||
|
||||
## Context
|
||||
|
||||
|
||||
@@ -48,11 +48,13 @@ type Server struct {
|
||||
port int
|
||||
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
|
||||
authService user.AuthService // Reference to auth service for cleanup
|
||||
cacheService cache.Service // Reference to cache service for cleanup
|
||||
isolatedRepo *user.PostgresRepository // Per-package isolated repo (BDD_SCHEMA_ISOLATION=true)
|
||||
isolatedSchemaName string // Per-package schema name to drop on Stop()
|
||||
schemaMutex sync.Mutex // Protects schema operations
|
||||
currentSchema string // Current schema being used
|
||||
originalSearchPath string // Original search_path to restore
|
||||
}
|
||||
|
||||
// getDatabaseHost returns the database host from environment variable or defaults to localhost
|
||||
@@ -148,9 +150,55 @@ func (s *Server) Start() error {
|
||||
// This is the ONLY place where we check env vars for v2 configuration
|
||||
v2Enabled := s.shouldEnableV2()
|
||||
|
||||
// Create real server instance from pkg/server
|
||||
// Create real server instance from pkg/server.
|
||||
// When BDD_SCHEMA_ISOLATION=true, each test package (process) gets its own
|
||||
// isolated PostgreSQL schema with its own connection pool + migrations.
|
||||
// This makes `go test ./features/...` parallel-safe because each feature
|
||||
// package runs in its own process and gets its own schema.
|
||||
cfg := createTestConfig(s.port, v2Enabled)
|
||||
realServer := server.NewServer(cfg, context.Background())
|
||||
var realServer *server.Server
|
||||
if isSchemaIsolationEnabled() {
|
||||
feature := os.Getenv("FEATURE")
|
||||
if feature == "" {
|
||||
feature = "bdd"
|
||||
}
|
||||
schemaName := generateSchemaName(feature, "package_root")
|
||||
log.Info().Str("schema", schemaName).Str("feature", feature).Msg("ISOLATION: Building per-package isolated repo")
|
||||
|
||||
// Connect a default repo briefly just to CREATE SCHEMA (uses cfg from env vars)
|
||||
bootstrapRepo, err := user.NewPostgresRepository(cfg)
|
||||
if err != nil {
|
||||
return fmt.Errorf("ISOLATION bootstrap repo failed: %w", err)
|
||||
}
|
||||
// Drop + recreate to ensure clean slate per process
|
||||
_ = bootstrapRepo.Exec(fmt.Sprintf("DROP SCHEMA IF EXISTS %s CASCADE", schemaName))
|
||||
if err := bootstrapRepo.Exec(fmt.Sprintf("CREATE SCHEMA %s", schemaName)); err != nil {
|
||||
bootstrapRepo.Close()
|
||||
return fmt.Errorf("ISOLATION CREATE SCHEMA failed: %w", err)
|
||||
}
|
||||
bootstrapRepo.Close()
|
||||
|
||||
// Build the per-package isolated repo (runs migrations in the new schema)
|
||||
dsn := user.BuildSchemaIsolatedDSN(cfg, schemaName)
|
||||
isolatedRepo, err := user.NewPostgresRepositoryFromDSN(cfg, dsn)
|
||||
if err != nil {
|
||||
return fmt.Errorf("ISOLATION isolated repo failed: %w", err)
|
||||
}
|
||||
s.isolatedRepo = isolatedRepo
|
||||
s.isolatedSchemaName = schemaName
|
||||
|
||||
// Build user service backed by the isolated repo
|
||||
jwtConfig := user.JWTConfig{
|
||||
Secret: cfg.GetJWTSecret(),
|
||||
ExpirationTime: time.Hour * 24,
|
||||
Issuer: "dance-lessons-coach",
|
||||
}
|
||||
isolatedUserService := user.NewUserService(isolatedRepo, jwtConfig, cfg.GetAdminMasterPassword())
|
||||
|
||||
realServer = server.NewServerWithUserRepo(cfg, context.Background(), isolatedRepo, isolatedUserService)
|
||||
} else {
|
||||
realServer = server.NewServer(cfg, context.Background())
|
||||
}
|
||||
|
||||
// Store auth service for cleanup
|
||||
s.authService = realServer.GetAuthService()
|
||||
@@ -639,6 +687,21 @@ func (s *Server) getCurrentSearchPath() (string, error) {
|
||||
}
|
||||
|
||||
func (s *Server) Stop() error {
|
||||
// Cleanup the per-package isolated schema + close its pool, if any.
|
||||
// (BDD_SCHEMA_ISOLATION=true path - see Start().)
|
||||
if s.isolatedRepo != nil {
|
||||
if s.isolatedSchemaName != "" {
|
||||
if err := s.isolatedRepo.Exec(fmt.Sprintf("DROP SCHEMA IF EXISTS %s CASCADE", s.isolatedSchemaName)); err != nil {
|
||||
log.Warn().Err(err).Str("schema", s.isolatedSchemaName).Msg("ISOLATION: failed to drop schema on Stop")
|
||||
}
|
||||
}
|
||||
if err := s.isolatedRepo.Close(); err != nil {
|
||||
log.Warn().Err(err).Msg("ISOLATION: failed to close isolated repo")
|
||||
}
|
||||
s.isolatedRepo = nil
|
||||
s.isolatedSchemaName = ""
|
||||
}
|
||||
|
||||
if s.httpServer != nil {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
|
||||
@@ -71,7 +71,21 @@ type Server struct {
|
||||
}
|
||||
|
||||
func NewServer(cfg *config.Config, readyCtx context.Context) *Server {
|
||||
// Create validator instance
|
||||
// Initialize default user repository and services (Postgres from cfg)
|
||||
userRepo, userService, err := initializeUserServices(cfg)
|
||||
if err != nil {
|
||||
log.Warn().Err(err).Msg("Failed to initialize user services, user functionality will be disabled")
|
||||
}
|
||||
return NewServerWithUserRepo(cfg, readyCtx, userRepo, userService)
|
||||
}
|
||||
|
||||
// NewServerWithUserRepo builds a Server with caller-provided userRepo + userService.
|
||||
// Used by BDD test infra to inject a per-scenario repository (e.g., one connected
|
||||
// to an isolated PostgreSQL schema). Pass nil for both to disable user functionality.
|
||||
//
|
||||
// The validator + cache services are still built from cfg internally; they don't
|
||||
// need per-scenario isolation today.
|
||||
func NewServerWithUserRepo(cfg *config.Config, readyCtx context.Context, userRepo user.UserRepository, userService user.UserService) *Server {
|
||||
validator, err := validation.GetValidatorFromConfig(cfg)
|
||||
if err != nil {
|
||||
log.Error().Err(err).Msg("Failed to create validator, continuing without validation")
|
||||
@@ -79,13 +93,6 @@ func NewServer(cfg *config.Config, readyCtx context.Context) *Server {
|
||||
log.Trace().Msg("Validator created successfully")
|
||||
}
|
||||
|
||||
// Initialize user repository and services
|
||||
userRepo, userService, err := initializeUserServices(cfg)
|
||||
if err != nil {
|
||||
log.Warn().Err(err).Msg("Failed to initialize user services, user functionality will be disabled")
|
||||
}
|
||||
|
||||
// Initialize cache service
|
||||
var cacheService cache.Service
|
||||
if cfg.GetCacheEnabled() {
|
||||
cacheService = cache.NewInMemoryService(
|
||||
|
||||
@@ -184,7 +184,15 @@ func BuildSchemaIsolatedDSN(cfg *config.Config, schemaName string) string {
|
||||
)
|
||||
}
|
||||
|
||||
// (Close already exists below; we reuse it.)
|
||||
// Exec runs a raw SQL statement against the repository's connection.
|
||||
// Used by BDD test infra for schema lifecycle (CREATE SCHEMA / DROP SCHEMA).
|
||||
// Avoid in production code paths -- prefer the typed Repository methods.
|
||||
func (r *PostgresRepository) Exec(sql string) error {
|
||||
if r.db == nil {
|
||||
return fmt.Errorf("Exec called on PostgresRepository with nil db")
|
||||
}
|
||||
return r.db.Exec(sql).Error
|
||||
}
|
||||
|
||||
// initializeDatabase sets up the PostgreSQL database connection and runs migrations
|
||||
func (r *PostgresRepository) initializeDatabase() error {
|
||||
|
||||
@@ -144,9 +144,21 @@ 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/..."
|
||||
# -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 "")
|
||||
# When BDD_SCHEMA_ISOLATION=true (T12 architecture):
|
||||
# each test PACKAGE gets its own isolated PostgreSQL schema with its own
|
||||
# connection pool + migrations (cf. pkg/bdd/testserver/server.go Start()).
|
||||
# Packages then run in parallel safely. ~2.85x speedup observed locally.
|
||||
# When unset:
|
||||
# fall back to -p 1 (sequential). Uses public schema with TRUNCATE-style
|
||||
# cleanup between scenarios.
|
||||
if [ "${BDD_SCHEMA_ISOLATION:-}" = "true" ]; then
|
||||
PARALLEL_FLAG=""
|
||||
echo "🔀 BDD_SCHEMA_ISOLATION=true → feature packages run in parallel"
|
||||
else
|
||||
PARALLEL_FLAG="-p 1"
|
||||
echo "🐌 BDD_SCHEMA_ISOLATION not set → feature packages run sequentially (-p 1)"
|
||||
fi
|
||||
GODOG_TAGS="$DEFAULT_TAGS" go test ./features/... -v $PARALLEL_FLAG -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user