Files
NaviWatcher/internal/musicbrainz/client_test.go
Vladimir Zagainov a5911c257c fix: address code review findings
Remove dead code: duplicate ExternalRelease/Artist/ParsedArtist structs in
model.go, ParseArtist/mbArtist/mbArtistData in client.go, ArtistTypeFilter
and related filtering functions in api.go, SyncArtistDiscographyWithFilter
in sync.go, and CacheStats/IsArtistCacheValid in cache.go.

Fix bugs: SaveExternalRelease now stores NULL instead of empty string for
zero CachedAt; sync upserts are now transactional with stale release cleanup;
getCachedReleases returns int instead of *CacheStats; doGet uses url.Values
for proper query encoding of MBID.

Fix tests: removed duplicate TestRun_GracefulShutdown, removed dead code
(_ = dbPath) from TestNewApp, fixed assertions in httptest handler goroutine
to avoid data race, increased rate limiter timing tolerance, removed
Client.Close() calls (no-op removed), fixed sync test cache expiry to use
UPDATE instead of 0 TTL races.

Fix formatting: cancel()}() formatting in main.go, error format string in sync.go.
2026-05-26 14:10:22 +03:00

204 lines
5.3 KiB
Go

package musicbrainz
import (
"context"
"net/http"
"net/http/httptest"
"sync"
"testing"
"time"
"golang.org/x/time/rate"
"naviwatcher/internal/config"
)
func TestNewClient_ValidConfig(t *testing.T) {
cfg := config.MusicBrainzConfig{
UserAgent: "naviwatcher/0.1.0 (test@example.com)",
}
client := NewClient(cfg)
if client == nil {
t.Fatal("NewClient() returned nil client")
}
if client.httpClient == nil {
t.Fatal("NewClient() returned client with nil http.Client")
}
if client.userAgent != cfg.UserAgent {
t.Errorf("NewClient().userAgent = %q, want %q", client.userAgent, cfg.UserAgent)
}
expectedBaseURL := "https://musicbrainz.org/ws/2"
if client.baseURL != expectedBaseURL {
t.Errorf("NewClient().baseURL = %q, want %q", client.baseURL, expectedBaseURL)
}
if client.rateLimiter == nil {
t.Fatal("NewClient() returned client with nil rate limiter")
}
}
func TestDoGet_Success(t *testing.T) {
var mu sync.Mutex
var gotUserAgent, gotAccept string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mu.Lock()
gotUserAgent = r.Header.Get("User-Agent")
gotAccept = r.Header.Get("Accept")
mu.Unlock()
w.Header().Set("Content-Type", "application/xml")
w.Write([]byte(`<metadata><test>ok</test></metadata>`))
}))
defer server.Close()
cfg := config.MusicBrainzConfig{
UserAgent: "naviwatcher/0.1.0 (test@example.com)",
}
rl := rate.NewLimiter(rate.Limit(1000), 1000) // high rate to avoid blocking in tests
client := &MusicBrainzClient{
httpClient: server.Client(),
userAgent: cfg.UserAgent,
baseURL: server.URL,
rateLimiter: rl,
}
body, err := client.doGet(context.Background(), "/test")
if err != nil {
t.Fatalf("doGet() error = %v", err)
}
if string(body) != `<metadata><test>ok</test></metadata>` {
t.Errorf("doGet() body = %q", string(body))
}
mu.Lock()
if gotUserAgent == "" {
t.Error("doGet() request missing User-Agent header")
}
if gotAccept != "application/xml" {
t.Errorf("doGet() Accept header = %q, want %q", gotAccept, "application/xml")
}
mu.Unlock()
}
func TestDoGet_Non200Status(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusServiceUnavailable)
w.Write([]byte("Rate limit exceeded"))
}))
defer server.Close()
cfg := config.MusicBrainzConfig{
UserAgent: "naviwatcher/0.1.0 (test@example.com)",
}
rl := rate.NewLimiter(rate.Limit(1000), 1000)
client := &MusicBrainzClient{
httpClient: server.Client(),
userAgent: cfg.UserAgent,
baseURL: server.URL,
rateLimiter: rl,
}
_, err := client.doGet(context.Background(), "/test")
if err == nil {
t.Fatal("doGet() expected error for non-200 status, got nil")
}
}
func TestDoGet_ServerUnreachable(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
server.Close()
cfg := config.MusicBrainzConfig{
UserAgent: "naviwatcher/0.1.0 (test@example.com)",
}
rl := rate.NewLimiter(rate.Limit(1000), 1000)
client := &MusicBrainzClient{
httpClient: server.Client(),
userAgent: cfg.UserAgent,
baseURL: server.URL,
rateLimiter: rl,
}
_, err := client.doGet(context.Background(), "/test")
if err == nil {
t.Fatal("doGet() expected error for unreachable server, got nil")
}
}
func TestDoGet_ContextCancellation(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(`<metadata><test>ok</test></metadata>`))
}))
defer server.Close()
cfg := config.MusicBrainzConfig{
UserAgent: "naviwatcher/0.1.0 (test@example.com)",
}
// Use a rate limiter with 0 burst to force blocking on Wait
rl := rate.NewLimiter(rate.Limit(0), 0)
client := &MusicBrainzClient{
httpClient: server.Client(),
userAgent: cfg.UserAgent,
baseURL: server.URL,
rateLimiter: rl,
}
ctx, cancel := context.WithCancel(context.Background())
cancel() // cancel immediately
_, err := client.doGet(ctx, "/test")
if err == nil {
t.Fatal("doGet() expected error for cancelled context, got nil")
}
}
func TestRateLimiter_OnePerSecond(t *testing.T) {
// Verify that the rate limiter enforces approximately 1 request per second
rl := rate.NewLimiter(rate.Limit(1), 1)
// First request should be immediate (burst of 1)
start := time.Now()
if err := rl.Wait(context.Background()); err != nil {
t.Fatalf("first Wait() error: %v", err)
}
elapsed := time.Since(start)
if elapsed > 200*time.Millisecond {
t.Errorf("first Wait() took %v, expected near-instant", elapsed)
}
// Second request should block for approximately 1 second
start = time.Now()
if err := rl.Wait(context.Background()); err != nil {
t.Fatalf("second Wait() error: %v", err)
}
elapsed = time.Since(start)
if elapsed < 800*time.Millisecond {
t.Errorf("second Wait() took %v, expected at least ~1s", elapsed)
}
if elapsed > 2*time.Second {
t.Errorf("second Wait() took %v, expected less than 2s", elapsed)
}
}
func TestRateLimiter_BurstBehavior(t *testing.T) {
// With burst=1, the first request should be immediate
rl := rate.NewLimiter(rate.Limit(1), 1)
start := time.Now()
rl.Wait(context.Background())
elapsed := time.Since(start)
if elapsed > 200*time.Millisecond {
t.Errorf("burst Wait() took %v, expected near-instant", elapsed)
}
}