diff --git a/Makefile b/Makefile index 8f7bc9d17e..8b17b88e20 100644 --- a/Makefile +++ b/Makefile @@ -1020,19 +1020,11 @@ endif TEST_PACKAGES ?= ./... -warm-go-cache-db-cleaner: - # ensure Go's build cache for the cleanercmd is fresh so that tests don't have to build from scratch. This - # could take some time and counts against the test's timeout, which can lead to flakes. - # c.f. https://github.com/coder/internal/issues/1026 - mkdir -p build - $(GIT_FLAGS) go build -o ./build/cleaner github.com/coder/coder/v2/coderd/database/dbtestutil/cleanercmd -.PHONY: warm-go-cache-db-cleaner - -test: warm-go-cache-db-cleaner +test: $(GIT_FLAGS) gotestsum --format standard-quiet $(GOTESTSUM_RETRY_FLAGS) --packages="$(TEST_PACKAGES)" -- $(GOTEST_FLAGS) .PHONY: test -test-cli: warm-go-cache-db-cleaner +test-cli: $(MAKE) test TEST_PACKAGES="./cli..." .PHONY: test-cli diff --git a/coderd/database/dbtestutil/cleaner.go b/coderd/database/dbtestutil/cleaner.go index 23da8c8fc7..851f4488f8 100644 --- a/coderd/database/dbtestutil/cleaner.go +++ b/coderd/database/dbtestutil/cleaner.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "os/exec" "os/signal" "time" @@ -21,36 +22,43 @@ const ( cleanerRespOK = "OK" envCleanerParentUUID = "DB_CLEANER_PARENT_UUID" envCleanerDSN = "DB_CLEANER_DSN" -) - -var ( - originalWorkingDir string - errGettingWorkingDir error + envCleanerMagic = "DB_CLEANER_MAGIC" + envCleanerMagicValue = "XEHdJqWehWek8AaWwopy" // 20 random characters to make this collision resistant ) func init() { - // We expect our tests to run from somewhere in the project tree where `go run` below in `startCleaner` will - // be able to resolve the command package. However, some of the tests modify the working directory during the run. - // So, we grab the working directory during package init, before tests are run, and then set that work dir on the - // subcommand process before it starts. - originalWorkingDir, errGettingWorkingDir = os.Getwd() + // We are hijacking the init() function here to do something very non-standard. + // + // We want to be able to run the cleaner as a subprocess of the test process so that it can outlive the test binary + // and still clean up, even if the test process times out or is killed. So, what we do is in startCleaner() below, + // which is called in the parent process, we exec our own binary and set a collision-resistant environment variable. + // Then here in the init(), which will run before main() and therefore before executing tests, we check for the + // environment variable, and if present we know this is the child process and we exec the cleaner. Instead of + // returning normally from init() we call os.Exit(). This prevents tests from being re-run in the child process (and + // recursion). + // + // If the magic value is not present, we know we are the parent process and init() returns normally. + magicValue := os.Getenv(envCleanerMagic) + if magicValue == envCleanerMagicValue { + RunCleaner() + os.Exit(0) + } } // 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, t TBSubset, parentUUID uuid.UUID, dsn string) (holdThis any, err error) { - cmd := cleanerCmd(t) +func startCleaner(ctx context.Context, _ TBSubset, parentUUID uuid.UUID, dsn string) (holdThis any, err error) { + bin, err := os.Executable() + if err != nil { + return nil, xerrors.Errorf("could not get executable path: %w", err) + } + cmd := exec.Command(bin) cmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", envCleanerParentUUID, parentUUID.String()), fmt.Sprintf("%s=%s", envCleanerDSN, dsn), + fmt.Sprintf("%s=%s", envCleanerMagic, envCleanerMagicValue), ) - // c.f. comment on `func init()` in this file. - if errGettingWorkingDir != nil { - return nil, xerrors.Errorf("failed to get working directory during init: %w", errGettingWorkingDir) - } - cmd.Dir = originalWorkingDir - // Here we don't actually use the reference to the stdin pipe, because we never write anything to it. When this // process exits, the pipe is closed by the OS and this triggers the cleaner to do its cleaning work. But, we do // need to hang on to a reference to it so that it doesn't get garbage collected and trigger cleanup early. @@ -177,8 +185,7 @@ func (c *cleaner) waitAndClean() { } // RunCleaner runs the test database cleaning process. It takes no arguments but uses stdio and environment variables -// for its operation. It is designed to be launched as the only task of a `main()` process, but is included in this -// package to share constants with the parent code that launches it above. +// for its operation. // // The cleaner is designed to run in a separate process from the main test suite, connected over stdio. If the main test // process ends (panics, times out, or is killed) without explicitly discarding the databases it clones, the cleaner diff --git a/coderd/database/dbtestutil/cleaner_posix.go b/coderd/database/dbtestutil/cleaner_posix.go deleted file mode 100644 index 3c8188e5d8..0000000000 --- a/coderd/database/dbtestutil/cleaner_posix.go +++ /dev/null @@ -1,34 +0,0 @@ -//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 deleted file mode 100644 index 4c743b90ed..0000000000 --- a/coderd/database/dbtestutil/cleaner_windows.go +++ /dev/null @@ -1,11 +0,0 @@ -//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/cleanercmd/main.go b/coderd/database/dbtestutil/cleanercmd/main.go deleted file mode 100644 index 038637320e..0000000000 --- a/coderd/database/dbtestutil/cleanercmd/main.go +++ /dev/null @@ -1,7 +0,0 @@ -package main - -import "github.com/coder/coder/v2/coderd/database/dbtestutil" - -func main() { - dbtestutil.RunCleaner() -}