Skip to content

Commit 7cfe95a

Browse files
authored
fix: parse duration strings in max-runtime config (#510)
## Summary - Adds `StringToTimeDurationHookFunc` to both mapstructure decoders (`decodeWithMetadata` and `weakDecodeConsistent`), enabling Go duration strings like `1h`, `30m`, `24h` to be parsed correctly for all `time.Duration` fields in INI config and Docker labels - Without this hook, mapstructure's `WeaklyTypedInput` attempted to parse duration strings as integers via `strconv.ParseInt`, causing config parsing failures - Affected fields: `max-runtime`, `notification-cooldown`, `config-poll-interval`, `docker-poll-interval`, `polling-fallback`, `poll-interval`, `restore-history-max-age`, webhook `timeout`, `retry-delay`, `preset-cache-ttl` Fixes #509 ## Test plan - [x] `TestDecodeWithMetadata_TimeDuration` — verifies `1h`, `30m`, `55s`, `1h30m`, `24h` all decode correctly - [x] `TestWeakDecodeConsistent_TimeDuration` — verifies duration parsing in Docker label decode path - [x] `TestBuildFromString_MaxRuntimeDuration` — end-to-end: INI config with `max-runtime = 1h` on a `job-run` - [x] `TestBuildFromString_AllDurationFields` — global `max-runtime = 2h` and `notification-cooldown = 5m` - [x] `TestBuildFromString_ServiceJobMaxRuntime` — service job with `max-runtime = 45m` - [x] Full test suite passes (`go test ./...`)
2 parents edadead + ca35768 commit 7cfe95a

File tree

2 files changed

+121
-0
lines changed

2 files changed

+121
-0
lines changed

cli/config_decode.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ func decodeWithMetadata(input map[string]any, output any) (*DecodeResult, error)
3131
WeaklyTypedInput: true,
3232
TagName: "mapstructure",
3333
MatchName: caseInsensitiveMatch,
34+
DecodeHook: mapstructure.StringToTimeDurationHookFunc(),
3435
}
3536

3637
decoder, err := mapstructure.NewDecoder(config)
@@ -65,6 +66,7 @@ func weakDecodeConsistent(input, output any) error {
6566
WeaklyTypedInput: true,
6667
TagName: "mapstructure",
6768
MatchName: caseInsensitiveMatch,
69+
DecodeHook: mapstructure.StringToTimeDurationHookFunc(),
6870
}
6971

7072
decoder, err := mapstructure.NewDecoder(config)

cli/config_decode_test.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package cli
55

66
import (
77
"testing"
8+
"time"
89

910
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
@@ -584,6 +585,124 @@ func TestMergeUsedKeys_AllNil(t *testing.T) {
584585
assert.Empty(t, merged)
585586
}
586587

588+
func TestDecodeWithMetadata_TimeDuration(t *testing.T) {
589+
t.Parallel()
590+
591+
type Config struct {
592+
Timeout time.Duration `mapstructure:"timeout"`
593+
}
594+
595+
tests := []struct {
596+
name string
597+
input string
598+
expected time.Duration
599+
}{
600+
{"hours", "1h", time.Hour},
601+
{"minutes", "30m", 30 * time.Minute},
602+
{"seconds", "55s", 55 * time.Second},
603+
{"combined", "1h30m", 90 * time.Minute},
604+
{"day-equivalent", "24h", 24 * time.Hour},
605+
}
606+
607+
for _, tt := range tests {
608+
t.Run(tt.name, func(t *testing.T) {
609+
t.Parallel()
610+
611+
var cfg Config
612+
_, err := decodeWithMetadata(map[string]any{"timeout": tt.input}, &cfg)
613+
require.NoError(t, err, "should parse duration string %q", tt.input)
614+
assert.Equal(t, tt.expected, cfg.Timeout)
615+
})
616+
}
617+
}
618+
619+
func TestWeakDecodeConsistent_TimeDuration(t *testing.T) {
620+
t.Parallel()
621+
622+
type Config struct {
623+
Timeout time.Duration `mapstructure:"timeout"`
624+
}
625+
626+
tests := []struct {
627+
name string
628+
input string
629+
expected time.Duration
630+
}{
631+
{"hours", "1h", time.Hour},
632+
{"minutes", "30m", 30 * time.Minute},
633+
{"seconds", "55s", 55 * time.Second},
634+
{"combined", "1h30m", 90 * time.Minute},
635+
}
636+
637+
for _, tt := range tests {
638+
t.Run(tt.name, func(t *testing.T) {
639+
t.Parallel()
640+
641+
var cfg Config
642+
err := weakDecodeConsistent(map[string]any{"timeout": tt.input}, &cfg)
643+
require.NoError(t, err, "should parse duration string %q", tt.input)
644+
assert.Equal(t, tt.expected, cfg.Timeout)
645+
})
646+
}
647+
}
648+
649+
func TestBuildFromString_MaxRuntimeDuration(t *testing.T) {
650+
t.Parallel()
651+
652+
configStr := `
653+
[job-run "test-job"]
654+
schedule = @every 5s
655+
image = busybox
656+
command = echo hello
657+
max-runtime = 1h
658+
`
659+
660+
logger, _ := test.NewTestLoggerWithHandler()
661+
cfg, err := BuildFromString(configStr, logger)
662+
require.NoError(t, err, "max-runtime = 1h should parse without error")
663+
require.Contains(t, cfg.RunJobs, "test-job")
664+
assert.Equal(t, time.Hour, cfg.RunJobs["test-job"].MaxRuntime)
665+
}
666+
667+
func TestBuildFromString_AllDurationFields(t *testing.T) {
668+
t.Parallel()
669+
670+
configStr := `
671+
[global]
672+
max-runtime = 2h
673+
notification-cooldown = 5m
674+
675+
[job-exec "test-exec"]
676+
schedule = @every 5s
677+
container = my-container
678+
command = echo hello
679+
`
680+
681+
logger, _ := test.NewTestLoggerWithHandler()
682+
cfg, err := BuildFromString(configStr, logger)
683+
require.NoError(t, err, "global duration fields should parse without error")
684+
assert.Equal(t, 2*time.Hour, cfg.Global.MaxRuntime)
685+
assert.Equal(t, 5*time.Minute, cfg.Global.NotificationCooldown)
686+
}
687+
688+
func TestBuildFromString_ServiceJobMaxRuntime(t *testing.T) {
689+
t.Parallel()
690+
691+
configStr := `
692+
[job-service-run "test-svc"]
693+
schedule = @every 5s
694+
image = busybox
695+
command = echo hello
696+
max-runtime = 45m
697+
`
698+
699+
logger, _ := test.NewTestLoggerWithHandler()
700+
cfg, err := BuildFromString(configStr, logger)
701+
require.NoError(t, err, "service job max-runtime = 45m should parse without error")
702+
require.Contains(t, cfg.ServiceJobs, "test-svc")
703+
assert.Equal(t, 45*time.Minute, cfg.ServiceJobs["test-svc"].MaxRuntime)
704+
}
705+
587706
func TestMergeUsedKeys_FalseValues(t *testing.T) {
588707
t.Parallel()
589708

0 commit comments

Comments
 (0)