From 7e94122724da6d0428a302a6cff51c4610b71667 Mon Sep 17 00:00:00 2001 From: Mad Max Date: Wed, 22 Apr 2026 14:33:47 +0200 Subject: [PATCH 1/2] fix: strip port/proto suffix from wait.ForSQL URL callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit testcontainers-go v0.42 (moby-modules migration) changed the signature of wait.ForSQL's URL callback from (host string, port nat.Port) to (host string, port string), and now passes port.String() which returns "/" (e.g. "5432/tcp"). Embedding that directly into the URL made pgx parse the dbname as "tcp/" — every connect attempt failed with "database tcp/ does not exist" and the wait loop timed out at 10s with a misleading "get state: context deadline exceeded" error. Strip the suffix in the callback before composing the DSN, and add testcontainer-backed regression tests. --- container.go | 5 ++- container_test.go | 102 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 container_test.go diff --git a/container.go b/container.go index b40bc0e..ba0894c 100644 --- a/container.go +++ b/container.go @@ -345,7 +345,10 @@ func setupPostgresTestContainer(ctx context.Context, cfg Config) (testcontainers "/var/lib/pg/data": "rw", }, WaitingFor: wait.ForSQL(port, "pgx", func(host string, port string) string { - return fmt.Sprintf("postgres://%s:%s@%s:%s/%s?sslmode=disable", cfg.User, cfg.Password, host, port, cfg.Database) + // testcontainers-go v0.42 passes the port as "/" (e.g. "5432/tcp"). + // Strip the protocol suffix so it doesn't leak into the URL path and corrupt the dbname. + portNum, _, _ := strings.Cut(port, "/") + return fmt.Sprintf("postgres://%s:%s@%s:%s/%s?sslmode=disable", cfg.User, cfg.Password, host, portNum, cfg.Database) }).WithStartupTimeout(10 * time.Second), } diff --git a/container_test.go b/container_test.go new file mode 100644 index 0000000..b228726 --- /dev/null +++ b/container_test.go @@ -0,0 +1,102 @@ +package brrr_test + +import ( + "context" + "os" + "testing" + "time" + + "github.com/modfin/brrr" +) + +var testContainer *brrr.Container + +func TestMain(m *testing.M) { + c, err := brrr.NewContainer(brrr.Config{ + User: "postgres", + Password: "postgres", + Database: "brrr_test", + }) + if err != nil { + panic("failed to start test container: " + err.Error()) + } + testContainer = c + code := m.Run() + if err := c.Close(); err != nil { + panic("failed to close test container: " + err.Error()) + } + os.Exit(code) +} + +// TestNewContainer_StartsCleanly is the regression test for the wait.ForSQL URL +// construction bug introduced when testcontainers-go migrated to moby modules in +// v0.42. The callback was receiving the port as "/" (e.g. "5432/tcp"), +// and embedding the suffix into the URL path, which corrupted the dbname and made +// the SQL wait strategy fail until the 10s startup timeout expired. +// If TestMain completes successfully, this test passes; it exists to name the +// guarantee explicitly so future regressions surface as a named failure. +func TestNewContainer_StartsCleanly(t *testing.T) { + if testContainer == nil { + t.Fatal("test container was not initialized in TestMain") + } +} + +func TestContainer_NewInstance_IsUsable(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + di, err := testContainer.NewInstance(ctx) + if err != nil { + t.Fatalf("NewInstance: %v", err) + } + t.Cleanup(func() { + if err := testContainer.CloseInstance(context.Background(), di); err != nil { + t.Errorf("CloseInstance: %v", err) + } + }) + + var got int + if err := di.Connection.QueryRow(ctx, "SELECT 1").Scan(&got); err != nil { + t.Fatalf("SELECT 1 against instance failed: %v", err) + } + if got != 1 { + t.Fatalf("expected 1, got %d", got) + } +} + +func TestContainer_NewInstance_InstancesAreIsolated(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + a, err := testContainer.NewInstance(ctx) + if err != nil { + t.Fatalf("NewInstance a: %v", err) + } + t.Cleanup(func() { _ = testContainer.CloseInstance(context.Background(), a) }) + + b, err := testContainer.NewInstance(ctx) + if err != nil { + t.Fatalf("NewInstance b: %v", err) + } + t.Cleanup(func() { _ = testContainer.CloseInstance(context.Background(), b) }) + + if a.Name == b.Name { + t.Fatalf("expected distinct instance names, both were %q", a.Name) + } + + if _, err := a.Connection.Exec(ctx, "CREATE TABLE isolation_probe (v int)"); err != nil { + t.Fatalf("create table on a: %v", err) + } + if _, err := a.Connection.Exec(ctx, "INSERT INTO isolation_probe (v) VALUES (42)"); err != nil { + t.Fatalf("insert on a: %v", err) + } + + var exists bool + err = b.Connection.QueryRow(ctx, "SELECT EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'isolation_probe')").Scan(&exists) + if err != nil { + t.Fatalf("lookup on b: %v", err) + } + if exists { + t.Fatal("expected isolation_probe to not exist on instance b; template contamination") + } +} From 33870a471a4c324f4b34debc111bc2c384a3127e Mon Sep 17 00:00:00 2001 From: Mad Max Date: Wed, 22 Apr 2026 14:36:38 +0200 Subject: [PATCH 2/2] ci: run tests on push/PR, fix SlogAdapter.Printf arg spread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a Test workflow that runs `go test -race` on push and PR. `go vet` (run by `-race`) flags that SlogAdapter.Printf passed its variadic arguments without spreading, so fmt.Sprintf saw a single []any instead of the intended args — fix it by forwarding with `v...`. --- .github/workflows/test.yml | 22 ++++++++++++++++++++++ container.go | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/test.yml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..948c33e --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,22 @@ +name: Test + +on: + push: + branches: [master] + pull_request: + branches: [master] + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: 'go.mod' + cache-dependency-path: 'go.sum' + + - name: Run tests + run: go test -race -v -timeout=3m ./... diff --git a/container.go b/container.go index ba0894c..19c6feb 100644 --- a/container.go +++ b/container.go @@ -374,5 +374,5 @@ type SlogAdapter struct { } func (s *SlogAdapter) Printf(format string, v ...any) { - s.logger.Info(fmt.Sprintf(format, v)) + s.logger.Info(fmt.Sprintf(format, v...)) }