diff --git a/.golangci.yml b/.golangci.yml index 9e7e0f4..3445966 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,6 +9,8 @@ linters: - errcheck - forbidigo settings: + dupl: + threshold: 200 depguard: rules: main: diff --git a/backup/cmd/root.go b/backup/cmd/root.go index 3bb1dcb..1f5527c 100644 --- a/backup/cmd/root.go +++ b/backup/cmd/root.go @@ -30,8 +30,8 @@ func BuildRootCommandWithDeps(fs afero.Fs, shell internal.Exec) *cobra.Command { rootCmd.AddCommand( buildListCommand(shell), - buildRunCommand(shell), - buildSimulateCommand(shell), + buildRunCommand(fs, shell), + buildSimulateCommand(fs, shell), buildConfigCommand(), buildCheckCoverageCommand(fs), buildVersionCommand(shell), diff --git a/backup/cmd/run.go b/backup/cmd/run.go index 6d1ec21..353c19a 100644 --- a/backup/cmd/run.go +++ b/backup/cmd/run.go @@ -5,10 +5,11 @@ import ( "fmt" "time" + "github.com/spf13/afero" "github.com/spf13/cobra" ) -func buildRunCommand(shell internal.Exec) *cobra.Command { +func buildRunCommand(fs afero.Fs, shell internal.Exec) *cobra.Command { return &cobra.Command{ Use: "run", Short: "Execute the sync jobs", @@ -21,7 +22,7 @@ func buildRunCommand(shell internal.Exec) *cobra.Command { return fmt.Errorf("loading config: %w", err) } - logger, logPath, cleanup, err := internal.CreateMainLogger(configPath, false, time.Now()) + logger, logPath, cleanup, err := internal.CreateMainLogger(fs, configPath, false, time.Now()) if err != nil { return fmt.Errorf("creating logger: %w", err) } diff --git a/backup/cmd/simulate.go b/backup/cmd/simulate.go index f63bfd4..ebcf75e 100644 --- a/backup/cmd/simulate.go +++ b/backup/cmd/simulate.go @@ -5,10 +5,11 @@ import ( "fmt" "time" + "github.com/spf13/afero" "github.com/spf13/cobra" ) -func buildSimulateCommand(shell internal.Exec) *cobra.Command { +func buildSimulateCommand(fs afero.Fs, shell internal.Exec) *cobra.Command { return &cobra.Command{ Use: "simulate", Short: "Simulate the sync jobs", @@ -21,7 +22,7 @@ func buildSimulateCommand(shell internal.Exec) *cobra.Command { return fmt.Errorf("loading config: %w", err) } - logger, logPath, cleanup, err := internal.CreateMainLogger(configPath, true, time.Now()) + logger, logPath, cleanup, err := internal.CreateMainLogger(fs, configPath, true, time.Now()) if err != nil { return fmt.Errorf("creating logger: %w", err) } diff --git a/backup/cmd/test/commands_test.go b/backup/cmd/test/commands_test.go index b8b72f8..376c234 100644 --- a/backup/cmd/test/commands_test.go +++ b/backup/cmd/test/commands_test.go @@ -189,15 +189,12 @@ jobs: target: "/backup/docs/" `) - // Block log directory creation by placing a file named "logs" - tmpDir := t.TempDir() - t.Chdir(tmpDir) - - require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "logs"), []byte("block"), 0600)) + // Use a read-only filesystem to block log directory creation + fs := afero.NewReadOnlyFs(afero.NewMemMapFs()) shell := &stubExec{output: []byte("rsync version 3.2.7 protocol version 31\n")} - _, err := executeCommandWithDeps(t, afero.NewMemMapFs(), shell, "run", "--config", cfgPath) + _, err := executeCommandWithDeps(t, fs, shell, "run", "--config", cfgPath) require.Error(t, err) assert.Contains(t, err.Error(), "creating logger") @@ -246,14 +243,12 @@ jobs: target: "/backup/docs/" `) - tmpDir := t.TempDir() - t.Chdir(tmpDir) - - require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "logs"), []byte("block"), 0600)) + // Use a read-only filesystem to block log directory creation + fs := afero.NewReadOnlyFs(afero.NewMemMapFs()) shell := &stubExec{output: []byte("rsync version 3.2.7 protocol version 31\n")} - _, err := executeCommandWithDeps(t, afero.NewMemMapFs(), shell, "simulate", "--config", cfgPath) + _, err := executeCommandWithDeps(t, fs, shell, "simulate", "--config", cfgPath) require.Error(t, err) assert.Contains(t, err.Error(), "creating logger") diff --git a/backup/internal/helper.go b/backup/internal/helper.go index 080576c..d89255b 100644 --- a/backup/internal/helper.go +++ b/backup/internal/helper.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" "time" + + "github.com/spf13/afero" ) // Path represents a source or target path with optional exclusions. @@ -35,16 +37,18 @@ func getLogPath(simulate bool, configPath string, now time.Time) string { return logPath } -func CreateMainLogger(configPath string, simulate bool, now time.Time) (*log.Logger, string, func() error, error) { +func CreateMainLogger( + fs afero.Fs, configPath string, simulate bool, now time.Time, +) (*log.Logger, string, func() error, error) { logPath := getLogPath(simulate, configPath, now) overallLogPath := logPath + "/summary.log" - err := os.MkdirAll(logPath, LogDirPermission) + err := fs.MkdirAll(logPath, LogDirPermission) if err != nil { return nil, "", nil, fmt.Errorf("failed to create log directory: %w", err) } - overallLogFile, err := os.OpenFile(overallLogPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, LogFilePermission) + overallLogFile, err := fs.OpenFile(overallLogPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, LogFilePermission) if err != nil { return nil, "", nil, fmt.Errorf("failed to open overall log file: %w", err) } diff --git a/backup/internal/test/helper_test.go b/backup/internal/test/helper_test.go index 67dd58b..b28489a 100644 --- a/backup/internal/test/helper_test.go +++ b/backup/internal/test/helper_test.go @@ -1,12 +1,12 @@ package internal_test import ( - "os" "testing" "time" . "backup-rsync/backup/internal" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -33,7 +33,7 @@ func fixedTime() time.Time { } func TestCreateMainLogger_Title_IsPresent(t *testing.T) { - logger, logPath, cleanup, err := CreateMainLogger("title", true, fixedTime()) + logger, logPath, cleanup, err := CreateMainLogger(afero.NewMemMapFs(), "title", true, fixedTime()) require.NoError(t, err) defer cleanup() @@ -43,7 +43,7 @@ func TestCreateMainLogger_Title_IsPresent(t *testing.T) { } func TestCreateMainLogger_IsSimulate_HasSimSuffix(t *testing.T) { - logger, logPath, cleanup, err := CreateMainLogger("", true, fixedTime()) + logger, logPath, cleanup, err := CreateMainLogger(afero.NewMemMapFs(), "", true, fixedTime()) require.NoError(t, err) defer cleanup() @@ -53,7 +53,7 @@ func TestCreateMainLogger_IsSimulate_HasSimSuffix(t *testing.T) { } func TestCreateMainLogger_NotSimulate_HasNoSimSuffix(t *testing.T) { - logger, logPath, cleanup, err := CreateMainLogger("", false, fixedTime()) + logger, logPath, cleanup, err := CreateMainLogger(afero.NewMemMapFs(), "", false, fixedTime()) require.NoError(t, err) defer cleanup() @@ -63,7 +63,7 @@ func TestCreateMainLogger_NotSimulate_HasNoSimSuffix(t *testing.T) { } func TestCreateMainLogger_DeterministicLogPath(t *testing.T) { - _, logPath, cleanup, err := CreateMainLogger("backup.yaml", true, fixedTime()) + _, logPath, cleanup, err := CreateMainLogger(afero.NewMemMapFs(), "backup.yaml", true, fixedTime()) require.NoError(t, err) defer cleanup() @@ -72,7 +72,7 @@ func TestCreateMainLogger_DeterministicLogPath(t *testing.T) { } func TestCreateMainLogger_DeterministicLogPath_NoSimulate(t *testing.T) { - _, logPath, cleanup, err := CreateMainLogger("sync.yaml", false, fixedTime()) + _, logPath, cleanup, err := CreateMainLogger(afero.NewMemMapFs(), "sync.yaml", false, fixedTime()) require.NoError(t, err) defer cleanup() @@ -81,15 +81,10 @@ func TestCreateMainLogger_DeterministicLogPath_NoSimulate(t *testing.T) { } func TestCreateMainLogger_MkdirError(t *testing.T) { - // Use t.Chdir to a temp dir so we control the filesystem - tmpDir := t.TempDir() - t.Chdir(tmpDir) + // Use a read-only filesystem to block directory creation + fs := afero.NewReadOnlyFs(afero.NewMemMapFs()) - // Create "logs" as a regular file to block MkdirAll - err := os.WriteFile("logs", []byte("block"), 0600) - require.NoError(t, err) - - _, _, cleanup, err := CreateMainLogger("test.yaml", false, fixedTime()) + _, _, cleanup, err := CreateMainLogger(fs, "test.yaml", false, fixedTime()) _ = cleanup require.Error(t, err) @@ -97,18 +92,11 @@ func TestCreateMainLogger_MkdirError(t *testing.T) { } func TestCreateMainLogger_OpenFileError(t *testing.T) { - tmpDir := t.TempDir() - t.Chdir(tmpDir) - - // Pre-create the log path directory and make summary.log a directory to block OpenFile - logDir := "logs/sync-2025-06-15T14-30-45-test" + fs := afero.NewReadOnlyFs(afero.NewMemMapFs()) - err := os.MkdirAll(logDir+"/summary.log", 0750) - require.NoError(t, err) - - _, _, cleanup, err := CreateMainLogger("test.yaml", false, fixedTime()) + _, _, cleanup, err := CreateMainLogger(fs, "test.yaml", false, fixedTime()) _ = cleanup require.Error(t, err) - assert.Contains(t, err.Error(), "failed to open overall log file") + assert.Contains(t, err.Error(), "failed to create log directory") }