Swap to using gotestsum for retrying flaky tests

.github/workflows/go-test.yml

@@ -67,7 +67,8 @@ jobs:
       env:
         DD_API_KEY: ${{ secrets.DD_API_KEY }}
       run: |
-          make retrying-test
+          go install gotest.tools/gotestsum@latest
+          make test
     # - name: Setup tmate session
     #   if: ${{ failure() }}
     #   uses: mxschmitt/action-tmate@v3

Makefile

@@ -1,16 +1,13 @@
 forcetest:
 	go clean -testcache
-	TZ='America/Los_Angeles' HISHTORY_TEST=1 HISHTORY_SKIP_INIT_IMPORT=1 go test -p 1 -timeout 60m ./...
+	make test
 
 test:
-	TZ='America/Los_Angeles' HISHTORY_TEST=1 HISHTORY_SKIP_INIT_IMPORT=1 go test -p 1 -timeout 60m ./...
+	TZ='America/Los_Angeles' HISHTORY_TEST=1 HISHTORY_SKIP_INIT_IMPORT=1 gotestsum --packages ./... --rerun-fails=10 --format testname -- -p 1 --rerun-fails-max-failures=30
 
 ftest:
 	go clean -testcache
-	TZ='America/Los_Angeles' HISHTORY_TEST=1 HISHTORY_SKIP_INIT_IMPORT=1 go test -v -p 1 -run "$(FILTER)" ./...
+	TZ='America/Los_Angeles' HISHTORY_TEST=1 HISHTORY_SKIP_INIT_IMPORT=1 gotestsum --packages ./... --rerun-fails=3 --format testname -- -p 1 -run "$(FILTER)"
-
-retrying-test:
-	for i in `seq 1 3`; do echo "Test attempt number $$i"; rm -rf ~/.hishtory/; make test && break; done
 
 acttest:
 	act push -j test -e .github/push_event.json --reuse --container-architecture linux/amd64

client/client_test.go

@@ -107,16 +107,16 @@ func TestParam(t *testing.T) {
 		t.Run("testCustomColumns/"+tester.ShellName(), func(t *testing.T) { testCustomColumns(t, tester) })
 		t.Run("testUninstall/"+tester.ShellName(), func(t *testing.T) { testUninstall(t, tester) })
 		t.Run("testPresaving/"+tester.ShellName(), func(t *testing.T) { testPresaving(t, tester) })
-		runTestsWithRetries(t, "testControlR/"+tester.ShellName(), func(t testing.TB) { testControlR(t, tester, tester.ShellName(), Online) })
+		t.Run("testControlR/"+tester.ShellName(), func(t *testing.T) { testControlR(t, tester, tester.ShellName(), Online) })
 	}
-	runTestsWithRetries(t, "testControlR/offline/bash", func(t testing.TB) { testControlR(t, bashTester{}, "bash", Offline) })
+	t.Run("testControlR/offline/bash", func(t *testing.T) { testControlR(t, bashTester{}, "bash", Offline) })
-	runTestsWithRetries(t, "testControlR/fish", func(t testing.TB) { testControlR(t, bashTester{}, "fish", Online) })
+	t.Run("testControlR/fish", func(t *testing.T) { testControlR(t, bashTester{}, "fish", Online) })
-	runTestsWithExtraRetries(t, "testTui/search", testTui_search, 10)
+	t.Run("testTui/search", testTui_search)
-	runTestsWithRetries(t, "testTui/general", testTui_general)
+	t.Run("testTui/general", testTui_general)
-	runTestsWithRetries(t, "testTui/scroll", testTui_scroll)
+	t.Run("testTui/scroll", testTui_scroll)
-	runTestsWithRetries(t, "testTui/resize", testTui_resize)
+	t.Run("testTui/resize", testTui_resize)
-	runTestsWithExtraRetries(t, "testTui/delete", testTui_delete, 10)
+	t.Run("testTui/delete", testTui_delete)
-	runTestsWithRetries(t, "testTui/color", testTui_color)
+	t.Run("testTui/color", testTui_color)
 
 	// Assert there are no leaked connections
 	assertNoLeakedConnections(t)
@@ -1635,7 +1635,7 @@ func setupTestTui(t testing.TB) (shellTester, string, *gorm.DB) {
 	return tester, userSecret, db
 }
 
