From f8a3552ad8b942d5ff98cc7722750a1640ebdc88 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 17:54:33 -0800 Subject: [PATCH 01/36] Remove a few direct DB insertions to prepare for parallel tests --- client/client_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 7aec368f..f71335f8 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1672,12 +1672,10 @@ func setupTestTui(t testing.TB, onlineStatus OnlineStatus) (shellTester, string, // Insert a couple hishtory entries db := hctx.GetDb(hctx.MakeContext()) e1 := testutils.MakeFakeHistoryEntry("ls ~/") - require.NoError(t, db.Create(e1).Error) if onlineStatus == Online { manuallySubmitHistoryEntry(t, userSecret, e1) } e2 := testutils.MakeFakeHistoryEntry("echo 'aaaaaa bbbb'") - require.NoError(t, db.Create(e2).Error) if onlineStatus == Online { manuallySubmitHistoryEntry(t, userSecret, e2) } @@ -1793,14 +1791,11 @@ func testTui_defaultFilter(t *testing.T) { // Setup defer testutils.BackupAndRestore(t)() tester, userSecret, _ := setupTestTui(t, Online) - db := hctx.GetDb(hctx.MakeContext()) e1 := testutils.MakeFakeHistoryEntry("exit 0") e1.ExitCode = 0 - require.NoError(t, db.Create(e1).Error) manuallySubmitHistoryEntry(t, userSecret, e1) e2 := testutils.MakeFakeHistoryEntry("exit 1") e2.ExitCode = 1 - require.NoError(t, db.Create(e2).Error) manuallySubmitHistoryEntry(t, userSecret, e2) // Configure a default filter From fbcdece1bf8f0909ecd1323716b7bf820df119d4 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 18:48:18 -0800 Subject: [PATCH 02/36] Revert "Remove a few direct DB insertions to prepare for parallel tests" This reverts commit f8a3552ad8b942d5ff98cc7722750a1640ebdc88. --- client/client_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/client/client_test.go b/client/client_test.go index f71335f8..7aec368f 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1672,10 +1672,12 @@ func setupTestTui(t testing.TB, onlineStatus OnlineStatus) (shellTester, string, // Insert a couple hishtory entries db := hctx.GetDb(hctx.MakeContext()) e1 := testutils.MakeFakeHistoryEntry("ls ~/") + require.NoError(t, db.Create(e1).Error) if onlineStatus == Online { manuallySubmitHistoryEntry(t, userSecret, e1) } e2 := testutils.MakeFakeHistoryEntry("echo 'aaaaaa bbbb'") + require.NoError(t, db.Create(e2).Error) if onlineStatus == Online { manuallySubmitHistoryEntry(t, userSecret, e2) } @@ -1791,11 +1793,14 @@ func testTui_defaultFilter(t *testing.T) { // Setup defer testutils.BackupAndRestore(t)() tester, userSecret, _ := setupTestTui(t, Online) + db := hctx.GetDb(hctx.MakeContext()) e1 := testutils.MakeFakeHistoryEntry("exit 0") e1.ExitCode = 0 + require.NoError(t, db.Create(e1).Error) manuallySubmitHistoryEntry(t, userSecret, e1) e2 := testutils.MakeFakeHistoryEntry("exit 1") e2.ExitCode = 1 + require.NoError(t, db.Create(e2).Error) manuallySubmitHistoryEntry(t, userSecret, e2) // Configure a default filter From d646939c0063f54c03cc5e1fb243190e5a1be147 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 19:17:23 -0800 Subject: [PATCH 03/36] Add rudimentary experiment of splitting tests into two chunks to make them faster --- .github/workflows/go-test.yml | 3 +- client/client_test.go | 56 +++++++++++++++++++++++++++++++++++ client/fuzz_test.go | 5 ++++ client/testutils.go | 14 +++++++++ 4 files changed, 77 insertions(+), 1 deletion(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index b0950bfd..d50eeb51 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -14,6 +14,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, macos-latest, macos-14] + tests: [BASIC, TUI] fail-fast: false steps: - uses: actions/checkout@v4 @@ -69,7 +70,7 @@ jobs: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} run: | go install gotest.tools/gotestsum@bc98120 - make test + SPLIT_TESTS=${{ matrix.tests }} make test - name: Extra Delay run: | diff --git a/client/client_test.go b/client/client_test.go index 7aec368f..1a677268 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -121,6 +121,7 @@ func TestParam(t *testing.T) { func testIntegration(t *testing.T, tester shellTester, onlineStatus OnlineStatus) { // Set up + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Run the test @@ -129,6 +130,7 @@ func testIntegration(t *testing.T, tester shellTester, onlineStatus OnlineStatus func testIntegrationWithNewDevice(t *testing.T, tester shellTester) { // Set up + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Run the test @@ -238,6 +240,7 @@ func installWithOnlineStatus(t testing.TB, tester shellTester, onlineStatus Onli func testBasicUserFlow(t *testing.T, tester shellTester, onlineStatus OnlineStatus) string { // Test install + tagAsBasicTest(t) userSecret := installWithOnlineStatus(t, tester, onlineStatus) assertOnlineStatus(t, onlineStatus) @@ -338,6 +341,7 @@ echo thisisrecorded`) func testAdvancedQuery(t *testing.T, tester shellTester) { // Set up + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Install hishtory @@ -561,18 +565,22 @@ func updateToHead(t *testing.T, tester shellTester) string { } func testUpdateFromHeadToRelease(t *testing.T, tester shellTester) { + tagAsBasicTest(t) testGenericUpdate(t, tester, installFromHead, updateToRelease) } func testUpdateFromPrevToRelease(t *testing.T, tester shellTester) { + tagAsBasicTest(t) testGenericUpdate(t, tester, installFromPrev, updateToRelease) } func testUpdateFromPrevToCurrent(t *testing.T, tester shellTester) { + tagAsBasicTest(t) testGenericUpdate(t, tester, installFromPrev, updateToHead) } func testUpdateFromPrevToReleaseViaProd(t *testing.T, tester shellTester) { + tagAsBasicTest(t) defer testutils.BackupAndRestoreEnv("HISHTORY_SERVER")() os.Setenv("HISHTORY_SERVER", "https://api.hishtory.dev") testGenericUpdate(t, tester, installFromPrev, updateToRelease) @@ -622,6 +630,7 @@ func testGenericUpdate(t *testing.T, tester shellTester, installInitialVersion f func testRepeatedCommandThenQuery(t *testing.T, tester shellTester) { // Set up + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -662,6 +671,7 @@ echo mycommand-3`) func testRepeatedCommandAndQuery(t *testing.T, tester shellTester) { // Set up + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -687,6 +697,7 @@ func testRepeatedCommandAndQuery(t *testing.T, tester shellTester) { func testRepeatedEnableDisable(t *testing.T, tester shellTester) { // Set up + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -718,6 +729,7 @@ hishtory enable`, i)) func testExcludeHiddenCommand(t *testing.T, tester shellTester) { // Set up + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -775,6 +787,7 @@ func waitForBackgroundSavesToComplete(t testing.TB) { func testTimestampsAreReasonablyCorrect(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -795,6 +808,7 @@ func testTimestampsAreReasonablyCorrect(t *testing.T, tester shellTester) { func testTableDisplayCwd(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -822,6 +836,7 @@ echo other`) func testHishtoryBackgroundSaving(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Check that we can find the go binary @@ -878,6 +893,7 @@ echo foo`) func testDisplayTable(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -940,6 +956,7 @@ func testDisplayTable(t *testing.T, tester shellTester) { func testRequestAndReceiveDbDump(t *testing.T, tester shellTester) { // Set up + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() secretKey := installHishtory(t, tester, "") @@ -1038,6 +1055,7 @@ echo other`) } func TestInstallViaPythonScriptWithCustomHishtoryPath(t *testing.T) { + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("HISHTORY_PATH")() altHishtoryPath := ".other-path" @@ -1052,6 +1070,7 @@ func TestInstallViaPythonScriptWithCustomHishtoryPath(t *testing.T) { } func TestInstallViaPythonScriptInOfflineMode(t *testing.T) { + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("HISHTORY_OFFLINE")() os.Setenv("HISHTORY_OFFLINE", "1") @@ -1108,6 +1127,7 @@ func testInstallViaPythonScriptChild(t *testing.T, tester shellTester) { } func TestInstallViaPythonScriptFromHead(t *testing.T) { + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() tester := zshTester{} @@ -1145,6 +1165,7 @@ func TestInstallViaPythonScriptFromHead(t *testing.T) { func testExportWithQuery(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -1203,6 +1224,7 @@ sleep 1`) func testHelpCommand(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -1219,6 +1241,7 @@ func testHelpCommand(t *testing.T, tester shellTester) { func TestStripBashTimePrefix(t *testing.T) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() tester := bashTester{} installHishtory(t, tester, "") @@ -1264,6 +1287,7 @@ func TestStripBashTimePrefix(t *testing.T) { func testReuploadHistoryEntries(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Init an initial device @@ -1314,6 +1338,7 @@ func testReuploadHistoryEntries(t *testing.T, tester shellTester) { func testHishtoryOffline(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Init an initial device @@ -1365,6 +1390,7 @@ func testHishtoryOffline(t *testing.T, tester shellTester) { func testInitialHistoryImport(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("HISHTORY_SKIP_INIT_IMPORT")() os.Setenv("HISHTORY_SKIP_INIT_IMPORT", "") @@ -1399,6 +1425,7 @@ hishtory export `+randomCmdUuid[:5]+` func testLocalRedaction(t *testing.T, tester shellTester, onlineStatus OnlineStatus) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installWithOnlineStatus(t, tester, onlineStatus) assertOnlineStatus(t, onlineStatus) @@ -1467,6 +1494,7 @@ ls /tmp`, randomCmdUuid, randomCmdUuid) func testRemoteRedaction(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Install hishtory client 1 @@ -1522,6 +1550,7 @@ ls /tmp`, randomCmdUuid, randomCmdUuid) func testConfigGetSet(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -1580,6 +1609,7 @@ func clearControlRSearchFromConfig(t testing.TB) { func testHandleUpgradedFeatures(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -1611,6 +1641,7 @@ func testHandleUpgradedFeatures(t *testing.T, tester shellTester) { func TestFish(t *testing.T) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() tester := bashTester{} installHishtory(t, tester, "") @@ -1686,6 +1717,7 @@ func setupTestTui(t testing.TB, onlineStatus OnlineStatus) (shellTester, string, func testTui_resize(t *testing.T) { // Setup + tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, userSecret, _ := setupTestTui(t, Online) @@ -1753,6 +1785,7 @@ func testTui_resize(t *testing.T) { func testTui_scroll(t *testing.T) { // Setup + tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, userSecret, _ := setupTestTui(t, Online) @@ -1791,6 +1824,7 @@ func testTui_scroll(t *testing.T) { func testTui_defaultFilter(t *testing.T) { // Setup + tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, userSecret, _ := setupTestTui(t, Online) db := hctx.GetDb(hctx.MakeContext()) @@ -1844,6 +1878,7 @@ func testTui_color(t *testing.T) { } // Setup + tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, _, _ := setupTestTui(t, Online) tester.RunInteractiveShell(t, ` hishtory config-set highlight-matches false`) @@ -1886,6 +1921,7 @@ func testTui_color(t *testing.T) { func testTui_delete(t *testing.T) { // Setup + tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, userSecret, _ := setupTestTui(t, Online) manuallySubmitHistoryEntry(t, userSecret, testutils.MakeFakeHistoryEntry("echo 'cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc'")) @@ -1928,6 +1964,7 @@ func testTui_delete(t *testing.T) { func testTui_search(t *testing.T, onlineStatus OnlineStatus) { // Setup + tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, _, _ := setupTestTui(t, onlineStatus) @@ -2040,6 +2077,7 @@ func testTui_search(t *testing.T, onlineStatus OnlineStatus) { func testTui_general(t *testing.T, onlineStatus OnlineStatus) { // Setup + tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, _, _ := setupTestTui(t, onlineStatus) @@ -2097,6 +2135,7 @@ func testTui_general(t *testing.T, onlineStatus OnlineStatus) { func testTui_errors(t *testing.T) { // Setup + tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, _, _ := setupTestTui(t, Online) @@ -2117,6 +2156,7 @@ func testTui_errors(t *testing.T) { func testTui_ai(t *testing.T) { // Setup + tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("OPENAI_API_KEY")() os.Setenv("OPENAI_API_KEY", "") @@ -2151,6 +2191,7 @@ func testTui_ai(t *testing.T) { func testControlR(t *testing.T, tester shellTester, shellName string, onlineStatus OnlineStatus) { // Setup + tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() installWithOnlineStatus(t, tester, onlineStatus) assertOnlineStatus(t, onlineStatus) @@ -2313,6 +2354,7 @@ func testControlR(t *testing.T, tester shellTester, shellName string, onlineStat func testCustomColumns(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2361,6 +2403,7 @@ echo bar`) func testPresavingDisabled(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2385,6 +2428,7 @@ func testPresavingDisabled(t *testing.T, tester shellTester) { func testPresavingOffline(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("HISHTORY_SIMULATE_NETWORK_ERROR")() userSecret := installHishtory(t, tester, "") @@ -2427,6 +2471,7 @@ func testPresavingOffline(t *testing.T, tester shellTester) { func testPresaving(t *testing.T, tester shellTester, shellName string) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") manuallySubmitHistoryEntry(t, userSecret, testutils.MakeFakeHistoryEntry("table_sizing aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")) @@ -2515,6 +2560,7 @@ func testTabCompletion(t *testing.T, tester shellTester, shellName string) { } // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2536,6 +2582,7 @@ func testTabCompletion(t *testing.T, tester shellTester, shellName string) { func testUninstall(t *testing.T, tester shellTester) { // Setup + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2570,6 +2617,7 @@ echo bar`) func TestTimestampFormat(t *testing.T) { // Setup + tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -2607,6 +2655,7 @@ func TestTimestampFormat(t *testing.T) { func TestSortByConsistentTimezone(t *testing.T) { // Setup + tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2646,6 +2695,7 @@ func TestSortByConsistentTimezone(t *testing.T) { func TestZDotDir(t *testing.T) { // Setup + tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("ZDOTDIR")() @@ -2680,6 +2730,7 @@ func TestZDotDir(t *testing.T) { func TestRemoveDuplicateRows(t *testing.T) { // Setup + tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2732,6 +2783,7 @@ echo foo`) func TestSetConfigNoCorruption(t *testing.T) { // Setup + tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2766,6 +2818,7 @@ func TestSetConfigNoCorruption(t *testing.T) { // Test that the config retrieved from the context is a reference and there are no consistency issues with it getting out of sync func TestCtxConfigIsReference(t *testing.T) { // Setup + tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2791,6 +2844,7 @@ func TestCtxConfigIsReference(t *testing.T) { } func testMultipleUsers(t *testing.T, tester shellTester) { + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Create all our devices @@ -2877,6 +2931,7 @@ func createSyntheticImportEntries(t testing.TB, numSyntheticEntries int) { func TestImportHistory(t *testing.T) { // Setup + tagAsBasicTest(t) tester := bashTester{} defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -2931,6 +2986,7 @@ func BenchmarkImport(b *testing.B) { } func TestAugmentedIsOfflineError(t *testing.T) { + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, zshTester{}, "") defer testutils.BackupAndRestoreEnv("HISHTORY_SIMULATE_NETWORK_ERROR")() diff --git a/client/fuzz_test.go b/client/fuzz_test.go index 3cb01b0f..5af928e1 100644 --- a/client/fuzz_test.go +++ b/client/fuzz_test.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "os" "regexp" "strconv" "strings" @@ -141,6 +142,10 @@ func FuzzTestMultipleUsers(f *testing.F) { if skipSlowTests() { f.Skip("skipping slow tests") } + s := os.Getenv("SPLIT_TESTS") + if s != "" && s != "BASIC" { + f.Skip() + } defer testutils.RunTestServer()() // Format: // $Op = $Key;$Device|$Command\n diff --git a/client/testutils.go b/client/testutils.go index be304352..30b518c2 100644 --- a/client/testutils.go +++ b/client/testutils.go @@ -366,3 +366,17 @@ func stripRequiredPrefix(t *testing.T, out, prefix string) string { func stripTuiCommandPrefix(t *testing.T, out string) string { return stripRequiredPrefix(t, out, "hishtory tquery") } + +func tagAsBasicTest(t *testing.T) { + s := os.Getenv("SPLIT_TESTS") + if s != "" && s != "BASIC" { + t.Skip() + } +} + +func tagAsTuiTest(t *testing.T) { + s := os.Getenv("SPLIT_TESTS") + if s != "" && s != "TUI" { + t.Skip() + } +} From ea7c7148730947086210d542dd2e4b0b43f267aa Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 19:23:33 -0800 Subject: [PATCH 04/36] Add missing tag --- client/client_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/client/client_test.go b/client/client_test.go index 1a677268..dbb5c7e1 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1085,6 +1085,7 @@ func TestInstallViaPythonScriptInOfflineMode(t *testing.T) { } func testInstallViaPythonScript(t *testing.T, tester shellTester) { + tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() testInstallViaPythonScriptChild(t, tester) From 06cc3eedbc5be6a09dbc3f274adcacd875eb25a8 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 20:19:30 -0800 Subject: [PATCH 05/36] Remove code that enforces that all goldens are used, since it is incompatible with how tests are currently split into chunks --- client/posttest/main.go | 73 ----------------------------------------- 1 file changed, 73 deletions(-) diff --git a/client/posttest/main.go b/client/posttest/main.go index 93c7b935..819ef640 100644 --- a/client/posttest/main.go +++ b/client/posttest/main.go @@ -2,14 +2,10 @@ package main import ( - "bufio" "fmt" "log" "os" - "path" "runtime" - "slices" - "strings" "github.com/DataDog/datadog-go/statsd" "gotest.tools/gotestsum/testjson" @@ -19,77 +15,8 @@ var GLOBAL_STATSD *statsd.Client = nil var NUM_TEST_RETRIES map[string]int -var UNUSED_GOLDENS []string = []string{"TestTui-Exit", "testControlR-ControlC-bash", "testControlR-ControlC-fish", - "testControlR-ControlC-zsh", "testControlR-SelectMultiline-bash", "testControlR-SelectMultiline-fish", - "testControlR-SelectMultiline-zsh", "testControlR-bash-Disabled", "testControlR-fish-Disabled", - "testControlR-zsh-Disabled", "testCustomColumns-query-isAction=false", "testCustomColumns-tquery-bash", - "testCustomColumns-tquery-zsh", "testUninstall-post-uninstall-bash", - "testUninstall-post-uninstall-zsh", "TestTui-ColoredOutput", - "TestTui-ColoredOutputWithCustomColorScheme", "TestTui-ColoredOutputWithSearch", "TestTui-ColoredOutputWithSearch-Highlight", - "TestTui-DefaultColorScheme", "TestTui-ColoredOutputWithDefaultFilter"} - func main() { exportMetrics() - checkGoldensUsed() -} - -func checkGoldensUsed() { - if os.Getenv("HISHTORY_FILTERED_TEST") != "" { - return - } - // Read the goldens that were used - usedGoldens := make([]string, 0) - usedGoldensFile, err := os.Open("/tmp/goldens-used.txt") - if err != nil { - log.Fatalf("failed to open /tmp/goldens-used.txt: %v", err) - } - defer usedGoldensFile.Close() - scanner := bufio.NewScanner(usedGoldensFile) - for scanner.Scan() { - usedGoldens = append(usedGoldens, strings.TrimSpace(scanner.Text())) - } - if err := scanner.Err(); err != nil { - log.Fatalf("failed to read lines from /tmp/goldens-used.txt: %v", err) - } - - // List all the goldens that exist - goldensDir := "client/testdata/" - files, err := os.ReadDir(goldensDir) - if err != nil { - panic(fmt.Errorf("failed to list files in %s: %w", goldensDir, err)) - } - - // And check for mismatches - var unusedGoldenErr error = nil - for _, f := range files { - goldenName := path.Base(f.Name()) - if !slices.Contains(usedGoldens, goldenName) { - if slices.Contains(UNUSED_GOLDENS, goldenName) { - // It is allowlisted to not be used - continue - } - if (runtime.GOOS == "darwin" && strings.Contains(goldenName, "-linux")) || - (runtime.GOOS == "linux" && strings.Contains(goldenName, "-darwin")) { - // It is for another OS - continue - } - unusedGoldenErr = fmt.Errorf("golden file %v was never used", goldenName) - fmt.Println(unusedGoldenErr) - } - } - if unusedGoldenErr != nil { - log.Fatalf("%v", unusedGoldenErr) - } - - // And print out anything that is in UNUSED_GOLDENS that was actually used, so we - // can manually trim UNUSED_GOLDENS - for _, g := range UNUSED_GOLDENS { - if slices.Contains(usedGoldens, g) { - fmt.Printf("Golden %s is in UNUSED_GOLDENS, but was actually used\n", g) - } - } - fmt.Println("Validated that all goldens in testdata/ were referenced!") - } func exportMetrics() { From 5a6be563c560361e265d38b90051dc88bf410513 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 20:26:26 -0800 Subject: [PATCH 06/36] Lay out the framework for checking goldens being used across all test runs --- .github/workflows/go-test.yml | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index d50eeb51..52ec6da9 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -80,23 +80,49 @@ jobs: uses: actions/upload-artifact@v3 if: success() || failure() with: - name: test-results-${{ matrix.os }}.json + name: test-results-${{ matrix.os }}-${{ matrix.tests }}.json path: /tmp/testrun.json - name: Upload failed test goldens uses: actions/upload-artifact@v3 if: success() || failure() with: - name: test-goldens-${{ matrix.os }}.zip + name: test-goldens-${{ matrix.os }}-${{ matrix.tests }}.zip path: /tmp/test-goldens/ - name: Upload test log uses: actions/upload-artifact@v3 if: success() || failure() with: - name: testlog-${{ matrix.os }}.txt + name: testlog-${{ matrix.os }}-${{ matrix.tests }}.txt path: /tmp/test.log + - name: Upload test log + uses: actions/upload-artifact@v3 + if: success() || failure() + with: + name: goldens-used-${{ matrix.os }-${{ matrix.tests }}.txt + path: /tmp/goldens-used.txt # - name: Setup tmate session # if: ${{ failure() }} # uses: mxschmitt/action-tmate@v3 # with: - # limit-access-to-actor: true \ No newline at end of file + # limit-access-to-actor: true + check-goldens: + runs-on: ubuntu-latest + strategy: + matrix: + os: [ubuntu-latest, macos-latest, macos-14] + fail-fast: false + steps: + - uses: actions/checkout@v4 + - name: Set up Go + uses: actions/setup-go@v3 + with: + go-version: 1.21 + - name: Download all artifacts + uses: actions/download-artifact@v4 + - name: Check all goldens were used + run: | + ls . + echo --- + ls * + # TODO: Actually do this check that was reverted in 06cc3eedbc5be6a09dbc3f274adcacd875eb25a8 From 25ecebef64f9ec59e7ef635b36028fe4ed990c47 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 20:35:40 -0800 Subject: [PATCH 07/36] Fix missing brace --- .github/workflows/go-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 52ec6da9..063d1242 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -98,7 +98,7 @@ jobs: uses: actions/upload-artifact@v3 if: success() || failure() with: - name: goldens-used-${{ matrix.os }-${{ matrix.tests }}.txt + name: goldens-used-${{ matrix.os }}-${{ matrix.tests }}.txt path: /tmp/goldens-used.txt # - name: Setup tmate session From 1deb3cffc6651859763cddfa379711d934f04782 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 20:26:46 -0800 Subject: [PATCH 08/36] Revert "Remove code that enforces that all goldens are used, since it is incompatible with how tests are currently split into chunks" This reverts commit 06cc3eedbc5be6a09dbc3f274adcacd875eb25a8. --- client/posttest/main.go | 73 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/client/posttest/main.go b/client/posttest/main.go index 819ef640..93c7b935 100644 --- a/client/posttest/main.go +++ b/client/posttest/main.go @@ -2,10 +2,14 @@ package main import ( + "bufio" "fmt" "log" "os" + "path" "runtime" + "slices" + "strings" "github.com/DataDog/datadog-go/statsd" "gotest.tools/gotestsum/testjson" @@ -15,8 +19,77 @@ var GLOBAL_STATSD *statsd.Client = nil var NUM_TEST_RETRIES map[string]int +var UNUSED_GOLDENS []string = []string{"TestTui-Exit", "testControlR-ControlC-bash", "testControlR-ControlC-fish", + "testControlR-ControlC-zsh", "testControlR-SelectMultiline-bash", "testControlR-SelectMultiline-fish", + "testControlR-SelectMultiline-zsh", "testControlR-bash-Disabled", "testControlR-fish-Disabled", + "testControlR-zsh-Disabled", "testCustomColumns-query-isAction=false", "testCustomColumns-tquery-bash", + "testCustomColumns-tquery-zsh", "testUninstall-post-uninstall-bash", + "testUninstall-post-uninstall-zsh", "TestTui-ColoredOutput", + "TestTui-ColoredOutputWithCustomColorScheme", "TestTui-ColoredOutputWithSearch", "TestTui-ColoredOutputWithSearch-Highlight", + "TestTui-DefaultColorScheme", "TestTui-ColoredOutputWithDefaultFilter"} + func main() { exportMetrics() + checkGoldensUsed() +} + +func checkGoldensUsed() { + if os.Getenv("HISHTORY_FILTERED_TEST") != "" { + return + } + // Read the goldens that were used + usedGoldens := make([]string, 0) + usedGoldensFile, err := os.Open("/tmp/goldens-used.txt") + if err != nil { + log.Fatalf("failed to open /tmp/goldens-used.txt: %v", err) + } + defer usedGoldensFile.Close() + scanner := bufio.NewScanner(usedGoldensFile) + for scanner.Scan() { + usedGoldens = append(usedGoldens, strings.TrimSpace(scanner.Text())) + } + if err := scanner.Err(); err != nil { + log.Fatalf("failed to read lines from /tmp/goldens-used.txt: %v", err) + } + + // List all the goldens that exist + goldensDir := "client/testdata/" + files, err := os.ReadDir(goldensDir) + if err != nil { + panic(fmt.Errorf("failed to list files in %s: %w", goldensDir, err)) + } + + // And check for mismatches + var unusedGoldenErr error = nil + for _, f := range files { + goldenName := path.Base(f.Name()) + if !slices.Contains(usedGoldens, goldenName) { + if slices.Contains(UNUSED_GOLDENS, goldenName) { + // It is allowlisted to not be used + continue + } + if (runtime.GOOS == "darwin" && strings.Contains(goldenName, "-linux")) || + (runtime.GOOS == "linux" && strings.Contains(goldenName, "-darwin")) { + // It is for another OS + continue + } + unusedGoldenErr = fmt.Errorf("golden file %v was never used", goldenName) + fmt.Println(unusedGoldenErr) + } + } + if unusedGoldenErr != nil { + log.Fatalf("%v", unusedGoldenErr) + } + + // And print out anything that is in UNUSED_GOLDENS that was actually used, so we + // can manually trim UNUSED_GOLDENS + for _, g := range UNUSED_GOLDENS { + if slices.Contains(usedGoldens, g) { + fmt.Printf("Golden %s is in UNUSED_GOLDENS, but was actually used\n", g) + } + } + fmt.Println("Validated that all goldens in testdata/ were referenced!") + } func exportMetrics() { From d8b9d77213ced61abf75529f0f4b6050391eee16 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 21:03:03 -0800 Subject: [PATCH 09/36] Add initial work towards checking that all goldens are used --- .github/workflows/go-test.yml | 2 ++ Makefile | 2 +- client/posttest/main.go | 8 ++++++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 063d1242..8cea3827 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -112,6 +112,7 @@ jobs: matrix: os: [ubuntu-latest, macos-latest, macos-14] fail-fast: false + needs: test steps: - uses: actions/checkout@v4 - name: Set up Go @@ -126,3 +127,4 @@ jobs: echo --- ls * # TODO: Actually do this check that was reverted in 06cc3eedbc5be6a09dbc3f274adcacd875eb25a8 + go run client/posttest/main.go check-goldens diff --git a/Makefile b/Makefile index 6b51d769..d78f18b0 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ forcetest: ## Force running all tests without a test cache make test test: ## Run all tests - TZ='America/Los_Angeles' HISHTORY_TEST=1 HISHTORY_SKIP_INIT_IMPORT=1 gotestsum --packages ./... --rerun-fails=10 --rerun-fails-max-failures=30 --format testname --jsonfile /tmp/testrun.json --post-run-command "go run client/posttest/main.go" -- -p 1 -timeout 90m + TZ='America/Los_Angeles' HISHTORY_TEST=1 HISHTORY_SKIP_INIT_IMPORT=1 gotestsum --packages ./... --rerun-fails=10 --rerun-fails-max-failures=30 --format testname --jsonfile /tmp/testrun.json --post-run-command "go run client/posttest/main.go export" -- -p 1 -timeout 90m ftest: ## Run a specific test specified via `make ftest FILTER=TestParam/testTui/color` go clean -testcache diff --git a/client/posttest/main.go b/client/posttest/main.go index 93c7b935..d9027be6 100644 --- a/client/posttest/main.go +++ b/client/posttest/main.go @@ -29,8 +29,12 @@ var UNUSED_GOLDENS []string = []string{"TestTui-Exit", "testControlR-ControlC-ba "TestTui-DefaultColorScheme", "TestTui-ColoredOutputWithDefaultFilter"} func main() { - exportMetrics() - checkGoldensUsed() + if os.Args[1] == "export" { + exportMetrics() + } + if os.Args[1] == "check-goldens" { + checkGoldensUsed() + } } func checkGoldensUsed() { From f3710b11de91cdcb01f5c15fb1d017ac08bb785c Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 21:23:05 -0800 Subject: [PATCH 10/36] Delete incorrect and unreferenced matrix --- .github/workflows/go-test.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 8cea3827..d990c99f 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -108,10 +108,6 @@ jobs: # limit-access-to-actor: true check-goldens: runs-on: ubuntu-latest - strategy: - matrix: - os: [ubuntu-latest, macos-latest, macos-14] - fail-fast: false needs: test steps: - uses: actions/checkout@v4 From 8b1d117543871bc4859e10f84195c1b89e6a6338 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 23:26:00 -0800 Subject: [PATCH 11/36] Upgrade actions/upload-artifact to see if that makes the download in the next job work --- .github/workflows/go-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index d990c99f..b1267970 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -95,7 +95,7 @@ jobs: name: testlog-${{ matrix.os }}-${{ matrix.tests }}.txt path: /tmp/test.log - name: Upload test log - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 if: success() || failure() with: name: goldens-used-${{ matrix.os }}-${{ matrix.tests }}.txt From 168d738b2de369e51a23fe114ce41552e6cd59f1 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 9 Feb 2024 23:27:09 -0800 Subject: [PATCH 12/36] Alternatively, try downloading the artifact by name --- .github/workflows/go-test.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index b1267970..4dba355d 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -115,8 +115,10 @@ jobs: uses: actions/setup-go@v3 with: go-version: 1.21 - - name: Download all artifacts + - name: Download artifact uses: actions/download-artifact@v4 + with: + name: goldens-used-ubuntu-TUI.txt - name: Check all goldens were used run: | ls . From f4694bdd28168c7d905f7903c0b3cab8cbb9b367 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 10:05:06 -0800 Subject: [PATCH 13/36] Update golden checker to read all the golden artifacts --- .github/workflows/go-test.yml | 4 +--- client/posttest/main.go | 32 +++++++++++++++++--------------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 4dba355d..178162cc 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -98,7 +98,7 @@ jobs: uses: actions/upload-artifact@v4 if: success() || failure() with: - name: goldens-used-${{ matrix.os }}-${{ matrix.tests }}.txt + name: goldens-used-${{ matrix.os }}-${{ matrix.tests }} path: /tmp/goldens-used.txt # - name: Setup tmate session @@ -117,8 +117,6 @@ jobs: go-version: 1.21 - name: Download artifact uses: actions/download-artifact@v4 - with: - name: goldens-used-ubuntu-TUI.txt - name: Check all goldens were used run: | ls . diff --git a/client/posttest/main.go b/client/posttest/main.go index d9027be6..63e89304 100644 --- a/client/posttest/main.go +++ b/client/posttest/main.go @@ -43,17 +43,24 @@ func checkGoldensUsed() { } // Read the goldens that were used usedGoldens := make([]string, 0) - usedGoldensFile, err := os.Open("/tmp/goldens-used.txt") - if err != nil { - log.Fatalf("failed to open /tmp/goldens-used.txt: %v", err) - } - defer usedGoldensFile.Close() - scanner := bufio.NewScanner(usedGoldensFile) - for scanner.Scan() { - usedGoldens = append(usedGoldens, strings.TrimSpace(scanner.Text())) + filenames := []string{ + "goldens-used-macos-14-BASIC/goldens-used.txt", "goldens-used-macos-14-TUI/goldens-used.txt", + "goldens-used-macos-latest-BASIC/goldens-used.txt", "goldens-used-macos-latest-TUI/goldens-used.txt", + "goldens-used-ubuntu-latest-BASIC/goldens-used.txt", "goldens-used-ubuntu-latest-TUI/goldens-used.txt", } - if err := scanner.Err(); err != nil { - log.Fatalf("failed to read lines from /tmp/goldens-used.txt: %v", err) + for _, filename := range filenames { + usedGoldensFile, err := os.Open(filename) + if err != nil { + log.Fatalf("failed to open /tmp/goldens-used.txt: %v", err) + } + defer usedGoldensFile.Close() + scanner := bufio.NewScanner(usedGoldensFile) + for scanner.Scan() { + usedGoldens = append(usedGoldens, strings.TrimSpace(scanner.Text())) + } + if err := scanner.Err(); err != nil { + log.Fatalf("failed to read lines from /tmp/goldens-used.txt: %v", err) + } } // List all the goldens that exist @@ -72,11 +79,6 @@ func checkGoldensUsed() { // It is allowlisted to not be used continue } - if (runtime.GOOS == "darwin" && strings.Contains(goldenName, "-linux")) || - (runtime.GOOS == "linux" && strings.Contains(goldenName, "-darwin")) { - // It is for another OS - continue - } unusedGoldenErr = fmt.Errorf("golden file %v was never used", goldenName) fmt.Println(unusedGoldenErr) } From ed22ae6e4f37a83b96e931096ce0998a269390ed Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 10:07:37 -0800 Subject: [PATCH 14/36] Swap to using glob to enumerate all golden files, rather than hardcoding them --- client/posttest/main.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/client/posttest/main.go b/client/posttest/main.go index 63e89304..48c65025 100644 --- a/client/posttest/main.go +++ b/client/posttest/main.go @@ -7,6 +7,7 @@ import ( "log" "os" "path" + "path/filepath" "runtime" "slices" "strings" @@ -43,15 +44,15 @@ func checkGoldensUsed() { } // Read the goldens that were used usedGoldens := make([]string, 0) - filenames := []string{ - "goldens-used-macos-14-BASIC/goldens-used.txt", "goldens-used-macos-14-TUI/goldens-used.txt", - "goldens-used-macos-latest-BASIC/goldens-used.txt", "goldens-used-macos-latest-TUI/goldens-used.txt", - "goldens-used-ubuntu-latest-BASIC/goldens-used.txt", "goldens-used-ubuntu-latest-TUI/goldens-used.txt", + filenames, err := filepath.Glob("*/goldens-used.txt") + if err != nil { + log.Fatalf("failed to list golden files: %v", err) } + fmt.Printf("Found used goldens in %#v\n", filenames) for _, filename := range filenames { usedGoldensFile, err := os.Open(filename) if err != nil { - log.Fatalf("failed to open /tmp/goldens-used.txt: %v", err) + log.Fatalf("failed to open %s: %v", filename, err) } defer usedGoldensFile.Close() scanner := bufio.NewScanner(usedGoldensFile) From 4efead762b7e880b8a39eca90b28254e559f4f51 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 10:09:59 -0800 Subject: [PATCH 15/36] Remove debugging commands --- .github/workflows/go-test.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 178162cc..43d33e4b 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -119,8 +119,4 @@ jobs: uses: actions/download-artifact@v4 - name: Check all goldens were used run: | - ls . - echo --- - ls * - # TODO: Actually do this check that was reverted in 06cc3eedbc5be6a09dbc3f274adcacd875eb25a8 go run client/posttest/main.go check-goldens From f0ce593ae993811f7db03f257c243a603263470b Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 13:01:30 -0800 Subject: [PATCH 16/36] Remove goldens that are actually used --- client/posttest/main.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/client/posttest/main.go b/client/posttest/main.go index 48c65025..fce6ab95 100644 --- a/client/posttest/main.go +++ b/client/posttest/main.go @@ -25,9 +25,7 @@ var UNUSED_GOLDENS []string = []string{"TestTui-Exit", "testControlR-ControlC-ba "testControlR-SelectMultiline-zsh", "testControlR-bash-Disabled", "testControlR-fish-Disabled", "testControlR-zsh-Disabled", "testCustomColumns-query-isAction=false", "testCustomColumns-tquery-bash", "testCustomColumns-tquery-zsh", "testUninstall-post-uninstall-bash", - "testUninstall-post-uninstall-zsh", "TestTui-ColoredOutput", - "TestTui-ColoredOutputWithCustomColorScheme", "TestTui-ColoredOutputWithSearch", "TestTui-ColoredOutputWithSearch-Highlight", - "TestTui-DefaultColorScheme", "TestTui-ColoredOutputWithDefaultFilter"} + "testUninstall-post-uninstall-zsh", "TestTui-ColoredOutputWithDefaultFilter"} func main() { if os.Args[1] == "export" { From 93d2b47f7d40ce815a8aa1c50dc859f7d54117bb Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 14:37:49 -0800 Subject: [PATCH 17/36] Remove another golden that is actually used --- client/posttest/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/posttest/main.go b/client/posttest/main.go index fce6ab95..e13ecaf1 100644 --- a/client/posttest/main.go +++ b/client/posttest/main.go @@ -25,7 +25,7 @@ var UNUSED_GOLDENS []string = []string{"TestTui-Exit", "testControlR-ControlC-ba "testControlR-SelectMultiline-zsh", "testControlR-bash-Disabled", "testControlR-fish-Disabled", "testControlR-zsh-Disabled", "testCustomColumns-query-isAction=false", "testCustomColumns-tquery-bash", "testCustomColumns-tquery-zsh", "testUninstall-post-uninstall-bash", - "testUninstall-post-uninstall-zsh", "TestTui-ColoredOutputWithDefaultFilter"} + "testUninstall-post-uninstall-zsh"} func main() { if os.Args[1] == "export" { From 8e7b078f039c8b6af3dcbb177b912f0c6e079fe1 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 16:44:53 -0800 Subject: [PATCH 18/36] Add more comprehensive support for test sharding --- .github/workflows/go-test.yml | 4 +- client/client_test.go | 187 ++++++++++++++-------------------- client/testutils.go | 14 --- 3 files changed, 80 insertions(+), 125 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 43d33e4b..827601b5 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -14,7 +14,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, macos-latest, macos-14] - tests: [BASIC, TUI] + test_shards: ["0", "1", "2", "3", "4"] fail-fast: false steps: - uses: actions/checkout@v4 @@ -70,7 +70,7 @@ jobs: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} run: | go install gotest.tools/gotestsum@bc98120 - SPLIT_TESTS=${{ matrix.tests }} make test + NUM_TEST_SHARDS=5 CURRENT_SHARD_NUM=${{ matrix.test_shards }} make test - name: Extra Delay run: | diff --git a/client/client_test.go b/client/client_test.go index dbb5c7e1..1efa5009 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -9,6 +9,7 @@ import ( "path" "regexp" "runtime" + "strconv" "strings" "sync" "testing" @@ -57,63 +58,88 @@ func TestMain(m *testing.M) { var shellTesters []shellTester = []shellTester{bashTester{}, zshTester{}} +var shardNumberAllocator int = 0 + +func markTestForSharding(t *testing.T, testShardNumber int) { + numTestShardsStr := os.Getenv("NUM_TEST_SHARDS") + currentShardNumberStr := os.Getenv("CURRENT_SHARD_NUM") + if numTestShardsStr != "" && currentShardNumberStr != "" { + numTestShards, err := strconv.Atoi(numTestShardsStr) + require.NoError(t, err) + currentShardNumber, err := strconv.Atoi(currentShardNumberStr) + require.NoError(t, err) + if testShardNumber%numTestShards != currentShardNumber { + t.Skip("Skipping sharded test") + } + } +} + +func wrapTestForSharding(test func(t *testing.T)) func(t *testing.T) { + shardNumberAllocator += 1 + return func(t *testing.T) { + testShardNumber := shardNumberAllocator + markTestForSharding(t, testShardNumber) + test(t) + } +} + func TestParam(t *testing.T) { if skipSlowTests() { shellTesters = shellTesters[:1] } for _, tester := range shellTesters { - t.Run("testRepeatedCommandThenQuery/"+tester.ShellName(), func(t *testing.T) { testRepeatedCommandThenQuery(t, tester) }) - t.Run("testRepeatedCommandAndQuery/"+tester.ShellName(), func(t *testing.T) { testRepeatedCommandAndQuery(t, tester) }) - t.Run("testRepeatedEnableDisable/"+tester.ShellName(), func(t *testing.T) { testRepeatedEnableDisable(t, tester) }) - t.Run("testExcludeHiddenCommand/"+tester.ShellName(), func(t *testing.T) { testExcludeHiddenCommand(t, tester) }) - t.Run("testUpdate/head->release/"+tester.ShellName(), func(t *testing.T) { testUpdateFromHeadToRelease(t, tester) }) - t.Run("testUpdate/prev->release/"+tester.ShellName(), func(t *testing.T) { testUpdateFromPrevToRelease(t, tester) }) - t.Run("testUpdate/prev->release/prod/"+tester.ShellName(), func(t *testing.T) { testUpdateFromPrevToReleaseViaProd(t, tester) }) - t.Run("testUpdate/prev->current/"+tester.ShellName(), func(t *testing.T) { testUpdateFromPrevToCurrent(t, tester) }) - t.Run("testAdvancedQuery/"+tester.ShellName(), func(t *testing.T) { testAdvancedQuery(t, tester) }) - t.Run("testIntegration/"+tester.ShellName(), func(t *testing.T) { testIntegration(t, tester, Online) }) - t.Run("testIntegration/offline/"+tester.ShellName(), func(t *testing.T) { testIntegration(t, tester, Offline) }) - t.Run("testIntegrationWithNewDevice/"+tester.ShellName(), func(t *testing.T) { testIntegrationWithNewDevice(t, tester) }) - t.Run("testHishtoryBackgroundSaving/"+tester.ShellName(), func(t *testing.T) { testHishtoryBackgroundSaving(t, tester) }) - t.Run("testDisplayTable/"+tester.ShellName(), func(t *testing.T) { testDisplayTable(t, tester) }) - t.Run("testTableDisplayCwd/"+tester.ShellName(), func(t *testing.T) { testTableDisplayCwd(t, tester) }) - t.Run("testTimestampsAreReasonablyCorrect/"+tester.ShellName(), func(t *testing.T) { testTimestampsAreReasonablyCorrect(t, tester) }) - t.Run("testRequestAndReceiveDbDump/"+tester.ShellName(), func(t *testing.T) { testRequestAndReceiveDbDump(t, tester) }) - t.Run("testInstallViaPythonScript/"+tester.ShellName(), func(t *testing.T) { testInstallViaPythonScript(t, tester) }) - t.Run("testExportWithQuery/"+tester.ShellName(), func(t *testing.T) { testExportWithQuery(t, tester) }) - t.Run("testHelpCommand/"+tester.ShellName(), func(t *testing.T) { testHelpCommand(t, tester) }) - t.Run("testReuploadHistoryEntries/"+tester.ShellName(), func(t *testing.T) { testReuploadHistoryEntries(t, tester) }) - t.Run("testHishtoryOffline/"+tester.ShellName(), func(t *testing.T) { testHishtoryOffline(t, tester) }) - t.Run("testInitialHistoryImport/"+tester.ShellName(), func(t *testing.T) { testInitialHistoryImport(t, tester) }) - t.Run("testLocalRedaction/"+tester.ShellName(), func(t *testing.T) { testLocalRedaction(t, tester, Online) }) - t.Run("testLocalRedaction/offline/"+tester.ShellName(), func(t *testing.T) { testLocalRedaction(t, tester, Offline) }) - t.Run("testRemoteRedaction/"+tester.ShellName(), func(t *testing.T) { testRemoteRedaction(t, tester) }) - t.Run("testMultipleUsers/"+tester.ShellName(), func(t *testing.T) { testMultipleUsers(t, tester) }) - t.Run("testConfigGetSet/"+tester.ShellName(), func(t *testing.T) { testConfigGetSet(t, tester) }) - t.Run("testHandleUpgradedFeatures/"+tester.ShellName(), func(t *testing.T) { testHandleUpgradedFeatures(t, tester) }) - 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, tester.ShellName()) }) - t.Run("testPresavingOffline/"+tester.ShellName(), func(t *testing.T) { testPresavingOffline(t, tester) }) - t.Run("testPresavingDisabled/"+tester.ShellName(), func(t *testing.T) { testPresavingDisabled(t, tester) }) - t.Run("testControlR/online/"+tester.ShellName(), func(t *testing.T) { testControlR(t, tester, tester.ShellName(), Online) }) - t.Run("testControlR/offline/"+tester.ShellName(), func(t *testing.T) { testControlR(t, tester, tester.ShellName(), Offline) }) - t.Run("testTabCompletion/"+tester.ShellName(), func(t *testing.T) { testTabCompletion(t, tester, tester.ShellName()) }) - } - t.Run("testTabCompletion/fish", func(t *testing.T) { testTabCompletion(t, zshTester{}, "fish") }) - t.Run("testPresaving/fish", func(t *testing.T) { testPresaving(t, zshTester{}, "fish") }) - t.Run("testControlR/fish", func(t *testing.T) { testControlR(t, bashTester{}, "fish", Online) }) - t.Run("testTui/search/online", func(t *testing.T) { testTui_search(t, Online) }) - t.Run("testTui/search/offline", func(t *testing.T) { testTui_search(t, Offline) }) - t.Run("testTui/general/online", func(t *testing.T) { testTui_general(t, Online) }) - t.Run("testTui/general/offline", func(t *testing.T) { testTui_general(t, Offline) }) - t.Run("testTui/scroll", testTui_scroll) - t.Run("testTui/resize", testTui_resize) - t.Run("testTui/delete", testTui_delete) - t.Run("testTui/color", testTui_color) - t.Run("testTui/errors", testTui_errors) - t.Run("testTui/ai", testTui_ai) - t.Run("testTui/defaultFilter", testTui_defaultFilter) + t.Run("testRepeatedCommandThenQuery/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testRepeatedCommandThenQuery(t, tester) })) + t.Run("testRepeatedCommandAndQuery/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testRepeatedCommandAndQuery(t, tester) })) + t.Run("testRepeatedEnableDisable/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testRepeatedEnableDisable(t, tester) })) + t.Run("testExcludeHiddenCommand/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testExcludeHiddenCommand(t, tester) })) + t.Run("testUpdate/head->release/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testUpdateFromHeadToRelease(t, tester) })) + t.Run("testUpdate/prev->release/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testUpdateFromPrevToRelease(t, tester) })) + t.Run("testUpdate/prev->release/prod/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testUpdateFromPrevToReleaseViaProd(t, tester) })) + t.Run("testUpdate/prev->current/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testUpdateFromPrevToCurrent(t, tester) })) + t.Run("testAdvancedQuery/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testAdvancedQuery(t, tester) })) + t.Run("testIntegration/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testIntegration(t, tester, Online) })) + t.Run("testIntegration/offline/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testIntegration(t, tester, Offline) })) + t.Run("testIntegrationWithNewDevice/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testIntegrationWithNewDevice(t, tester) })) + t.Run("testHishtoryBackgroundSaving/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testHishtoryBackgroundSaving(t, tester) })) + t.Run("testDisplayTable/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testDisplayTable(t, tester) })) + t.Run("testTableDisplayCwd/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testTableDisplayCwd(t, tester) })) + t.Run("testTimestampsAreReasonablyCorrect/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testTimestampsAreReasonablyCorrect(t, tester) })) + t.Run("testRequestAndReceiveDbDump/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testRequestAndReceiveDbDump(t, tester) })) + t.Run("testInstallViaPythonScript/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testInstallViaPythonScript(t, tester) })) + t.Run("testExportWithQuery/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testExportWithQuery(t, tester) })) + t.Run("testHelpCommand/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testHelpCommand(t, tester) })) + t.Run("testReuploadHistoryEntries/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testReuploadHistoryEntries(t, tester) })) + t.Run("testHishtoryOffline/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testHishtoryOffline(t, tester) })) + t.Run("testInitialHistoryImport/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testInitialHistoryImport(t, tester) })) + t.Run("testLocalRedaction/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testLocalRedaction(t, tester, Online) })) + t.Run("testLocalRedaction/offline/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testLocalRedaction(t, tester, Offline) })) + t.Run("testRemoteRedaction/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testRemoteRedaction(t, tester) })) + t.Run("testMultipleUsers/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testMultipleUsers(t, tester) })) + t.Run("testConfigGetSet/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testConfigGetSet(t, tester) })) + t.Run("testHandleUpgradedFeatures/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testHandleUpgradedFeatures(t, tester) })) + t.Run("testCustomColumns/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testCustomColumns(t, tester) })) + t.Run("testUninstall/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testUninstall(t, tester) })) + t.Run("testPresaving/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testPresaving(t, tester, tester.ShellName()) })) + t.Run("testPresavingOffline/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testPresavingOffline(t, tester) })) + t.Run("testPresavingDisabled/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testPresavingDisabled(t, tester) })) + t.Run("testControlR/online/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testControlR(t, tester, tester.ShellName(), Online) })) + t.Run("testControlR/offline/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testControlR(t, tester, tester.ShellName(), Offline) })) + t.Run("testTabCompletion/"+tester.ShellName(), wrapTestForSharding(func(t *testing.T) { testTabCompletion(t, tester, tester.ShellName()) })) + } + t.Run("testTabCompletion/fish", wrapTestForSharding(func(t *testing.T) { testTabCompletion(t, zshTester{}, "fish") })) + t.Run("testPresaving/fish", wrapTestForSharding(func(t *testing.T) { testPresaving(t, zshTester{}, "fish") })) + t.Run("testControlR/fish", wrapTestForSharding(func(t *testing.T) { testControlR(t, bashTester{}, "fish", Online) })) + t.Run("testTui/search/online", wrapTestForSharding(func(t *testing.T) { testTui_search(t, Online) })) + t.Run("testTui/search/offline", wrapTestForSharding(func(t *testing.T) { testTui_search(t, Offline) })) + t.Run("testTui/general/online", wrapTestForSharding(func(t *testing.T) { testTui_general(t, Online) })) + t.Run("testTui/general/offline", wrapTestForSharding(func(t *testing.T) { testTui_general(t, Offline) })) + t.Run("testTui/scroll", wrapTestForSharding(testTui_scroll)) + t.Run("testTui/resize", wrapTestForSharding(testTui_resize)) + t.Run("testTui/delete", wrapTestForSharding(testTui_delete)) + t.Run("testTui/color", wrapTestForSharding(testTui_color)) + t.Run("testTui/errors", wrapTestForSharding(testTui_errors)) + t.Run("testTui/ai", wrapTestForSharding(testTui_ai)) + t.Run("testTui/defaultFilter", wrapTestForSharding(testTui_defaultFilter)) // Assert there are no leaked connections assertNoLeakedConnections(t) @@ -121,7 +147,6 @@ func TestParam(t *testing.T) { func testIntegration(t *testing.T, tester shellTester, onlineStatus OnlineStatus) { // Set up - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Run the test @@ -130,7 +155,6 @@ func testIntegration(t *testing.T, tester shellTester, onlineStatus OnlineStatus func testIntegrationWithNewDevice(t *testing.T, tester shellTester) { // Set up - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Run the test @@ -240,7 +264,6 @@ func installWithOnlineStatus(t testing.TB, tester shellTester, onlineStatus Onli func testBasicUserFlow(t *testing.T, tester shellTester, onlineStatus OnlineStatus) string { // Test install - tagAsBasicTest(t) userSecret := installWithOnlineStatus(t, tester, onlineStatus) assertOnlineStatus(t, onlineStatus) @@ -341,7 +364,6 @@ echo thisisrecorded`) func testAdvancedQuery(t *testing.T, tester shellTester) { // Set up - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Install hishtory @@ -565,22 +587,18 @@ func updateToHead(t *testing.T, tester shellTester) string { } func testUpdateFromHeadToRelease(t *testing.T, tester shellTester) { - tagAsBasicTest(t) testGenericUpdate(t, tester, installFromHead, updateToRelease) } func testUpdateFromPrevToRelease(t *testing.T, tester shellTester) { - tagAsBasicTest(t) testGenericUpdate(t, tester, installFromPrev, updateToRelease) } func testUpdateFromPrevToCurrent(t *testing.T, tester shellTester) { - tagAsBasicTest(t) testGenericUpdate(t, tester, installFromPrev, updateToHead) } func testUpdateFromPrevToReleaseViaProd(t *testing.T, tester shellTester) { - tagAsBasicTest(t) defer testutils.BackupAndRestoreEnv("HISHTORY_SERVER")() os.Setenv("HISHTORY_SERVER", "https://api.hishtory.dev") testGenericUpdate(t, tester, installFromPrev, updateToRelease) @@ -630,7 +648,6 @@ func testGenericUpdate(t *testing.T, tester shellTester, installInitialVersion f func testRepeatedCommandThenQuery(t *testing.T, tester shellTester) { // Set up - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -671,7 +688,6 @@ echo mycommand-3`) func testRepeatedCommandAndQuery(t *testing.T, tester shellTester) { // Set up - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -697,7 +713,6 @@ func testRepeatedCommandAndQuery(t *testing.T, tester shellTester) { func testRepeatedEnableDisable(t *testing.T, tester shellTester) { // Set up - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -729,7 +744,6 @@ hishtory enable`, i)) func testExcludeHiddenCommand(t *testing.T, tester shellTester) { // Set up - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -787,7 +801,6 @@ func waitForBackgroundSavesToComplete(t testing.TB) { func testTimestampsAreReasonablyCorrect(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -808,7 +821,6 @@ func testTimestampsAreReasonablyCorrect(t *testing.T, tester shellTester) { func testTableDisplayCwd(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -836,7 +848,6 @@ echo other`) func testHishtoryBackgroundSaving(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Check that we can find the go binary @@ -893,7 +904,6 @@ echo foo`) func testDisplayTable(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -956,7 +966,6 @@ func testDisplayTable(t *testing.T, tester shellTester) { func testRequestAndReceiveDbDump(t *testing.T, tester shellTester) { // Set up - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() secretKey := installHishtory(t, tester, "") @@ -1055,7 +1064,6 @@ echo other`) } func TestInstallViaPythonScriptWithCustomHishtoryPath(t *testing.T) { - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("HISHTORY_PATH")() altHishtoryPath := ".other-path" @@ -1070,7 +1078,6 @@ func TestInstallViaPythonScriptWithCustomHishtoryPath(t *testing.T) { } func TestInstallViaPythonScriptInOfflineMode(t *testing.T) { - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("HISHTORY_OFFLINE")() os.Setenv("HISHTORY_OFFLINE", "1") @@ -1085,7 +1092,6 @@ func TestInstallViaPythonScriptInOfflineMode(t *testing.T) { } func testInstallViaPythonScript(t *testing.T, tester shellTester) { - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() testInstallViaPythonScriptChild(t, tester) @@ -1128,7 +1134,6 @@ func testInstallViaPythonScriptChild(t *testing.T, tester shellTester) { } func TestInstallViaPythonScriptFromHead(t *testing.T) { - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() tester := zshTester{} @@ -1166,7 +1171,6 @@ func TestInstallViaPythonScriptFromHead(t *testing.T) { func testExportWithQuery(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -1225,7 +1229,6 @@ sleep 1`) func testHelpCommand(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -1242,7 +1245,6 @@ func testHelpCommand(t *testing.T, tester shellTester) { func TestStripBashTimePrefix(t *testing.T) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() tester := bashTester{} installHishtory(t, tester, "") @@ -1288,7 +1290,6 @@ func TestStripBashTimePrefix(t *testing.T) { func testReuploadHistoryEntries(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Init an initial device @@ -1339,7 +1340,6 @@ func testReuploadHistoryEntries(t *testing.T, tester shellTester) { func testHishtoryOffline(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Init an initial device @@ -1391,7 +1391,6 @@ func testHishtoryOffline(t *testing.T, tester shellTester) { func testInitialHistoryImport(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("HISHTORY_SKIP_INIT_IMPORT")() os.Setenv("HISHTORY_SKIP_INIT_IMPORT", "") @@ -1426,7 +1425,6 @@ hishtory export `+randomCmdUuid[:5]+` func testLocalRedaction(t *testing.T, tester shellTester, onlineStatus OnlineStatus) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installWithOnlineStatus(t, tester, onlineStatus) assertOnlineStatus(t, onlineStatus) @@ -1495,7 +1493,6 @@ ls /tmp`, randomCmdUuid, randomCmdUuid) func testRemoteRedaction(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Install hishtory client 1 @@ -1551,7 +1548,6 @@ ls /tmp`, randomCmdUuid, randomCmdUuid) func testConfigGetSet(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -1610,7 +1606,6 @@ func clearControlRSearchFromConfig(t testing.TB) { func testHandleUpgradedFeatures(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -1642,7 +1637,6 @@ func testHandleUpgradedFeatures(t *testing.T, tester shellTester) { func TestFish(t *testing.T) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() tester := bashTester{} installHishtory(t, tester, "") @@ -1718,7 +1712,6 @@ func setupTestTui(t testing.TB, onlineStatus OnlineStatus) (shellTester, string, func testTui_resize(t *testing.T) { // Setup - tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, userSecret, _ := setupTestTui(t, Online) @@ -1786,7 +1779,6 @@ func testTui_resize(t *testing.T) { func testTui_scroll(t *testing.T) { // Setup - tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, userSecret, _ := setupTestTui(t, Online) @@ -1825,7 +1817,6 @@ func testTui_scroll(t *testing.T) { func testTui_defaultFilter(t *testing.T) { // Setup - tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, userSecret, _ := setupTestTui(t, Online) db := hctx.GetDb(hctx.MakeContext()) @@ -1879,7 +1870,6 @@ func testTui_color(t *testing.T) { } // Setup - tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, _, _ := setupTestTui(t, Online) tester.RunInteractiveShell(t, ` hishtory config-set highlight-matches false`) @@ -1922,7 +1912,6 @@ func testTui_color(t *testing.T) { func testTui_delete(t *testing.T) { // Setup - tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, userSecret, _ := setupTestTui(t, Online) manuallySubmitHistoryEntry(t, userSecret, testutils.MakeFakeHistoryEntry("echo 'cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc'")) @@ -1965,7 +1954,6 @@ func testTui_delete(t *testing.T) { func testTui_search(t *testing.T, onlineStatus OnlineStatus) { // Setup - tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, _, _ := setupTestTui(t, onlineStatus) @@ -2078,7 +2066,6 @@ func testTui_search(t *testing.T, onlineStatus OnlineStatus) { func testTui_general(t *testing.T, onlineStatus OnlineStatus) { // Setup - tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, _, _ := setupTestTui(t, onlineStatus) @@ -2136,7 +2123,6 @@ func testTui_general(t *testing.T, onlineStatus OnlineStatus) { func testTui_errors(t *testing.T) { // Setup - tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() tester, _, _ := setupTestTui(t, Online) @@ -2157,7 +2143,6 @@ func testTui_errors(t *testing.T) { func testTui_ai(t *testing.T) { // Setup - tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("OPENAI_API_KEY")() os.Setenv("OPENAI_API_KEY", "") @@ -2192,7 +2177,6 @@ func testTui_ai(t *testing.T) { func testControlR(t *testing.T, tester shellTester, shellName string, onlineStatus OnlineStatus) { // Setup - tagAsTuiTest(t) defer testutils.BackupAndRestore(t)() installWithOnlineStatus(t, tester, onlineStatus) assertOnlineStatus(t, onlineStatus) @@ -2355,7 +2339,6 @@ func testControlR(t *testing.T, tester shellTester, shellName string, onlineStat func testCustomColumns(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2404,7 +2387,6 @@ echo bar`) func testPresavingDisabled(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2429,7 +2411,6 @@ func testPresavingDisabled(t *testing.T, tester shellTester) { func testPresavingOffline(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("HISHTORY_SIMULATE_NETWORK_ERROR")() userSecret := installHishtory(t, tester, "") @@ -2472,7 +2453,6 @@ func testPresavingOffline(t *testing.T, tester shellTester) { func testPresaving(t *testing.T, tester shellTester, shellName string) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") manuallySubmitHistoryEntry(t, userSecret, testutils.MakeFakeHistoryEntry("table_sizing aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")) @@ -2561,7 +2541,6 @@ func testTabCompletion(t *testing.T, tester shellTester, shellName string) { } // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2583,7 +2562,6 @@ func testTabCompletion(t *testing.T, tester shellTester, shellName string) { func testUninstall(t *testing.T, tester shellTester) { // Setup - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2618,7 +2596,6 @@ echo bar`) func TestTimestampFormat(t *testing.T) { // Setup - tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -2656,7 +2633,6 @@ func TestTimestampFormat(t *testing.T) { func TestSortByConsistentTimezone(t *testing.T) { // Setup - tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2696,7 +2672,6 @@ func TestSortByConsistentTimezone(t *testing.T) { func TestZDotDir(t *testing.T) { // Setup - tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("ZDOTDIR")() @@ -2731,7 +2706,6 @@ func TestZDotDir(t *testing.T) { func TestRemoveDuplicateRows(t *testing.T) { // Setup - tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2784,7 +2758,6 @@ echo foo`) func TestSetConfigNoCorruption(t *testing.T) { // Setup - tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2819,7 +2792,6 @@ func TestSetConfigNoCorruption(t *testing.T) { // Test that the config retrieved from the context is a reference and there are no consistency issues with it getting out of sync func TestCtxConfigIsReference(t *testing.T) { // Setup - tagAsBasicTest(t) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2845,7 +2817,6 @@ func TestCtxConfigIsReference(t *testing.T) { } func testMultipleUsers(t *testing.T, tester shellTester) { - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() // Create all our devices @@ -2932,7 +2903,6 @@ func createSyntheticImportEntries(t testing.TB, numSyntheticEntries int) { func TestImportHistory(t *testing.T) { // Setup - tagAsBasicTest(t) tester := bashTester{} defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -2987,7 +2957,6 @@ func BenchmarkImport(b *testing.B) { } func TestAugmentedIsOfflineError(t *testing.T) { - tagAsBasicTest(t) defer testutils.BackupAndRestore(t)() installHishtory(t, zshTester{}, "") defer testutils.BackupAndRestoreEnv("HISHTORY_SIMULATE_NETWORK_ERROR")() diff --git a/client/testutils.go b/client/testutils.go index 30b518c2..be304352 100644 --- a/client/testutils.go +++ b/client/testutils.go @@ -366,17 +366,3 @@ func stripRequiredPrefix(t *testing.T, out, prefix string) string { func stripTuiCommandPrefix(t *testing.T, out string) string { return stripRequiredPrefix(t, out, "hishtory tquery") } - -func tagAsBasicTest(t *testing.T) { - s := os.Getenv("SPLIT_TESTS") - if s != "" && s != "BASIC" { - t.Skip() - } -} - -func tagAsTuiTest(t *testing.T) { - s := os.Getenv("SPLIT_TESTS") - if s != "" && s != "TUI" { - t.Skip() - } -} From 7fb25c22c15d937e65bfb5c870b613d7cd7b62ee Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 19:35:01 -0800 Subject: [PATCH 19/36] Fix references to test shards and increase shard count --- .github/workflows/go-test.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 827601b5..50edea94 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -14,7 +14,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, macos-latest, macos-14] - test_shards: ["0", "1", "2", "3", "4"] + test_shard: ["0", "1", "2", "3", "4", "5", "6"] fail-fast: false steps: - uses: actions/checkout@v4 @@ -70,7 +70,7 @@ jobs: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} run: | go install gotest.tools/gotestsum@bc98120 - NUM_TEST_SHARDS=5 CURRENT_SHARD_NUM=${{ matrix.test_shards }} make test + NUM_TEST_SHARDS=6 CURRENT_SHARD_NUM=${{ matrix.test_shard }} make test - name: Extra Delay run: | @@ -80,25 +80,25 @@ jobs: uses: actions/upload-artifact@v3 if: success() || failure() with: - name: test-results-${{ matrix.os }}-${{ matrix.tests }}.json + name: test-results-${{ matrix.os }}-${{ matrix.test_shard }}.json path: /tmp/testrun.json - name: Upload failed test goldens uses: actions/upload-artifact@v3 if: success() || failure() with: - name: test-goldens-${{ matrix.os }}-${{ matrix.tests }}.zip + name: test-goldens-${{ matrix.os }}-${{ matrix.test_shard }}.zip path: /tmp/test-goldens/ - name: Upload test log uses: actions/upload-artifact@v3 if: success() || failure() with: - name: testlog-${{ matrix.os }}-${{ matrix.tests }}.txt + name: testlog-${{ matrix.os }}-${{ matrix.test_shard }}.txt path: /tmp/test.log - name: Upload test log uses: actions/upload-artifact@v4 if: success() || failure() with: - name: goldens-used-${{ matrix.os }}-${{ matrix.tests }} + name: goldens-used-${{ matrix.os }}-${{ matrix.test_shard }} path: /tmp/goldens-used.txt # - name: Setup tmate session From f2c3aebe3af559cb318f155735e3cfedea936b35 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 19:59:26 -0800 Subject: [PATCH 20/36] Shard the fuzz test --- client/client_test.go | 36 +++++++++++++++++++++++++++++------- client/fuzz_test.go | 6 ++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 1efa5009..c783274a 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -60,15 +60,37 @@ var shellTesters []shellTester = []shellTester{bashTester{}, zshTester{}} var shardNumberAllocator int = 0 -func markTestForSharding(t *testing.T, testShardNumber int) { +func numTestShards() int { numTestShardsStr := os.Getenv("NUM_TEST_SHARDS") + if numTestShardsStr == "" { + return 0 + } + numTestShards, err := strconv.Atoi(numTestShardsStr) + if err != nil { + panic(fmt.Errorf("failed to parse NUM_TEST_SHARDS: %v", err)) + } + return numTestShards +} + +func currentShardNumber() int { currentShardNumberStr := os.Getenv("CURRENT_SHARD_NUM") - if numTestShardsStr != "" && currentShardNumberStr != "" { - numTestShards, err := strconv.Atoi(numTestShardsStr) - require.NoError(t, err) - currentShardNumber, err := strconv.Atoi(currentShardNumberStr) - require.NoError(t, err) - if testShardNumber%numTestShards != currentShardNumber { + if currentShardNumberStr == "" { + return 0 + } + currentShardNumber, err := strconv.Atoi(currentShardNumberStr) + if err != nil { + panic(fmt.Errorf("failed to parse CURRENT_SHARD_NUM: %v", err)) + } + return currentShardNumber +} + +func isShardedTestRun() bool { + return numTestShards() != 0 && currentShardNumber() != 0 +} + +func markTestForSharding(t *testing.T, testShardNumber int) { + if isShardedTestRun() { + if testShardNumber%numTestShards() == currentShardNumber() { t.Skip("Skipping sharded test") } } diff --git a/client/fuzz_test.go b/client/fuzz_test.go index 5af928e1..9e8c500a 100644 --- a/client/fuzz_test.go +++ b/client/fuzz_test.go @@ -142,6 +142,12 @@ func FuzzTestMultipleUsers(f *testing.F) { if skipSlowTests() { f.Skip("skipping slow tests") } + if isShardedTestRun() { + if currentShardNumber() == 0 { + f.Skip("Skipping sharded test") + } + } + s := os.Getenv("SPLIT_TESTS") if s != "" && s != "BASIC" { f.Skip() From c7d5ab0a43e7f0ea879fc8209893b5bf60c20b45 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 22:30:50 -0800 Subject: [PATCH 21/36] Add debug prints --- .github/workflows/go-test.yml | 4 ++-- client/client_test.go | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 50edea94..82881bb2 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -19,7 +19,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: Set up Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v4 with: go-version: 1.21 - name: Linux Setup @@ -112,7 +112,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: Set up Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v4 with: go-version: 1.21 - name: Download artifact diff --git a/client/client_test.go b/client/client_test.go index c783274a..20807a10 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -58,8 +58,6 @@ func TestMain(m *testing.M) { var shellTesters []shellTester = []shellTester{bashTester{}, zshTester{}} -var shardNumberAllocator int = 0 - func numTestShards() int { numTestShardsStr := os.Getenv("NUM_TEST_SHARDS") if numTestShardsStr == "" { @@ -89,6 +87,7 @@ func isShardedTestRun() bool { } func markTestForSharding(t *testing.T, testShardNumber int) { + fmt.Printf("DDWORKENDEBUG: markTestForSharding: isShardedTestRun()=%#v testShardNumber=%#v numTestShards()=%#v currentShardNumber()=%#v", isShardedTestRun(), testShardNumber, numTestShards(), currentShardNumber()) if isShardedTestRun() { if testShardNumber%numTestShards() == currentShardNumber() { t.Skip("Skipping sharded test") @@ -96,6 +95,8 @@ func markTestForSharding(t *testing.T, testShardNumber int) { } } +var shardNumberAllocator int = 0 + func wrapTestForSharding(test func(t *testing.T)) func(t *testing.T) { shardNumberAllocator += 1 return func(t *testing.T) { From c97f38154634391261e3b7a7314b7aa092ce2797 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 23:00:50 -0800 Subject: [PATCH 22/36] Mark additional tests for sharding --- client/client_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/client/client_test.go b/client/client_test.go index 20807a10..70be5d22 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1087,6 +1087,7 @@ echo other`) } func TestInstallViaPythonScriptWithCustomHishtoryPath(t *testing.T) { + markTestForSharding(t, 0) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("HISHTORY_PATH")() altHishtoryPath := ".other-path" @@ -1101,6 +1102,7 @@ func TestInstallViaPythonScriptWithCustomHishtoryPath(t *testing.T) { } func TestInstallViaPythonScriptInOfflineMode(t *testing.T) { + markTestForSharding(t, 1) defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("HISHTORY_OFFLINE")() os.Setenv("HISHTORY_OFFLINE", "1") @@ -1157,6 +1159,7 @@ func testInstallViaPythonScriptChild(t *testing.T, tester shellTester) { } func TestInstallViaPythonScriptFromHead(t *testing.T) { + markTestForSharding(t, 2) defer testutils.BackupAndRestore(t)() tester := zshTester{} @@ -1268,6 +1271,7 @@ func testHelpCommand(t *testing.T, tester shellTester) { func TestStripBashTimePrefix(t *testing.T) { // Setup + markTestForSharding(t, 4) defer testutils.BackupAndRestore(t)() tester := bashTester{} installHishtory(t, tester, "") @@ -1660,6 +1664,7 @@ func testHandleUpgradedFeatures(t *testing.T, tester shellTester) { func TestFish(t *testing.T) { // Setup + markTestForSharding(t, 5) defer testutils.BackupAndRestore(t)() tester := bashTester{} installHishtory(t, tester, "") @@ -2619,6 +2624,7 @@ echo bar`) func TestTimestampFormat(t *testing.T) { // Setup + markTestForSharding(t, 6) tester := zshTester{} defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -2656,6 +2662,7 @@ func TestTimestampFormat(t *testing.T) { func TestSortByConsistentTimezone(t *testing.T) { // Setup + markTestForSharding(t, 7) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2695,6 +2702,7 @@ func TestSortByConsistentTimezone(t *testing.T) { func TestZDotDir(t *testing.T) { // Setup + markTestForSharding(t, 8) tester := zshTester{} defer testutils.BackupAndRestore(t)() defer testutils.BackupAndRestoreEnv("ZDOTDIR")() @@ -2729,6 +2737,7 @@ func TestZDotDir(t *testing.T) { func TestRemoveDuplicateRows(t *testing.T) { // Setup + markTestForSharding(t, 9) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2781,6 +2790,7 @@ echo foo`) func TestSetConfigNoCorruption(t *testing.T) { // Setup + markTestForSharding(t, 10) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2815,6 +2825,7 @@ func TestSetConfigNoCorruption(t *testing.T) { // Test that the config retrieved from the context is a reference and there are no consistency issues with it getting out of sync func TestCtxConfigIsReference(t *testing.T) { // Setup + markTestForSharding(t, 11) tester := zshTester{} defer testutils.BackupAndRestore(t)() installHishtory(t, tester, "") @@ -2926,6 +2937,7 @@ func createSyntheticImportEntries(t testing.TB, numSyntheticEntries int) { func TestImportHistory(t *testing.T) { // Setup + markTestForSharding(t, 11) tester := bashTester{} defer testutils.BackupAndRestore(t)() userSecret := installHishtory(t, tester, "") @@ -2980,6 +2992,7 @@ func BenchmarkImport(b *testing.B) { } func TestAugmentedIsOfflineError(t *testing.T) { + markTestForSharding(t, 12) defer testutils.BackupAndRestore(t)() installHishtory(t, zshTester{}, "") defer testutils.BackupAndRestoreEnv("HISHTORY_SIMULATE_NETWORK_ERROR")() From f20b2a08c41adb73bf4d8c1a66f87284f1f1d290 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 23:02:29 -0800 Subject: [PATCH 23/36] Fix logic error that broke test sharding --- client/client_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 70be5d22..430167d4 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -61,7 +61,7 @@ var shellTesters []shellTester = []shellTester{bashTester{}, zshTester{}} func numTestShards() int { numTestShardsStr := os.Getenv("NUM_TEST_SHARDS") if numTestShardsStr == "" { - return 0 + return -1 } numTestShards, err := strconv.Atoi(numTestShardsStr) if err != nil { @@ -73,7 +73,7 @@ func numTestShards() int { func currentShardNumber() int { currentShardNumberStr := os.Getenv("CURRENT_SHARD_NUM") if currentShardNumberStr == "" { - return 0 + return -1 } currentShardNumber, err := strconv.Atoi(currentShardNumberStr) if err != nil { @@ -83,13 +83,13 @@ func currentShardNumber() int { } func isShardedTestRun() bool { - return numTestShards() != 0 && currentShardNumber() != 0 + return numTestShards() != -1 && currentShardNumber() != -1 } func markTestForSharding(t *testing.T, testShardNumber int) { fmt.Printf("DDWORKENDEBUG: markTestForSharding: isShardedTestRun()=%#v testShardNumber=%#v numTestShards()=%#v currentShardNumber()=%#v", isShardedTestRun(), testShardNumber, numTestShards(), currentShardNumber()) if isShardedTestRun() { - if testShardNumber%numTestShards() == currentShardNumber() { + if testShardNumber%numTestShards() != currentShardNumber() { t.Skip("Skipping sharded test") } } From 7e580888d20db4dbd600e3740896a435dabcf13c Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 23:04:33 -0800 Subject: [PATCH 24/36] Remove debug print --- client/client_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/client/client_test.go b/client/client_test.go index 430167d4..68a4341a 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -87,7 +87,6 @@ func isShardedTestRun() bool { } func markTestForSharding(t *testing.T, testShardNumber int) { - fmt.Printf("DDWORKENDEBUG: markTestForSharding: isShardedTestRun()=%#v testShardNumber=%#v numTestShards()=%#v currentShardNumber()=%#v", isShardedTestRun(), testShardNumber, numTestShards(), currentShardNumber()) if isShardedTestRun() { if testShardNumber%numTestShards() != currentShardNumber() { t.Skip("Skipping sharded test") From 102a3b54ac994647d03a470a56fe8dcf2c581a56 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 23:05:03 -0800 Subject: [PATCH 25/36] Fix incorrect logic with skipping the fuzz test --- client/fuzz_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/fuzz_test.go b/client/fuzz_test.go index 9e8c500a..5a896577 100644 --- a/client/fuzz_test.go +++ b/client/fuzz_test.go @@ -143,7 +143,7 @@ func FuzzTestMultipleUsers(f *testing.F) { f.Skip("skipping slow tests") } if isShardedTestRun() { - if currentShardNumber() == 0 { + if currentShardNumber() != 0 { f.Skip("Skipping sharded test") } } From 43a65066ee842251f36c65dbe8cb81fb68977028 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 23:07:52 -0800 Subject: [PATCH 26/36] Move sharding functions to testutils and add some comments --- client/client_test.go | 48 ------------------------------------- client/testutils.go | 56 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 68a4341a..adc9dd69 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -9,7 +9,6 @@ import ( "path" "regexp" "runtime" - "strconv" "strings" "sync" "testing" @@ -58,53 +57,6 @@ func TestMain(m *testing.M) { var shellTesters []shellTester = []shellTester{bashTester{}, zshTester{}} -func numTestShards() int { - numTestShardsStr := os.Getenv("NUM_TEST_SHARDS") - if numTestShardsStr == "" { - return -1 - } - numTestShards, err := strconv.Atoi(numTestShardsStr) - if err != nil { - panic(fmt.Errorf("failed to parse NUM_TEST_SHARDS: %v", err)) - } - return numTestShards -} - -func currentShardNumber() int { - currentShardNumberStr := os.Getenv("CURRENT_SHARD_NUM") - if currentShardNumberStr == "" { - return -1 - } - currentShardNumber, err := strconv.Atoi(currentShardNumberStr) - if err != nil { - panic(fmt.Errorf("failed to parse CURRENT_SHARD_NUM: %v", err)) - } - return currentShardNumber -} - -func isShardedTestRun() bool { - return numTestShards() != -1 && currentShardNumber() != -1 -} - -func markTestForSharding(t *testing.T, testShardNumber int) { - if isShardedTestRun() { - if testShardNumber%numTestShards() != currentShardNumber() { - t.Skip("Skipping sharded test") - } - } -} - -var shardNumberAllocator int = 0 - -func wrapTestForSharding(test func(t *testing.T)) func(t *testing.T) { - shardNumberAllocator += 1 - return func(t *testing.T) { - testShardNumber := shardNumberAllocator - markTestForSharding(t, testShardNumber) - test(t) - } -} - func TestParam(t *testing.T) { if skipSlowTests() { shellTesters = shellTesters[:1] diff --git a/client/testutils.go b/client/testutils.go index be304352..50164171 100644 --- a/client/testutils.go +++ b/client/testutils.go @@ -366,3 +366,59 @@ func stripRequiredPrefix(t *testing.T, out, prefix string) string { func stripTuiCommandPrefix(t *testing.T, out string) string { return stripRequiredPrefix(t, out, "hishtory tquery") } + +// Wrap the given test so that it can be run on Github Actions with sharding. This +// makes it possible to run only 1/N tests on each of N github action jobs, speeding +// up test execution through parallelization. This is necessary since the wrapped +// integration tests rely on OS-level globals (the shell history) that can't otherwise +// be parallelized. +func wrapTestForSharding(test func(t *testing.T)) func(t *testing.T) { + shardNumberAllocator += 1 + return func(t *testing.T) { + testShardNumber := shardNumberAllocator + markTestForSharding(t, testShardNumber) + test(t) + } +} + +var shardNumberAllocator int = 0 + +// Returns whether this is a sharded test run. false during all normal non-github action operations. +func isShardedTestRun() bool { + return numTestShards() != -1 && currentShardNumber() != -1 +} + +// Get the total number of test shards +func numTestShards() int { + numTestShardsStr := os.Getenv("NUM_TEST_SHARDS") + if numTestShardsStr == "" { + return -1 + } + numTestShards, err := strconv.Atoi(numTestShardsStr) + if err != nil { + panic(fmt.Errorf("failed to parse NUM_TEST_SHARDS: %v", err)) + } + return numTestShards +} + +// Get the current shard number +func currentShardNumber() int { + currentShardNumberStr := os.Getenv("CURRENT_SHARD_NUM") + if currentShardNumberStr == "" { + return -1 + } + currentShardNumber, err := strconv.Atoi(currentShardNumberStr) + if err != nil { + panic(fmt.Errorf("failed to parse CURRENT_SHARD_NUM: %v", err)) + } + return currentShardNumber +} + +// Mark the given test for sharding with the given test ID number. +func markTestForSharding(t *testing.T, testShardNumber int) { + if isShardedTestRun() { + if testShardNumber%numTestShards() != currentShardNumber() { + t.Skip("Skipping sharded test") + } + } +} From 00c98d5b10145530701436436bfeddb52bd5844b Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 23:09:04 -0800 Subject: [PATCH 27/36] Upgrade all setup-go actions to enable caching of deps --- .github/workflows/docker-compose-test.yml | 2 +- .github/workflows/pre-commit.yml | 2 +- .github/workflows/server-releaser.yml | 2 +- .github/workflows/slsa-releaser.yml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/docker-compose-test.yml b/.github/workflows/docker-compose-test.yml index ca89ed3a..a76f7eee 100644 --- a/.github/workflows/docker-compose-test.yml +++ b/.github/workflows/docker-compose-test.yml @@ -12,7 +12,7 @@ jobs: steps: - uses: actions/checkout@v3 - name: Set up Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v4 with: go-version: 1.21 - name: Docker Compose setup diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 93879c15..4a516463 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -11,7 +11,7 @@ jobs: steps: - uses: actions/checkout@v3 - uses: actions/setup-python@v3 - - uses: actions/setup-go@v3 + - uses: actions/setup-go@v4 with: go-version: 1.21 - name: Install dependencies diff --git a/.github/workflows/server-releaser.yml b/.github/workflows/server-releaser.yml index d0e52af7..21955ecb 100644 --- a/.github/workflows/server-releaser.yml +++ b/.github/workflows/server-releaser.yml @@ -23,7 +23,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: Set up Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v4 with: go-version: 1.21 - name: Build server binary diff --git a/.github/workflows/slsa-releaser.yml b/.github/workflows/slsa-releaser.yml index ea3d7826..625cae1c 100644 --- a/.github/workflows/slsa-releaser.yml +++ b/.github/workflows/slsa-releaser.yml @@ -182,7 +182,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: Set up Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v4 with: go-version: 1.21 - uses: actions/download-artifact@fb598a63ae348fa914e94cd0ff38f362e927b741 From 60f923ca081fd710f8613a1ff9586023a35ab78f Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Feb 2024 23:10:34 -0800 Subject: [PATCH 28/36] Remove goldens that don't exist --- client/posttest/main.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/posttest/main.go b/client/posttest/main.go index e13ecaf1..94fc3745 100644 --- a/client/posttest/main.go +++ b/client/posttest/main.go @@ -24,8 +24,7 @@ var UNUSED_GOLDENS []string = []string{"TestTui-Exit", "testControlR-ControlC-ba "testControlR-ControlC-zsh", "testControlR-SelectMultiline-bash", "testControlR-SelectMultiline-fish", "testControlR-SelectMultiline-zsh", "testControlR-bash-Disabled", "testControlR-fish-Disabled", "testControlR-zsh-Disabled", "testCustomColumns-query-isAction=false", "testCustomColumns-tquery-bash", - "testCustomColumns-tquery-zsh", "testUninstall-post-uninstall-bash", - "testUninstall-post-uninstall-zsh"} + "testCustomColumns-tquery-zsh"} func main() { if os.Args[1] == "export" { From d5eb739bb37f99b78d5406f839c08df6b7862df5 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Feb 2024 08:35:27 -0800 Subject: [PATCH 29/36] Remove new line --- .github/workflows/go-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 82881bb2..bb231d9a 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -119,4 +119,4 @@ jobs: uses: actions/download-artifact@v4 - name: Check all goldens were used run: | - go run client/posttest/main.go check-goldens + go run client/posttest/main.go check-goldens \ No newline at end of file From 7bd6d6ef7776f4641d4624ff0dff43963322270a Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Feb 2024 09:22:38 -0800 Subject: [PATCH 30/36] Reduce delay --- .github/workflows/go-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index bb231d9a..29425df1 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -75,7 +75,7 @@ jobs: run: | # Add an extra short delay to allow datadog to flush metrics - sleep 300 # 5 minutes + sleep 90 - name: Upload test results json uses: actions/upload-artifact@v3 if: success() || failure() From ce01d4ca5abe3231f4b9e6e3b1df8965a7581049 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Feb 2024 09:23:06 -0800 Subject: [PATCH 31/36] Correct stage name --- .github/workflows/go-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 29425df1..a62f6c62 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -94,7 +94,7 @@ jobs: with: name: testlog-${{ matrix.os }}-${{ matrix.test_shard }}.txt path: /tmp/test.log - - name: Upload test log + - name: Upload used goldens uses: actions/upload-artifact@v4 if: success() || failure() with: From 02b5e1f7f34b63577f89d984ad2054cbce3cf27e Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Feb 2024 09:25:25 -0800 Subject: [PATCH 32/36] Remove incorrect skip code from the first version of sharding --- client/fuzz_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/client/fuzz_test.go b/client/fuzz_test.go index 5a896577..57dc4d33 100644 --- a/client/fuzz_test.go +++ b/client/fuzz_test.go @@ -148,10 +148,6 @@ func FuzzTestMultipleUsers(f *testing.F) { } } - s := os.Getenv("SPLIT_TESTS") - if s != "" && s != "BASIC" { - f.Skip() - } defer testutils.RunTestServer()() // Format: // $Op = $Key;$Device|$Command\n From ffb06feca36d670471e198b38528626d24c5b102 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Feb 2024 09:43:50 -0800 Subject: [PATCH 33/36] Remove unused import --- client/fuzz_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/client/fuzz_test.go b/client/fuzz_test.go index 57dc4d33..ab1b59ee 100644 --- a/client/fuzz_test.go +++ b/client/fuzz_test.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "os" "regexp" "strconv" "strings" From fa65aeda36b6eaa8a37ac281774412b1521dd539 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Feb 2024 09:48:06 -0800 Subject: [PATCH 34/36] Reduce number of test shards to match GitHub's limit of 5 concurrent macos jobs --- .github/workflows/go-test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index a62f6c62..63ae9b2e 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -14,7 +14,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, macos-latest, macos-14] - test_shard: ["0", "1", "2", "3", "4", "5", "6"] + test_shard: ["0", "1", "2", "3", "4"] fail-fast: false steps: - uses: actions/checkout@v4 @@ -70,7 +70,7 @@ jobs: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} run: | go install gotest.tools/gotestsum@bc98120 - NUM_TEST_SHARDS=6 CURRENT_SHARD_NUM=${{ matrix.test_shard }} make test + NUM_TEST_SHARDS=5 CURRENT_SHARD_NUM=${{ matrix.test_shard }} make test - name: Extra Delay run: | From 7bce688fc58f6c273ad076d3aa67f031e4aff85d Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Feb 2024 10:30:54 -0800 Subject: [PATCH 35/36] Use cask for installing homebrew to speed up github actions --- .github/workflows/go-test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 63ae9b2e..b8509415 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -46,10 +46,10 @@ jobs: sudo scutil --set HostName ghaction-runner-hostname - name: MacOS Docker Setup if: ${{ matrix.os == 'macos-latest' || matrix.os == 'macos-14 '}} - continue-on-error: true + continue-on-error: true # Since colima is flaky, and a failure here only impacts our metrics run: | # Install docker so it can be used for datadog - brew install docker + brew install --cask docker colima start sudo ln -sf $HOME/.colima/default/docker.sock /var/run/docker.sock - name: Set up Datadog From b3804efcb79920ec48621e2bc8308d3c3e963d2b Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Feb 2024 10:35:03 -0800 Subject: [PATCH 36/36] More cleanup for unused goldens --- client/posttest/main.go | 5 +---- client/testdata/testControlR-SelectMultiline-bash | 2 -- client/testdata/testControlR-SelectMultiline-zsh | 3 --- client/testdata/testControlR-bash-Disabled | 2 -- client/testdata/testControlR-zsh-Disabled | 2 -- 5 files changed, 1 insertion(+), 13 deletions(-) delete mode 100644 client/testdata/testControlR-SelectMultiline-bash delete mode 100644 client/testdata/testControlR-SelectMultiline-zsh delete mode 100644 client/testdata/testControlR-bash-Disabled delete mode 100644 client/testdata/testControlR-zsh-Disabled diff --git a/client/posttest/main.go b/client/posttest/main.go index 94fc3745..a118e681 100644 --- a/client/posttest/main.go +++ b/client/posttest/main.go @@ -20,10 +20,7 @@ var GLOBAL_STATSD *statsd.Client = nil var NUM_TEST_RETRIES map[string]int -var UNUSED_GOLDENS []string = []string{"TestTui-Exit", "testControlR-ControlC-bash", "testControlR-ControlC-fish", - "testControlR-ControlC-zsh", "testControlR-SelectMultiline-bash", "testControlR-SelectMultiline-fish", - "testControlR-SelectMultiline-zsh", "testControlR-bash-Disabled", "testControlR-fish-Disabled", - "testControlR-zsh-Disabled", "testCustomColumns-query-isAction=false", "testCustomColumns-tquery-bash", +var UNUSED_GOLDENS []string = []string{"testCustomColumns-query-isAction=false", "testCustomColumns-tquery-bash", "testCustomColumns-tquery-zsh"} func main() { diff --git a/client/testdata/testControlR-SelectMultiline-bash b/client/testdata/testControlR-SelectMultiline-bash deleted file mode 100644 index 0dd1add8..00000000 --- a/client/testdata/testControlR-SelectMultiline-bash +++ /dev/null @@ -1,2 +0,0 @@ -bash-5.2$ source /Users/david/.bashrc -bash-5.2$ ls -Slah / \ No newline at end of file diff --git a/client/testdata/testControlR-SelectMultiline-zsh b/client/testdata/testControlR-SelectMultiline-zsh deleted file mode 100644 index 9752a558..00000000 --- a/client/testdata/testControlR-SelectMultiline-zsh +++ /dev/null @@ -1,3 +0,0 @@ -david@Davids-MacBook-Air hishtory % ls \ --Slah \ -/ \ No newline at end of file diff --git a/client/testdata/testControlR-bash-Disabled b/client/testdata/testControlR-bash-Disabled deleted file mode 100644 index 0858bb26..00000000 --- a/client/testdata/testControlR-bash-Disabled +++ /dev/null @@ -1,2 +0,0 @@ -bash-5.2$ source /Users/david/.bashrc -(reverse-i-search)`': \ No newline at end of file diff --git a/client/testdata/testControlR-zsh-Disabled b/client/testdata/testControlR-zsh-Disabled deleted file mode 100644 index 16c87004..00000000 --- a/client/testdata/testControlR-zsh-Disabled +++ /dev/null @@ -1,2 +0,0 @@ -david@Davids-MacBook-Air hishtory % -bck-i-search: _ \ No newline at end of file