Replace log.Fatal in tests, add t.Helper, cover NewFile

file_handling_test.go

@@ -1,6 +1,80 @@
 package main
 
-import "testing"
+import (
+	"path/filepath"
+	"testing"
+)
 
+// TestNewFile exercises NewFile's path-resolution behavior. It does NOT cover
+// the filepath.Abs error path: NewFile currently calls log.Fatalf on that
+// failure, which would kill the test binary, and that surface is not reachable
+// on most platforms in any case. The error path will become testable when
+// NewFile is refactored to return an error (see issue #6).
 func TestNewFile(t *testing.T) {
+	tmp := t.TempDir()
+
+	tests := []struct {
+		name string
+		// input is the raw path passed to NewFile.
+		input string
+		// want is the absolute path the resulting *File should expose. If
+		// empty, the test computes the expected value via filepath.Abs(input)
+		// at runtime (useful for inputs that are inherently relative to the
+		// test process's working directory).
+		want string
+	}{
+		{
+			name:  "absolute path is returned cleaned",
+			input: filepath.Join(tmp, "foo"),
+			want:  filepath.Join(tmp, "foo"),
+		},
+		{
+			name:  "absolute path with redundant separators is cleaned",
+			input: tmp + "//foo///bar",
+			want:  filepath.Join(tmp, "foo", "bar"),
+		},
+		{
+			name:  "absolute path with .. is resolved",
+			input: filepath.Join(tmp, "a", "..", "b"),
+			want:  filepath.Join(tmp, "b"),
+		},
+		{
+			name:  "relative path is resolved to absolute",
+			input: "relative/path",
+			// want is computed below because it depends on the test
+			// process's working directory.
+		},
+		{
+			name:  "relative path with .. is resolved",
+			input: "a/../b",
+		},
+		{
+			name:  "dot is resolved to the working directory",
+			input: ".",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			want := tc.want
+			if want == "" {
+				abs, err := filepath.Abs(tc.input)
+				if err != nil {
+					t.Fatalf("filepath.Abs(%q) returned unexpected error: %v", tc.input, err)
+				}
+				want = abs
+			}
+
+			got := NewFile(tc.input)
+			if got == nil {
+				t.Fatalf("NewFile(%q) returned nil", tc.input)
+			}
+			if got.Path != want {
+				t.Errorf("NewFile(%q).Path = %q; want %q", tc.input, got.Path, want)
+			}
+			if !filepath.IsAbs(got.Path) {
+				t.Errorf("NewFile(%q).Path = %q; want an absolute path", tc.input, got.Path)
+			}
+		})
+	}
 }

