Improve test coverage to exceed 90%

- Add tests for error handling in FTP container
- Add more comprehensive tests for file operations
- Enhance testing for stdout/stderr capture
- Add tests for large outputs and binary content
- Add tests for directory operations

Coverage: 90.1% for main package, 80.6% for containers

Author: umputun

capture_test.go

@@ -210,4 +210,79 @@ func TestCaptureFunctionErrors(t *testing.T) {
 			panic("test panic")
 		})
 	})
+
+	t.Run("stdout and stderr with panic", func(t *testing.T) {
+		defer func() {
+			if r := recover(); r == nil {
+				t.Errorf("Expected panic")
+			}
+		}()
+
+		CaptureStdoutAndStderr(t, func() {
+			panic("test panic")
+		})
+	})
+}
+
+// TestCaptureWithOutput tests capturing output when the function has early returns
+func TestCaptureWithOutput(t *testing.T) {
+	t.Run("stdout with basic output", func(t *testing.T) {
+		output := CaptureStdout(t, func() {
+			fmt.Fprintln(os.Stdout, "before early return")
+		})
+		require.Equal(t, "before early return\n", output)
+		require.NotContains(t, output, "after early return")
+	})
+
+	t.Run("stderr with basic output", func(t *testing.T) {
+		output := CaptureStderr(t, func() {
+			fmt.Fprintln(os.Stderr, "before early return")
+		})
+		require.Equal(t, "before early return\n", output)
+		require.NotContains(t, output, "after early return")
+	})
+
+	t.Run("stdout and stderr with basic output", func(t *testing.T) {
+		stdout, stderr := CaptureStdoutAndStderr(t, func() {
+			fmt.Fprintln(os.Stdout, "stdout before early return")
+			fmt.Fprintln(os.Stderr, "stderr before early return")
+		})
+		require.Equal(t, "stdout before early return\n", stdout)
+		require.Equal(t, "stderr before early return\n", stderr)
+		require.NotContains(t, stdout, "after early return")
+		require.NotContains(t, stderr, "after early return")
+	})
+}
+
+// TestCaptureWithLargeOutput tests capturing large amounts of output
+func TestCaptureWithLargeOutput(t *testing.T) {
+	const largeSize = 1000
+	largeData := strings.Repeat("a", largeSize)
+
+	t.Run("large stdout", func(t *testing.T) {
+		output := CaptureStdout(t, func() {
+			fmt.Fprint(os.Stdout, largeData)
+		})
+		require.Len(t, output, largeSize)
+		require.Equal(t, largeData, output)
+	})
+
+	t.Run("large stderr", func(t *testing.T) {
+		output := CaptureStderr(t, func() {
+			fmt.Fprint(os.Stderr, largeData)
+		})
+		require.Len(t, output, largeSize)
+		require.Equal(t, largeData, output)
+	})
+
+	t.Run("large stdout and stderr", func(t *testing.T) {
+		stdout, stderr := CaptureStdoutAndStderr(t, func() {
+			fmt.Fprint(os.Stdout, largeData)
+			fmt.Fprint(os.Stderr, largeData)
+		})
+		require.Len(t, stdout, largeSize)
+		require.Len(t, stderr, largeSize)
+		require.Equal(t, largeData, stdout)
+		require.Equal(t, largeData, stderr)
+	})
 }

containers/ftp_test.go

@@ -63,13 +63,79 @@ func TestFTPContainer(t *testing.T) {
 		require.NoError(t, os.WriteFile(localPath, content, 0o600))
 	}
 
