mirror of
https://github.com/cheat/cheat.git
synced 2026-03-07 19:23:34 +01:00
test: audit and clean up test suite
Delete tautological, no-assertion, and permanently-skipped tests. Rewrite false-confidence tests that couldn't detect the bugs they claimed to test. Decouple brittle assertions from error message strings and third-party library output. Fix misleading test names and error messages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -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 := "[38;2;181;137;0mecho[0m[38;2;147;161;161m"
|
||||
want += " [0m[38;2;42;161;152m'foo'[0m"
|
||||
// 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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user