Skip to content

Commit 2ded9d6

Browse files
authored
test: remove external compilation of db cleaner entirely (coder#20240)
fixes coder/internal#1026 Thru a (perhaps too-) clever hack of `init()` functions, I've managed to remove the need to separately compile the cleaner binary. This should fix the flakes we are seeing were the binary compilation takes 10s of seconds on macOS. The cleaner is encorporated directly into the test binary and we self-exec as the subprocess.
1 parent 97cb19e commit 2ded9d6

File tree

5 files changed

+29
-82
lines changed

5 files changed

+29
-82
lines changed

Makefile

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,19 +1020,11 @@ endif
10201020

10211021
TEST_PACKAGES ?= ./...
10221022

1023-
warm-go-cache-db-cleaner:
1024-
# ensure Go's build cache for the cleanercmd is fresh so that tests don't have to build from scratch. This
1025-
# could take some time and counts against the test's timeout, which can lead to flakes.
1026-
# c.f. https://github.com/coder/internal/issues/1026
1027-
mkdir -p build
1028-
$(GIT_FLAGS) go build -o ./build/cleaner github.com/coder/coder/v2/coderd/database/dbtestutil/cleanercmd
1029-
.PHONY: warm-go-cache-db-cleaner
1030-
1031-
test: warm-go-cache-db-cleaner
1023+
test:
10321024
$(GIT_FLAGS) gotestsum --format standard-quiet $(GOTESTSUM_RETRY_FLAGS) --packages="$(TEST_PACKAGES)" -- $(GOTEST_FLAGS)
10331025
.PHONY: test
10341026

1035-
test-cli: warm-go-cache-db-cleaner
1027+
test-cli:
10361028
$(MAKE) test TEST_PACKAGES="./cli..."
10371029
.PHONY: test-cli
10381030

coderd/database/dbtestutil/cleaner.go

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"io"
88
"os"
9+
"os/exec"
910
"os/signal"
1011
"time"
1112

@@ -21,36 +22,43 @@ const (
2122
cleanerRespOK = "OK"
2223
envCleanerParentUUID = "DB_CLEANER_PARENT_UUID"
2324
envCleanerDSN = "DB_CLEANER_DSN"
24-
)
25-
26-
var (
27-
originalWorkingDir string
28-
errGettingWorkingDir error
25+
envCleanerMagic = "DB_CLEANER_MAGIC"
26+
envCleanerMagicValue = "XEHdJqWehWek8AaWwopy" // 20 random characters to make this collision resistant
2927
)
3028

3129
func init() {
32-
// We expect our tests to run from somewhere in the project tree where `go run` below in `startCleaner` will
33-
// be able to resolve the command package. However, some of the tests modify the working directory during the run.
34-
// So, we grab the working directory during package init, before tests are run, and then set that work dir on the
35-
// subcommand process before it starts.
36-
originalWorkingDir, errGettingWorkingDir = os.Getwd()
30+
// We are hijacking the init() function here to do something very non-standard.
31+
//
32+
// We want to be able to run the cleaner as a subprocess of the test process so that it can outlive the test binary
33+
// and still clean up, even if the test process times out or is killed. So, what we do is in startCleaner() below,
34+
// which is called in the parent process, we exec our own binary and set a collision-resistant environment variable.
35+
// Then here in the init(), which will run before main() and therefore before executing tests, we check for the
36+
// environment variable, and if present we know this is the child process and we exec the cleaner. Instead of
37+
// returning normally from init() we call os.Exit(). This prevents tests from being re-run in the child process (and
38+
// recursion).
39+
//
40+
// If the magic value is not present, we know we are the parent process and init() returns normally.
41+
magicValue := os.Getenv(envCleanerMagic)
42+
if magicValue == envCleanerMagicValue {
43+
RunCleaner()
44+
os.Exit(0)
45+
}
3746
}
3847

3948
// startCleaner starts the cleaner in a subprocess. holdThis is an opaque reference that needs to be kept from being
4049
// garbage collected until we are done with all test databases (e.g. the end of the process).
41-
func startCleaner(ctx context.Context, t TBSubset, parentUUID uuid.UUID, dsn string) (holdThis any, err error) {
42-
cmd := cleanerCmd(t)
50+
func startCleaner(ctx context.Context, _ TBSubset, parentUUID uuid.UUID, dsn string) (holdThis any, err error) {
51+
bin, err := os.Executable()
52+
if err != nil {
53+
return nil, xerrors.Errorf("could not get executable path: %w", err)
54+
}
55+
cmd := exec.Command(bin)
4356
cmd.Env = append(os.Environ(),
4457
fmt.Sprintf("%s=%s", envCleanerParentUUID, parentUUID.String()),
4558
fmt.Sprintf("%s=%s", envCleanerDSN, dsn),
59+
fmt.Sprintf("%s=%s", envCleanerMagic, envCleanerMagicValue),
4660
)
4761

48-
// c.f. comment on `func init()` in this file.
49-
if errGettingWorkingDir != nil {
50-
return nil, xerrors.Errorf("failed to get working directory during init: %w", errGettingWorkingDir)
51-
}
52-
cmd.Dir = originalWorkingDir
53-
5462
// Here we don't actually use the reference to the stdin pipe, because we never write anything to it. When this
5563
// process exits, the pipe is closed by the OS and this triggers the cleaner to do its cleaning work. But, we do
5664
// 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() {
177185
}
178186

179187
// RunCleaner runs the test database cleaning process. It takes no arguments but uses stdio and environment variables
180-
// for its operation. It is designed to be launched as the only task of a `main()` process, but is included in this
181-
// package to share constants with the parent code that launches it above.
188+
// for its operation.
182189
//
183190
// The cleaner is designed to run in a separate process from the main test suite, connected over stdio. If the main test
184191
// process ends (panics, times out, or is killed) without explicitly discarding the databases it clones, the cleaner

coderd/database/dbtestutil/cleaner_posix.go

Lines changed: 0 additions & 34 deletions
This file was deleted.

coderd/database/dbtestutil/cleaner_windows.go

Lines changed: 0 additions & 11 deletions
This file was deleted.

coderd/database/dbtestutil/cleanercmd/main.go

Lines changed: 0 additions & 7 deletions
This file was deleted.

0 commit comments

Comments
 (0)