Merge pull request #19 from Snider/improve-test-coverage

Improve test coverage and fix cmd tests

cmd/run_test.go

@@ -110,3 +110,13 @@ func createDummyTim(t *testing.T) string {
 	}
 	return timPath
 }
+
+// TestHelperProcess isn't a real test. It's used as a helper for tests that need to mock exec.Command.
+func TestHelperProcess(t *testing.T) {
+	if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
+		return
+	}
+	// The rest of the arguments are the command and its arguments.
+	// In our case, we don't need to do anything with them.
+	os.Exit(0)
+}

pkg/datanode/datanode_test.go

@@ -1,7 +1,10 @@
 package datanode
 
 import (
+	"archive/tar"
+	"bytes"
 	"errors"
+	"io"
 	"io/fs"
 	"os"
 	"path/filepath"
@@ -348,6 +351,65 @@ func TestWalk_Ugly(t *testing.T) {
 	}
 }
 
+func TestWalk_Options(t *testing.T) {
+	dn := New()
+	dn.AddData("root.txt", []byte("root"))
+	dn.AddData("a/a1.txt", []byte("a1"))
+	dn.AddData("a/b/b1.txt", []byte("b1"))
+	dn.AddData("c/c1.txt", []byte("c1"))
+
+	t.Run("MaxDepth", func(t *testing.T) {
+		var paths []string
+		err := dn.Walk(".", func(path string, d fs.DirEntry, err error) error {
+			paths = append(paths, path)
+			return nil
+		}, WalkOptions{MaxDepth: 1})
+		if err != nil {
+			t.Fatalf("Walk failed: %v", err)
+		}
+		expected := []string{".", "a", "c", "root.txt"}
+		sort.Strings(paths)
+		if !reflect.DeepEqual(paths, expected) {
+			t.Errorf("expected paths %v, got %v", expected, paths)
+		}
+	})
+
+	t.Run("Filter", func(t *testing.T) {
+		var paths []string
+		err := dn.Walk(".", func(path string, d fs.DirEntry, err error) error {
+			paths = append(paths, path)
+			return nil
+		}, WalkOptions{Filter: func(path string, d fs.DirEntry) bool {
+			return !strings.HasPrefix(path, "a")
+		}})
+		if err != nil {
+			t.Fatalf("Walk failed: %v", err)
+		}
+		expected := []string{".", "c", "c/c1.txt", "root.txt"}
+		sort.Strings(paths)
+		if !reflect.DeepEqual(paths, expected) {
+			t.Errorf("expected paths %v, got %v", expected, paths)
+		}
+	})
+
+	t.Run("SkipErrors", func(t *testing.T) {
+		// Mock a walk failure by passing a non-existent root with SkipErrors.
+		// Normally, WalkDir calls fn with an error for the root if it doesn't exist.
+		var called bool
+		err := dn.Walk("nonexistent", func(path string, d fs.DirEntry, err error) error {
+			called = true
+			return err
+		}, WalkOptions{SkipErrors: true})
+
+		if err != nil {
+			t.Errorf("expected no error with SkipErrors, got %v", err)
+		}
+		if called {
+			t.Error("callback should NOT be called if error is skipped internally")
+		}
+	})
+}
+
 func TestCopyFile_Good(t *testing.T) {
 	dn := New()
 	dn.AddData("foo.txt", []byte("foo"))
@@ -397,6 +459,127 @@ func TestCopyFile_Ugly(t *testing.T) {
 	}
 }
 
