diff --git a/README.md b/README.md index 788eb3e..c90f774 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,9 @@ watchpr unkin/argocd-apps#42 # Multiple PRs, custom interval; refs accept #N or :N watchpr --interval 30s unkin/argocd-apps#42 unkin/terraform-vault:98 +# --interval takes a duration (30s, 2m, 1h30m) or a bare number of seconds +watchpr --interval 30 unkin/argocd-apps#42 + # One-shot: print current state and exit 0 (great for scripts) watchpr --once unkin/argocd-apps#42 watchpr --once --json unkin/argocd-apps#42 diff --git a/cmd/watchpr/main.go b/cmd/watchpr/main.go index f26db99..7a0739b 100644 --- a/cmd/watchpr/main.go +++ b/cmd/watchpr/main.go @@ -7,6 +7,7 @@ // watchpr owner/repo#12 owner/repo:15 // watchpr --once --json owner/repo#12 // watchpr --interval 30s owner/repo#12 +// watchpr --interval 30 owner/repo#12 package main import ( @@ -34,7 +35,7 @@ func main() { // tests can invoke Execute and assert the exit behaviour without spawning a // process. func newRootCmd() *cobra.Command { - var interval time.Duration + var intervalFlag string var once, jsonMode bool root := &cobra.Command{ @@ -50,6 +51,10 @@ func newRootCmd() *cobra.Command { if len(args) == 0 { return fmt.Errorf("no PR references given (e.g. owner/repo#12)") } + interval, err := agent.ParseDurationFlag("interval", intervalFlag) + if err != nil { + return err + } refs := make([]agent.PRRef, 0, len(args)) for _, a := range args { ref, err := agent.ParsePRRef(a) @@ -68,7 +73,7 @@ func newRootCmd() *cobra.Command { root.SetVersionTemplate("{{.Version}}\n") f := root.Flags() - f.DurationVar(&interval, "interval", 60*time.Second, "Polling interval") + f.StringVar(&intervalFlag, "interval", "60s", "Polling interval: a duration (30s, 2m, 1h30m) or a bare number of seconds") f.BoolVar(&once, "once", false, "Check once, print current state, and exit") f.BoolVar(&jsonMode, "json", false, "Emit JSON") diff --git a/cmd/watchpr/main_test.go b/cmd/watchpr/main_test.go index 1433541..06b784f 100644 --- a/cmd/watchpr/main_test.go +++ b/cmd/watchpr/main_test.go @@ -4,6 +4,7 @@ import ( "io" "net/http" "net/http/httptest" + "strings" "testing" ) @@ -70,3 +71,37 @@ func TestOnceRunsAnonymouslyWhenNoTokenIsAvailable(t *testing.T) { t.Errorf("sent %d Authorization headers, want none", authHeaders) } } + +// A bare integer interval means seconds and must survive flag parsing: the +// command should fail on the missing PR reference, not on the flag value. +func TestExecuteBareIntervalIsSeconds(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"--interval", "15"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + if err == nil { + t.Fatal("Execute() = nil, want the no-references error") + } + if !strings.Contains(err.Error(), "no PR references given") { + t.Fatalf("Execute() error = %v, want the no-references error", err) + } +} + +// An unparseable interval is rejected before any Vault/Gitea call, with an +// error naming the flag and showing valid forms. +func TestExecuteBadIntervalErrors(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"--interval", "soon", "unkin/repo#1"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + if err == nil { + t.Fatal("Execute() = nil, want error for an unparseable --interval") + } + for _, want := range []string{"--interval", "30s"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("Execute() error %q does not mention %q", err, want) + } + } +} diff --git a/internal/agent/parse.go b/internal/agent/parse.go index 3816ccf..ac5f379 100644 --- a/internal/agent/parse.go +++ b/internal/agent/parse.go @@ -4,6 +4,7 @@ import ( "fmt" "strconv" "strings" + "time" ) // PRRef identifies a single pull request by repository and number. @@ -48,6 +49,25 @@ func ParsePRRef(s string) (PRRef, error) { return PRRef{Owner: owner, Repo: repo, Number: n}, nil } +// ParseDurationFlag parses a duration flag value, accepting either a Go +// duration string ("30s", "1h30m") or a bare integer read as seconds ("15"). +// flag names the flag so the error says which value was rejected. +func ParseDurationFlag(flag, value string) (time.Duration, error) { + s := strings.TrimSpace(value) + d, err := time.ParseDuration(s) + if err != nil { + n, nerr := strconv.Atoi(s) + if nerr != nil { + return 0, fmt.Errorf("invalid --%s value %q: want a duration such as 30s, 2m or 1h30m, or a bare number of seconds such as 15", flag, value) + } + d = time.Duration(n) * time.Second + } + if d <= 0 { + return 0, fmt.Errorf("invalid --%s value %q: must be greater than zero", flag, value) + } + return d, nil +} + // ParseRepo validates and splits an "owner/repo" string. func ParseRepo(s string) (owner, repo string, err error) { s = strings.TrimSpace(s) diff --git a/internal/agent/parse_test.go b/internal/agent/parse_test.go index 8449470..d3d8703 100644 --- a/internal/agent/parse_test.go +++ b/internal/agent/parse_test.go @@ -1,6 +1,10 @@ package agent -import "testing" +import ( + "strings" + "testing" + "time" +) func TestParsePRRef(t *testing.T) { tests := []struct { @@ -77,3 +81,54 @@ func TestParseRepo(t *testing.T) { } } } + +func TestParseDurationFlag(t *testing.T) { + tests := []struct { + in string + want time.Duration + wantErr bool + }{ + {"15", 15 * time.Second, false}, + {"15s", 15 * time.Second, false}, + {"2m", 2 * time.Minute, false}, + {"1h30m", 90 * time.Minute, false}, + {"500ms", 500 * time.Millisecond, false}, + {" 45 ", 45 * time.Second, false}, + {"0", 0, true}, + {"0s", 0, true}, + {"-5", 0, true}, + {"-5s", 0, true}, + {"15x", 0, true}, + {"", 0, true}, + } + for _, tt := range tests { + got, err := ParseDurationFlag("interval", tt.in) + if tt.wantErr { + if err == nil { + t.Errorf("ParseDurationFlag(%q): expected error, got %v", tt.in, got) + } + continue + } + if err != nil { + t.Errorf("ParseDurationFlag(%q): unexpected error: %v", tt.in, err) + continue + } + if got != tt.want { + t.Errorf("ParseDurationFlag(%q) = %v, want %v", tt.in, got, tt.want) + } + } +} + +// The error must name the flag and show valid forms instead of surfacing +// time.ParseDuration's "missing unit" wording. +func TestParseDurationFlagErrorMessage(t *testing.T) { + _, err := ParseDurationFlag("interval", "soon") + if err == nil { + t.Fatal("ParseDurationFlag(\"soon\"): expected error") + } + for _, want := range []string{"--interval", `"soon"`, "30s", "seconds"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err, want) + } + } +}