find_replace_test.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"errors"
-	"log"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -19,18 +18,19 @@ import (
 // If a baseName is not provided, a random file name is generated. Returns the
 // directory where the file was created, the file's directory entry, and the
 // actual name of the file.
-func newTestFile(path string, baseName string, content string) *File {
+func newTestFile(tb testing.TB, path string, baseName string, content string) *File {
+	tb.Helper()
 	f, err := os.CreateTemp(path, baseName)
 	if err != nil {
-		log.Fatal(err)
+		tb.Fatalf("CreateTemp(%q, %q): %v", path, baseName, err)
 	}
 	if _, err := f.Write([]byte(content)); err != nil {
 		defer os.Remove(f.Name())
-		log.Fatal(err)
+		tb.Fatalf("write to %v: %v", f.Name(), err)
 	}
 	if err := f.Close(); err != nil {
 		defer os.Remove(f.Name())
-		log.Fatal(err)
+		tb.Fatalf("close %v: %v", f.Name(), err)
 	}
 
 	return NewFile(f.Name())
@@ -41,10 +41,11 @@ func newTestFile(path string, baseName string, content string) *File {
 // a baseName is not provided, a random file name is generated. Returns the
 // directory where the file was created, the file's directory entry, and the
 // actual name of the file.
-func newTestDir(path string, baseName string) *File {
+func newTestDir(tb testing.TB, path string, baseName string) *File {
+	tb.Helper()
 	dirPath, err := os.MkdirTemp(path, baseName)
 	if err != nil {
-		log.Fatal(err)
+		tb.Fatalf("MkdirTemp(%q, %q): %v", path, baseName, err)
 	}
 	return NewFile(dirPath)
 }
@@ -59,13 +60,15 @@ func expectedPathAfterRename(f *File, fr *findReplace) string {
 
 // assertFileExists ensures that the given File exists
 func assertFileExists(t *testing.T, f *File) {
+	t.Helper()
 	if _, err := os.Stat(f.Path); errors.Is(err, os.ErrNotExist) {
 		t.Errorf("test file %v does not exist", f.Path)
 	}
 }
 
 // assertFileNonexistent ensures that the File does not exist
 func assertFileNonexistent(t *testing.T, f *File) {
+	t.Helper()
 	if _, err := os.Stat(f.Path); !errors.Is(err, os.ErrNotExist) {
 		if err == nil {
 			t.Errorf("test file %v exists", f.Path)
@@ -76,6 +79,7 @@ func assertFileNonexistent(t *testing.T, f *File) {
 }
 
 func assertPathExistsAfterRename(t *testing.T, f *File, expectedPath string) *File {
+	t.Helper()
 	assertFileNonexistent(t, f)
 	newFile := NewFile(expectedPath)
 	assertFileExists(t, newFile)
@@ -95,50 +99,50 @@ func TestWalkDir(t *testing.T) {
 	find := "wh"
 	replace := "f"
 
-	d := newTestDir("", "*")
+	d := newTestDir(t, "", "*")
 	defer os.Remove(d.Path)
 
 	// d1: who/
-	d1 := newTestDir(d.Path, "who")
+	d1 := newTestDir(t, d.Path, "who")
 	defer os.Remove(d1.Path)
 
 	// d1d1: who/what/
-	d1d1 := newTestDir(d1.Path, "what")
+	d1d1 := newTestDir(t, d1.Path, "what")
 	defer os.Remove(d1d1.Path)
 
 	// d1d1f1: who/what/when (contains "where")
 	d1d1f1Contents := "where"
-	d1d1f1 := newTestFile(d1d1.Path, "when", d1d1f1Contents)
+	d1d1f1 := newTestFile(t, d1d1.Path, "when", d1d1f1Contents)
 	defer os.Remove(d1d1f1.Path)
 
 	// d2: what/
-	d2 := newTestDir(d.Path, "what")
+	d2 := newTestDir(t, d.Path, "what")
 	defer os.Remove(d2.Path)
 
 	// d2d1: what/when/
-	d2d1 := newTestDir(d2.Path, "when")
+	d2d1 := newTestDir(t, d2.Path, "when")
 	defer os.Remove(d2d1.Path)
 
 	// d2d1d1: what/when/where (directories with no files)
-	d2d1d1 := newTestDir(d2d1.Path, "where")
+	d2d1d1 := newTestDir(t, d2d1.Path, "where")
 	defer os.Remove(d2d1d1.Path)
 
 	// d3: when/
-	d3 := newTestDir(d.Path, "when")
+	d3 := newTestDir(t, d.Path, "when")
 	defer os.Remove(d3.Path)
 
 	// d3f1: when/where (contains "why")
 	d3f1Contents := "why"
-	d3f1 := newTestFile(d3.Path, "where", d3f1Contents)
+	d3f1 := newTestFile(t, d3.Path, "where", d3f1Contents)
 	defer os.Remove(d3f1.Path)
 
 	// d4: where/ (empty directory in base dir)
-	d4 := newTestDir(d.Path, "where")
+	d4 := newTestDir(t, d.Path, "where")
 	defer os.Remove(d4.Path)
 
 	// f1: why (file in base dir contains "wh")
 	f1Contents := "wh\nwh\nwh\n"
-	f1 := newTestFile(d.Path, "why", f1Contents)
+	f1 := newTestFile(t, d.Path, "why", f1Contents)
 	defer os.Remove(f1.Path)
 
 	fr := findReplace{find: find, replace: replace}
@@ -193,7 +197,7 @@ func TestHandleFileWithDir(t *testing.T) {
 	find := "ph"
 	replace := "f"
 
-	f := newTestDir("", initial)
+	f := newTestDir(t, "", initial)
 	defer os.Remove(f.Path)
 	expectedPath := filepath.Join(f.Dir(), strings.Replace(f.Base(), find, replace, -1))
 	defer os.Remove(expectedPath)
@@ -211,7 +215,7 @@ func TestHandleFileWithIgnoredDir(t *testing.T) {
 
 	dirPath := filepath.Join(os.TempDir(), initial)
 	if err := os.Mkdir(dirPath, 0700); err != nil {
-		log.Fatal(err)
+		t.Fatalf("Mkdir(%q): %v", dirPath, err)
 	}
 	f := NewFile(dirPath)
 	defer os.Remove(f.Path)
@@ -233,7 +237,7 @@ func TestHandleFileWithFile(t *testing.T) {
 	replace := "f"
 	want := "alfa"
 
-	f := newTestFile("", initial, initial)
+	f := newTestFile(t, "", initial, initial)
 	defer os.Remove(f.Path)
 	expectedName := strings.Replace(f.Base(), find, replace, -1)
 	expectedPath := filepath.Join(f.Dir(), expectedName)
@@ -255,7 +259,7 @@ func TestRenameFile(t *testing.T) {
 	find := "ph"
 	replace := "f"
 
-	f := newTestFile("", initial, "")
+	f := newTestFile(t, "", initial, "")
 	defer os.Remove(f.Path)
 	expectedName := strings.Replace(f.Base(), find, replace, -1)
 	expectedPath := filepath.Join(f.Dir(), expectedName)
@@ -270,6 +274,7 @@ func TestRenameFile(t *testing.T) {
 // assertNewContentsOfFile ensures that the contents of the file at the given
 // path exactly match the desired string.
 func assertNewContentsOfFile(t *testing.T, path string, initial string, find string, replace string, want string) {
+	t.Helper()
 	got := NewFile(path).Read()
 	if got != want {
 		t.Errorf("replace %v with %v in %v, but got %v; want %v", find, replace, initial, got, want)
@@ -282,7 +287,7 @@ func TestReplaceContents(t *testing.T) {
 	replace := "f"
 	want := "alfa"
 
-	f := newTestFile("", "*", initial)
+	f := newTestFile(t, "", "*", initial)
 	defer os.Remove(f.Path)
 	fr := findReplace{find: find, replace: replace}
 	fr.ReplaceContents(f)
@@ -295,7 +300,7 @@ func TestReplaceContentsEntireFile(t *testing.T) {
 	replace := "beta"
 	want := "beta"
 
-	f := newTestFile("", "*", initial)
+	f := newTestFile(t, "", "*", initial)
 	defer os.Remove(f.Path)
 	fr := findReplace{find: find, replace: replace}
 	fr.ReplaceContents(f)
@@ -308,7 +313,7 @@ func TestReplaceContentsMultipleMatchesSingleLine(t *testing.T) {
 	replace := "f"
 	want := "alfaalfa"
 
-	f := newTestFile("", "*", initial)
+	f := newTestFile(t, "", "*", initial)
 	defer os.Remove(f.Path)
 	fr := findReplace{find: find, replace: replace}
 	fr.ReplaceContents(f)
@@ -321,7 +326,7 @@ func TestReplaceContentsMultipleMatchesMultipleLines(t *testing.T) {
 	replace := "f"
 	want := "alfa\nalfa"
 
-	f := newTestFile("", "*", initial)
+	f := newTestFile(t, "", "*", initial)
 	defer os.Remove(f.Path)
 	fr := findReplace{find: find, replace: replace}
 	fr.ReplaceContents(f)
@@ -334,15 +339,16 @@ func TestReplaceContentsNoMatches(t *testing.T) {
 	replace := "xyz"
 	want := "alpha"
 
-	f := newTestFile("", "*", initial)
+	f := newTestFile(t, "", "*", initial)
 	defer os.Remove(f.Path)
 	fr := findReplace{find: find, replace: replace}
 	fr.ReplaceContents(f)
 	assertNewContentsOfFile(t, f.Path, initial, find, replace, want)
 }
 
 func CloneRepoToTestDir(b *testing.B, repoUrl string) *File {
-	d := newTestDir("", "*")
+	b.Helper()
+	d := newTestDir(b, "", "*")
 	defer os.Remove(d.Path)
 
 	cmd := exec.Command("git", "clone", "--depth=1", "--single-branch", repoUrl, ".")

strings_test.go

@@ -7,6 +7,7 @@ import (
 // assertRandomStringLength ensures that the generated string matches the
 // desired length.
 func assertRandomStringLength(t *testing.T, ask int, want int) {
+	t.Helper()
 	got := len(RandomString(ask))
 	if got != want {
 		t.Errorf("len(RandomString(%v)) = %v; want %v", ask, got, want)