arcodange/dance-lessons-coach
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

✨ feat(user): foundation for parallel-safe BDD isolation (T12 stage 1/2) #34

Merged
arcodange merged 1 commits from feat/bdd-parallel-isolation into main 2026-05-03 18:03:44 +02:00
Conversation 0 Commits 1 Files Changed 2 +179
arcodange commented 2026-05-03 18:03:29 +02:00
Owner
Copy Link

Summary

First half of T12 (parallel-safe BDD scenario isolation, ADR-0025 follow-up).

PR #28 had to revert BDD_SCHEMA_ISOLATION because the existing SetupScenarioSchema created empty per-scenario schemas — no migrations were run on them. This PR adds the missing factory that opens a *PostgresRepository from an arbitrary DSN AND runs AutoMigrate on that connection.

What's new

NewPostgresRepositoryFromDSN(cfg, dsn) (*PostgresRepository, error)

Opens a Postgres repo via an explicit DSN, runs AutoMigrate. The DSN can include search_path=<schema> so the migrations create tables in a per-scenario schema instead of public.

BuildSchemaIsolatedDSN(cfg, schemaName) string

Builds a DSN with search_path=<schema> from a base config object.

Existing Close() reused

For per-scenario teardown.

Verification

New integration test TestNewPostgresRepositoryFromDSN_SchemaIsolation verifies:

  • AutoMigrate creates the users table inside the per-scenario schema
  • A CreateUser through the isolated repo writes ONLY into the per-scenario schema
  • public.users sees ZERO rows for the test user (proves no leak)
  • The per-scenario schema sees exactly 1 row

Skips gracefully when DLC_DATABASE_HOST is not set (CI safe).

$ go test -count=1 ./pkg/user/... -run TestNewPostgresRepositoryFromDSN_SchemaIsolation
ok dance-lessons-coach/pkg/user 0.514s

Out of scope (T12 stage 2/2)

  • Wiring this factory into pkg/bdd/testserver/SetupScenarioSchema
  • Spawning a fresh *server.Server per scenario (requires a NewServerWithUserRepo factory)
  • Removing -p 1 from scripts/run-bdd-tests.sh after parallel safety is achieved

That follow-up PR will be smaller because this one establishes the contract.

Architectural note (per code-reviewer SOLID/DDD checklist)

  • SRP : factory does ONE thing (open connection + AutoMigrate)
  • OCP : extends the package without modifying existing callers
  • DDD modular : the test infra concern (per-scenario isolation) stays at the boundary; production code is untouched
  • Cognitive load : 1 file changed, ~70 lines added, 1 dedicated test file (~110 lines)

Test plan

  • go build ./pkg/user/... PASS
  • go test ./pkg/... PASS (all packages, no regression)
  • New integration test PASS with Postgres up
  • CI passes
## Summary First half of T12 (parallel-safe BDD scenario isolation, ADR-0025 follow-up). PR #28 had to revert `BDD_SCHEMA_ISOLATION` because the existing `SetupScenarioSchema` created empty per-scenario schemas — no migrations were run on them. This PR adds the **missing factory** that opens a `*PostgresRepository` from an arbitrary DSN AND runs AutoMigrate on that connection. ## What's new ### `NewPostgresRepositoryFromDSN(cfg, dsn) (*PostgresRepository, error)` Opens a Postgres repo via an explicit DSN, runs AutoMigrate. The DSN can include `search_path=<schema>` so the migrations create tables in a per-scenario schema instead of public. ### `BuildSchemaIsolatedDSN(cfg, schemaName) string` Builds a DSN with `search_path=<schema>` from a base config object. ### Existing `Close()` reused For per-scenario teardown. ## Verification New integration test `TestNewPostgresRepositoryFromDSN_SchemaIsolation` verifies: - AutoMigrate creates the `users` table inside the per-scenario schema - A `CreateUser` through the isolated repo writes ONLY into the per-scenario schema - `public.users` sees ZERO rows for the test user (proves no leak) - The per-scenario schema sees exactly 1 row Skips gracefully when `DLC_DATABASE_HOST` is not set (CI safe). ``` $ go test -count=1 ./pkg/user/... -run TestNewPostgresRepositoryFromDSN_SchemaIsolation ok dance-lessons-coach/pkg/user 0.514s ``` ## Out of scope (T12 stage 2/2) - Wiring this factory into `pkg/bdd/testserver/SetupScenarioSchema` - Spawning a fresh `*server.Server` per scenario (requires a `NewServerWithUserRepo` factory) - Removing `-p 1` from `scripts/run-bdd-tests.sh` after parallel safety is achieved That follow-up PR will be smaller because this one establishes the contract. ## Architectural note (per code-reviewer SOLID/DDD checklist) - **SRP** : factory does ONE thing (open connection + AutoMigrate) - **OCP** : extends the package without modifying existing callers - **DDD modular** : the test infra concern (per-scenario isolation) stays at the boundary; production code is untouched - **Cognitive load** : 1 file changed, ~70 lines added, 1 dedicated test file (~110 lines) ## Test plan - [x] `go build ./pkg/user/...` PASS - [x] `go test ./pkg/...` PASS (all packages, no regression) - [x] New integration test PASS with Postgres up - [ ] CI passes
arcodange added 1 commit 2026-05-03 18:03:30 +02:00
✨ feat(user): NewPostgresRepositoryFromDSN factory + integration test (T12 stage 1/2) 7a2b1a0a87 
First building block for parallel-safe BDD scenario isolation (T12 plan,
ADR-0025 follow-up). PR #28 had to revert BDD_SCHEMA_ISOLATION because
SetupScenarioSchema created an empty schema without migrations -- the
production server's repo never saw it. This PR adds the missing piece:
a factory that opens a *PostgresRepository connected via an arbitrary
DSN AND runs AutoMigrate against it, so a per-scenario schema actually
gets the users table.

Public API additions in pkg/user/postgres_repository.go:

- NewPostgresRepositoryFromDSN(cfg, dsn) (*PostgresRepository, error)
  Opens the repo from an explicit DSN (overrides cfg's host/port/etc),
  runs AutoMigrate -- creates tables in whatever schema the DSN's
  search_path points to.

- BuildSchemaIsolatedDSN(cfg, schemaName) string
  Builds a DSN with `search_path=<schemaName>` from a base config.

The existing NewPostgresRepository(cfg) is unchanged. Existing Close()
method is reused.

Integration test in postgres_repository_isolated_test.go proves:
- AutoMigrate creates `users` table in the per-scenario schema (not public)
- A CreateUser through the isolated repo writes into the per-scenario schema
- public.users sees ZERO rows for the test username
- The per-scenario schema users table sees exactly 1 row

Test skips gracefully when DLC_DATABASE_HOST is not set.

Out of scope (T12 stage 2/2 next):
- Wiring this factory into pkg/bdd/testserver/SetupScenarioSchema
- Spawning a fresh server.Server per scenario (requires NewServerWithUserRepo)
- Removing -p 1 from scripts/run-bdd-tests.sh after parallel safety is achieved

Per code-reviewer skill SOLID/DDD section :
- SRP : factory has single responsibility (open + migrate, no business logic)
- OCP : the new factory extends the package without changing existing callers
- Cognitive load : 1 file, 50 lines added, 1 dedicated test file

🤖 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
arcodange merged commit 4452620df8 into main 2026-05-03 18:03:44 +02:00
arcodange deleted branch feat/bdd-parallel-isolation 2026-05-03 18:03:44 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
arcodange
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: arcodange/dance-lessons-coach#34
Delete Branch "feat/bdd-parallel-isolation"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?