From be762dbc229a3fa3d8274e9266a3206915b03da8 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sun, 22 Feb 2026 23:39:27 +0100 Subject: [PATCH] test(discovery): optimize discovery tests for faster execution Reduces `pkg/discovery` test suite runtime by ~75% (from ~17s to ~4s) by eliminating unnecessary network timeouts and reducing wait intervals. - Refactor `discovery.Service` to use an injectable `http.Client`, allowing UPnP enrichment tests to use `httptest.Server` instead of waiting for 5s network timeouts. - Make `DNSDiscovery` forward timeout configurable and reduce it from 2s to 100ms in unit tests. - Decrease discovery and context timeouts in mDNS and Unified discovery tests to the minimum required for stable verification (typically 100-200ms). --- pkg/discovery/dns.go | 6 ++++- pkg/discovery/dns_test.go | 5 ++-- pkg/discovery/mdns_test.go | 4 ++-- pkg/discovery/unified_test.go | 12 +++++----- pkg/discovery/upnp.go | 39 +++++++++++++++---------------- pkg/discovery/upnp_enrich_test.go | 4 +++- pkg/discovery/upnp_test.go | 37 ++++++++++++++++++----------- 7 files changed, 62 insertions(+), 45 deletions(-) diff --git a/pkg/discovery/dns.go b/pkg/discovery/dns.go index 934e09f..5404dd2 100644 --- a/pkg/discovery/dns.go +++ b/pkg/discovery/dns.go @@ -32,6 +32,9 @@ type DNSDiscovery struct { // Address for loop prevention bindAddr string + // Forward timeout + timeout time.Duration + // Log throttling lastLog map[string]time.Time lastLogMu sync.Mutex @@ -54,6 +57,7 @@ func NewDNSDiscovery(upstreamDNS []string, serviceIP string) *DNSDiscovery { upstreamDNS: upstreamDNS, serviceIP: serviceIP, discovered: make(map[string]*DiscoveredHost), + timeout: 2 * time.Second, lastLog: make(map[string]time.Time), } } @@ -295,7 +299,7 @@ func (d *DNSDiscovery) forward(w dns.ResponseWriter, r *dns.Msg) { } c := new(dns.Client) - c.Timeout = 2 * time.Second + c.Timeout = d.timeout for _, upstream := range d.upstreamDNS { // Add port 53 if not present diff --git a/pkg/discovery/dns_test.go b/pkg/discovery/dns_test.go index c5cdf2d..82826ef 100644 --- a/pkg/discovery/dns_test.go +++ b/pkg/discovery/dns_test.go @@ -400,6 +400,7 @@ func TestDNSDiscovery_ForwardTimeout(t *testing.T) { // Use an IP that is unroutable or doesn't exist on the network to ensure timeout upstreamDNS := []string{"192.0.2.1:53"} // TEST-NET-1, usually non-routable d := NewDNSDiscovery(upstreamDNS, serviceIP) + d.timeout = 100 * time.Millisecond m := new(dns.Msg) m.SetQuestion("google.com.", dns.TypeA) @@ -410,8 +411,8 @@ func TestDNSDiscovery_ForwardTimeout(t *testing.T) { d.forward(rw, m) duration := time.Since(start) - if duration < 2*time.Second { - t.Errorf("Expected forward to take at least 2 seconds (timeout), but took %v", duration) + if duration < 100*time.Millisecond { + t.Errorf("Expected forward to take at least 100ms (timeout), but took %v", duration) } if rw.msg == nil || rw.msg.Rcode != dns.RcodeServerFailure { diff --git a/pkg/discovery/mdns_test.go b/pkg/discovery/mdns_test.go index d0b7064..960a9a3 100644 --- a/pkg/discovery/mdns_test.go +++ b/pkg/discovery/mdns_test.go @@ -21,9 +21,9 @@ func TestNewMDNSDiscoveryService(t *testing.T) { } func TestMDNSDiscoverDevices(t *testing.T) { - service := NewMDNSDiscoveryService(2 * time.Second) + service := NewMDNSDiscoveryService(100 * time.Millisecond) - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) defer cancel() // Note: This test will attempt actual mDNS discovery diff --git a/pkg/discovery/unified_test.go b/pkg/discovery/unified_test.go index 050d171..d46281e 100644 --- a/pkg/discovery/unified_test.go +++ b/pkg/discovery/unified_test.go @@ -78,11 +78,11 @@ func TestUnifiedDiscoveryWithCustomConfig(t *testing.T) { func TestUnifiedDiscoverDevices(t *testing.T) { cfg := config.DefaultConfig() - cfg.DiscoveryTimeout = 2 * time.Second + cfg.DiscoveryTimeout = 100 * time.Millisecond cfg.CacheEnabled = false // Disable cache for testing service := NewUnifiedDiscoveryService(cfg) - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) defer cancel() devices, err := service.DiscoverDevices(ctx) @@ -119,13 +119,13 @@ func TestUnifiedDiscoverDevices(t *testing.T) { func TestUnifiedDiscoveryOnlyMDNS(t *testing.T) { cfg := config.DefaultConfig() - cfg.DiscoveryTimeout = 1 * time.Second + cfg.DiscoveryTimeout = 100 * time.Millisecond cfg.UPnPEnabled = false // Disable UPnP cfg.MDNSEnabled = true // Enable only mDNS cfg.CacheEnabled = false service := NewUnifiedDiscoveryService(cfg) - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) defer cancel() devices, err := service.DiscoverDevices(ctx) @@ -141,13 +141,13 @@ func TestUnifiedDiscoveryOnlyMDNS(t *testing.T) { func TestUnifiedDiscoveryOnlySSDP(t *testing.T) { cfg := config.DefaultConfig() - cfg.DiscoveryTimeout = 1 * time.Second + cfg.DiscoveryTimeout = 100 * time.Millisecond cfg.UPnPEnabled = true // Enable only UPnP cfg.MDNSEnabled = false // Disable mDNS cfg.CacheEnabled = false service := NewUnifiedDiscoveryService(cfg) - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) defer cancel() devices, err := service.DiscoverDevices(ctx) diff --git a/pkg/discovery/upnp.go b/pkg/discovery/upnp.go index 835d717..c781e66 100644 --- a/pkg/discovery/upnp.go +++ b/pkg/discovery/upnp.go @@ -20,11 +20,12 @@ import ( // Service handles UPnP SSDP discovery of SoundTouch devices type Service struct { - timeout time.Duration - cache map[string]*models.DiscoveredDevice - cacheTTL time.Duration - mutex sync.RWMutex - config *config.Config + timeout time.Duration + cache map[string]*models.DiscoveredDevice + cacheTTL time.Duration + mutex sync.RWMutex + config *config.Config + httpClient *http.Client } // NewService creates a new UPnP discovery service @@ -34,11 +35,12 @@ func NewService(timeout time.Duration) *Service { } return &Service{ - timeout: timeout, - cache: make(map[string]*models.DiscoveredDevice), - cacheTTL: defaultCacheTTL, - mutex: sync.RWMutex{}, - config: config.DefaultConfig(), + timeout: timeout, + cache: make(map[string]*models.DiscoveredDevice), + cacheTTL: defaultCacheTTL, + mutex: sync.RWMutex{}, + config: config.DefaultConfig(), + httpClient: &http.Client{Timeout: 5 * time.Second}, } } @@ -55,11 +57,12 @@ func NewServiceWithConfig(cfg *config.Config) *Service { } return &Service{ - timeout: timeout, - cache: make(map[string]*models.DiscoveredDevice), - cacheTTL: cacheTTL, - mutex: sync.RWMutex{}, - config: cfg, + timeout: timeout, + cache: make(map[string]*models.DiscoveredDevice), + cacheTTL: cacheTTL, + mutex: sync.RWMutex{}, + config: cfg, + httpClient: &http.Client{Timeout: 5 * time.Second}, } } @@ -423,11 +426,7 @@ func (d *Service) parseLocationURL(location, usn string) (*models.DiscoveredDevi func (d *Service) enrichDeviceInfo(device *models.DiscoveredDevice, location string) error { log.Printf("UPnP: Attempting to enrich device info by fetching %s", location) - client := &http.Client{ - Timeout: 5 * time.Second, - } - - resp, err := client.Get(location) + resp, err := d.httpClient.Get(location) if err != nil { log.Printf("UPnP: Failed to fetch device description from %s: %v", location, err) return err diff --git a/pkg/discovery/upnp_enrich_test.go b/pkg/discovery/upnp_enrich_test.go index c8b52d8..8c48a88 100644 --- a/pkg/discovery/upnp_enrich_test.go +++ b/pkg/discovery/upnp_enrich_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/gesellix/bose-soundtouch/pkg/models" ) @@ -32,7 +33,8 @@ func TestEnrichDeviceInfo(t *testing.T) { Name: "Initial Name", } - service := &Service{} + service := NewService(1 * time.Second) + service.httpClient = server.Client() err := service.enrichDeviceInfo(device, server.URL) if err != nil { diff --git a/pkg/discovery/upnp_test.go b/pkg/discovery/upnp_test.go index 89ce24a..9f17ce0 100644 --- a/pkg/discovery/upnp_test.go +++ b/pkg/discovery/upnp_test.go @@ -2,6 +2,9 @@ package discovery import ( "context" + "fmt" + "net/http" + "net/http/httptest" "strings" "testing" "time" @@ -109,18 +112,30 @@ func TestParseLocationURL_Invalid(t *testing.T) { } func TestParseResponse_ValidMediaRenderer(t *testing.T) { - service := NewService(1 * time.Second) + xmlPayload := ` + + + Test Device + SoundTouch 10 + AABBCCDDEEFF + +` + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/xml") + fmt.Fprint(w, xmlPayload) + })) + defer server.Close() - validResponse := `HTTP/1.1 200 OK + service := NewService(1 * time.Second) + service.httpClient = server.Client() + + validResponse := fmt.Sprintf(`HTTP/1.1 200 OK Cache-Control: max-age=1800 -Date: Mon, 22 Jun 1998 09:55:21 GMT -EXT: -Location: http://192.168.1.100:8090/device.xml -Server: Linux/3.14.0 UPnP/1.0 Bose-SoundTouch/1.0 +Location: %s ST: urn:schemas-upnp-org:device:MediaRenderer:1 USN: uuid:12345678-1234-5678-9012-123456789012::urn:schemas-upnp-org:device:MediaRenderer:1 -` +`, server.URL) device, err := service.parseResponse(validResponse) if err != nil { @@ -131,12 +146,8 @@ USN: uuid:12345678-1234-5678-9012-123456789012::urn:schemas-upnp-org:device:Medi t.Fatal("Expected device, got nil") } - if device.Host != "192.168.1.100" { - t.Errorf("Expected host '192.168.1.100', got '%s'", device.Host) - } - - if device.UPnPLocation != "http://192.168.1.100:8090/device.xml" { - t.Errorf("Expected UPnP location 'http://192.168.1.100:8090/device.xml', got '%s'", device.UPnPLocation) + if device.Name != "Test Device" { + t.Errorf("Expected name 'Test Device', got '%s'", device.Name) } }