Fix lastRefreshError not being reset on success (#11)

go.mod

@@ -4,6 +4,7 @@ go 1.14
 
 require (
 	github.com/exzz/netatmo-api-go v0.0.0-20171026152754-41589231f446
+	github.com/google/go-cmp v0.4.0
 	github.com/prometheus/client_golang v1.7.1
 	github.com/sirupsen/logrus v1.6.0
 	github.com/spf13/pflag v1.0.5

internal/collector/collector.go

@@ -112,12 +112,15 @@ var (
 		nil)
 )
 
+// ReadFunction defines the interface for reading from the Netatmo API.
+type ReadFunction func() (*netatmo.DeviceCollection, error)
+
 // NetatmoCollector is a Prometheus collector for Netatmo sensor values.
 type NetatmoCollector struct {
 	Log                 logrus.FieldLogger
 	RefreshInterval     time.Duration
 	StaleThreshold      time.Duration
-	Client              *netatmo.Client
+	ReadFunction        ReadFunction
 	lastRefresh         time.Time
 	lastRefreshError    error
 	lastRefreshDuration time.Duration
@@ -175,10 +178,10 @@ func (c *NetatmoCollector) RefreshData(now time.Time) {
 		c.lastRefreshDuration = time.Since(start)
 	}(time.Now())
 
-	devices, err := c.Client.Read()
+	devices, err := c.ReadFunction()
+	c.lastRefreshError = err
 	if err != nil {
 		c.Log.Errorf("Error during refresh: %s", err)
-		c.lastRefreshError = err
 		return
 	}
 

internal/collector/collector_test.go

@@ -0,0 +1,105 @@
+package collector
+
+import (
+	"errors"
+	"testing"
+	"time"
+
+	netatmo "github.com/exzz/netatmo-api-go"
+	"github.com/google/go-cmp/cmp"
+	"github.com/sirupsen/logrus"
+)
+
+func TestRefreshData(t *testing.T) {
+	testData := &netatmo.DeviceCollection{}
+	testError := errors.New("test error")
+	tt := []struct {
+		desc         string
+		time         time.Time
+		readFunction ReadFunction
+		wantTime     time.Time
+		wantData     *netatmo.DeviceCollection
+		wantError    error
+	}{
+		{
+			desc: "success",
+			time: time.Unix(0, 0),
+			readFunction: func() (*netatmo.DeviceCollection, error) {
+				return testData, nil
+			},
+			wantTime:  time.Unix(0, 0),
+			wantData:  testData,
+			wantError: nil,
+		},
+		{
+			desc: "error",
+			time: time.Unix(0, 0),
+			readFunction: func() (*netatmo.DeviceCollection, error) {
+				return nil, testError
+			},
+			wantTime:  time.Time{},
+			wantData:  nil,
+			wantError: testError,
+		},
+	}
+
+	for _, tc := range tt {
+		tc := tc
+		t.Run(tc.desc, func(t *testing.T) {
+			t.Parallel()
+
+			c := &NetatmoCollector{
+				Log:          logrus.New(),
+				ReadFunction: tc.readFunction,
+			}
+			c.RefreshData(tc.time)
+
+			if c.cacheTimestamp != tc.wantTime {
+				t.Errorf("got time %s, want %s", c.cacheTimestamp, tc.wantTime)
+			}
+
+			if diff := cmp.Diff(c.cachedData, tc.wantData); diff != "" {
+				t.Errorf("data differs: -got+want\n%s", diff)
+			}
+
+			if c.lastRefreshError != tc.wantError {
+				t.Errorf("got error %q, want %q", c.lastRefreshError, tc.wantError)
+			}
+		})
+	}
+}
+
+func TestRefreshDataResetError(t *testing.T) {
+	testData := &netatmo.DeviceCollection{}
+	testError := errors.New("test error")
+	successFunc := func() (*netatmo.DeviceCollection, error) {
+		return testData, nil
+	}
+	errorFunc := func() (*netatmo.DeviceCollection, error) {
+		return nil, testError
+	}
+
+	c := &NetatmoCollector{
+		Log:          logrus.New(),
+		ReadFunction: successFunc,
+	}
+	c.RefreshData(time.Unix(0, 0))
+
+	if c.lastRefreshError != nil {
+		t.Errorf("got error %q, want none", c.lastRefreshError)
+	}
+
+	c.ReadFunction = errorFunc
+	c.RefreshData(time.Unix(1, 0))
+
+	if c.lastRefreshError != testError {
+		t.Errorf("got error %q, want %q", c.lastRefreshError, testError)
+	}
+
+	c.ReadFunction = successFunc
+	c.RefreshData(time.Unix(0, 0))
+
+	if c.lastRefreshError != nil {
+		t.Errorf("got error %q, want none", c.lastRefreshError)
+	}
+}

main.go

@@ -40,7 +40,7 @@ func main() {
 
 	metrics := &collector.NetatmoCollector{
 		Log:             log,
-		Client:          client,
+		ReadFunction:    client.Read,
 		RefreshInterval: cfg.RefreshInterval,
 		StaleThreshold:  cfg.StaleDuration,
 	}