Accept a bare integer as seconds for watchpr --interval
- add agent.ParseDurationFlag: bare integer means seconds, duration strings still parse - take --interval as a string and parse it in the command - reject unparseable and non-positive values with an error naming the flag
This commit is contained in:
@@ -70,6 +70,9 @@ watchpr unkin/argocd-apps#42
|
|||||||
# Multiple PRs, custom interval; refs accept #N or :N
|
# Multiple PRs, custom interval; refs accept #N or :N
|
||||||
watchpr --interval 30s unkin/argocd-apps#42 unkin/terraform-vault:98
|
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)
|
# One-shot: print current state and exit 0 (great for scripts)
|
||||||
watchpr --once unkin/argocd-apps#42
|
watchpr --once unkin/argocd-apps#42
|
||||||
watchpr --once --json unkin/argocd-apps#42
|
watchpr --once --json unkin/argocd-apps#42
|
||||||
|
|||||||
+7
-2
@@ -7,6 +7,7 @@
|
|||||||
// watchpr owner/repo#12 owner/repo:15
|
// watchpr owner/repo#12 owner/repo:15
|
||||||
// watchpr --once --json owner/repo#12
|
// watchpr --once --json owner/repo#12
|
||||||
// watchpr --interval 30s owner/repo#12
|
// watchpr --interval 30s owner/repo#12
|
||||||
|
// watchpr --interval 30 owner/repo#12
|
||||||
package main
|
package main
|
||||||
|
|
||||||
import (
|
import (
|
||||||
@@ -34,7 +35,7 @@ func main() {
|
|||||||
// tests can invoke Execute and assert the exit behaviour without spawning a
|
// tests can invoke Execute and assert the exit behaviour without spawning a
|
||||||
// process.
|
// process.
|
||||||
func newRootCmd() *cobra.Command {
|
func newRootCmd() *cobra.Command {
|
||||||
var interval time.Duration
|
var intervalFlag string
|
||||||
var once, jsonMode bool
|
var once, jsonMode bool
|
||||||
|
|
||||||
root := &cobra.Command{
|
root := &cobra.Command{
|
||||||
@@ -50,6 +51,10 @@ func newRootCmd() *cobra.Command {
|
|||||||
if len(args) == 0 {
|
if len(args) == 0 {
|
||||||
return fmt.Errorf("no PR references given (e.g. owner/repo#12)")
|
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))
|
refs := make([]agent.PRRef, 0, len(args))
|
||||||
for _, a := range args {
|
for _, a := range args {
|
||||||
ref, err := agent.ParsePRRef(a)
|
ref, err := agent.ParsePRRef(a)
|
||||||
@@ -68,7 +73,7 @@ func newRootCmd() *cobra.Command {
|
|||||||
root.SetVersionTemplate("{{.Version}}\n")
|
root.SetVersionTemplate("{{.Version}}\n")
|
||||||
|
|
||||||
f := root.Flags()
|
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(&once, "once", false, "Check once, print current state, and exit")
|
||||||
f.BoolVar(&jsonMode, "json", false, "Emit JSON")
|
f.BoolVar(&jsonMode, "json", false, "Emit JSON")
|
||||||
|
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -70,3 +71,37 @@ func TestOnceRunsAnonymouslyWhenNoTokenIsAvailable(t *testing.T) {
|
|||||||
t.Errorf("sent %d Authorization headers, want none", authHeaders)
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
// PRRef identifies a single pull request by repository and number.
|
// 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
|
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.
|
// ParseRepo validates and splits an "owner/repo" string.
|
||||||
func ParseRepo(s string) (owner, repo string, err error) {
|
func ParseRepo(s string) (owner, repo string, err error) {
|
||||||
s = strings.TrimSpace(s)
|
s = strings.TrimSpace(s)
|
||||||
|
|||||||
@@ -1,6 +1,10 @@
|
|||||||
package agent
|
package agent
|
||||||
|
|
||||||
import "testing"
|
import (
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
func TestParsePRRef(t *testing.T) {
|
func TestParsePRRef(t *testing.T) {
|
||||||
tests := []struct {
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user