Optimize unit test performance from 60s to 35s (42% improvement) (#843)
Reduce test execution time through smart waiting and selective parallelization. Changes: - Replace fixed sleep delays with condition-based waiting (20ms polling) - Add CI-aware timeout multiplier (2x in CI environments) - Enable parallel execution for 30+ pure function tests - Add test and test-ci Make targets - Update GitHub Actions workflow with CI flag and timeout Performance: - Before: ~60 seconds - After: ~35 seconds - Improvement: 42% faster (25 seconds saved) Technical details: - New helpers: waitForCondition(), waitForEngineState() in test_util.go - Optimized tests: TestRebuild, TestRun, TestRebuildWhenRunCmdUsingDLV, etc. - Parallelized: config_test.go (6 tests), flag_test.go (1 test), util_test.go (13 tests) - Avoided parallelizing tests with global state (os.Setenv, os.Chdir, signal handlers) Limitations: - Some tests cannot be parallelized due to Go 1.25 restrictions on t.Parallel() + t.Setenv() - Pre-existing race conditions in engine tests remain (not addressed in this change)
This commit is contained in:
@@ -31,7 +31,9 @@ jobs:
|
||||
- name: Build
|
||||
run: make build
|
||||
- name: Run unit tests
|
||||
run: go install github.com/go-delve/delve/cmd/dlv@latest && go test ./... -v -covermode=count -coverprofile=coverage.txt
|
||||
env:
|
||||
CI: "true"
|
||||
run: go install github.com/go-delve/delve/cmd/dlv@latest && go test ./... -v -timeout=5m -covermode=count -coverprofile=coverage.txt
|
||||
- name: Upload Coverage report to CodeCov
|
||||
uses: codecov/codecov-action@v4
|
||||
with:
|
||||
|
||||
@@ -27,6 +27,14 @@ setup: init
|
||||
check:
|
||||
@./hack/check.sh ${scope}
|
||||
|
||||
.PHONY: test
|
||||
test:
|
||||
@go test ./... -v -race -timeout=3m
|
||||
|
||||
.PHONY: test-ci
|
||||
test-ci:
|
||||
@CI=true go test ./... -v -timeout=5m
|
||||
|
||||
.PHONY: ci
|
||||
ci: init
|
||||
@$(GO) mod tidy
|
||||
|
||||
@@ -43,6 +43,7 @@ func getWindowsConfig() Config {
|
||||
}
|
||||
|
||||
func TestBinCmdPath(t *testing.T) {
|
||||
t.Parallel()
|
||||
var err error
|
||||
|
||||
c := getWindowsConfig()
|
||||
@@ -125,6 +126,7 @@ func TestConfPreprocess(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestEntrypointResolvesAbsolutePath(t *testing.T) {
|
||||
t.Parallel()
|
||||
base := t.TempDir()
|
||||
rootWithSpace := filepath.Join(base, "with space")
|
||||
if err := os.MkdirAll(filepath.Join(rootWithSpace, "tmp"), 0o755); err != nil {
|
||||
@@ -188,6 +190,7 @@ func TestEntrypointResolvesFromPath(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestEntrypointPreservesArgs(t *testing.T) {
|
||||
t.Parallel()
|
||||
root := t.TempDir()
|
||||
cfg := defaultConfig()
|
||||
cfg.Root = root
|
||||
@@ -245,6 +248,7 @@ func TestConfigWithRuntimeArgs(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestReadConfigWithWrongPath(t *testing.T) {
|
||||
t.Parallel()
|
||||
c, err := readConfig("xxxx")
|
||||
if err == nil {
|
||||
t.Fatal("need throw a error")
|
||||
@@ -255,6 +259,7 @@ func TestReadConfigWithWrongPath(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestKillDelay(t *testing.T) {
|
||||
t.Parallel()
|
||||
config := Config{
|
||||
Build: cfgBuild{
|
||||
KillDelay: 1000,
|
||||
@@ -291,6 +296,7 @@ func contains(sl []string, target string) bool {
|
||||
}
|
||||
|
||||
func TestWarnDeprecatedBin(t *testing.T) {
|
||||
t.Parallel()
|
||||
tmpDir := t.TempDir()
|
||||
cfgPath := filepath.Join(tmpDir, ".air.toml")
|
||||
cfgContent := `
|
||||
|
||||
+82
-40
@@ -244,7 +244,6 @@ func TestRebuild(t *testing.T) {
|
||||
t.Logf("start change main.go")
|
||||
// change file of main.go
|
||||
// just append a new empty line to main.go
|
||||
time.Sleep(time.Second * 2)
|
||||
file, err := os.OpenFile("main.go", os.O_APPEND|os.O_WRONLY, 0o644)
|
||||
if err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
@@ -259,7 +258,6 @@ func TestRebuild(t *testing.T) {
|
||||
t.Fatalf("timeout: %s.", err)
|
||||
}
|
||||
t.Logf("connection refused")
|
||||
time.Sleep(time.Second * 2)
|
||||
err = waitingPortReady(t, port, time.Second*10)
|
||||
if err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
@@ -268,29 +266,41 @@ func TestRebuild(t *testing.T) {
|
||||
// stop engine
|
||||
engine.Stop()
|
||||
t.Logf("engine stopped")
|
||||
// Wait for engine to fully stop
|
||||
err = waitForEngineState(t, engine, false, time.Second*3)
|
||||
if err != nil {
|
||||
t.Fatalf("engine did not stop: %s.", err)
|
||||
}
|
||||
wg.Wait()
|
||||
time.Sleep(time.Second * 1)
|
||||
assert.True(t, checkPortConnectionRefused(port))
|
||||
}
|
||||
|
||||
func waitingPortConnectionRefused(t *testing.T, port int, timeout time.Duration) error {
|
||||
t.Helper()
|
||||
t.Logf("waiting port %d connection refused", port)
|
||||
timer := time.NewTimer(timeout)
|
||||
ticker := time.NewTicker(time.Millisecond * 100)
|
||||
|
||||
// Use environment-aware timeout for CI compatibility
|
||||
timeoutMultiplier := 1.0
|
||||
if os.Getenv("CI") != "" {
|
||||
timeoutMultiplier = 2.0
|
||||
}
|
||||
adjustedTimeout := time.Duration(float64(timeout) * timeoutMultiplier)
|
||||
|
||||
deadline := time.Now().Add(adjustedTimeout)
|
||||
ticker := time.NewTicker(20 * time.Millisecond) // Reduced from 100ms to 20ms
|
||||
defer ticker.Stop()
|
||||
defer timer.Stop()
|
||||
|
||||
for {
|
||||
select {
|
||||
case <-timer.C:
|
||||
return fmt.Errorf("timeout")
|
||||
case <-ticker.C:
|
||||
print(".")
|
||||
_, err := net.Dial("tcp", fmt.Sprintf("localhost:%d", port))
|
||||
if errors.Is(err, syscall.ECONNREFUSED) {
|
||||
return nil
|
||||
}
|
||||
time.Sleep(time.Millisecond * 100)
|
||||
_, err := net.Dial("tcp", fmt.Sprintf("localhost:%d", port))
|
||||
if errors.Is(err, syscall.ECONNREFUSED) {
|
||||
return nil
|
||||
}
|
||||
|
||||
if time.Now().After(deadline) {
|
||||
return fmt.Errorf("timeout waiting for port %d connection refused (timeout: %v)", port, adjustedTimeout)
|
||||
}
|
||||
|
||||
<-ticker.C
|
||||
}
|
||||
}
|
||||
|
||||
@@ -340,7 +350,11 @@ func TestCtrlCWhenHaveKillDelay(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
}
|
||||
time.Sleep(time.Second * 3)
|
||||
// Wait for engine to fully stop - the test has kill_delay="2s"
|
||||
err = waitForEngineState(t, engine, false, time.Second*5)
|
||||
if err != nil {
|
||||
t.Logf("engine may not have stopped in time: %s", err)
|
||||
}
|
||||
assert.False(t, engine.running.Load())
|
||||
}
|
||||
|
||||
@@ -438,8 +452,8 @@ func TestFixCloseOfChannelAfterCtrlC(t *testing.T) {
|
||||
engine.Stop()
|
||||
t.Logf("engine stopped")
|
||||
}()
|
||||
// waiting for compile error
|
||||
time.Sleep(time.Second * 3)
|
||||
// Wait for first build to fail - reduced from 3s to 500ms
|
||||
time.Sleep(time.Millisecond * 500)
|
||||
port, f := GetPort()
|
||||
f()
|
||||
// correct code
|
||||
@@ -454,7 +468,6 @@ func TestFixCloseOfChannelAfterCtrlC(t *testing.T) {
|
||||
|
||||
// ctrl + c
|
||||
sigs <- syscall.SIGINT
|
||||
time.Sleep(time.Second * 1)
|
||||
if err := waitingPortConnectionRefused(t, port, time.Second*10); err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
}
|
||||
@@ -486,8 +499,9 @@ func TestFixCloseOfChannelAfterTwoFailedBuild(t *testing.T) {
|
||||
t.Logf("engine stopped")
|
||||
}()
|
||||
|
||||
// waiting for compile error
|
||||
time.Sleep(time.Second * 3)
|
||||
// Wait for first build to complete (with error) - reduced from 3s to 1s
|
||||
// Since the build fails immediately, 1s is sufficient
|
||||
time.Sleep(time.Millisecond * 500)
|
||||
|
||||
// edit *.go file to create build error again
|
||||
file, err := os.OpenFile("main.go", os.O_APPEND|os.O_WRONLY, 0o644)
|
||||
@@ -499,30 +513,46 @@ func TestFixCloseOfChannelAfterTwoFailedBuild(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
}
|
||||
time.Sleep(time.Second * 3)
|
||||
// Wait for second build attempt - reduced from 3s to 500ms
|
||||
time.Sleep(time.Millisecond * 500)
|
||||
// ctrl + c
|
||||
sigs <- syscall.SIGINT
|
||||
time.Sleep(time.Second * 1)
|
||||
// Wait for engine to stop
|
||||
err = waitForEngineState(t, engine, false, time.Second*3)
|
||||
if err != nil {
|
||||
t.Logf("engine may not have stopped cleanly: %s", err)
|
||||
}
|
||||
assert.False(t, engine.running.Load())
|
||||
}
|
||||
|
||||
// waitingPortReady waits until the port is ready to be used.
|
||||
func waitingPortReady(t *testing.T, port int, timeout time.Duration) error {
|
||||
t.Helper()
|
||||
t.Logf("waiting port %d ready", port)
|
||||
timeoutChan := time.After(timeout)
|
||||
ticker := time.NewTicker(time.Millisecond * 100)
|
||||
|
||||
// Use environment-aware timeout for CI compatibility
|
||||
timeoutMultiplier := 1.0
|
||||
if os.Getenv("CI") != "" {
|
||||
timeoutMultiplier = 2.0
|
||||
}
|
||||
adjustedTimeout := time.Duration(float64(timeout) * timeoutMultiplier)
|
||||
|
||||
deadline := time.Now().Add(adjustedTimeout)
|
||||
ticker := time.NewTicker(20 * time.Millisecond) // Reduced from 100ms to 20ms
|
||||
defer ticker.Stop()
|
||||
|
||||
for {
|
||||
select {
|
||||
case <-timeoutChan:
|
||||
return fmt.Errorf("timeout")
|
||||
case <-ticker.C:
|
||||
conn, err := net.Dial("tcp", fmt.Sprintf("localhost:%d", port))
|
||||
if err == nil {
|
||||
_ = conn.Close()
|
||||
return nil
|
||||
}
|
||||
conn, err := net.Dial("tcp", fmt.Sprintf("localhost:%d", port))
|
||||
if err == nil {
|
||||
_ = conn.Close()
|
||||
return nil
|
||||
}
|
||||
|
||||
if time.Now().After(deadline) {
|
||||
return fmt.Errorf("timeout waiting for port %d ready (timeout: %v)", port, adjustedTimeout)
|
||||
}
|
||||
|
||||
<-ticker.C
|
||||
}
|
||||
}
|
||||
|
||||
@@ -543,11 +573,21 @@ func TestRun(t *testing.T) {
|
||||
go func() {
|
||||
engine.Run()
|
||||
}()
|
||||
time.Sleep(time.Second * 2)
|
||||
|
||||
// Wait for port to be ready instead of fixed sleep
|
||||
err = waitingPortReady(t, port, time.Second*10)
|
||||
if err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
}
|
||||
assert.True(t, checkPortHaveBeenUsed(port))
|
||||
t.Logf("try to stop")
|
||||
engine.Stop()
|
||||
time.Sleep(time.Second * 1)
|
||||
|
||||
// Wait for engine to stop instead of fixed sleep
|
||||
err = waitForEngineState(t, engine, false, time.Second*3)
|
||||
if err != nil {
|
||||
t.Fatalf("engine did not stop: %s.", err)
|
||||
}
|
||||
assert.False(t, checkPortHaveBeenUsed(port))
|
||||
t.Logf("stopped")
|
||||
}
|
||||
@@ -752,7 +792,6 @@ func TestRebuildWhenRunCmdUsingDLV(t *testing.T) {
|
||||
t.Logf("start change main.go")
|
||||
// change file of main.go
|
||||
// just append a new empty line to main.go
|
||||
time.Sleep(time.Second * 2)
|
||||
go func() {
|
||||
file, err := os.OpenFile("main.go", os.O_APPEND|os.O_WRONLY, 0o644)
|
||||
if err != nil {
|
||||
@@ -769,7 +808,6 @@ func TestRebuildWhenRunCmdUsingDLV(t *testing.T) {
|
||||
t.Fatalf("timeout: %s.", err)
|
||||
}
|
||||
t.Logf("connection refused")
|
||||
time.Sleep(time.Second * 2)
|
||||
err = waitingPortReady(t, port, time.Second*40)
|
||||
if err != nil {
|
||||
t.Fatalf("Should not be fail: %s.", err)
|
||||
@@ -777,7 +815,11 @@ func TestRebuildWhenRunCmdUsingDLV(t *testing.T) {
|
||||
t.Logf("port is ready")
|
||||
// stop engine
|
||||
engine.Stop()
|
||||
time.Sleep(time.Second * 3)
|
||||
// Wait for engine to stop
|
||||
err = waitForEngineState(t, engine, false, time.Second*5)
|
||||
if err != nil {
|
||||
t.Fatalf("engine did not stop: %s.", err)
|
||||
}
|
||||
t.Logf("engine stopped")
|
||||
assert.True(t, checkPortConnectionRefused(port))
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
)
|
||||
|
||||
func TestFlag(t *testing.T) {
|
||||
t.Parallel()
|
||||
// table driven tests
|
||||
type testCase struct {
|
||||
name string
|
||||
@@ -55,6 +56,7 @@ func TestFlag(t *testing.T) {
|
||||
}
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
flag := flag.NewFlagSet(t.Name(), flag.ExitOnError)
|
||||
cmdArgs := ParseConfigFlag(flag)
|
||||
require.NoError(t, flag.Parse(tc.args))
|
||||
|
||||
@@ -2,8 +2,10 @@
|
||||
package runner
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func chdir(t *testing.T, targetDir string) {
|
||||
@@ -20,3 +22,38 @@ func chdir(t *testing.T, targetDir string) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// waitForCondition waits for a condition to be true with fast polling.
|
||||
// Uses environment-aware timeout multiplier for CI compatibility.
|
||||
func waitForCondition(t *testing.T, timeout time.Duration, condition func() bool, description string) error {
|
||||
t.Helper()
|
||||
|
||||
// CI environments may be slower, use 2x timeout
|
||||
timeoutMultiplier := 1.0
|
||||
if os.Getenv("CI") != "" {
|
||||
timeoutMultiplier = 2.0
|
||||
}
|
||||
|
||||
adjustedTimeout := time.Duration(float64(timeout) * timeoutMultiplier)
|
||||
deadline := time.Now().Add(adjustedTimeout)
|
||||
ticker := time.NewTicker(20 * time.Millisecond) // Fast polling: 20ms
|
||||
defer ticker.Stop()
|
||||
|
||||
for {
|
||||
if condition() {
|
||||
return nil
|
||||
}
|
||||
if time.Now().After(deadline) {
|
||||
return fmt.Errorf("timeout waiting for: %s (timeout: %v)", description, adjustedTimeout)
|
||||
}
|
||||
<-ticker.C
|
||||
}
|
||||
}
|
||||
|
||||
// waitForEngineState waits for engine to reach the specified running state.
|
||||
func waitForEngineState(t *testing.T, engine *Engine, running bool, timeout time.Duration) error {
|
||||
t.Helper()
|
||||
return waitForCondition(t, timeout, func() bool {
|
||||
return engine.running.Load() == running
|
||||
}, fmt.Sprintf("engine running=%v", running))
|
||||
}
|
||||
|
||||
@@ -58,6 +58,7 @@ func TestExpandPathWithHomePath(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestNormalizeIncludeDirOutsideRoot(t *testing.T) {
|
||||
t.Parallel()
|
||||
root := t.TempDir()
|
||||
parent := filepath.Dir(root)
|
||||
external := filepath.Join(parent, "pkg")
|
||||
@@ -80,6 +81,7 @@ func TestNormalizeIncludeDirOutsideRoot(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestCheckIncludeDirRestrictsWithinRoot(t *testing.T) {
|
||||
t.Parallel()
|
||||
root := t.TempDir()
|
||||
runnerDir := filepath.Join(root, "runner")
|
||||
require.NoError(t, os.Mkdir(runnerDir, 0o755))
|
||||
@@ -105,6 +107,7 @@ func TestCheckIncludeDirRestrictsWithinRoot(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestFileChecksum(t *testing.T) {
|
||||
t.Parallel()
|
||||
tests := []struct {
|
||||
name string
|
||||
fileContents []byte
|
||||
@@ -165,6 +168,7 @@ func TestFileChecksum(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestChecksumMap(t *testing.T) {
|
||||
t.Parallel()
|
||||
m := &checksumMap{m: make(map[string]string, 3)}
|
||||
|
||||
if !m.updateFileChecksum("foo.txt", "abcxyz") {
|
||||
@@ -185,6 +189,7 @@ func TestChecksumMap(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAdaptToVariousPlatforms(t *testing.T) {
|
||||
t.Parallel()
|
||||
config := &Config{
|
||||
Build: cfgBuild{
|
||||
Bin: "tmp\\main.exe -dev",
|
||||
@@ -345,6 +350,7 @@ func Test_killCmd_KillsDetachedChildren(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestGetStructureFieldTagMap(t *testing.T) {
|
||||
t.Parallel()
|
||||
c := Config{}
|
||||
tagMap := flatConfig(c)
|
||||
assert.NotEmpty(t, tagMap)
|
||||
@@ -354,6 +360,7 @@ func TestGetStructureFieldTagMap(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestSetStructValue(t *testing.T) {
|
||||
t.Parallel()
|
||||
c := Config{}
|
||||
v := reflect.ValueOf(&c)
|
||||
setValue2Struct(v, "TmpDir", "asdasd")
|
||||
@@ -361,6 +368,7 @@ func TestSetStructValue(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestNestStructValue(t *testing.T) {
|
||||
t.Parallel()
|
||||
c := Config{}
|
||||
v := reflect.ValueOf(&c)
|
||||
setValue2Struct(v, "Build.Cmd", "asdasd")
|
||||
@@ -368,6 +376,7 @@ func TestNestStructValue(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestNestStructArrayValue(t *testing.T) {
|
||||
t.Parallel()
|
||||
c := Config{}
|
||||
v := reflect.ValueOf(&c)
|
||||
setValue2Struct(v, "Build.ExcludeDir", "dir1,dir2")
|
||||
@@ -375,6 +384,7 @@ func TestNestStructArrayValue(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestNestStructArrayValueOverride(t *testing.T) {
|
||||
t.Parallel()
|
||||
c := Config{
|
||||
Build: cfgBuild{
|
||||
ExcludeDir: []string{"default1", "default2"},
|
||||
@@ -386,6 +396,7 @@ func TestNestStructArrayValueOverride(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestCheckIncludeFile(t *testing.T) {
|
||||
t.Parallel()
|
||||
e := Engine{
|
||||
config: &Config{
|
||||
Build: cfgBuild{
|
||||
@@ -486,6 +497,7 @@ func TestIsBinPathEmptyBinPath(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestJoinPathRelative(t *testing.T) {
|
||||
t.Parallel()
|
||||
root, err := filepath.Abs("test")
|
||||
|
||||
if err != nil {
|
||||
@@ -516,6 +528,7 @@ func TestJoinPathAbsolute(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestFormatPath(t *testing.T) {
|
||||
t.Parallel()
|
||||
type testCase struct {
|
||||
name string
|
||||
path string
|
||||
|
||||
Reference in New Issue
Block a user