Skip to content

Commit 72f58c0

Browse files
authored
fix: limit test parallelism in make test (#19465)
In order to get `make test` to reliably pass again on our dogfood workspaces, we're having to resort to setting parallelism. It also reworks our CI to call the `make test` target, instead of rolling a different command. Behavior changes: * sets 8 packages x 8 tests in parallel by default on `make test` * by default, removes the `-short` flag. In my testing it makes only a few seconds difference on ~200s, or 1-2% * by default, removes the `-count=1` flag that busts Go's test cache. With a fresh cache and no code changes, `make test` executes in ~15 seconds. Signed-off-by: Spike Curtis <spike@coder.com>
1 parent 8d0bc48 commit 72f58c0

File tree

2 files changed

+31
-15
lines changed

2 files changed

+31
-15
lines changed

.github/workflows/ci.yaml

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -465,30 +465,28 @@ jobs:
465465
# running in parallel, and dbtestutil.NewDB starts to take more than
466466
# 10s to complete sometimes causing test timeouts. With 16x8=128 tests
467467
# Postgres tends not to choke.
468-
NUM_PARALLEL_PACKAGES=8
469-
NUM_PARALLEL_TESTS=16
468+
export TEST_NUM_PARALLEL_PACKAGES=8
469+
export TEST_NUM_PARALLEL_TESTS=16
470470
# Only the CLI and Agent are officially supported on Windows and the rest are too flaky
471-
PACKAGES="./cli/... ./enterprise/cli/... ./agent/..."
471+
export TEST_PACKAGES="./cli/... ./enterprise/cli/... ./agent/..."
472472
elif [ "${RUNNER_OS}" == "macOS" ]; then
473473
# Our macOS runners have 8 cores. We set NUM_PARALLEL_TESTS to 16
474474
# because the tests complete faster and Postgres doesn't choke. It seems
475475
# that macOS's tmpfs is faster than the one on Windows.
476-
NUM_PARALLEL_PACKAGES=8
477-
NUM_PARALLEL_TESTS=16
476+
export TEST_NUM_PARALLEL_PACKAGES=8
477+
export TEST_NUM_PARALLEL_TESTS=16
478478
# Only the CLI and Agent are officially supported on macOS and the rest are too flaky
479-
PACKAGES="./cli/... ./enterprise/cli/... ./agent/..."
479+
export TEST_PACKAGES="./cli/... ./enterprise/cli/... ./agent/..."
480480
elif [ "${RUNNER_OS}" == "Linux" ]; then
481481
# Our Linux runners have 8 cores.
482-
NUM_PARALLEL_PACKAGES=8
483-
NUM_PARALLEL_TESTS=8
484-
PACKAGES="./..."
482+
export TEST_NUM_PARALLEL_PACKAGES=8
483+
export TEST_NUM_PARALLEL_TESTS=8
485484
fi
486485
487486
# by default, run tests with cache
488-
TESTCOUNT=""
489487
if [ "${GITHUB_REF}" == "refs/heads/main" ]; then
490488
# on main, run tests without cache
491-
TESTCOUNT="-count=1"
489+
export TEST_COUNT="1"
492490
fi
493491
494492
mkdir -p "$RUNNER_TEMP/sym"
@@ -498,8 +496,7 @@ jobs:
498496
# invalidated. See scripts/normalize_path.sh for more details.
499497
normalize_path_with_symlinks "$RUNNER_TEMP/sym" "$(dirname "$(which terraform)")"
500498
501-
gotestsum --format standard-quiet --packages "$PACKAGES" \
502-
-- -timeout=20m -v -p $NUM_PARALLEL_PACKAGES -parallel=$NUM_PARALLEL_TESTS $TESTCOUNT
499+
make test
503500
504501
- name: Upload failed test db dumps
505502
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2

Makefile

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -958,12 +958,31 @@ else
958958
GOTESTSUM_RETRY_FLAGS :=
959959
endif
960960

961+
# default to 8x8 parallelism to avoid overwhelming our workspaces. Hopefully we can remove these defaults
962+
# when we get our test suite's resource utilization under control.
963+
GOTEST_FLAGS := -v -p $(or $(TEST_NUM_PARALLEL_PACKAGES),"8") -parallel=$(or $(TEST_NUM_PARALLEL_TESTS),"8")
964+
965+
# The most common use is to set TEST_COUNT=1 to avoid Go's test cache.
966+
ifdef TEST_COUNT
967+
GOTEST_FLAGS += -count=$(TEST_COUNT)
968+
endif
969+
970+
ifdef TEST_SHORT
971+
GOTEST_FLAGS += -short
972+
endif
973+
974+
ifdef RUN
975+
GOTEST_FLAGS += -run $(RUN)
976+
endif
977+
978+
TEST_PACKAGES ?= ./...
979+
961980
test:
962-
$(GIT_FLAGS) gotestsum --format standard-quiet $(GOTESTSUM_RETRY_FLAGS) --packages="./..." -- -v -short -count=1 $(if $(RUN),-run $(RUN))
981+
$(GIT_FLAGS) gotestsum --format standard-quiet $(GOTESTSUM_RETRY_FLAGS) --packages="$(TEST_PACKAGES)" -- $(GOTEST_FLAGS)
963982
.PHONY: test
964983

965984
test-cli:
966-
$(GIT_FLAGS) gotestsum --format standard-quiet $(GOTESTSUM_RETRY_FLAGS) --packages="./cli/..." -- -v -short -count=1
985+
$(MAKE) test TEST_PACKAGES="./cli..."
967986
.PHONY: test-cli
968987

969988
# sqlc-cloud-is-setup will fail if no SQLc auth token is set. Use this as a

0 commit comments

Comments
 (0)