feat(inputs.huebridge): Re-work after PR review

plugins/inputs/huebridge/bridgemetadata.go

@@ -99,57 +99,51 @@ func fetchRoomAssignments(bridgeClient hue.BridgeClient) (map[string]string, err
 
 func (metadata *bridgeMetadata) resolveResourceRoom(resourceId string, resourceName string) string {
 	roomName := metadata.roomAssignments[resourceName]
+	if roomName != "" {
+		return roomName
+	}
 	// If resource does not have a room assigned directly, iterate upwards via
 	// its owners until we find a room or there is no more owner. The latter
 	// may happen (e.g. for Motion Sensors) resulting in room name
 	// "<unassigned>".
-	if roomName == "" {
-		// No room so far, search via the the owner hierarchy...
-		currentResourceId := resourceId
-		for {
-			// Try next owner
-			currentResourceId = metadata.resourceTree[currentResourceId]
-			if currentResourceId == "" {
-				// No owner, no room
-				break
-			}
-			roomName = metadata.roomAssignments[currentResourceId]
-			if roomName != "" {
-				// Room name found, done
-				break
-			}
+	currentResourceId := resourceId
+	for {
+		// Try next owner
+		currentResourceId = metadata.resourceTree[currentResourceId]
+		if currentResourceId == "" {
+			// No owner left but no room found
+			break
+		}
+		roomName = metadata.roomAssignments[currentResourceId]
+		if roomName != "" {
+			// Room name found, done
+			return roomName
 		}
 	}
-	if roomName == "" {
-		roomName = "<unassigned>"
-	}
-	return roomName
+	return "<unassigned>"
 }
 
 func (metadata *bridgeMetadata) resolveDeviceName(resourceId string) string {
 	deviceName := metadata.deviceNames[resourceId]
+	if deviceName != "" {
+		return deviceName
+	}
 	// If resource does not have a device name assigned directly, iterate
 	// upwards via its owners until we find a room or there is no more
 	// owner. The latter may happen resulting in device name "<undefined>".
-	if deviceName == "" {
-		// No device so far, search via the the owner hierarchy...
-		currentResourceId := resourceId
-		for {
-			// Try next owner
-			currentResourceId = metadata.resourceTree[currentResourceId]
-			if currentResourceId == "" {
-				// No owner, no device
-				break
-			}
-			deviceName = metadata.deviceNames[currentResourceId]
-			if deviceName != "" {
-				// Device name found, done
-				break
-			}
+	currentResourceId := resourceId
+	for {
+		// Try next owner
+		currentResourceId = metadata.resourceTree[currentResourceId]
+		if currentResourceId == "" {
+			// No owner left but no device found
+			break
+		}
+		deviceName = metadata.deviceNames[currentResourceId]
+		if deviceName != "" {
+			// Device name found, done
+			return deviceName
 		}
 	}
-	if deviceName == "" {
-		deviceName = "<undefined>"
-	}
-	return deviceName
+	return "<undefined>"
 }

plugins/inputs/huebridge/huebridge_test.go

@@ -16,26 +16,30 @@ import (
 )
 
 func TestConfig(t *testing.T) {
+	// Verify plugin can be loaded from config
 	conf := config.NewConfig()
 	require.NoError(t, conf.LoadConfig("testdata/conf/huebridge.conf"))
 	require.Len(t, conf.Inputs, 1)
 	h, ok := conf.Inputs[0].Input.(*HueBridge)
 	require.True(t, ok)
 
+	// Verify successful Init
 	require.NoError(t, h.Init())
 
+	// Verify everything is setup according to config file
 	require.Len(t, h.Bridges, 4)
-	require.NotEmpty(t, h.RemoteClientId)
-	require.NotEmpty(t, h.RemoteClientSecret)
-	require.NotEmpty(t, h.RemoteCallbackUrl)
-	require.NotEmpty(t, h.RemoteTokenDir)
+	require.Equal(t, "client", h.RemoteClientId)
+	require.Equal(t, "secret", h.RemoteClientSecret)
+	require.Equal(t, "url", h.RemoteCallbackUrl)
+	require.Equal(t, "dir", h.RemoteTokenDir)
 	require.Len(t, h.RoomAssignments, 2)
 	require.Equal(t, config.Duration(60*time.Second), h.Timeout)
 	require.Equal(t, "secret", h.TLSKeyPwd)
 	require.True(t, h.InsecureSkipVerify)
 }
 
 func TestInitSuccess(t *testing.T) {
+	// Create plugin instance with all types of URL schemes
 	h := &HueBridge{
 		Bridges: []string{
 			"address://12345678:secret@localhost/",
@@ -55,13 +59,15 @@ func TestInitSuccess(t *testing.T) {
 		Log:     &testutil.Logger{Name: "huebridge"},
 	}
 
+	// Verify successful Init
 	require.NoError(t, h.Init())
 
-	require.Len(t, h.configuredBridges, 4)
+	// Verify successful configuration of all bridge URLs
+	require.Len(t, h.configuredBridges, len(h.Bridges))
 }
 
 func TestInitIgnoreInvalidUrls(t *testing.T) {
-	// The following URLs must all be ignored during Init
+	// The following URLs are all invalid must all be ignored during Init
 	h := &HueBridge{
 		Bridges: []string{
 			"invalid://12345678:[email protected]/",
@@ -75,16 +81,18 @@ func TestInitIgnoreInvalidUrls(t *testing.T) {
 		Log:     &testutil.Logger{Name: "huebridge"},
 	}
 
+	// Verify successful Init
 	require.NoError(t, h.Init())
 
+	// Verify no bridge have been configured
 	require.Len(t, h.configuredBridges, 0)
 }
 
 func TestGatherLocal(t *testing.T) {
+	// Start mock server and make plugin targing it
 	bridgeMock := mock.Start()
 	require.NotNil(t, bridgeMock)
 	defer bridgeMock.Shutdown()
-
 	h := &HueBridge{
 		Bridges: []string{
 			fmt.Sprintf("address://%s:%s@%s/", mock.MockBridgeId, mock.MockBridgeUsername, bridgeMock.Server().Host),
@@ -94,21 +102,25 @@ func TestGatherLocal(t *testing.T) {
 		Log:             &testutil.Logger{Name: "huebridge"},
 	}
 
+	// Verify successful Init
 	require.NoError(t, h.Init())
 
+	// Verify successfull Gather
 	acc := &testutil.Accumulator{}
-
 	require.NoError(t, acc.GatherError(h.Gather))
-	testMetric(t, acc, "testdata/metrics/huebridge.txt", telegraf.Gauge)
+
+	// Verify collected metrics are as expected
+	expectedMetrics := loadExpectedMetrics(t, "testdata/metrics/huebridge.txt", telegraf.Gauge)
+	testutil.RequireMetricsEqual(t, expectedMetrics, acc.GetTelegrafMetrics(), testutil.IgnoreTime(), testutil.SortMetrics())
 }
 
-func testMetric(t *testing.T, acc *testutil.Accumulator, file string, vt telegraf.ValueType) {
+func loadExpectedMetrics(t *testing.T, file string, vt telegraf.ValueType) []telegraf.Metric {
 	parser := &influx.Parser{}
 	require.NoError(t, parser.Init())
 	expectedMetrics, err := testutil.ParseMetricsFromFile(file, parser)
 	require.NoError(t, err)
 	for index := range expectedMetrics {
 		expectedMetrics[index].SetType(vt)
 	}
-	testutil.RequireMetricsEqual(t, expectedMetrics, acc.GetTelegrafMetrics(), testutil.IgnoreTime(), testutil.SortMetrics())
+	return expectedMetrics
 }