fix: parse day and fractional-second components in FromTimeSpan

pkg/machinepolicies/duration_formatter.go

@@ -3,6 +3,7 @@ package machinepolicies
 import (
 	"fmt"
 	"strconv"
+	"strings"
 	"time"
 )
 
@@ -30,20 +31,67 @@ func ToTimeSpan(duration time.Duration) string {
 	return fmt.Sprintf("%02d.%02d:%02d:%02d.%05d", days, hours, minutes, seconds, secondsFraction)
 }
 
+// FromTimeSpan parses a .NET time span, "[d.]hh:mm:ss[.fffffff]", into a duration. The day and
+// fractional-second components are optional, and the server does not pad the day component to a
+// fixed width, so the fields cannot be read at fixed offsets. Input that does not parse yields a
+// zero duration.
 func FromTimeSpan(timeSpan string) time.Duration {
-	if len(timeSpan) == 8 {
-		hours, _ := strconv.ParseInt(timeSpan[0:2], 10, 64)
-		minutes, _ := strconv.ParseInt(timeSpan[3:5], 10, 64)
-		seconds, _ := strconv.ParseInt(timeSpan[6:8], 10, 64)
-		duration, _ := time.ParseDuration(fmt.Sprintf("%dh%dm%ds", hours, minutes, seconds))
-		return duration
-	}
-
-	days, _ := strconv.ParseInt(timeSpan[0:0], 10, 32)
-	hours, _ := strconv.ParseInt(timeSpan[2:4], 10, 64)
-	hours += (days * 24)
-	minutes, _ := strconv.ParseInt(timeSpan[5:7], 10, 64)
-	seconds, _ := strconv.ParseInt(timeSpan[8:10], 10, 64)
-	duration, _ := time.ParseDuration(fmt.Sprintf("%dh%dm%ds", hours, minutes, seconds))
-	return duration
+	var days int64
+	remainder := timeSpan
+
+	// Both the day separator and the fractional-second separator are ".", so a leading segment is
+	// only the day component when the rest still holds a complete "hh:mm:ss".
+	if index := strings.Index(remainder, "."); index >= 0 && strings.Count(remainder[index+1:], ":") == 2 {
+		parsedDays, err := strconv.ParseInt(remainder[:index], 10, 64)
+		if err != nil {
+			return 0
+		}
+		days = parsedDays
+		remainder = remainder[index+1:]
+	}
+
+	var fraction time.Duration
+	if index := strings.Index(remainder, "."); index >= 0 {
+		digits := remainder[index+1:]
+		parsedFraction, err := strconv.ParseInt(digits, 10, 64)
+		if err != nil {
+			return 0
+		}
+		// The digits are a decimal fraction of a second, however many of them there are.
+		scale := pow10(len(digits))
+		fraction = time.Duration(parsedFraction * int64(time.Second) / scale)
+		remainder = remainder[:index]
+	}
+
+	fields := strings.Split(remainder, ":")
+	if len(fields) != 3 {
+		return 0
+	}
+
+	hours, err := strconv.ParseInt(fields[0], 10, 64)
+	if err != nil {
+		return 0
+	}
+	minutes, err := strconv.ParseInt(fields[1], 10, 64)
+	if err != nil {
+		return 0
+	}
+	seconds, err := strconv.ParseInt(fields[2], 10, 64)
+	if err != nil {
+		return 0
+	}
+
+	return time.Duration(days)*24*time.Hour +
+		time.Duration(hours)*time.Hour +
+		time.Duration(minutes)*time.Minute +
+		time.Duration(seconds)*time.Second +
+		fraction
+}
+
+func pow10(exponent int) int64 {
+	result := int64(1)
+	for i := 0; i < exponent; i++ {
+		result *= 10
+	}
+	return result
 }

pkg/machinepolicies/duration_formatter_test.go