+	t.Run("Connect", func(t *testing.T) {
+		// test the connect function directly
+		conn, err := ftpContainer.connect(ctx)
+		require.NoError(t, err)
+		require.NotNil(t, conn)
+
+		// test connection state
+		pwd, err := conn.CurrentDir()
+		require.NoError(t, err)
+		require.NotEmpty(t, pwd)
+
+		// close properly
+		err = conn.Quit()
+		require.NoError(t, err)
+	})
+
+	t.Run("SaveAndRestoreCurrentDirectory", func(t *testing.T) {
+		// first we need to make sure the testdir exists
+		testdirPath := filepath.Join(tempDir, "testdir")
+		moreFilePath := filepath.Join(testdirPath, "more.txt")
+		require.NoError(t, os.MkdirAll(testdirPath, 0o750))
+		require.NoError(t, os.WriteFile(moreFilePath, []byte("Test content"), 0o600))
+		err := ftpContainer.SaveFile(ctx, moreFilePath, "testdir/more.txt")
+		require.NoError(t, err, "Failed to create directory for test")
+
+		conn, err := ftpContainer.connect(ctx)
+		require.NoError(t, err)
+		defer func() {
+			err := conn.Quit()
+			require.NoError(t, err)
+		}()
+
+		// test saving current directory
+		originalDir, err := ftpContainer.saveCurrentDirectory(conn)
+		require.NoError(t, err)
+		require.NotEmpty(t, originalDir)
+
+		// change directory - now testdir should exist
+		err = conn.ChangeDir("testdir")
+		require.NoError(t, err)
+
+		// verify we're in a different directory
+		currentDir, err := conn.CurrentDir()
+		require.NoError(t, err)
+		require.NotEqual(t, originalDir, currentDir)
+
+		// restore to original directory
+		ftpContainer.restoreWorkingDirectory(conn, originalDir)
+
+		// verify we're back in the original directory
+		restoredDir, err := conn.CurrentDir()
+		require.NoError(t, err)
+		require.Equal(t, originalDir, restoredDir)
+
+		// test with empty original directory (should be a no-op)
+		ftpContainer.restoreWorkingDirectory(conn, "")
+	})
+
 	t.Run("Upload", func(t *testing.T) {
 		for filename := range testFiles {
 			localPath := filepath.Join(tempDir, filename)
 			remotePath := filename
 			err := ftpContainer.SaveFile(ctx, localPath, remotePath)
 			require.NoError(t, err, "Failed to upload file %s", filename)
 		}
+
+		// test with invalid local path
+		err := ftpContainer.SaveFile(ctx, "/path/does/not/exist.txt", "remote.txt")
+		require.Error(t, err)
+
+		// test with invalid path traversal attempt
+		err = ftpContainer.SaveFile(ctx, "../../../etc/passwd", "remote.txt")
+		require.Error(t, err)
 	})
 
 	t.Run("ListHome", func(t *testing.T) {
@@ -94,6 +160,15 @@ func TestFTPContainer(t *testing.T) {
 		assert.True(t, foundFiles["test1.txt"], "test1.txt should be in home directory")
 		assert.True(t, foundFiles["test2.txt"], "test2.txt should be in home directory")
 		assert.True(t, foundFiles["testdir"], "testdir should be in home directory")
+
+		// test empty path and "." path
+		emptyEntries, err := ftpContainer.ListFiles(ctx, "")
+		require.NoError(t, err)
+		require.NotEmpty(t, emptyEntries)
+
+		dotEntries, err := ftpContainer.ListFiles(ctx, ".")
+		require.NoError(t, err)
+		require.NotEmpty(t, dotEntries)
 	})
 
 	t.Run("ListSubdir", func(t *testing.T) {
@@ -135,5 +210,121 @@ func TestFTPContainer(t *testing.T) {
 			require.Equal(t, originalContent, downloadedContent, "Content mismatch for file %s", filename)
 			t.Logf("Verified content for downloaded file %s", filename)
 		}
+
+		// test with non-existent remote file
+		err := ftpContainer.GetFile(ctx, "non-existent-file.txt", filepath.Join(downloadDir, "non-existent.txt"))
+		require.Error(t, err)
+
+		// test with path traversal attempt
+		err = ftpContainer.GetFile(ctx, "file.txt", "../../../etc/malicious.txt")
+		require.Error(t, err)
+	})
+
+	t.Run("SplitPath", func(t *testing.T) {
+		testCases := []struct {
+			path     string
+			expected []string
+		}{
+			{path: "foo/bar/baz", expected: []string{"foo", "bar", "baz"}},
+			{path: "/foo/bar/baz", expected: []string{"foo", "bar", "baz"}},
+			{path: "foo/bar/baz/", expected: []string{"foo", "bar", "baz"}},
+			{path: "/foo/bar/baz/", expected: []string{"foo", "bar", "baz"}},
+			{path: "", expected: []string{}},
+			{path: "/", expected: []string{}},
+		}
+
+		for _, tc := range testCases {
+			t.Run(tc.path, func(t *testing.T) {
+				result := splitPath(tc.path)
+				require.Equal(t, tc.expected, result)
+			})
+		}
 	})
+
+	t.Run("CreateDirRecursive", func(t *testing.T) {
+		conn, err := ftpContainer.connect(ctx)
+		require.NoError(t, err)
+		defer func() {
+			err := conn.Quit()
+			require.NoError(t, err)
+		}()
+
+		// create a deep directory structure
+		err = ftpContainer.createDirRecursive(conn, "deep/nested/directory/structure")
+		require.NoError(t, err)
+
+		// verify it exists by listing files
+		entries, err := ftpContainer.ListFiles(ctx, "deep/nested/directory")
+		require.NoError(t, err)
+
+		foundStructure := false
+		for _, entry := range entries {
+			if entry.Name == "structure" && entry.Type == 1 {
+				foundStructure = true
+				break
+			}
+		}
+		assert.True(t, foundStructure, "deep/nested/directory/structure should exist")
+
+		// test with empty path (should be a no-op)
+		err = ftpContainer.createDirRecursive(conn, "")
+		require.NoError(t, err)
+	})
+}
+
+// TestFTPContainerErrorHandling tests error handling in FTP container
+func TestFTPContainerErrorHandling(t *testing.T) {
+	t.Run("TestHandleMakeDirFailure", func(t *testing.T) {
+		if testing.Short() {
+			t.Skip("skipping FTP error handling test in short mode")
+		}
+
+		// create a test container
+		ctx := context.Background()
+		ftpContainer := NewFTPTestContainer(ctx, t)
+		defer func() {
+			err := ftpContainer.Close(context.Background())
+			require.NoError(t, err)
+		}()
+
+		// connect to the container
+		conn, err := ftpContainer.connect(ctx)
+		require.NoError(t, err)
+		defer conn.Quit()
+
+		// test the handleMakeDirFailure function with a directory that already exists
+		// first create the directory normally
+		err = conn.MakeDir("testdir2")
+		require.NoError(t, err)
+
+		// now simulate a failure but where the directory actually exists
+		err = ftpContainer.handleMakeDirFailure(conn, "testdir2", fmt.Errorf("simulated error"))
+		require.NoError(t, err, "Should handle the case where directory exists but MakeDir failed")
+
+		// test with a non-existent directory
+		err = ftpContainer.handleMakeDirFailure(conn, "definitely_not_exists_dir", fmt.Errorf("simulated error"))
+		require.Error(t, err, "Should fail when directory doesn't exist")
+	})
+}
+
+// Test utility methods separately
+func TestSplitPath(t *testing.T) {
+	testCases := []struct {
+		path     string
+		expected []string
+	}{
+		{path: "foo/bar/baz", expected: []string{"foo", "bar", "baz"}},
+		{path: "/foo/bar/baz", expected: []string{"foo", "bar", "baz"}},
+		{path: "foo/bar/baz/", expected: []string{"foo", "bar", "baz"}},
+		{path: "/foo/bar/baz/", expected: []string{"foo", "bar", "baz"}},
+		{path: "", expected: []string{}},
+		{path: "/", expected: []string{}},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.path, func(t *testing.T) {
+			result := splitPath(tc.path)
+			require.Equal(t, tc.expected, result)
+		})
+	}
 }