-func testTui_resize(t testing.TB) {
+func testTui_resize(t *testing.T) {
 	// Setup
 	defer testutils.BackupAndRestore(t)()
 	tester, userSecret, _ := setupTestTui(t)
@@ -1675,7 +1675,7 @@ func testTui_resize(t testing.TB) {
 	testutils.CompareGoldens(t, out, "TestTui-LongQuery")
 }
 
-func testTui_scroll(t testing.TB) {
+func testTui_scroll(t *testing.T) {
 	// Setup
 	defer testutils.BackupAndRestore(t)()
 	tester, userSecret, _ := setupTestTui(t)
@@ -1713,7 +1713,7 @@ func testTui_scroll(t testing.TB) {
 	assertNoLeakedConnections(t)
 }
 
-func testTui_color(t testing.TB) {
+func testTui_color(t *testing.T) {
 	if runtime.GOOS == "linux" {
 		// For some reason, this test fails on linux. Since this test isn't critical and is expected to be
 		// flaky, we can just skip it on linux.
@@ -1744,7 +1744,7 @@ func testTui_color(t testing.TB) {
 	testutils.CompareGoldens(t, out, "TestTui-ColoredOutputWithSearch-BetaMode")
 }
 
-func testTui_delete(t testing.TB) {
+func testTui_delete(t *testing.T) {
 	// Setup
 	defer testutils.BackupAndRestore(t)()
 	tester, userSecret, _ := setupTestTui(t)
@@ -1786,7 +1786,7 @@ func testTui_delete(t testing.TB) {
 	assertNoLeakedConnections(t)
 }
 
-func testTui_search(t testing.TB) {
+func testTui_search(t *testing.T) {
 	// Setup
 	defer testutils.BackupAndRestore(t)()
 	tester, _, _ := setupTestTui(t)
@@ -1847,7 +1847,7 @@ func testTui_search(t testing.TB) {
 	testutils.CompareGoldens(t, out, "TestTui-InvalidSearchBecomesValid")
 }
 
-func testTui_general(t testing.TB) {
+func testTui_general(t *testing.T) {
 	// Setup
 	defer testutils.BackupAndRestore(t)()
 	tester, _, _ := setupTestTui(t)
@@ -1907,7 +1907,7 @@ func testTui_general(t testing.TB) {
 	assertNoLeakedConnections(t)
 }
 
-func testControlR(t testing.TB, tester shellTester, shellName string, onlineStatus OnlineStatus) {
+func testControlR(t *testing.T, tester shellTester, shellName string, onlineStatus OnlineStatus) {
 	// Setup
 	defer testutils.BackupAndRestore(t)()
 	installWithOnlineStatus(t, tester, onlineStatus)

client/testutils.go

@@ -114,89 +114,6 @@ func (z zshTester) ShellName() string {
 	return "zsh"
 }
 
-func runTestsWithRetries(parentT *testing.T, testName string, testFunc func(t testing.TB)) {
-	numRetries := 3
-	if testutils.IsGithubAction() {
-		numRetries = 5
-	}
-	runTestsWithExtraRetries(parentT, testName, testFunc, numRetries)
-}
-
-func runTestsWithExtraRetries(parentT *testing.T, testName string, testFunc func(t testing.TB), numRetries int) {
-	for i := 1; i <= numRetries; i++ {
-		rt := &retryingTester{nil, i == numRetries, true, testName, numRetries}
-		parentT.Run(fmt.Sprintf("%s/%d", testName, i), func(t *testing.T) {
-			rt.T = t
-			testFunc(rt)
-		})
-		if rt.succeeded {
-			if GLOBAL_STATSD != nil {
-				GLOBAL_STATSD.Incr("test_status", []string{"result:passed", "test:" + testName, "os:" + runtime.GOOS}, 1.0)
-				GLOBAL_STATSD.Distribution("test_retry_count", float64(i), []string{"test:" + testName, "os:" + runtime.GOOS}, 1.0)
-			}
-			break
-		} else {
-			if GLOBAL_STATSD != nil {
-				GLOBAL_STATSD.Incr("test_status", []string{"result:failed", "test:" + testName, "os:" + runtime.GOOS}, 1.0)
-			}
-		}
-	}
-}
-
-type retryingTester struct {
-	*testing.T
-	isFinalRun bool
-	succeeded  bool
-	testName   string
-	numRetries int
-}
-
-func (t *retryingTester) Fatalf(format string, args ...any) {
-	t.T.Helper()
-	t.succeeded = false
-	if t.isFinalRun {
-		if GLOBAL_STATSD != nil {
-			GLOBAL_STATSD.Incr("test_failure", []string{"test:" + t.testName, "os:" + runtime.GOOS}, 1.0)
-			GLOBAL_STATSD.Distribution("test_retry_count", float64(t.numRetries), []string{"test:" + t.testName, "os:" + runtime.GOOS}, 1.0)
-		}
-		t.T.Fatalf(format, args...)
-	} else {
-		testutils.TestLog(t.T, fmt.Sprintf("retryingTester: Ignoring fatalf for non-final run: %#v", fmt.Sprintf(format, args...)))
-	}
-	t.SkipNow()
-}
-
-func (t *retryingTester) Errorf(format string, args ...any) {
-	t.T.Helper()
-	t.succeeded = false
-	if t.isFinalRun {
-		t.T.Errorf(format, args...)
-	} else {
-		testutils.TestLog(t.T, fmt.Sprintf("retryingTester: Ignoring errorf for non-final run: %#v", fmt.Sprintf(format, args...)))
-	}
-	t.SkipNow()
-}
-
-func (t *retryingTester) FailNow() {
-	t.succeeded = false
-	if t.isFinalRun {
-		t.T.FailNow()
-	} else {
-		testutils.TestLog(t.T, "retryingTester: Ignoring FailNow for non-final run")
-		// Still terminate execution via SkipNow() since FailNow() means we should stop the current test
-		t.T.SkipNow()
-	}
-}
-
-func (t *retryingTester) Fail() {
-	t.succeeded = false
-	if t.isFinalRun {
-		t.T.Fail()
-	} else {
-		testutils.TestLog(t.T, "retryingTester: Ignoring Fail for non-final run")
-	}
-}
-
 type OnlineStatus int64
 
 const (