test: change to explicitly compiling dbcleaner on posix (coder#20206)

spikecurtis · web-flow · commit e2076beb9f7a · 2025-10-08T16:18:02.000+04:00
relates to: coder/internal#1026 On POSIX systems (macOS and Linux) we compile the dbcleaner binary into a temp directory. This allows us to explicitly separate the compilation step and report the time it takes. We suspect this might be a contributing factor in the above linked flakes we see on macOS. This doesn't work on Windows because Go tests clean up the temp directory at the end of the test and the dbcleaner binary will still be executing. On Windows you cannot delete a file being executed nor the directory. However, we are not seeing any flakes on Windows so the old behavior seems to be OK.
diff --git a/coderd/database/dbtestutil/broker.go b/coderd/database/dbtestutil/broker.go
@@ -150,7 +150,7 @@ func (b *Broker) init(t TBSubset) error {
 		b.uuid = uuid.New()
 		ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
 		defer cancel()
-		b.cleanerFD, err = startCleaner(ctx, b.uuid, coderTestingParams.DSN())
+		b.cleanerFD, err = startCleaner(ctx, t, b.uuid, coderTestingParams.DSN())
 		if err != nil {
 			return xerrors.Errorf("start test db cleaner: %w", err)
 		}
diff --git a/coderd/database/dbtestutil/cleaner.go b/coderd/database/dbtestutil/cleaner.go
@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"os/exec"
 	"os/signal"
 	"time"
 
@@ -39,8 +38,8 @@ func init() {
 
 // startCleaner starts the cleaner in a subprocess. holdThis is an opaque reference that needs to be kept from being
 // garbage collected until we are done with all test databases (e.g. the end of the process).
-func startCleaner(ctx context.Context, parentUUID uuid.UUID, dsn string) (holdThis any, err error) {
-	cmd := exec.Command("go", "run", "github.com/coder/coder/v2/coderd/database/dbtestutil/cleanercmd")
+func startCleaner(ctx context.Context, t TBSubset, parentUUID uuid.UUID, dsn string) (holdThis any, err error) {
+	cmd := cleanerCmd(t)
 	cmd.Env = append(os.Environ(),
 		fmt.Sprintf("%s=%s", envCleanerParentUUID, parentUUID.String()),
 		fmt.Sprintf("%s=%s", envCleanerDSN, dsn),
diff --git a/coderd/database/dbtestutil/cleaner_posix.go b/coderd/database/dbtestutil/cleaner_posix.go
@@ -0,0 +1,34 @@
+//go:build !windows
+
+package dbtestutil
+
+import (
+	"os/exec"
+	"path/filepath"
+	"time"
+)
+
+const timeFormat = "2006-01-02 15:04:05.000"
+
+// cleanerCmd builds the cleaner binary in a temporary directory and returns a command to execute it. We can do this on
+// POSIX because it's OK to delete the temporary directory after the test: the binary can still run. This is not
+// possible on Windows because cleaning the temporary directory will fail if the binary is still running.
+// c.f. cleaner_windows.go.
+func cleanerCmd(t TBSubset) *exec.Cmd {
+	start := time.Now()
+	t.Logf("[%s] starting cleaner binary build", start.Format(timeFormat))
+	tempDir := t.TempDir()
+	cleanerBinary := filepath.Join(tempDir, "cleaner")
+
+	buildCmd := exec.Command("go", "build", "-o", cleanerBinary, "github.com/coder/coder/v2/coderd/database/dbtestutil/cleanercmd")
+	output, err := buildCmd.CombinedOutput()
+	if err != nil {
+		t.Logf("failed to build cleaner binary: %v", err)
+		t.Logf("output: %s", string(output))
+		// Fall back to go run if build fails
+		return exec.Command("go", "run", "github.com/coder/coder/v2/coderd/database/dbtestutil/cleanercmd")
+	}
+	t.Logf("[%s] cleaner binary %s built in %s", time.Now().Format(timeFormat), cleanerBinary, time.Since(start))
+
+	return exec.Command(cleanerBinary)
+}
diff --git a/coderd/database/dbtestutil/cleaner_windows.go b/coderd/database/dbtestutil/cleaner_windows.go
@@ -0,0 +1,11 @@
+//go:build windows
+
+package dbtestutil
+
+import "os/exec"
+
+// cleanerCmd returns a command to execute the cleaner binary. We do this with go run on Windows because we can't
+// delete the temporary directory after the test: the binary will still be running. c.f. cleaner_posix.go.
+func cleanerCmd(_ TBSubset) *exec.Cmd {
+	return exec.Command("go", "run", "github.com/coder/coder/v2/coderd/database/dbtestutil/cleanercmd")
+}
diff --git a/coderd/database/dbtestutil/postgres.go b/coderd/database/dbtestutil/postgres.go
@@ -166,6 +166,7 @@ type TBSubset interface {
 	Cleanup(func())
 	Helper()
 	Logf(format string, args ...any)
+	TempDir() string
 }
 
 // Open creates a new PostgreSQL database instance.
diff --git a/coderd/database/gen/dump/main.go b/coderd/database/gen/dump/main.go
@@ -35,6 +35,10 @@ func (*mockTB) Logf(format string, args ...any) {
 	_, _ = fmt.Printf(format, args...)
 }
 
+func (*mockTB) TempDir() string {
+	panic("not implemented")
+}
+
 func main() {
 	t := &mockTB{}
 	defer func() {

Original file line number	Diff line number	Diff line change
`@@ -150,7 +150,7 @@ func (b *Broker) init(t TBSubset) error {`
`150`	`150`	`b.uuid = uuid.New()`
`151`	`151`	`ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)`
`152`	`152`	`defer cancel()`
`153`		`- b.cleanerFD, err = startCleaner(ctx, b.uuid, coderTestingParams.DSN())`
	`153`	`+ b.cleanerFD, err = startCleaner(ctx, t, b.uuid, coderTestingParams.DSN())`
`154`	`154`	`if err != nil {`
`155`	`155`	`return xerrors.Errorf("start test db cleaner: %w", err)`
`156`	`156`	`}`
Original file line number	Diff line number	Diff line change
`@@ -166,6 +166,7 @@ type TBSubset interface {`
`166`	`166`	`Cleanup(func())`
`167`	`167`	`Helper()`
`168`	`168`	`Logf(format string, args ...any)`
	`169`	`+ TempDir() string`
`169`	`170`	`}`
`170`	`171`
`171`	`172`	`// Open creates a new PostgreSQL database instance.`