health: move health metrics test to health_test

Updates #13420

Signed-off-by: Kristoffer Dalby <[email protected]>

health/health.go

@@ -331,7 +331,7 @@ func (t *Tracker) SetMetricsRegistry(reg *usermetric.Registry) {
 	)
 
 	t.metricHealthMessage.Set(metricHealthMessageLabel{
-		Type: "warning",
+		Type: MetricLabelWarning,
 	}, expvar.Func(func() any {
 		if t.nil() {
 			return 0
@@ -1283,6 +1283,8 @@ func (t *Tracker) LastNoiseDialWasRecent() bool {
 	return dur < 2*time.Minute
 }
 
+const MetricLabelWarning = "warning"
+
 type metricHealthMessageLabel struct {
 	// TODO: break down by warnable.severity as well?
 	Type string

health/health_test.go

@@ -7,11 +7,13 @@ import (
 	"fmt"
 	"reflect"
 	"slices"
+	"strconv"
 	"testing"
 	"time"
 
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/opt"
+	"tailscale.com/util/usermetric"
 )
 
 func TestAppendWarnableDebugFlags(t *testing.T) {
@@ -273,7 +275,7 @@ func TestShowUpdateWarnable(t *testing.T) {
 		wantShow     bool
 	}{
 		{
-			desc:         "nil CientVersion",
+			desc:         "nil ClientVersion",
 			check:        true,
 			cv:           nil,
 			wantWarnable: nil,
@@ -348,3 +350,47 @@ func TestShowUpdateWarnable(t *testing.T) {
 		})
 	}
 }
+
+func TestHealthMetric(t *testing.T) {
+	tests := []struct {
+		desc            string
+		check           bool
+		apply           opt.Bool
+		cv              *tailcfg.ClientVersion
+		wantMetricCount int
+	}{
+		// When running in dev, and not initialising the client, there will be two warnings
+		// by default:
+		// - is-using-unstable-version
+		// - wantrunning-false
+		{
+			desc:            "base-warnings",
+			check:           true,
+			cv:              nil,
+			wantMetricCount: 2,
+		},
+		// with: update-available
+		{
+			desc:            "update-warning",
+			check:           true,
+			cv:              &tailcfg.ClientVersion{RunningLatest: false, LatestVersion: "1.2.3"},
+			wantMetricCount: 3,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			tr := &Tracker{
+				checkForUpdates: tt.check,
+				applyUpdates:    tt.apply,
+				latestVersion:   tt.cv,
+			}
+			tr.SetMetricsRegistry(&usermetric.Registry{})
+			if val := tr.metricHealthMessage.Get(metricHealthMessageLabel{Type: MetricLabelWarning}).String(); val != strconv.Itoa(tt.wantMetricCount) {
+				t.Fatalf("metric value: %q, want: %q", val, strconv.Itoa(tt.wantMetricCount))
+			}
+			for _, w := range tr.CurrentState().Warnings {
+				t.Logf("warning: %v", w)
+			}
+		})
+	}
+}

tsnet/tsnet_test.go

@@ -38,7 +38,6 @@ import (
 	"golang.org/x/net/proxy"
 	"tailscale.com/client/tailscale"
 	"tailscale.com/cmd/testwrapper/flakytest"
-	"tailscale.com/health"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/store/mem"
 	"tailscale.com/net/netns"
@@ -822,16 +821,6 @@ func TestUDPConn(t *testing.T) {
 	}
 }
 
-// testWarnable is a Warnable that is used within this package for testing purposes only.
-var testWarnable = health.Register(&health.Warnable{
-	Code:     "test-warnable-tsnet",
-	Title:    "Test warnable",
-	Severity: health.SeverityLow,
-	Text: func(args health.Args) string {
-		return args[health.ArgError]
-	},
-})
-
 func parseMetrics(m []byte) (map[string]float64, error) {
 	metrics := make(map[string]float64)
 
@@ -1045,11 +1034,6 @@ func TestUserMetrics(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	status1, err := lc1.Status(ctxLc)
-	if err != nil {
-		t.Fatal(err)
-	}
-
 	parsedMetrics1, err := parseMetrics(metrics1)
 	if err != nil {
 		t.Fatal(err)
@@ -1075,11 +1059,6 @@ func TestUserMetrics(t *testing.T) {
 		t.Errorf("metrics1, tailscaled_approved_routes: got %v, want %v", got, want)
 	}
 
-	// Validate the health counter metric against the status of the node
-	if got, want := parsedMetrics1[`tailscaled_health_messages{type="warning"}`], float64(len(status1.Health)); got != want {
-		t.Errorf("metrics1, tailscaled_health_messages: got %v, want %v", got, want)
-	}
-
 	// Verify that the amount of data recorded in bytes is higher or equal to the
 	// 10 megabytes sent.
 	inboundBytes1 := parsedMetrics1[`tailscaled_inbound_bytes_total{path="direct_ipv4"}`]
@@ -1097,11 +1076,6 @@ func TestUserMetrics(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	status2, err := lc2.Status(ctx)
-	if err != nil {
-		t.Fatal(err)
-	}
-
 	parsedMetrics2, err := parseMetrics(metrics2)
 	if err != nil {
 		t.Fatal(err)
@@ -1119,11 +1093,6 @@ func TestUserMetrics(t *testing.T) {
 		t.Errorf("metrics2, tailscaled_approved_routes: got %v, want %v", got, want)
 	}
 
-	// Validate the health counter metric against the status of the node
-	if got, want := parsedMetrics2[`tailscaled_health_messages{type="warning"}`], float64(len(status2.Health)); got != want {
-		t.Errorf("metrics2, tailscaled_health_messages: got %v, want %v", got, want)
-	}
-
 	// Verify that the amount of data recorded in bytes is higher or equal than the
 	// 10 megabytes sent.
 	outboundBytes2 := parsedMetrics2[`tailscaled_outbound_bytes_total{path="direct_ipv4"}`]