@@ -0,0 +1,94 @@
+package machinepolicies
+
+import (
+	"testing"
+	"time"
+)
+
+func TestToTimeSpan(t *testing.T) {
+	halfSecond, _ := time.ParseDuration("0.5s")
+	second, _ := time.ParseDuration("1111ms")
+	twoHours, _ := time.ParseDuration("120m")
+	fourtySevenHours, _ := time.ParseDuration("47h")
+	twoDays, _ := time.ParseDuration("48h")
+
+	testCases := []struct {
+		duration time.Duration
+		expected string
+	}{
+		{halfSecond, "00:00:00.50000"},
+		{second, "00:00:01.11100"},
+		{time.Second, "00:00:01"},
+		{time.Minute, "00:01:00"},
+		{time.Hour, "01:00:00"},
+		{twoHours, "02:00:00"},
+		{fourtySevenHours, "01.23:00:00"},
+		{twoDays, "02.00:00:00"},
+	}
+
+	for _, testCase := range testCases {
+		if actual := ToTimeSpan(testCase.duration); actual != testCase.expected {
+			t.Errorf("ToTimeSpan(%s) = %q, want %q", testCase.duration, actual, testCase.expected)
+		}
+	}
+}
+
+func TestFromTimeSpan(t *testing.T) {
+	testCases := []struct {
+		timeSpan string
+		expected time.Duration
+	}{
+		// hh:mm:ss
+		{"00:00:00", 0},
+		{"00:00:01", time.Second},
+		{"00:01:00", time.Minute},
+		{"01:00:00", time.Hour},
+		{"02:00:00", 2 * time.Hour},
+		// d.hh:mm:ss, as written by ToTimeSpan
+		{"00.00:00:01", time.Second},
+		{"00.47:00:00", 47 * time.Hour},
+		{"01.23:00:00", 47 * time.Hour},
+		{"02.00:00:00", 48 * time.Hour},
+		// d.hh:mm:ss, as returned by the Octopus server, which does not pad the days
+		{"1.00:00:00", 24 * time.Hour},
+		{"7.12:30:00", 7*24*time.Hour + 12*time.Hour + 30*time.Minute},
+		{"37500.00:00:00", 900000 * time.Hour},
+		// fractional seconds
+		{"00:00:00.50000", 500 * time.Millisecond},
+		{"00:00:01.11100", 1111 * time.Millisecond},
+		{"01.00:00:00.50000", 24*time.Hour + 500*time.Millisecond},
+		// .NET renders fractional seconds as seven digits
+		{"00:00:00.5000000", 500 * time.Millisecond},
+		// malformed input yields a zero duration rather than a panic
+		{"", 0},
+		{"not-a-timespan", 0},
+		{"00:00", 0},
+	}
+
+	for _, testCase := range testCases {
+		if actual := FromTimeSpan(testCase.timeSpan); actual != testCase.expected {
+			t.Errorf("FromTimeSpan(%q) = %s, want %s", testCase.timeSpan, actual, testCase.expected)
+		}
+	}
+}
+
+func TestTimeSpanRoundTrip(t *testing.T) {
+	durations := []time.Duration{
+		0,
+		time.Second,
+		time.Minute,
+		time.Hour,
+		47 * time.Hour,
+		48 * time.Hour,
+		7 * 24 * time.Hour,
+		900000 * time.Hour,
+		500 * time.Millisecond,
+		24*time.Hour + 12*time.Hour + 30*time.Minute + 15*time.Second,
+	}
+
+	for _, duration := range durations {
+		if actual := FromTimeSpan(ToTimeSpan(duration)); actual != duration {
+			t.Errorf("FromTimeSpan(ToTimeSpan(%s)) = %s, want %s", duration, actual, duration)
+		}
+	}
+}

pkg/machines/duration_formatter.go

@@ -3,6 +3,7 @@ package machines
 import (
 	"fmt"
 	"strconv"
+	"strings"
 	"time"
 )
 
@@ -30,20 +31,67 @@ func ToTimeSpan(duration time.Duration) string {
 	return fmt.Sprintf("%02d.%02d:%02d:%02d.%05d", days, hours, minutes, seconds, secondsFraction)
 }
 