+func TestToTar_Good(t *testing.T) {
+	dn := New()
+	dn.AddData("foo.txt", []byte("foo"))
+	dn.AddData("bar/baz.txt", []byte("baz"))
+
+	tarball, err := dn.ToTar()
+	if err != nil {
+		t.Fatalf("ToTar failed: %v", err)
+	}
+	if len(tarball) == 0 {
+		t.Fatal("expected non-empty tarball")
+	}
+
+	// Verify tar content
+	tr := tar.NewReader(bytes.NewReader(tarball))
+	files := make(map[string]string)
+	for {
+		header, err := tr.Next()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			t.Fatalf("tar.Next failed: %v", err)
+		}
+		content, err := io.ReadAll(tr)
+		if err != nil {
+			t.Fatalf("read tar content failed: %v", err)
+		}
+		files[header.Name] = string(content)
+	}
+
+	if files["foo.txt"] != "foo" {
+		t.Errorf("expected foo.txt content 'foo', got %q", files["foo.txt"])
+	}
+	if files["bar/baz.txt"] != "baz" {
+		t.Errorf("expected bar/baz.txt content 'baz', got %q", files["bar/baz.txt"])
+	}
+}
+
+func TestFromTar_Good(t *testing.T) {
+	// Create a tarball
+	buf := new(bytes.Buffer)
+	tw := tar.NewWriter(buf)
+
+	files := []struct{ Name, Body string }{
+		{"foo.txt", "foo"},
+		{"bar/baz.txt", "baz"},
+	}
+	for _, file := range files {
+		hdr := &tar.Header{
+			Name: file.Name,
+			Mode: 0600,
+			Size: int64(len(file.Body)),
+			Typeflag: tar.TypeReg,
+		}
+		if err := tw.WriteHeader(hdr); err != nil {
+			t.Fatalf("WriteHeader failed: %v", err)
+		}
+		if _, err := tw.Write([]byte(file.Body)); err != nil {
+			t.Fatalf("Write failed: %v", err)
+		}
+	}
+	if err := tw.Close(); err != nil {
+		t.Fatalf("Close failed: %v", err)
+	}
+
+	dn, err := FromTar(buf.Bytes())
+	if err != nil {
+		t.Fatalf("FromTar failed: %v", err)
+	}
+
+	// Verify DataNode content
+	exists, _ := dn.Exists("foo.txt")
+	if !exists {
+		t.Error("foo.txt missing")
+	}
+	exists, _ = dn.Exists("bar/baz.txt")
+	if !exists {
+		t.Error("bar/baz.txt missing")
+	}
+}
+
+func TestTarRoundTrip_Good(t *testing.T) {
+	dn1 := New()
+	dn1.AddData("a.txt", []byte("a"))
+	dn1.AddData("b/c.txt", []byte("c"))
+
+	tarball, err := dn1.ToTar()
+	if err != nil {
+		t.Fatalf("ToTar failed: %v", err)
+	}
+
+	dn2, err := FromTar(tarball)
+	if err != nil {
+		t.Fatalf("FromTar failed: %v", err)
+	}
+
+	// Verify dn2 matches dn1
+	exists, _ := dn2.Exists("a.txt")
+	if !exists {
+		t.Error("a.txt missing in dn2")
+	}
+	exists, _ = dn2.Exists("b/c.txt")
+	if !exists {
+		t.Error("b/c.txt missing in dn2")
+	}
+}
+
+func TestFromTar_Bad(t *testing.T) {
+	// Pass invalid data (truncated header)
+	// A valid tar header is 512 bytes.
+	truncated := make([]byte, 100)
+	_, err := FromTar(truncated)
+	if err == nil {
+		t.Error("expected error for truncated tar header, got nil")
+	} else if err != io.EOF && err != io.ErrUnexpectedEOF {
+		// Verify it's some sort of read error or EOF related
+		// Depending on implementation details of archive/tar
+	}
+}
+
 func toSortedNames(entries []fs.DirEntry) []string {
 	var names []string
 	for _, e := range entries {

pkg/tim/run_test.go

@@ -4,6 +4,7 @@ import (
 	"archive/tar"
 	"os"
 	"os/exec"
+	"strings"
 	"testing"
 )
 
@@ -31,6 +32,72 @@ func TestRun(t *testing.T) {
 	}
 }
 
+func TestRun_BadInput(t *testing.T) {
+	// Test non-existent file
+	err := Run("nonexistent.tim")
+	if err == nil {
+		t.Error("expected error for nonexistent file, got nil")
+	}
+
+	// Test invalid file (not a tar)
+	f, err := os.CreateTemp("", "bad-tim-*.tim")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(f.Name())
+	f.Write([]byte("not a tar file"))
+	f.Close()
+
+	// Mock ExecCommand to fail if run called
+	origExecCommand := ExecCommand
+	ExecCommand = func(command string, args ...string) *exec.Cmd {
+		// If we reach here, it means we tried to run runc.
+		// For a bad tar, we might still reach here.
+		// Let's just return a command that fails.
+		return exec.Command("false")
+	}
+	t.Cleanup(func() { ExecCommand = origExecCommand })
+
+	err = Run(f.Name())
+	if err == nil {
+		t.Error("expected error when running bad tim file")
+	}
+}
+
+func TestRun_ZipSlip(t *testing.T) {
+	// Create a malicious tim file with ../ path
+	file, err := os.CreateTemp("", "zipslip-*.tim")
+	if err != nil {
+		t.Fatalf("failed to create temp file: %v", err)
+	}
+	defer os.Remove(file.Name())
+	defer file.Close()
+
+	tw := tar.NewWriter(file)
+
+	hdr := &tar.Header{
+		Name: "../evil.txt",
+		Mode: 0600,
+		Size: 4,
+		Typeflag: tar.TypeReg,
+	}
+	if err := tw.WriteHeader(hdr); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := tw.Write([]byte("evil")); err != nil {
+		t.Fatal(err)
+	}
+	tw.Close()
+
+	err = Run(file.Name())
+	if err == nil {
+		t.Fatal("expected error for zip slip attempt, got nil")
+	}
+	if !strings.Contains(err.Error(), "invalid file path") {
+		t.Errorf("expected 'invalid file path' error, got: %v", err)
+	}
+}
+
 // createDummyTim creates a valid, empty tim file for testing.
 func createDummyTim(t *testing.T) string {
 	t.Helper()