From e2076beb9f7af4394dfe36b797e9847ea5a1f2e3 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Wed, 8 Oct 2025 16:18:02 +0400 Subject: [PATCH] test: change to explicitly compiling dbcleaner on posix (#20206) relates to: https://github.com/coder/internal/issues/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. --- coderd/database/dbtestutil/broker.go | 2 +- coderd/database/dbtestutil/cleaner.go | 5 ++- coderd/database/dbtestutil/cleaner_posix.go | 34 +++++++++++++++++++ coderd/database/dbtestutil/cleaner_windows.go | 11 ++++++ coderd/database/dbtestutil/postgres.go | 1 + coderd/database/gen/dump/main.go | 4 +++ 6 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 coderd/database/dbtestutil/cleaner_posix.go create mode 100644 coderd/database/dbtestutil/cleaner_windows.go diff --git a/coderd/database/dbtestutil/broker.go b/coderd/database/dbtestutil/broker.go index 4e814b93d0..8a0d47b7b1 100644 --- 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 index 8387cc18fe..23da8c8fc7 100644 --- 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 new file mode 100644 index 0000000000..3c8188e5d8 --- /dev/null +++ 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 new file mode 100644 index 0000000000..4c743b90ed --- /dev/null +++ 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 index 567fae0daf..a55a99f972 100644 --- 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 index 1d84339eec..25bcbcd396 100644 --- 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() {