file_utils_test.go

@@ -3,6 +3,8 @@ package testutils
 import (
 	"os"
 	"path/filepath"
+	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -74,4 +76,48 @@ func TestWriteTestFile(t *testing.T) {
 		_, err = os.Stat(filePath)
 		require.True(t, os.IsNotExist(err), "File should be removed after cleanup")
 	})
+
+	t.Run("with large content", func(t *testing.T) {
+		// create a large string (100KB)
+		largeContent := strings.Repeat("abcdefghij", 10*1024) // 10 chars * 10K = 100KB
+		filePath := WriteTestFile(t, largeContent)
+
+		// verify the file exists and has correct size
+		info, err := os.Stat(filePath)
+		require.NoError(t, err)
+		require.Equal(t, int64(len(largeContent)), info.Size())
+
+		// verify the content
+		data, err := os.ReadFile(filePath) // #nosec G304 -- safe file access in test
+		require.NoError(t, err)
+		require.Equal(t, largeContent, string(data))
+	})
+
+	t.Run("with binary content", func(t *testing.T) {
+		// create some binary data
+		binaryContent := string([]byte{0, 1, 2, 3, 4, 5, 255, 254, 253})
+		filePath := WriteTestFile(t, binaryContent)
+
+		// verify the content
+		data, err := os.ReadFile(filePath) // #nosec G304 -- safe file access in test
+		require.NoError(t, err)
+		require.Equal(t, binaryContent, string(data))
+	})
+
+	t.Run("file permissions", func(t *testing.T) {
+		content := "permission test"
+		filePath := WriteTestFile(t, content)
+
+		// check that the file has the expected permissions (0o600)
+		info, err := os.Stat(filePath)
+		require.NoError(t, err)
+
+		// on Unix systems, we can check the exact permission bits
+		if runtime.GOOS != "windows" {
+			// 0o600 = rw------- (read/write for owner only)
+			expectedPerm := os.FileMode(0o600)
+			actualPerm := info.Mode().Perm()
+			require.Equal(t, expectedPerm, actualPerm, "File should have 0600 permissions")
+		}
+	})
 }