Fix flakey test failures by removing cache=shared which is a discouraged mode (https://www.sqlite.org/sharedcache.html). WAL is sufficient for our purposes. Plus fix a bug where the TUI would go into an infinite loop if there were zero results.

client/client_test.go

@@ -310,7 +310,7 @@ func installHishtory(t *testing.T, tester shellTester, userSecret string) string
 	return matches[1]
 }
 
-func initFromOnlineStatus(t *testing.T, tester shellTester, onlineStatus OnlineStatus) string {
+func installWithOnlineStatus(t *testing.T, tester shellTester, onlineStatus OnlineStatus) string {
 	if onlineStatus == Online {
 		return installHishtory(t, tester, "")
 	} else {
@@ -330,7 +330,7 @@ func assertOnlineStatus(t *testing.T, onlineStatus OnlineStatus) {
 
 func testBasicUserFlow(t *testing.T, tester shellTester, onlineStatus OnlineStatus) string {
 	// Test install
-	userSecret := initFromOnlineStatus(t, tester, onlineStatus)
+	userSecret := installWithOnlineStatus(t, tester, onlineStatus)
 	assertOnlineStatus(t, onlineStatus)
 
 	// Test the status subcommand
@@ -1512,7 +1512,7 @@ echo UUID-fishcommand
 func testLocalRedaction(t *testing.T, tester shellTester, onlineStatus OnlineStatus) {
 	// Setup
 	defer testutils.BackupAndRestore(t)()
-	initFromOnlineStatus(t, tester, onlineStatus)
+	installWithOnlineStatus(t, tester, onlineStatus)
 	assertOnlineStatus(t, onlineStatus)
 
 	// Record some commands
@@ -1797,8 +1797,8 @@ func TestTui(t *testing.T) {
 
 	// Insert a couple hishtory entries
 	db := hctx.GetDb(hctx.MakeContext())
-	db.Create(testutils.MakeFakeHistoryEntry("ls ~/"))
+	testutils.Check(t, db.Create(testutils.MakeFakeHistoryEntry("ls ~/")).Error)
-	db.Create(testutils.MakeFakeHistoryEntry("echo 'aaaaaa bbbb'"))
+	testutils.Check(t, db.Create(testutils.MakeFakeHistoryEntry("echo 'aaaaaa bbbb'")).Error)
 
 	// Check the initial output when there is no search
 	out := captureTerminalOutput(t, tester, []string{"hishtory SPACE tquery ENTER"})
@@ -1874,7 +1874,7 @@ func captureTerminalOutputWithShellNameAndDimensions(t *testing.T, tester shellT
 	fullCommand += " sleep 0.5\n"
 	fullCommand += " tmux capture-pane -t foo -p\n"
 	fullCommand += " tmux kill-session -t foo\n"
-	hctx.GetLogger().Infof("Running tmux command: " + fullCommand)
+	testutils.TestLog(t, "Running tmux command: "+fullCommand)
 	return strings.TrimSpace(tester.RunInteractiveShell(t, fullCommand))
 }
 
@@ -1884,7 +1884,7 @@ func testControlR(t *testing.T, tester shellTester, shellName string, onlineStat
 	}
 	// Setup
 	defer testutils.BackupAndRestore(t)()
-	initFromOnlineStatus(t, tester, onlineStatus)
+	installWithOnlineStatus(t, tester, onlineStatus)
 	assertOnlineStatus(t, onlineStatus)
 
 	// Disable recording so that all our testing commands don't get recorded
@@ -1897,11 +1897,18 @@ func testControlR(t *testing.T, tester shellTester, shellName string, onlineStat
 	e1.CurrentWorkingDirectory = "/etc/"
 	e1.Hostname = "server"
 	e1.ExitCode = 127
-	db.Create(e1)
+	testutils.Check(t, db.Create(e1).Error)
-	db.Create(testutils.MakeFakeHistoryEntry("ls ~/foo/"))
+	testutils.Check(t, db.Create(testutils.MakeFakeHistoryEntry("ls ~/foo/")).Error)
-	db.Create(testutils.MakeFakeHistoryEntry("ls ~/bar/"))
+	testutils.Check(t, db.Create(testutils.MakeFakeHistoryEntry("ls ~/bar/")).Error)
-	db.Create(testutils.MakeFakeHistoryEntry("echo 'aaaaaa bbbb'"))
+	testutils.Check(t, db.Create(testutils.MakeFakeHistoryEntry("echo 'aaaaaa bbbb'")).Error)
-	db.Create(testutils.MakeFakeHistoryEntry("echo 'bar' &"))
+	testutils.Check(t, db.Create(testutils.MakeFakeHistoryEntry("echo 'bar' &")).Error)
+
+	// Check that they're there
+	var historyEntries []*data.HistoryEntry
+	db.Model(&data.HistoryEntry{}).Find(&historyEntries)
+	if len(historyEntries) != 5 {
+		t.Fatalf("expected to find 5 history entries, actual found %d: %#v", len(historyEntries), historyEntries)
+	}
 
 	// And check that the control-r binding brings up the search
 	out := captureTerminalOutputWithShellName(t, tester, shellName, []string{"C-R"})
@@ -2030,6 +2037,21 @@ func testControlR(t *testing.T, tester shellTester, shellName string, onlineStat
 		// This bit is broken on actions since actions run as a different user
 		compareGoldens(t, out, "testControlR-"+shellName+"-Disabled")
 	}
+
+	// Re-enable control-r
+	out, err := tester.RunInteractiveShellRelaxed(t, `hishtory config-set enable-control-r true`)
+	testutils.Check(t, err)
+	if out != "" {
+		t.Fatalf("config-set out is unexpected: %#v", out)
+	}
+
+	// And check that the control-r bindings work again
+	out = captureTerminalOutputWithShellName(t, tester, "fish", []string{"C-R", "-pipefail SPACE -exit_code:0"})
+	if !strings.Contains(out, "\n\n\n") {
+		t.Fatalf("failed to find separator in %#v", out)
+	}
+	out = strings.TrimSpace(strings.Split(out, "\n\n\n")[1])
+	compareGoldens(t, out, "testControlR-Final")
 }
 
 func testCustomColumns(t *testing.T, tester shellTester) {

client/hctx/hctx.go

@@ -87,7 +87,7 @@ func OpenLocalSqliteDb() (*gorm.DB, error) {
 		},
 	)
 	dbFilePath := path.Join(homedir, data.HISHTORY_PATH, data.DB_PATH)
-	dsn := fmt.Sprintf("file:%s?cache=shared&mode=rwc&_journal_mode=WAL", dbFilePath)
+	dsn := fmt.Sprintf("file:%s?mode=rwc&_journal_mode=WAL", dbFilePath)
 	db, err := gorm.Open(sqlite.Open(dsn), &gorm.Config{SkipDefaultTransaction: true, Logger: newLogger})
 	if err != nil {
 		return nil, fmt.Errorf("failed to connect to the DB: %v", err)

client/lib/goldens/testControlR-Final

@@ -0,0 +1,26 @@
+Search Query: > -pipefail -exit_code:0
+
+┌─────────────────────────────────────────────────────────────────────────────┐
+│ Hostname                       Exit Code  Command                  foo      │
+│─────────────────────────────────────────────────────────────────────────────│
+│ localhost                      2          echo 'bar' &                      │
+│ localhost                      2          echo 'aaaaaa bbbb'                │
+│ localhost                      2          ls ~/bar/                         │
+│ localhost                      2          ls ~/foo/                         │
+│ server                         127        ls ~/                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+│                                                                             │
+└─────────────────────────────────────────────────────────────────────────────┘

client/lib/lib.go

@@ -409,6 +409,7 @@ func AddToDbIfNew(db *gorm.DB, entry data.HistoryEntry) {
 	tx.Limit(1).Find(&results)
 	if len(results) == 0 {
 		db.Create(entry)
+		// TODO: check the error here and bubble it up
 	}
 }
 

client/lib/lib_test.go

@@ -190,7 +190,7 @@ func TestPersist(t *testing.T) {
 	db := hctx.GetDb(hctx.MakeContext())
 
 	entry := testutils.MakeFakeHistoryEntry("ls ~/")
-	db.Create(entry)
+	testutils.Check(t, db.Create(entry).Error)
 	var historyEntries []*data.HistoryEntry
 	result := db.Find(&historyEntries)
 	testutils.Check(t, result.Error)
@@ -211,9 +211,9 @@ func TestSearch(t *testing.T) {
 
 	// Insert data
 	entry1 := testutils.MakeFakeHistoryEntry("ls /foo")
-	db.Create(entry1)
+	testutils.Check(t, db.Create(entry1).Error)
 	entry2 := testutils.MakeFakeHistoryEntry("ls /bar")
-	db.Create(entry2)
+	testutils.Check(t, db.Create(entry2).Error)
 
 	// Search for data
 	results, err := Search(ctx, db, "ls", 5)

client/lib/tui.go

@@ -242,8 +242,7 @@ func getRows(ctx *context.Context, columnNames []string, query string, numEntrie
 	return rows, len(data), nil
 }
 
-func calculateColumnWidths(rows []table.Row) []int {
+func calculateColumnWidths(rows []table.Row, numColumns int) []int {
-	numColumns := len(rows[0])
 	neededColumnWidth := make([]int, numColumns)
 	for _, row := range rows {
 		for i, v := range row {
@@ -266,11 +265,20 @@ func makeTableColumns(ctx *context.Context, columnNames []string, rows []table.R
 		if err != nil {
 			return nil, err
 		}
+		if len(allRows) == 0 || len(allRows[0]) == 0 {
+			// There are truly zero history entries. Let's still display a table in this case rather than erroring out.
+			allRows = make([]table.Row, 0)
+			row := make([]string, 0)
+			for range columnNames {
+				row = append(row, " ")
+			}
+			allRows = append(allRows, row)
+		}
 		return makeTableColumns(ctx, columnNames, allRows)
 	}
 
 	// Calculate the minimum amount of space that we need for each column for the current actual search
-	columnWidths := calculateColumnWidths(rows)
+	columnWidths := calculateColumnWidths(rows, len(columnNames))
 	totalWidth := 20
 	for i, name := range columnNames {
 		columnWidths[i] = max(columnWidths[i], len(name))
@@ -285,7 +293,7 @@ func makeTableColumns(ctx *context.Context, columnNames []string, rows []table.R
 		}
 		bigQueryResults = bigRows
 	}
-	maximumColumnWidths := calculateColumnWidths(bigQueryResults)
+	maximumColumnWidths := calculateColumnWidths(bigQueryResults, len(columnNames))
 
 	// Get the actual terminal width. If we're below this, opportunistically add some padding aiming for the maximum column widths
 	terminalWidth, _, err := getTerminalSize()

shared/testutils/testutils.go

@@ -67,7 +67,7 @@ func BackupAndRestoreWithId(t *testing.T, id string) func() {
 	}
 	for _, file := range renameFiles {
 		touchFile(file)
-		_ = os.Rename(file, getBackPath(file, id))
+		Check(t, os.Rename(file, getBackPath(file, id)))
 	}
 	copyFiles := []string{
 		path.Join(homedir, ".zshrc"),
@@ -76,13 +76,21 @@ func BackupAndRestoreWithId(t *testing.T, id string) func() {
 	}
 	for _, file := range copyFiles {
 		touchFile(file)
-		_ = copy(file, getBackPath(file, id))
+		Check(t, copy(file, getBackPath(file, id)))
 	}
 	configureZshrc(homedir)
 	touchFile(path.Join(homedir, ".bash_history"))
 	touchFile(path.Join(homedir, ".zsh_history"))
 	touchFile(path.Join(homedir, ".local/share/fish/fish_history"))
 	return func() {
+		if runtime.GOOS != "windows" {
+			cmd := exec.Command("killall", "hishtory", "tmux")
+			stdout, err := cmd.Output()
+			if err != nil && err.Error() != "exit status 1" {
+				t.Fatalf("failed to execute killall hishtory, stdout=%#v: %v", string(stdout), err)
+			}
+		}
+		Check(t, os.RemoveAll(path.Join(homedir, data.HISHTORY_PATH)))
 		Check(t, os.MkdirAll(path.Join(homedir, data.HISHTORY_PATH), os.ModePerm))
 		for _, file := range renameFiles {
 			checkError(os.Rename(getBackPath(file, id), file))
@@ -90,13 +98,6 @@ func BackupAndRestoreWithId(t *testing.T, id string) func() {
 		for _, file := range copyFiles {
 			checkError(copy(getBackPath(file, id), file))
 		}
-		if runtime.GOOS != "windows" {
-			cmd := exec.Command("killall", "hishtory")
-			stdout, err := cmd.Output()
-			if err != nil && err.Error() != "exit status 1" {
-				t.Fatalf("failed to execute killall hishtory, stdout=%#v: %v", string(stdout), err)
-			}
-		}
 		checkError(os.Chdir(initialWd))
 	}
 }
@@ -285,3 +286,15 @@ func MakeFakeHistoryEntry(command string) data.HistoryEntry {
 func IsGithubAction() bool {
 	return os.Getenv("GITHUB_ACTION") != ""
 }
+
+func TestLog(t *testing.T, line string) {
+	f, err := os.OpenFile("/tmp/test.log", os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0644)
+	if err != nil {
+		Check(t, err)
+	}
+	defer f.Close()
+	_, err = f.WriteString(line + "\n")
+	if err != nil {
+		Check(t, err)
+	}
+}