From feb9b08a0d6e07e4611cc17bd6bdd9b41a420be3 Mon Sep 17 00:00:00 2001 From: Jaap de Haan <261428+jdehaan@users.noreply.github.com> Date: Sun, 30 Nov 2025 16:51:30 +0000 Subject: [PATCH 1/3] fix: Early close of overall log file --- backup/internal/helper.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/backup/internal/helper.go b/backup/internal/helper.go index 887a53f..0b9aed8 100644 --- a/backup/internal/helper.go +++ b/backup/internal/helper.go @@ -38,13 +38,6 @@ func createFileLogger() (*log.Logger, string) { log.Fatalf("Failed to open overall log file: %v", err) } - defer func() { - err := overallLogFile.Close() - if err != nil { - log.Fatalf("Failed to close overall log file: %v", err) - } - }() - logger := log.New(overallLogFile, "", log.LstdFlags) return logger, logPath From 86f72a81aa5185f560c9e8ec4212194361f59f7c Mon Sep 17 00:00:00 2001 From: Jaap de Haan <261428+jdehaan@users.noreply.github.com> Date: Sun, 30 Nov 2025 17:04:45 +0000 Subject: [PATCH 2/3] fix: Simplify calls to Execute --- backup/internal/job.go | 2 +- backup/internal/rsync.go | 6 +++++- backup/internal/test/rsync_test.go | 14 ++++++-------- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/backup/internal/job.go b/backup/internal/job.go index d475578..6775225 100644 --- a/backup/internal/job.go +++ b/backup/internal/job.go @@ -18,7 +18,7 @@ func (job Job) Apply(rsync RSyncCommand, logPath string) string { return "SUCCESS" } - out, err := rsync.Executor.Execute(rsync.BinPath, args...) + out, err := rsync.Run(args...) fmt.Printf("Output:\n%s\n", string(out)) if err != nil { diff --git a/backup/internal/rsync.go b/backup/internal/rsync.go index e52ccf6..75f1c29 100644 --- a/backup/internal/rsync.go +++ b/backup/internal/rsync.go @@ -31,7 +31,7 @@ func (command RSyncCommand) GetVersionInfo() (string, error) { return "", fmt.Errorf("%w: \"%s\"", ErrInvalidRsyncPath, rsyncPath) } - output, err := command.Executor.Execute(rsyncPath, "--version") + output, err := command.Run("--version") if err != nil { return "", fmt.Errorf("error fetching rsync version: %w", err) } @@ -65,3 +65,7 @@ func (command RSyncCommand) ArgumentsForJob(job Job, logPath string) []string { return args } + +func (command RSyncCommand) Run(args ...string) ([]byte, error) { + return command.Executor.Execute(command.BinPath, args...) +} diff --git a/backup/internal/test/rsync_test.go b/backup/internal/test/rsync_test.go index 707ecd7..7c42a3a 100644 --- a/backup/internal/test/rsync_test.go +++ b/backup/internal/test/rsync_test.go @@ -14,16 +14,14 @@ var errCommandNotFound = errors.New("command not found") const rsyncPath = "/usr/bin/rsync" -func TestBuildRsyncCmd(t *testing.T) { +func TestArgumentsForJob(t *testing.T) { job := *NewJob( WithSource("/home/user/Music/"), WithTarget("/target/user/music/home"), WithExclusions([]string{"*.tmp", "node_modules/"}), ) command := internal.RSyncCommand{ - BinPath: rsyncPath, Simulate: true, - Executor: nil, } args := command.ArgumentsForJob(job, "") @@ -36,7 +34,7 @@ func TestBuildRsyncCmd(t *testing.T) { assert.Equal(t, strings.Join(expectedArgs, " "), strings.Join(args, " ")) } -func TestFetchRsyncVersion_Success(t *testing.T) { +func TestGetVersionInfo_Success(t *testing.T) { rsync := internal.RSyncCommand{ BinPath: rsyncPath, Executor: &MockCommandExecutor{ @@ -51,7 +49,7 @@ func TestFetchRsyncVersion_Success(t *testing.T) { assert.Equal(t, "rsync version 3.2.3 protocol version 31\n", versionInfo) } -func TestFetchRsyncVersion_CommandError(t *testing.T) { +func TestGetVersionInfo_CommandError(t *testing.T) { rsync := internal.RSyncCommand{ BinPath: rsyncPath, Executor: &MockCommandExecutor{ @@ -66,7 +64,7 @@ func TestFetchRsyncVersion_CommandError(t *testing.T) { assert.Empty(t, versionInfo) } -func TestFetchRsyncVersion_InvalidOutput(t *testing.T) { +func TestGetVersionInfo_InvalidOutput(t *testing.T) { rsync := internal.RSyncCommand{ BinPath: rsyncPath, Executor: &MockCommandExecutor{ @@ -81,7 +79,7 @@ func TestFetchRsyncVersion_InvalidOutput(t *testing.T) { assert.Empty(t, versionInfo) } -func TestFetchRsyncVersion_EmptyPath(t *testing.T) { +func TestGetVersionInfo_EmptyPath(t *testing.T) { rsync := internal.RSyncCommand{ BinPath: "", Executor: &MockCommandExecutor{ @@ -97,7 +95,7 @@ func TestFetchRsyncVersion_EmptyPath(t *testing.T) { assert.Empty(t, versionInfo) } -func TestFetchRsyncVersion_IncompletePath(t *testing.T) { +func TestGetVersionInfo_IncompletePath(t *testing.T) { rsync := internal.RSyncCommand{ BinPath: "bin/rsync", Executor: &MockCommandExecutor{ From f1ca82f509ecddb3b6d0913fe258ba907adea165 Mon Sep 17 00:00:00 2001 From: Jaap de Haan <261428+jdehaan@users.noreply.github.com> Date: Sun, 30 Nov 2025 17:09:17 +0000 Subject: [PATCH 3/3] fix: Introduce NewListCommand, NewRSyncSimulateCommand --- backup/cmd/list.go | 3 +-- backup/cmd/simulate.go | 3 +-- backup/internal/job.go | 22 +--------------- backup/internal/rsync.go | 41 ++++++++++++++++++++++++++---- backup/internal/test/rsync_test.go | 5 +--- 5 files changed, 40 insertions(+), 34 deletions(-) diff --git a/backup/cmd/list.go b/backup/cmd/list.go index f387b41..c0e8d4a 100644 --- a/backup/cmd/list.go +++ b/backup/cmd/list.go @@ -14,8 +14,7 @@ func buildListCommand() *cobra.Command { configPath, _ := cmd.Flags().GetString("config") rsyncPath, _ := cmd.Flags().GetString("rsync-path") cfg := internal.LoadResolvedConfig(configPath) - command := internal.NewRSyncCommand(rsyncPath) - command.ListOnly = true + command := internal.NewListCommand(rsyncPath) cfg.Apply(command) }, diff --git a/backup/cmd/simulate.go b/backup/cmd/simulate.go index b6d815f..42dd756 100644 --- a/backup/cmd/simulate.go +++ b/backup/cmd/simulate.go @@ -15,8 +15,7 @@ func buildSimulateCommand() *cobra.Command { rsyncPath, _ := cmd.Flags().GetString("rsync-path") cfg := internal.LoadResolvedConfig(configPath) - command := internal.NewRSyncCommand(rsyncPath) - command.Simulate = true + command := internal.NewRSyncSimulateCommand(rsyncPath) cfg.Apply(command) }, diff --git a/backup/internal/job.go b/backup/internal/job.go index 6775225..41fb004 100644 --- a/backup/internal/job.go +++ b/backup/internal/job.go @@ -1,29 +1,9 @@ package internal -import ( - "fmt" - "strings" -) - func (job Job) Apply(rsync RSyncCommand, logPath string) string { if !job.Enabled { return "SKIPPED" } - args := rsync.ArgumentsForJob(job, logPath) - fmt.Printf("Job: %s\n", job.Name) - fmt.Printf("Command: rsync %s %s\n", rsync.BinPath, strings.Join(args, " ")) - - if rsync.ListOnly { - return "SUCCESS" - } - - out, err := rsync.Run(args...) - fmt.Printf("Output:\n%s\n", string(out)) - - if err != nil { - return "FAILURE" - } - - return "SUCCESS" + return rsync.Run(job, logPath) } diff --git a/backup/internal/rsync.go b/backup/internal/rsync.go index 75f1c29..1b26db0 100644 --- a/backup/internal/rsync.go +++ b/backup/internal/rsync.go @@ -24,6 +24,22 @@ func NewRSyncCommand(binPath string) RSyncCommand { } } +func NewRSyncSimulateCommand(binPath string) RSyncCommand { + return RSyncCommand{ + BinPath: binPath, + Simulate: true, + Executor: &RealSync{}, + } +} + +func NewListCommand(binPath string) RSyncCommand { + return RSyncCommand{ + BinPath: binPath, + ListOnly: true, + Executor: &RealSync{}, + } +} + func (command RSyncCommand) GetVersionInfo() (string, error) { rsyncPath := command.BinPath @@ -31,7 +47,7 @@ func (command RSyncCommand) GetVersionInfo() (string, error) { return "", fmt.Errorf("%w: \"%s\"", ErrInvalidRsyncPath, rsyncPath) } - output, err := command.Run("--version") + output, err := command.Executor.Execute("--version") if err != nil { return "", fmt.Errorf("error fetching rsync version: %w", err) } @@ -44,7 +60,7 @@ func (command RSyncCommand) GetVersionInfo() (string, error) { return string(output), nil } -func (command RSyncCommand) ArgumentsForJob(job Job, logPath string) []string { +func ArgumentsForJob(job Job, logPath string, simulate bool) []string { args := []string{"-aiv", "--stats"} if job.Delete { args = append(args, "--delete") @@ -59,13 +75,28 @@ func (command RSyncCommand) ArgumentsForJob(job Job, logPath string) []string { } args = append(args, job.Source, job.Target) - if command.Simulate { + if simulate { args = append([]string{"--dry-run"}, args...) } return args } -func (command RSyncCommand) Run(args ...string) ([]byte, error) { - return command.Executor.Execute(command.BinPath, args...) +func (rsync RSyncCommand) Run(job Job, logPath string) string { + args := ArgumentsForJob(job, logPath, rsync.Simulate) + fmt.Printf("Job: %s\n", job.Name) + fmt.Printf("Command: rsync %s %s\n", rsync.BinPath, strings.Join(args, " ")) + + if rsync.ListOnly { + return "SUCCESS" + } + + out, err := rsync.Executor.Execute(rsync.BinPath, args...) + fmt.Printf("Output:\n%s\n", string(out)) + + if err != nil { + return "FAILURE" + } + + return "SUCCESS" } diff --git a/backup/internal/test/rsync_test.go b/backup/internal/test/rsync_test.go index 7c42a3a..b049dfa 100644 --- a/backup/internal/test/rsync_test.go +++ b/backup/internal/test/rsync_test.go @@ -20,10 +20,7 @@ func TestArgumentsForJob(t *testing.T) { WithTarget("/target/user/music/home"), WithExclusions([]string{"*.tmp", "node_modules/"}), ) - command := internal.RSyncCommand{ - Simulate: true, - } - args := command.ArgumentsForJob(job, "") + args := internal.ArgumentsForJob(job, "", true) expectedArgs := []string{ "--dry-run", "-aiv", "--stats", "--delete",