diff --git a/cmd/cheat/search_bench_test.go b/cmd/cheat/search_bench_test.go index 45ab0d9..08efc5a 100644 --- a/cmd/cheat/search_bench_test.go +++ b/cmd/cheat/search_bench_test.go @@ -9,7 +9,6 @@ import ( "os/exec" "path/filepath" "testing" - "time" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" @@ -108,23 +107,15 @@ cheatpaths: cmd := exec.Command(cheatBin, tc.args...) cmd.Env = env - // Capture output to prevent spamming var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr - start := time.Now() err := cmd.Run() - elapsed := time.Since(start) - if err != nil { b.Fatalf("Command failed: %v\nStderr: %s", err, stderr.String()) } - // Report custom metric - b.ReportMetric(float64(elapsed.Nanoseconds())/1e6, "ms/op") - - // Ensure we got some results if stdout.Len() == 0 { b.Fatal("No output from search") } diff --git a/internal/cheatpath/cheatpath_test.go b/internal/cheatpath/cheatpath_test.go index f8c06bf..0787dec 100644 --- a/internal/cheatpath/cheatpath_test.go +++ b/internal/cheatpath/cheatpath_test.go @@ -88,26 +88,3 @@ func TestCheatpathValidate(t *testing.T) { }) } } - -func TestCheatpathStruct(t *testing.T) { - // Test that the struct fields work as expected - cp := Cheatpath{ - Name: "test", - Path: "/test/path", - ReadOnly: true, - Tags: []string{"tag1", "tag2"}, - } - - if cp.Name != "test" { - t.Errorf("expected Name to be 'test', got %q", cp.Name) - } - if cp.Path != "/test/path" { - t.Errorf("expected Path to be '/test/path', got %q", cp.Path) - } - if !cp.ReadOnly { - t.Error("expected ReadOnly to be true") - } - if len(cp.Tags) != 2 || cp.Tags[0] != "tag1" || cp.Tags[1] != "tag2" { - t.Errorf("expected Tags to be [tag1 tag2], got %v", cp.Tags) - } -} diff --git a/internal/config/config_extended_test.go b/internal/config/config_extended_test.go index c997030..7060f40 100644 --- a/internal/config/config_extended_test.go +++ b/internal/config/config_extended_test.go @@ -3,7 +3,6 @@ package config import ( "os" "path/filepath" - "runtime" "testing" "github.com/cheat/cheat/internal/mock" @@ -19,7 +18,7 @@ func TestConfigYAMLErrors(t *testing.T) { defer os.RemoveAll(tempDir) invalidYAML := filepath.Join(tempDir, "invalid.yml") - err = os.WriteFile(invalidYAML, []byte("invalid: yaml: content:\n - no closing"), 0644) + err = os.WriteFile(invalidYAML, []byte("cheatpaths: [{unclosed\n"), 0644) if err != nil { t.Fatalf("failed to write invalid yaml: %v", err) } @@ -335,7 +334,10 @@ cheatpaths: } // Verify symlink was resolved - if len(conf.Cheatpaths) > 0 && conf.Cheatpaths[0].Path != targetDir { + if len(conf.Cheatpaths) == 0 { + t.Fatal("expected at least one cheatpath, got none") + } + if conf.Cheatpaths[0].Path != targetDir { t.Errorf("expected symlink to be resolved to %s, got %s", targetDir, conf.Cheatpaths[0].Path) } } @@ -380,70 +382,3 @@ cheatpaths: t.Errorf("expected broken cheatpath to be filtered out, got %d cheatpaths", len(conf.Cheatpaths)) } } - -// TestConfigTildeExpansionError tests tilde expansion error handling -func TestConfigTildeExpansionError(t *testing.T) { - // This is tricky to test without mocking homedir.Expand - // We'll create a config with an invalid home reference - tempDir, err := os.MkdirTemp("", "cheat-config-test-*") - if err != nil { - t.Fatalf("failed to create temp dir: %v", err) - } - defer os.RemoveAll(tempDir) - - // Create config with user that likely doesn't exist - configContent := `--- -editor: vim -cheatpaths: - - name: test - path: ~nonexistentuser12345/cheat - readonly: true -` - configFile := filepath.Join(tempDir, "config.yml") - err = os.WriteFile(configFile, []byte(configContent), 0644) - if err != nil { - t.Fatalf("failed to write config: %v", err) - } - - // Load config - this may or may not fail depending on the system - // but we're testing that it doesn't panic - _, _ = New(map[string]interface{}{}, configFile, false) -} - -// TestConfigGetCwdError tests error handling when os.Getwd fails -func TestConfigGetCwdError(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Windows does not allow removing the current directory") - } - - // This is difficult to test without being able to break os.Getwd - // We'll create a scenario where the current directory is removed - - // Create and enter a temp directory - tempDir, err := os.MkdirTemp("", "cheat-config-test-*") - if err != nil { - t.Fatalf("failed to create temp dir: %v", err) - } - - oldCwd, err := os.Getwd() - if err != nil { - t.Fatalf("failed to get cwd: %v", err) - } - defer os.Chdir(oldCwd) - - err = os.Chdir(tempDir) - if err != nil { - t.Fatalf("failed to change dir: %v", err) - } - - // Remove the directory we're in - err = os.RemoveAll(tempDir) - if err != nil { - t.Fatalf("failed to remove temp dir: %v", err) - } - - // Now os.Getwd should fail - _, err = New(map[string]interface{}{}, mock.Path("conf/empty.yml"), false) - // This might not fail on all systems, so we just ensure no panic - _ = err -} diff --git a/internal/config/init_test.go b/internal/config/init_test.go index 8017f23..adf1052 100644 --- a/internal/config/init_test.go +++ b/internal/config/init_test.go @@ -4,7 +4,6 @@ import ( "os" "path/filepath" "runtime" - "strings" "testing" ) @@ -90,9 +89,6 @@ func TestInitWriteError(t *testing.T) { if err == nil { t.Error("expected error when writing to invalid path, got nil") } - if err != nil && !strings.Contains(err.Error(), "failed to create") { - t.Errorf("expected 'failed to create' error, got: %v", err) - } } // TestInitExistingFile tests that Init overwrites existing files diff --git a/internal/config/pager_test.go b/internal/config/pager_test.go index fba3e04..96065d1 100644 --- a/internal/config/pager_test.go +++ b/internal/config/pager_test.go @@ -2,6 +2,7 @@ package config import ( "os" + "path/filepath" "runtime" "testing" ) @@ -44,29 +45,20 @@ func TestPager(t *testing.T) { os.Setenv("PAGER", "") pager := Pager() - // Should find one of the fallback pagers or return empty string + if pager == "" { + return // no pager found is acceptable + } + + // Should find one of the known fallback pagers validPagers := map[string]bool{ - "": true, // no pager found "pager": true, "less": true, "more": true, } - // Check if it's a path to one of these - found := false - for p := range validPagers { - if p == "" && pager == "" { - found = true - break - } - if p != "" && (pager == p || len(pager) >= len(p) && pager[len(pager)-len(p):] == p) { - found = true - break - } - } - - if !found { - t.Errorf("unexpected pager value: %s", pager) + base := filepath.Base(pager) + if !validPagers[base] { + t.Errorf("unexpected pager value: %s (base: %s)", pager, base) } }) diff --git a/internal/config/validate_test.go b/internal/config/validate_test.go index d5f37bd..16827da 100644 --- a/internal/config/validate_test.go +++ b/internal/config/validate_test.go @@ -71,19 +71,28 @@ func TestInvalidateMissingCheatpaths(t *testing.T) { } } -// TestMissingInvalidFormatters asserts that configs which contain invalid +// TestInvalidateInvalidFormatter asserts that configs which contain invalid // formatters are invalidated -func TestMissingInvalidFormatters(t *testing.T) { +func TestInvalidateInvalidFormatter(t *testing.T) { - // mock a config + // mock a config with a valid editor and cheatpaths but invalid formatter conf := Config{ - Colorize: true, - Editor: "vim", + Colorize: true, + Editor: "vim", + Formatter: "html", + Cheatpaths: []cheatpath.Cheatpath{ + cheatpath.Cheatpath{ + Name: "foo", + Path: "/foo", + ReadOnly: false, + Tags: []string{}, + }, + }, } - // assert that no errors are returned + // assert that the config is invalidated due to the formatter if err := conf.Validate(); err == nil { - t.Errorf("failed to invalidate config without formatter") + t.Errorf("failed to invalidate config with invalid formatter") } } diff --git a/internal/installer/run_test.go b/internal/installer/run_test.go index ef32469..729c805 100644 --- a/internal/installer/run_test.go +++ b/internal/installer/run_test.go @@ -1,7 +1,6 @@ package installer import ( - "fmt" "io" "os" "path/filepath" @@ -245,10 +244,10 @@ cheatpaths: if strings.Contains(contentStr, "PERSONAL_PATH") { t.Error("PERSONAL_PATH was not replaced") } - if strings.Contains(contentStr, "EDITOR_PATH") && !strings.Contains(contentStr, fmt.Sprintf("editor: %s", "")) { + if strings.Contains(contentStr, "EDITOR_PATH") { t.Error("EDITOR_PATH was not replaced") } - if strings.Contains(contentStr, "PAGER_PATH") && !strings.Contains(contentStr, fmt.Sprintf("pager: %s", "")) { + if strings.Contains(contentStr, "PAGER_PATH") { t.Error("PAGER_PATH was not replaced") } if strings.Contains(contentStr, "WORK_PATH") { diff --git a/internal/sheet/colorize_test.go b/internal/sheet/colorize_test.go index 7e86248..553a2e7 100644 --- a/internal/sheet/colorize_test.go +++ b/internal/sheet/colorize_test.go @@ -1,6 +1,7 @@ package sheet import ( + "strings" "testing" "github.com/cheat/cheat/internal/config" @@ -16,45 +17,26 @@ func TestColorize(t *testing.T) { } // mock a sheet + original := "echo 'foo'" s := Sheet{ - Text: "echo 'foo'", + Text: original, } // colorize the sheet text s.Colorize(conf) - // initialize expectations - want := "echo" - want += " 'foo'" + // assert that the text was modified (colorization applied) + if s.Text == original { + t.Error("Colorize did not modify sheet text") + } - // assert - if s.Text != want { - t.Errorf("failed to colorize sheet: want: %s, got: %s", want, s.Text) + // assert that ANSI escape codes are present + if !strings.Contains(s.Text, "\x1b[") && !strings.Contains(s.Text, "[0m") { + t.Errorf("colorized text does not contain ANSI escape codes: %q", s.Text) + } + + // assert that the original content is still present within the colorized output + if !strings.Contains(s.Text, "echo") || !strings.Contains(s.Text, "foo") { + t.Errorf("colorized text lost original content: %q", s.Text) } } - -// TestColorizeError tests the error handling in Colorize -func TestColorizeError(_ *testing.T) { - // Create a sheet with content - sheet := Sheet{ - Text: "some text", - Syntax: "invalidlexer12345", // Use an invalid lexer that might cause issues - } - - // Create a config with invalid formatter/style - conf := config.Config{ - Formatter: "invalidformatter", - Style: "invalidstyle", - } - - // Store original text - originalText := sheet.Text - - // Colorize should not panic even with invalid settings - sheet.Colorize(conf) - - // The text might be unchanged if there was an error, or it might be colorized - // We're mainly testing that it doesn't panic - _ = sheet.Text - _ = originalText -} diff --git a/internal/sheet/copy_error_test.go b/internal/sheet/copy_error_test.go index ea0d001..1104285 100644 --- a/internal/sheet/copy_error_test.go +++ b/internal/sheet/copy_error_test.go @@ -10,15 +10,12 @@ import ( // TestCopyErrors tests error cases for the Copy method func TestCopyErrors(t *testing.T) { tests := []struct { - name string - setup func() (*Sheet, string, func()) - wantErr bool - errMsg string + name string + setup func() (*Sheet, string, func()) }{ { name: "source file does not exist", setup: func() (*Sheet, string, func()) { - // Create a sheet with non-existent path sheet := &Sheet{ Title: "test", Path: "/non/existent/file.txt", @@ -30,13 +27,10 @@ func TestCopyErrors(t *testing.T) { } return sheet, dest, cleanup }, - wantErr: true, - errMsg: "failed to open cheatsheet", }, { name: "destination directory creation fails", setup: func() (*Sheet, string, func()) { - // Create a source file src, err := os.CreateTemp("", "copy-test-src-*") if err != nil { t.Fatalf("failed to create temp file: %v", err) @@ -50,13 +44,11 @@ func TestCopyErrors(t *testing.T) { CheatPath: "test", } - // Create a file where we want a directory blockerFile := filepath.Join(os.TempDir(), "copy-blocker-file") if err := os.WriteFile(blockerFile, []byte("blocker"), 0644); err != nil { t.Fatalf("failed to create blocker file: %v", err) } - // Try to create dest under the blocker file (will fail) dest := filepath.Join(blockerFile, "subdir", "dest.txt") cleanup := func() { @@ -65,13 +57,10 @@ func TestCopyErrors(t *testing.T) { } return sheet, dest, cleanup }, - wantErr: true, - errMsg: "failed to create directory", }, { name: "destination file creation fails", setup: func() (*Sheet, string, func()) { - // Create a source file src, err := os.CreateTemp("", "copy-test-src-*") if err != nil { t.Fatalf("failed to create temp file: %v", err) @@ -85,7 +74,6 @@ func TestCopyErrors(t *testing.T) { CheatPath: "test", } - // Create a directory where we want the file destDir := filepath.Join(os.TempDir(), "copy-test-dir") if err := os.Mkdir(destDir, 0755); err != nil && !os.IsExist(err) { t.Fatalf("failed to create dest dir: %v", err) @@ -97,8 +85,6 @@ func TestCopyErrors(t *testing.T) { } return sheet, destDir, cleanup }, - wantErr: true, - errMsg: "failed to create outfile", }, } @@ -108,43 +94,27 @@ func TestCopyErrors(t *testing.T) { defer cleanup() err := sheet.Copy(dest) - if (err != nil) != tt.wantErr { - t.Errorf("Copy() error = %v, wantErr %v", err, tt.wantErr) - return - } - if err != nil && tt.errMsg != "" { - if !contains(err.Error(), tt.errMsg) { - t.Errorf("Copy() error = %v, want error containing %q", err, tt.errMsg) - } + if err == nil { + t.Error("Copy() expected error, got nil") } }) } } -// TestCopyIOError tests the io.Copy error case -func TestCopyIOError(t *testing.T) { - // This is difficult to test without mocking io.Copy - // The error case would occur if the source file is modified - // or removed after opening but before copying - t.Skip("Skipping io.Copy error test - requires file system race condition") -} - -// TestCopyCleanupOnError verifies that partially written files are cleaned up on error -func TestCopyCleanupOnError(t *testing.T) { +// TestCopyUnreadableSource verifies that Copy returns an error when the source +// file cannot be opened (e.g., permission denied). +func TestCopyUnreadableSource(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("chmod does not restrict reads on Windows") } - // Create a source file that we'll make unreadable after opening - src, err := os.CreateTemp("", "copy-test-cleanup-*") + src, err := os.CreateTemp("", "copy-test-unreadable-*") if err != nil { t.Fatalf("failed to create temp file: %v", err) } defer os.Remove(src.Name()) - // Write some content - content := "test content for cleanup" - if _, err := src.WriteString(content); err != nil { + if _, err := src.WriteString("test content"); err != nil { t.Fatalf("failed to write content: %v", err) } src.Close() @@ -155,38 +125,21 @@ func TestCopyCleanupOnError(t *testing.T) { CheatPath: "test", } - // Destination path - dest := filepath.Join(os.TempDir(), "copy-cleanup-test.txt") - defer os.Remove(dest) // Clean up if test fails + dest := filepath.Join(os.TempDir(), "copy-unreadable-test.txt") + defer os.Remove(dest) - // Make the source file unreadable (simulating a read error during copy) - // This is platform-specific, but should work on Unix-like systems if err := os.Chmod(src.Name(), 0000); err != nil { t.Skip("Cannot change file permissions on this platform") } - defer os.Chmod(src.Name(), 0644) // Restore permissions for cleanup + defer os.Chmod(src.Name(), 0644) - // Attempt to copy - this should fail during io.Copy err = sheet.Copy(dest) if err == nil { - t.Error("Expected Copy to fail with permission error") + t.Error("expected Copy to fail with permission error") } - // Verify the destination file was cleaned up + // Destination should not exist since the error occurs before it is created if _, err := os.Stat(dest); !os.IsNotExist(err) { - t.Error("Destination file should have been removed after copy failure") + t.Error("destination file should not exist after open failure") } } - -func contains(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsHelper(s, substr)) -} - -func containsHelper(s, substr string) bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false -} diff --git a/internal/sheet/parse_test.go b/internal/sheet/parse_test.go index ca0925c..b43f46e 100644 --- a/internal/sheet/parse_test.go +++ b/internal/sheet/parse_test.go @@ -38,7 +38,7 @@ To foo the bar: baz` t.Errorf("failed to parse tags: want: %s, got: %s", want, fm.Tags[0]) } if len(fm.Tags) != 1 { - t.Errorf("failed to parse tags: want: len 0, got: len %d", len(fm.Tags)) + t.Errorf("failed to parse tags: want: len 1, got: len %d", len(fm.Tags)) } } diff --git a/internal/sheet/search_fuzz_test.go b/internal/sheet/search_fuzz_test.go index 79c98e6..d6a3c3b 100644 --- a/internal/sheet/search_fuzz_test.go +++ b/internal/sheet/search_fuzz_test.go @@ -122,69 +122,3 @@ func FuzzSearchRegex(f *testing.F) { } }) } - -// FuzzSearchCatastrophicBacktracking specifically tests for regex patterns -// that could cause performance issues -func FuzzSearchCatastrophicBacktracking(f *testing.F) { - // Seed with patterns known to potentially cause issues - f.Add("a", 10, 5) - f.Add("x", 20, 3) - - f.Fuzz(func(t *testing.T, char string, repeats int, groups int) { - // Limit the size to avoid memory issues in the test - if repeats > 30 || repeats < 0 || groups > 10 || groups < 0 || len(char) > 5 { - t.Skip("Skipping invalid or overly large test case") - } - - // Construct patterns that might cause backtracking - patterns := []string{ - strings.Repeat(char, repeats), - "(" + char + "+)+", - "(" + char + "*)*", - "(" + char + "|" + char + ")*", - } - - // Add nested groups - if groups > 0 && groups < 10 { - nested := char - for i := 0; i < groups; i++ { - nested = "(" + nested + ")+" - } - patterns = append(patterns, nested) - } - - // Test text that might trigger backtracking - testText := strings.Repeat(char, repeats) + "x" - - for _, pattern := range patterns { - // Try to compile the pattern - reg, err := regexp.Compile(pattern) - if err != nil { - // Invalid pattern, skip - continue - } - - // Test with timeout - done := make(chan bool, 1) - - go func() { - defer func() { - if r := recover(); r != nil { - t.Errorf("Search panicked with backtracking pattern %q: %v", pattern, r) - } - done <- true - }() - - sheet := Sheet{Text: testText} - _ = sheet.Search(reg) - }() - - select { - case <-done: - // Completed successfully - case <-time.After(50 * time.Millisecond): - t.Logf("Warning: potential backtracking issue with pattern %q (completed slowly)", pattern) - } - } - }) -} diff --git a/internal/sheets/filter_test.go b/internal/sheets/filter_test.go index 9e0f33d..82f537a 100644 --- a/internal/sheets/filter_test.go +++ b/internal/sheets/filter_test.go @@ -18,28 +18,26 @@ func TestFilterSingleTag(t *testing.T) { map[string]sheet.Sheet{ "foo": sheet.Sheet{Title: "foo", Tags: []string{"alpha", "bravo"}}, - "bar": sheet.Sheet{Title: "bar", Tags: []string{"bravo", "charlie"}}, + "bar": sheet.Sheet{Title: "bar", Tags: []string{"charlie"}}, }, map[string]sheet.Sheet{ - "baz": sheet.Sheet{Title: "baz", Tags: []string{"alpha", "bravo"}}, + "baz": sheet.Sheet{Title: "baz", Tags: []string{"alpha"}}, "bat": sheet.Sheet{Title: "bat", Tags: []string{"bravo", "charlie"}}, }, } // filter the cheatsheets - filtered := Filter(cheatpaths, []string{"bravo"}) + filtered := Filter(cheatpaths, []string{"alpha"}) // assert that the expect results were returned want := []map[string]sheet.Sheet{ map[string]sheet.Sheet{ "foo": sheet.Sheet{Title: "foo", Tags: []string{"alpha", "bravo"}}, - "bar": sheet.Sheet{Title: "bar", Tags: []string{"bravo", "charlie"}}, }, map[string]sheet.Sheet{ - "baz": sheet.Sheet{Title: "baz", Tags: []string{"alpha", "bravo"}}, - "bat": sheet.Sheet{Title: "bat", Tags: []string{"bravo", "charlie"}}, + "baz": sheet.Sheet{Title: "baz", Tags: []string{"alpha"}}, }, }