fix: watcher missconfiguration checks (#23)

* fix: watcher missconfiguration checks

Fixes some of the checks for initialising the WatchConfig and adds test coverage.

AB#9753

* use builtin time compare as it is clearer

---------

Co-authored-by: Robin Bryce <[email protected]>

.gitignore

@@ -1,2 +1,3 @@
 .local
 **/test_results
+.vscode

massifs/watcher/watcher.go

@@ -62,16 +62,16 @@ func (w Watcher) ConfigString() string {
 }
 
 func ConfigDefaults(cfg *WatchConfig) error {
-	if cfg.Since.UnixMilli() == 0 || cfg.IDSince != "" && cfg.Horizon != 0 {
+	if cfg.Since.Equal(time.Time{}) && cfg.IDSince == "" && cfg.Horizon == 0 {
 		return fmt.Errorf("provide horizon on its own or either of the since parameters.")
-
 	}
 	// If horizon is provided, the since values are derived
 
 	if cfg.Interval == 0 {
 		cfg.Interval = DefaultInterval
 	}
 
+	var sinceUnset bool
 	if cfg.Horizon == 0 {
 		// temporarily force a horizon
 		cfg.Horizon = time.Second * 30
@@ -80,9 +80,10 @@ func ConfigDefaults(cfg *WatchConfig) error {
 	// since defaults to now (but will get trumped by horizon if that was provided)
 	if cfg.Since.UnixMilli() == 0 {
 		cfg.Since = time.Now()
+		sinceUnset = true
 	}
-	// horizon trumps since
-	if cfg.Horizon > 0 {
+	// explicit horizon trumps since, and the default horizon trumps the default since
+	if cfg.Horizon > 0 && sinceUnset {
 		cfg.Since = time.Now().Add(-cfg.Horizon)
 	}
 	if cfg.IDSince == "" {

massifs/watcher/watcher_test.go

@@ -0,0 +1,77 @@
+package watcher
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type checkWatchConfig func(t *testing.T, cfg WatchConfig)
+
+func TestConfigDefaults(t *testing.T) {
+	hourSince := time.Now().Add(-time.Hour)
+
+	type args struct {
+		cfg *WatchConfig
+	}
+	tests := []struct {
+		name      string
+		args      args
+		errPrefix string
+		check     checkWatchConfig
+	}{
+		{
+			name: "horizon or since options are required",
+			args: args{
+				cfg: &WatchConfig{},
+			},
+			errPrefix: "provide horizon on its own or either of the since",
+		},
+
+		{
+			name: "poll with since an hour in the past",
+			args: args{
+				cfg: &WatchConfig{
+					Since: hourSince,
+				},
+			},
+			check: func(t *testing.T, cfg WatchConfig) {
+				assert.Equal(t, hourSince, cfg.Since)
+				assert.Equal(t, time.Second, cfg.Interval)
+				assert.NotEqual(t, "", cfg.IDSince) // should be set to IDTimeHex
+			},
+		},
+
+		{
+			name: "bad hex string for idtimestamp errors",
+			args: args{
+				cfg: &WatchConfig{
+					IDSince: "thisisnothex",
+				},
+			},
+			errPrefix: "encoding/hex: invalid byte",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ConfigDefaults(tt.args.cfg)
+			if err != nil {
+				if tt.errPrefix == "" {
+					t.Errorf("NewWatchConfig() unexpected error = %v", err)
+				}
+				if !strings.HasPrefix(err.Error(), tt.errPrefix) {
+					t.Errorf("NewWatchConfig() unexpected error = %v, expected prefix: %s", err, tt.errPrefix)
+				}
+			} else {
+				if tt.errPrefix != "" {
+					t.Errorf("NewWatchConfig() expected error prefix = %s", tt.errPrefix)
+				}
+			}
+			if tt.check != nil {
+				tt.check(t, *tt.args.cfg)
+			}
+		})
+	}
+}