Bookhoard/bookhoard
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 12 Wiki Activity

fix(tests): use errors.Is() for error comparison and improve resource cleanup in analytics tests

Browse Source 
Replace direct error equality check with errors.Is() in media_scanner_hash_test.

In analytics_test.go, improve defer patterns by capturing resp.Body as a named parameter
to avoid stale references, and add require.NoError() checks on all json.NewDecoder().Decode()
calls that were previously silently ignoring decode errors.
This commit is contained in:
john-okeefe
2026-04-20 21:20:38 -04:00
parent 9ccff320a1
commit 8baecad379
2 changed files with 89 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/server/tests/analytics_test.go
+87 -35
View File
@@ -4,6 +4,7 @@ import (
"bookhoard/internal/handlers"
"bytes"
"encoding/json"
"io"
"net/http"
"testing"
"time"
@@ -21,7 +22,9 @@ func TestAnalyticsReadingStats(t *testing.T) {
req, _ := http.NewRequest("GET", setup.Server.URL+"/api/analytics/reading-stats", nil)
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
})
@@ -32,12 +35,15 @@ func TestAnalyticsReadingStats(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result handlers.ReadingStatsResponse
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
assert.GreaterOrEqual(t, result.TotalBooksRead, 0)
assert.GreaterOrEqual(t, result.TotalPagesRead, 0)
@@ -53,7 +59,9 @@ func TestAnalyticsReadingStats(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusOK, resp.StatusCode)
})
@@ -64,7 +72,9 @@ func TestAnalyticsReadingStats(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
})
@@ -75,7 +85,9 @@ func TestAnalyticsReadingStats(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
})
@@ -86,12 +98,15 @@ func TestAnalyticsReadingStats(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result map[string]interface{}
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
// Should return zero values for empty history
assert.Equal(t, 0.0, result["total_books_read"])
@@ -108,7 +123,9 @@ func TestAnalyticsDeviceUsage(t *testing.T) {
req, _ := http.NewRequest("GET", setup.Server.URL+"/api/analytics/device-usage", nil)
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
})
@@ -119,19 +136,19 @@ func TestAnalyticsDeviceUsage(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result handlers.DeviceUsageResponse
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
assert.NotNil(t, result.Devices)
assert.Equal(t, 0, len(result.Devices))
})
t.Run("GetDeviceUsage_WithAuth_WithDevices", func(t *testing.T) {
// First create a device
// First, create a device
deviceReq := map[string]interface{}{
"device_name": "Test Kobo",
"device_type": "kobo",
@@ -144,7 +161,9 @@ func TestAnalyticsDeviceUsage(t *testing.T) {
resp, err := client.Do(deviceReqHTTP)
require.NoError(t, err)
resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
// Now get device usage
req, _ := http.NewRequest("GET", setup.Server.URL+"/api/analytics/device-usage", nil)
@@ -152,12 +171,15 @@ func TestAnalyticsDeviceUsage(t *testing.T) {
resp, err = client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result map[string]interface{}
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
devices, ok := result["devices"].([]interface{})
assert.True(t, ok)
@@ -171,12 +193,15 @@ func TestAnalyticsDeviceUsage(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result map[string]interface{}
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
assert.Contains(t, result, "devices")
@@ -203,7 +228,9 @@ func TestAnalyticsPopularBooks(t *testing.T) {
req, _ := http.NewRequest("GET", setup.Server.URL+"/api/analytics/popular-books", nil)
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
})
@@ -214,12 +241,15 @@ func TestAnalyticsPopularBooks(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result handlers.PopularBooksResponse
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
assert.NotNil(t, result.Books)
// Default limit is 10, but may be fewer if no reading history
@@ -232,12 +262,15 @@ func TestAnalyticsPopularBooks(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result map[string]interface{}
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
books := result["books"].([]interface{})
assert.True(t, len(books) <= 5)
@@ -249,20 +282,23 @@ func TestAnalyticsPopularBooks(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
// Should default to 10 on invalid limit
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result map[string]interface{}
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
books := result["books"].([]interface{})
assert.True(t, len(books) <= 10)
})
t.Run("GetPopularBooks_ResponseStructure", func(t *testing.T) {
// First create a book and some reading history
// First, create a book and some reading history
bookID := createTestMediaItemID(t, setup)
// Create reading history for the book
@@ -280,7 +316,9 @@ func TestAnalyticsPopularBooks(t *testing.T) {
resp, err := client.Do(historyHTTP)
require.NoError(t, err)
resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
// Now get popular books
req, _ := http.NewRequest("GET", setup.Server.URL+"/api/analytics/popular-books", nil)
@@ -288,12 +326,15 @@ func TestAnalyticsPopularBooks(t *testing.T) {
resp, err = client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result map[string]interface{}
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
books := result["books"].([]interface{})
@@ -315,12 +356,15 @@ func TestAnalyticsPopularBooks(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result map[string]interface{}
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
books := result["books"].([]interface{})
// Should return empty array if no reading history
@@ -342,13 +386,16 @@ func TestAnalyticsEdgeCases(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
// Should succeed but return empty stats
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result map[string]interface{}
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
assert.Equal(t, 0.0, result["total_books_read"])
})
@@ -359,13 +406,16 @@ func TestAnalyticsEdgeCases(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
// Should handle limit=0 gracefully
assert.Equal(t, http.StatusOK, resp.StatusCode)
var result map[string]interface{}
json.NewDecoder(resp.Body).Decode(&result)
err = json.NewDecoder(resp.Body).Decode(&result)
require.NoError(t, err)
books := result["books"].([]interface{})
assert.Equal(t, 0, len(books))
@@ -377,7 +427,9 @@ func TestAnalyticsEdgeCases(t *testing.T) {
resp, err := client.Do(req)
require.NoError(t, err)
defer resp.Body.Close()
defer func(Body io.ReadCloser) {
_ = Body.Close()
}(resp.Body)
// Should handle large limit
assert.Equal(t, http.StatusOK, resp.StatusCode)