+// FromTimeSpan parses a .NET time span, "[d.]hh:mm:ss[.fffffff]", into a duration. The day and
+// fractional-second components are optional, and the server does not pad the day component to a
+// fixed width, so the fields cannot be read at fixed offsets. Input that does not parse yields a
+// zero duration.
 func FromTimeSpan(timeSpan string) time.Duration {
-	if len(timeSpan) == 8 {
-		hours, _ := strconv.ParseInt(timeSpan[0:2], 10, 64)
-		minutes, _ := strconv.ParseInt(timeSpan[3:5], 10, 64)
-		seconds, _ := strconv.ParseInt(timeSpan[6:8], 10, 64)
-		duration, _ := time.ParseDuration(fmt.Sprintf("%dh%dm%ds", hours, minutes, seconds))
-		return duration
-	}
-
-	days, _ := strconv.ParseInt(timeSpan[0:0], 10, 32)
-	hours, _ := strconv.ParseInt(timeSpan[2:4], 10, 64)
-	hours += (days * 24)
-	minutes, _ := strconv.ParseInt(timeSpan[5:7], 10, 64)
-	seconds, _ := strconv.ParseInt(timeSpan[8:10], 10, 64)
-	duration, _ := time.ParseDuration(fmt.Sprintf("%dh%dm%ds", hours, minutes, seconds))
-	return duration
+	var days int64
+	remainder := timeSpan
+
+	// Both the day separator and the fractional-second separator are ".", so a leading segment is
+	// only the day component when the rest still holds a complete "hh:mm:ss".
+	if index := strings.Index(remainder, "."); index >= 0 && strings.Count(remainder[index+1:], ":") == 2 {
+		parsedDays, err := strconv.ParseInt(remainder[:index], 10, 64)
+		if err != nil {
+			return 0
+		}
+		days = parsedDays
+		remainder = remainder[index+1:]
+	}
+
+	var fraction time.Duration
+	if index := strings.Index(remainder, "."); index >= 0 {
+		digits := remainder[index+1:]
+		parsedFraction, err := strconv.ParseInt(digits, 10, 64)
+		if err != nil {
+			return 0
+		}
+		// The digits are a decimal fraction of a second, however many of them there are.
+		scale := pow10(len(digits))
+		fraction = time.Duration(parsedFraction * int64(time.Second) / scale)
+		remainder = remainder[:index]
+	}
+
+	fields := strings.Split(remainder, ":")
+	if len(fields) != 3 {
+		return 0
+	}
+
+	hours, err := strconv.ParseInt(fields[0], 10, 64)
+	if err != nil {
+		return 0
+	}
+	minutes, err := strconv.ParseInt(fields[1], 10, 64)
+	if err != nil {
+		return 0
+	}
+	seconds, err := strconv.ParseInt(fields[2], 10, 64)
+	if err != nil {
+		return 0
+	}
+
+	return time.Duration(days)*24*time.Hour +
+		time.Duration(hours)*time.Hour +
+		time.Duration(minutes)*time.Minute +
+		time.Duration(seconds)*time.Second +
+		fraction
+}
+
+func pow10(exponent int) int64 {
+	result := int64(1)
+	for i := 0; i < exponent; i++ {
+		result *= 10
+	}
+	return result
 }

pkg/machines/duration_formatter_test.go

