From edbc2beddd98e8c07ee13ceaeb6f4e14d65656d2 Mon Sep 17 00:00:00 2001 From: Tyler Koenig Date: Sat, 27 Jun 2026 11:53:10 -0400 Subject: [PATCH] fix(test): check errors instead of discarding with bare _ Replace bare _ error discards with t.Fatalf checks across sqlstore_test, crypto_test, server_test, and checker_test. --- internal/monitor/checker_test.go | 50 +++++++++++++++++++++++------ internal/server/server_test.go | 55 +++++++++++++++++++++++++------- internal/store/crypto_test.go | 10 ++++-- internal/store/sqlstore_test.go | 20 +++++++++--- 4 files changed, 107 insertions(+), 28 deletions(-) diff --git a/internal/monitor/checker_test.go b/internal/monitor/checker_test.go index 558b404..6468a0b 100644 --- a/internal/monitor/checker_test.go +++ b/internal/monitor/checker_test.go @@ -130,8 +130,14 @@ func TestRunCheck_Port_Open(t *testing.T) { } defer ln.Close() - _, portStr, _ := net.SplitHostPort(ln.Addr().String()) - port, _ := strconv.Atoi(portStr) + _, portStr, err := net.SplitHostPort(ln.Addr().String()) + if err != nil { + t.Fatalf("SplitHostPort: %v", err) + } + port, err := strconv.Atoi(portStr) + if err != nil { + t.Fatalf("Atoi: %v", err) + } site := models.SiteConfig{ID: 1, Type: "port", Hostname: "127.0.0.1", Port: port, Timeout: 2} result := RunCheck(context.Background(), site, nil, nil, false, true) @@ -149,8 +155,14 @@ func TestRunCheck_Port_Closed(t *testing.T) { if err != nil { t.Fatal(err) } - _, portStr, _ := net.SplitHostPort(ln.Addr().String()) - port, _ := strconv.Atoi(portStr) + _, portStr, err := net.SplitHostPort(ln.Addr().String()) + if err != nil { + t.Fatalf("SplitHostPort: %v", err) + } + port, err := strconv.Atoi(portStr) + if err != nil { + t.Fatalf("Atoi: %v", err) + } ln.Close() site := models.SiteConfig{ID: 1, Type: "port", Hostname: "127.0.0.1", Port: port, Timeout: 1} @@ -168,8 +180,14 @@ func TestRunPortCheck_UsesPinnedIP(t *testing.T) { } defer ln.Close() - _, portStr, _ := net.SplitHostPort(ln.Addr().String()) - port, _ := strconv.Atoi(portStr) + _, portStr, err := net.SplitHostPort(ln.Addr().String()) + if err != nil { + t.Fatalf("SplitHostPort: %v", err) + } + port, err := strconv.Atoi(portStr) + if err != nil { + t.Fatalf("Atoi: %v", err) + } // Pass a pinned IP — runPortCheck should dial it instead of resolving Hostname. site := models.SiteConfig{ID: 1, Type: "port", Hostname: "will-not-resolve.invalid", Port: port, Timeout: 2} @@ -187,8 +205,14 @@ func TestRunPortCheck_NilPinnedIP_UsesHostname(t *testing.T) { } defer ln.Close() - _, portStr, _ := net.SplitHostPort(ln.Addr().String()) - port, _ := strconv.Atoi(portStr) + _, portStr, err := net.SplitHostPort(ln.Addr().String()) + if err != nil { + t.Fatalf("SplitHostPort: %v", err) + } + port, err := strconv.Atoi(portStr) + if err != nil { + t.Fatalf("Atoi: %v", err) + } site := models.SiteConfig{ID: 1, Type: "port", Hostname: "127.0.0.1", Port: port, Timeout: 2} result := runPortCheck(context.Background(), site, nil) @@ -205,8 +229,14 @@ func TestRunCheck_Port_BlocksPrivateByDefault(t *testing.T) { } defer ln.Close() - _, portStr, _ := net.SplitHostPort(ln.Addr().String()) - port, _ := strconv.Atoi(portStr) + _, portStr, err := net.SplitHostPort(ln.Addr().String()) + if err != nil { + t.Fatalf("SplitHostPort: %v", err) + } + port, err := strconv.Atoi(portStr) + if err != nil { + t.Fatalf("Atoi: %v", err) + } site := models.SiteConfig{ID: 1, Type: "port", Hostname: "127.0.0.1", Port: port, Timeout: 2} result := RunCheck(context.Background(), site, nil, nil, false, false) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index d92ec4b..7394fd4 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -77,8 +77,12 @@ func (m *mockStore) GetActiveMaintenanceWindows(_ context.Context) ([]models.Mai // --- Helpers --- -func freePort() int { - ln, _ := net.Listen("tcp", "127.0.0.1:0") +func freePort(t *testing.T) int { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("freePort: %v", err) + } port := ln.Addr().(*net.TCPAddr).Port ln.Close() return port @@ -95,7 +99,7 @@ func newTestServer(t *testing.T, clusterKey string, enableStatus bool) *testServ t.Helper() ms := newMockStore() eng := monitor.NewEngine(ms) - port := freePort() + port := freePort(t) srv := Start(ServerConfig{ Port: port, @@ -285,7 +289,10 @@ func TestImport_MethodNotAllowed(t *testing.T) { func TestImport_Unauthorized(t *testing.T) { ts := newTestServer(t, "secret", false) - body, _ := json.Marshal(models.Backup{}) + body, err := json.Marshal(models.Backup{}) + if err != nil { + t.Fatalf("json.Marshal: %v", err) + } resp, err := authReq("POST", ts.baseURL+"/api/backup/import", "wrong", body) if err != nil { t.Fatal(err) @@ -301,7 +308,10 @@ func TestImport_Success(t *testing.T) { backup := models.Backup{ Sites: []models.SiteConfig{{Name: "imported", URL: "http://example.com"}}, } - body, _ := json.Marshal(backup) + body, err := json.Marshal(backup) + if err != nil { + t.Fatalf("json.Marshal: %v", err) + } resp, err := authReq("POST", ts.baseURL+"/api/backup/import", "secret", body) if err != nil { t.Fatal(err) @@ -333,9 +343,12 @@ func TestImport_InvalidJSON(t *testing.T) { func TestProbeRegister_Success(t *testing.T) { ts := newTestServer(t, "secret", false) - body, _ := json.Marshal(map[string]string{ + body, err := json.Marshal(map[string]string{ "id": "node-1", "name": "US East", "region": "us-east", }) + if err != nil { + t.Fatalf("json.Marshal: %v", err) + } resp, err := authReq("POST", ts.baseURL+"/api/probe/register", "secret", body) if err != nil { t.Fatal(err) @@ -356,7 +369,10 @@ func TestProbeRegister_Success(t *testing.T) { func TestProbeRegister_MissingID(t *testing.T) { ts := newTestServer(t, "secret", false) - body, _ := json.Marshal(map[string]string{"name": "test"}) + body, err := json.Marshal(map[string]string{"name": "test"}) + if err != nil { + t.Fatalf("json.Marshal: %v", err) + } resp, err := authReq("POST", ts.baseURL+"/api/probe/register", "secret", body) if err != nil { t.Fatal(err) @@ -369,7 +385,10 @@ func TestProbeRegister_MissingID(t *testing.T) { func TestProbeRegister_Unauthorized(t *testing.T) { ts := newTestServer(t, "secret", false) - body, _ := json.Marshal(map[string]string{"id": "node-1"}) + body, err := json.Marshal(map[string]string{"id": "node-1"}) + if err != nil { + t.Fatalf("json.Marshal: %v", err) + } resp, err := authReq("POST", ts.baseURL+"/api/probe/register", "wrong", body) if err != nil { t.Fatal(err) @@ -384,12 +403,15 @@ func TestProbeRegister_Unauthorized(t *testing.T) { func TestProbeResults_Success(t *testing.T) { ts := newTestServer(t, "secret", false) - body, _ := json.Marshal(map[string]any{ + body, err := json.Marshal(map[string]any{ "node_id": "node-1", "results": []map[string]any{ {"site_id": 1, "latency_ns": 5000000, "is_up": true}, }, }) + if err != nil { + t.Fatalf("json.Marshal: %v", err) + } resp, err := authReq("POST", ts.baseURL+"/api/probe/results", "secret", body) if err != nil { t.Fatal(err) @@ -402,9 +424,12 @@ func TestProbeResults_Success(t *testing.T) { func TestProbeResults_MissingNodeID(t *testing.T) { ts := newTestServer(t, "secret", false) - body, _ := json.Marshal(map[string]any{ + body, err := json.Marshal(map[string]any{ "results": []map[string]any{}, }) + if err != nil { + t.Fatalf("json.Marshal: %v", err) + } resp, err := authReq("POST", ts.baseURL+"/api/probe/results", "secret", body) if err != nil { t.Fatal(err) @@ -573,7 +598,10 @@ func TestClientIP_TrustedProxyHandling(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r, _ := http.NewRequest(http.MethodGet, "/", nil) + r, err := http.NewRequest(http.MethodGet, "/", nil) + if err != nil { + t.Fatalf("http.NewRequest: %v", err) + } r.RemoteAddr = tt.remoteAddr if tt.xff != "" { r.Header.Set("X-Forwarded-For", tt.xff) @@ -591,7 +619,10 @@ func TestRateLimit_SpoofedXFFCannotBypass(t *testing.T) { rl := NewRateLimiter(60, nil) // no trusted proxies allowed := 0 for i := 0; i < 200; i++ { - r, _ := http.NewRequest(http.MethodGet, "/", nil) + r, err := http.NewRequest(http.MethodGet, "/", nil) + if err != nil { + t.Fatalf("http.NewRequest: %v", err) + } r.RemoteAddr = "203.0.113.9:5000" r.Header.Set("X-Forwarded-For", fmt.Sprintf("9.9.9.%d", i%256)) if rl.Allow(clientIP(r, rl.trusted)) { diff --git a/internal/store/crypto_test.go b/internal/store/crypto_test.go index 75b5a05..c7c53d6 100644 --- a/internal/store/crypto_test.go +++ b/internal/store/crypto_test.go @@ -75,8 +75,14 @@ func TestEncryptorUniqueCiphertexts(t *testing.T) { t.Fatal(err) } - a, _ := enc.Encrypt("same") - b, _ := enc.Encrypt("same") + a, err := enc.Encrypt("same") + if err != nil { + t.Fatalf("first encrypt: %v", err) + } + b, err := enc.Encrypt("same") + if err != nil { + t.Fatalf("second encrypt: %v", err) + } if a == b { t.Error("two encryptions of same plaintext should produce different ciphertexts") } diff --git a/internal/store/sqlstore_test.go b/internal/store/sqlstore_test.go index 8174b01..f720e74 100644 --- a/internal/store/sqlstore_test.go +++ b/internal/store/sqlstore_test.go @@ -343,7 +343,10 @@ func TestDeleteSiteCascade(t *testing.T) { if err := s.AddSite(context.Background(), site); err != nil { t.Fatalf("AddSite: %v", err) } - sites, _ := s.GetSites(context.Background()) + sites, err := s.GetSites(context.Background()) + if err != nil { + t.Fatalf("GetSites: %v", err) + } siteID := sites[0].ID if err := s.SaveCheck(context.Background(), siteID, 1000, true); err != nil { @@ -366,17 +369,26 @@ func TestDeleteSiteCascade(t *testing.T) { t.Fatalf("DeleteSite: %v", err) } - history, _ := s.LoadAllHistory(context.Background(), 100) + history, err := s.LoadAllHistory(context.Background(), 100) + if err != nil { + t.Fatalf("LoadAllHistory: %v", err) + } if len(history[siteID]) != 0 { t.Errorf("expected 0 check_history rows, got %d", len(history[siteID])) } - changes, _ := s.GetStateChanges(context.Background(), siteID, 100) + changes, err := s.GetStateChanges(context.Background(), siteID, 100) + if err != nil { + t.Fatalf("GetStateChanges: %v", err) + } if len(changes) != 0 { t.Errorf("expected 0 state_changes rows, got %d", len(changes)) } - windows, _ := s.GetActiveMaintenanceWindows(context.Background()) + windows, err := s.GetActiveMaintenanceWindows(context.Background()) + if err != nil { + t.Fatalf("GetActiveMaintenanceWindows: %v", err) + } for _, w := range windows { if w.MonitorID == siteID { t.Errorf("orphaned maintenance window found: id=%d", w.ID)