diff --git a/cmd/restic/cmd_forget.go b/cmd/restic/cmd_forget.go index 82313c6ef..ac3601584 100644 --- a/cmd/restic/cmd_forget.go +++ b/cmd/restic/cmd_forget.go @@ -100,15 +100,16 @@ func init() { } func verifyForgetOptions(opts *ForgetOptions) error { - var negValFound = false - if opts.Last < -1 || opts.Hourly < -1 || opts.Daily < -1 || opts.Weekly < -1 || opts.Monthly < -1 || opts.Yearly < -1 { - negValFound = true + return errors.Fatal("negative values other than -1 are not allowed for --keep-*") } - if negValFound { - return errors.Fatal("negative values other than -1 are not allowed for --keep-* options") + for _, d := range []restic.Duration{opts.Within, opts.WithinHourly, opts.WithinDaily, + opts.WithinMonthly, opts.WithinWeekly, opts.WithinYearly} { + if d.Hours < 0 || d.Days < 0 || d.Months < 0 || d.Years < 0 { + return errors.Fatal("durations containing negative values are not allowed for --keep-within*") + } } return nil diff --git a/cmd/restic/cmd_forget_test.go b/cmd/restic/cmd_forget_test.go index c45d72815..9fd5c7bb0 100644 --- a/cmd/restic/cmd_forget_test.go +++ b/cmd/restic/cmd_forget_test.go @@ -1,25 +1,65 @@ package main import ( - "fmt" "testing" + "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) -func TestPreventNegativeForgetOptionValues(t *testing.T) { - invalidForgetOpts := []ForgetOptions{ - {Last: -2}, - {Hourly: -2}, - {Daily: -2}, - {Weekly: -2}, - {Monthly: -2}, - {Yearly: -2}, +func TestForgetOptionValues(t *testing.T) { + const negValErrorMsg = "Fatal: negative values other than -1 are not allowed for --keep-*" + const negDurationValErrorMsg = "Fatal: durations containing negative values are not allowed for --keep-within*" + testCases := []struct { + input ForgetOptions + expectsError bool + errorMsg string + }{ + {ForgetOptions{Last: 1}, false, ""}, + {ForgetOptions{Hourly: 1}, false, ""}, + {ForgetOptions{Daily: 1}, false, ""}, + {ForgetOptions{Weekly: 1}, false, ""}, + {ForgetOptions{Monthly: 1}, false, ""}, + {ForgetOptions{Yearly: 1}, false, ""}, + {ForgetOptions{Last: 0}, false, ""}, + {ForgetOptions{Hourly: 0}, false, ""}, + {ForgetOptions{Daily: 0}, false, ""}, + {ForgetOptions{Weekly: 0}, false, ""}, + {ForgetOptions{Monthly: 0}, false, ""}, + {ForgetOptions{Yearly: 0}, false, ""}, + {ForgetOptions{Last: -1}, false, ""}, + {ForgetOptions{Hourly: -1}, false, ""}, + {ForgetOptions{Daily: -1}, false, ""}, + {ForgetOptions{Weekly: -1}, false, ""}, + {ForgetOptions{Monthly: -1}, false, ""}, + {ForgetOptions{Yearly: -1}, false, ""}, + {ForgetOptions{Last: -2}, true, negValErrorMsg}, + {ForgetOptions{Hourly: -2}, true, negValErrorMsg}, + {ForgetOptions{Daily: -2}, true, negValErrorMsg}, + {ForgetOptions{Weekly: -2}, true, negValErrorMsg}, + {ForgetOptions{Monthly: -2}, true, negValErrorMsg}, + {ForgetOptions{Yearly: -2}, true, negValErrorMsg}, + {ForgetOptions{Within: restic.ParseDurationOrPanic("1y2m3d3h")}, false, ""}, + {ForgetOptions{WithinHourly: restic.ParseDurationOrPanic("1y2m3d3h")}, false, ""}, + {ForgetOptions{WithinDaily: restic.ParseDurationOrPanic("1y2m3d3h")}, false, ""}, + {ForgetOptions{WithinWeekly: restic.ParseDurationOrPanic("1y2m3d3h")}, false, ""}, + {ForgetOptions{WithinMonthly: restic.ParseDurationOrPanic("2y4m6d8h")}, false, ""}, + {ForgetOptions{WithinYearly: restic.ParseDurationOrPanic("2y4m6d8h")}, false, ""}, + {ForgetOptions{Within: restic.ParseDurationOrPanic("-1y2m3d3h")}, true, negDurationValErrorMsg}, + {ForgetOptions{WithinHourly: restic.ParseDurationOrPanic("1y-2m3d3h")}, true, negDurationValErrorMsg}, + {ForgetOptions{WithinDaily: restic.ParseDurationOrPanic("1y2m-3d3h")}, true, negDurationValErrorMsg}, + {ForgetOptions{WithinWeekly: restic.ParseDurationOrPanic("1y2m3d-3h")}, true, negDurationValErrorMsg}, + {ForgetOptions{WithinMonthly: restic.ParseDurationOrPanic("-2y4m6d8h")}, true, negDurationValErrorMsg}, + {ForgetOptions{WithinYearly: restic.ParseDurationOrPanic("2y-4m6d8h")}, true, negDurationValErrorMsg}, } - for _, opts := range invalidForgetOpts { - err := verifyForgetOptions(&opts) - rtest.Assert(t, err != nil, fmt.Sprintf("should have returned error for %+v", opts)) - rtest.Equals(t, "Fatal: negative values other than -1 are not allowed for --keep-* options", err.Error()) + for _, testCase := range testCases { + err := verifyForgetOptions(&testCase.input) + if testCase.expectsError { + rtest.Assert(t, err != nil, "should have returned error for input %+v", testCase.input) + rtest.Equals(t, testCase.errorMsg, err.Error()) + } else { + rtest.Assert(t, err == nil, "expected no error for input %+v", testCase.input) + } } }