@@ -11,22 +11,84 @@ func TestToTimeSpan(t *testing.T) {
 	twoHours, _ := time.ParseDuration("120m")
 	fourtySevenHours, _ := time.ParseDuration("47h")
 	twoDays, _ := time.ParseDuration("48h")
-	t.Logf("500ms: %s", ToTimeSpan(halfSecond))
-	t.Logf("1000ms: %s", ToTimeSpan(second))
-	t.Logf("1s: %s", ToTimeSpan(time.Second))
-	t.Logf("1m: %s", ToTimeSpan(time.Minute))
-	t.Logf("1h: %s", ToTimeSpan(time.Hour))
-	t.Logf("120m: %s", ToTimeSpan(twoHours))
-	t.Logf("47h: %s", ToTimeSpan(fourtySevenHours))
-	t.Logf("48h: %s", ToTimeSpan(twoDays))
+
+	testCases := []struct {
+		duration time.Duration
+		expected string
+	}{
+		{halfSecond, "00:00:00.50000"},
+		{second, "00:00:01.11100"},
+		{time.Second, "00:00:01"},
+		{time.Minute, "00:01:00"},
+		{time.Hour, "01:00:00"},
+		{twoHours, "02:00:00"},
+		{fourtySevenHours, "01.23:00:00"},
+		{twoDays, "02.00:00:00"},
+	}
+
+	for _, testCase := range testCases {
+		if actual := ToTimeSpan(testCase.duration); actual != testCase.expected {
+			t.Errorf("ToTimeSpan(%s) = %q, want %q", testCase.duration, actual, testCase.expected)
+		}
+	}
 }
 
 func TestFromTimeSpan(t *testing.T) {
-	t.Logf("1s: %s", FromTimeSpan("00.00:00:01"))
-	t.Logf("1m: %s", FromTimeSpan("00.00:01:00"))
-	t.Logf("1h: %s", FromTimeSpan("00.01:00:00"))
-	t.Logf("120m: %s", FromTimeSpan("00.02:00:00"))
-	t.Logf("47h: %s", FromTimeSpan("00.47:00:00"))
-	t.Logf("48h: %s", FromTimeSpan("00.48:00:00"))
-	t.Logf("2d: %s", FromTimeSpan("02.00:00:00"))
+	testCases := []struct {
+		timeSpan string
+		expected time.Duration
+	}{
+		// hh:mm:ss
+		{"00:00:00", 0},
+		{"00:00:01", time.Second},
+		{"00:01:00", time.Minute},
+		{"01:00:00", time.Hour},
+		{"02:00:00", 2 * time.Hour},
+		// d.hh:mm:ss, as written by ToTimeSpan
+		{"00.00:00:01", time.Second},
+		{"00.47:00:00", 47 * time.Hour},
+		{"01.23:00:00", 47 * time.Hour},
+		{"02.00:00:00", 48 * time.Hour},
+		// d.hh:mm:ss, as returned by the Octopus server, which does not pad the days
+		{"1.00:00:00", 24 * time.Hour},
+		{"7.12:30:00", 7*24*time.Hour + 12*time.Hour + 30*time.Minute},
+		{"37500.00:00:00", 900000 * time.Hour},
+		// fractional seconds
+		{"00:00:00.50000", 500 * time.Millisecond},
+		{"00:00:01.11100", 1111 * time.Millisecond},
+		{"01.00:00:00.50000", 24*time.Hour + 500*time.Millisecond},
+		// .NET renders fractional seconds as seven digits
+		{"00:00:00.5000000", 500 * time.Millisecond},
+		// malformed input yields a zero duration rather than a panic
+		{"", 0},
+		{"not-a-timespan", 0},
+		{"00:00", 0},
+	}
+
+	for _, testCase := range testCases {
+		if actual := FromTimeSpan(testCase.timeSpan); actual != testCase.expected {
+			t.Errorf("FromTimeSpan(%q) = %s, want %s", testCase.timeSpan, actual, testCase.expected)
+		}
+	}
+}
+
+func TestTimeSpanRoundTrip(t *testing.T) {
+	durations := []time.Duration{
+		0,
+		time.Second,
+		time.Minute,
+		time.Hour,
+		47 * time.Hour,
+		48 * time.Hour,
+		7 * 24 * time.Hour,
+		900000 * time.Hour,
+		500 * time.Millisecond,
+		24*time.Hour + 12*time.Hour + 30*time.Minute + 15*time.Second,
+	}
+
+	for _, duration := range durations {
+		if actual := FromTimeSpan(ToTimeSpan(duration)); actual != duration {
+			t.Errorf("FromTimeSpan(ToTimeSpan(%s)) = %s, want %s", duration, actual, duration)
+		}
+	}
 }