Skip to content

Commit fbae4ed

Browse files
luispaterluispater
committed
Rename error variable for clarity in MKV subtitle track extraction loop.
Refactor tests for error handling and environment variable management to improve clarity and consistency.
1 parent 9046c8e commit fbae4ed

File tree

8 files changed

+63
-55
lines changed

8 files changed

+63
-55
lines changed

‎internal/helpers/helpers_test.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -206,35 +206,35 @@ func TestGetGenerationConfig(t *testing.T) {
206206

207207
for _, tt := range tests {
208208
t.Run(tt.name, func(t *testing.T) {
209-
config := GetGenerationConfig(tt.temperature, tt.topP, tt.topK)
209+
generationConfig := GetGenerationConfig(tt.temperature, tt.topP, tt.topK)
210210

211211
// Check all expected keys are present
212212
for _, key := range tt.wantKeys {
213-
if _, exists := config[key]; !exists {
214-
t.Errorf("Expected key %q to be present in config", key)
213+
if _, exists := generationConfig[key]; !exists {
214+
t.Errorf("Expected key %q to be present in generationConfig", key)
215215
}
216216
}
217217

218218
// Check specific values
219-
if config["response_mime_type"] != "application/json" {
220-
t.Errorf("Expected response_mime_type to be 'application/json', got %v", config["response_mime_type"])
219+
if generationConfig["response_mime_type"] != "application/json" {
220+
t.Errorf("Expected response_mime_type to be 'application/json', got %v", generationConfig["response_mime_type"])
221221
}
222222

223223
if tt.temperature != nil {
224-
if config["temperature"] != *tt.temperature {
225-
t.Errorf("Expected temperature to be %v, got %v", *tt.temperature, config["temperature"])
224+
if generationConfig["temperature"] != *tt.temperature {
225+
t.Errorf("Expected temperature to be %v, got %v", *tt.temperature, generationConfig["temperature"])
226226
}
227227
}
228228

229229
if tt.topP != nil {
230-
if config["top_p"] != *tt.topP {
231-
t.Errorf("Expected top_p to be %v, got %v", *tt.topP, config["top_p"])
230+
if generationConfig["top_p"] != *tt.topP {
231+
t.Errorf("Expected top_p to be %v, got %v", *tt.topP, generationConfig["top_p"])
232232
}
233233
}
234234

235235
if tt.topK != nil {
236-
if config["top_k"] != *tt.topK {
237-
t.Errorf("Expected top_k to be %v, got %v", *tt.topK, config["top_k"])
236+
if generationConfig["top_k"] != *tt.topK {
237+
t.Errorf("Expected top_k to be %v, got %v", *tt.topK, generationConfig["top_k"])
238238
}
239239
}
240240
})

‎internal/logger/logger_test.go‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -53,28 +53,28 @@ func TestSupportsColor(t *testing.T) {
5353
originalForceColor := os.Getenv("FORCE_COLOR")
5454

5555
// Test NO_COLOR set
56-
os.Setenv("NO_COLOR", "1")
56+
_ = os.Setenv("NO_COLOR", "1")
5757
if supportsColor() {
5858
t.Error("Expected supportsColor to return false when NO_COLOR is set")
5959
}
6060

6161
// Test FORCE_COLOR set
62-
os.Unsetenv("NO_COLOR")
63-
os.Setenv("FORCE_COLOR", "1")
62+
_ = os.Unsetenv("NO_COLOR")
63+
_ = os.Setenv("FORCE_COLOR", "1")
6464
if !supportsColor() {
6565
t.Error("Expected supportsColor to return true when FORCE_COLOR is set")
6666
}
6767

6868
// Restore original environment
6969
if originalNoColor == "" {
70-
os.Unsetenv("NO_COLOR")
70+
_ = os.Unsetenv("NO_COLOR")
7171
} else {
72-
os.Setenv("NO_COLOR", originalNoColor)
72+
_ = os.Setenv("NO_COLOR", originalNoColor)
7373
}
7474
if originalForceColor == "" {
75-
os.Unsetenv("FORCE_COLOR")
75+
_ = os.Unsetenv("FORCE_COLOR")
7676
} else {
77-
os.Setenv("FORCE_COLOR", originalForceColor)
77+
_ = os.Setenv("FORCE_COLOR", originalForceColor)
7878
}
7979
}
8080

@@ -182,8 +182,8 @@ func TestGetStoredMessages(t *testing.T) {
182182
// Verify it's a copy (modifying returned slice shouldn't affect original)
183183
if len(messages) > 0 {
184184
messages[0].Message = "modified"
185-
originalMessages := GetStoredMessages()
186-
if originalMessages[0].Message == "modified" {
185+
om := GetStoredMessages()
186+
if om[0].Message == "modified" {
187187
t.Error("GetStoredMessages should return a copy, not the original slice")
188188
}
189189
}
@@ -234,7 +234,7 @@ func TestNewProgressBar(t *testing.T) {
234234

235235
func TestProgressBar_Update(t *testing.T) {
236236
pb := NewProgressBar(100, "Test")
237-
237+
238238
// Set quiet mode to avoid output during test
239239
originalQuiet := quietMode
240240
quietMode = true
@@ -253,7 +253,7 @@ func TestProgressBar_Update(t *testing.T) {
253253

254254
func TestProgressBar_SetSuffix(t *testing.T) {
255255
pb := NewProgressBar(100, "Test")
256-
256+
257257
// Set quiet mode to avoid output during test
258258
originalQuiet := quietMode
259259
quietMode = true
@@ -268,7 +268,7 @@ func TestProgressBar_SetSuffix(t *testing.T) {
268268

269269
func TestProgressBar_SetLoading(t *testing.T) {
270270
pb := NewProgressBar(100, "Test")
271-
271+
272272
// Set quiet mode to avoid output during test
273273
originalQuiet := quietMode
274274
quietMode = true
@@ -287,7 +287,7 @@ func TestProgressBar_SetLoading(t *testing.T) {
287287

288288
func TestProgressBar_SetThinking(t *testing.T) {
289289
pb := NewProgressBar(100, "Test")
290-
290+
291291
// Set quiet mode to avoid output during test
292292
originalQuiet := quietMode
293293
quietMode = true
@@ -306,7 +306,7 @@ func TestProgressBar_SetThinking(t *testing.T) {
306306

307307
func TestProgressBar_SetSending(t *testing.T) {
308308
pb := NewProgressBar(100, "Test")
309-
309+
310310
// Set quiet mode to avoid output during test
311311
originalQuiet := quietMode
312312
quietMode = true
@@ -325,7 +325,7 @@ func TestProgressBar_SetSending(t *testing.T) {
325325

326326
func TestProgressBar_AddMessage(t *testing.T) {
327327
pb := NewProgressBar(100, "Test")
328-
328+
329329
// Set quiet mode to avoid output during test
330330
originalQuiet := quietMode
331331
quietMode = true
@@ -348,7 +348,7 @@ func TestProgressBar_AddMessage(t *testing.T) {
348348

349349
func TestProgressBar_ThreadSafety(t *testing.T) {
350350
pb := NewProgressBar(1000, "Test")
351-
351+
352352
// Set quiet mode to avoid output during test
353353
originalQuiet := quietMode
354354
quietMode = true
@@ -377,8 +377,10 @@ func TestSaveLogsToFile(t *testing.T) {
377377
if err != nil {
378378
t.Fatal(err)
379379
}
380-
tempFile.Close()
381-
defer os.Remove(tempFile.Name())
380+
_ = tempFile.Close()
381+
defer func() {
382+
_ = os.Remove(tempFile.Name())
383+
}()
382384

383385
// Store some test messages
384386
logMutex.Lock()
@@ -443,7 +445,7 @@ func TestConstants(t *testing.T) {
443445
func TestLoggingFunctionsQuietMode(t *testing.T) {
444446
// Save original state
445447
originalQuiet := quietMode
446-
448+
447449
// Clear existing messages and enable quiet mode
448450
logMutex.Lock()
449451
originalMessages := logMessages
@@ -475,7 +477,7 @@ func TestLoggingFunctionsQuietMode(t *testing.T) {
475477
func TestLoggingFunctionsNormalMode(t *testing.T) {
476478
// Save original state
477479
originalQuiet := quietMode
478-
480+
479481
// Clear existing messages
480482
logMutex.Lock()
481483
originalMessages := logMessages
@@ -486,7 +488,7 @@ func TestLoggingFunctionsNormalMode(t *testing.T) {
486488

487489
// Call logging functions
488490
Info("test info")
489-
Warning("test warning")
491+
Warning("test warning")
490492
Error("test error")
491493
Success("test success")
492494
Highlight("test highlight")
@@ -524,4 +526,4 @@ func TestLoggingFunctionsNormalMode(t *testing.T) {
524526
logMutex.Lock()
525527
logMessages = originalMessages
526528
logMutex.Unlock()
527-
}
529+
}

‎internal/providers/gemini.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,9 @@ func (g *GeminiProvider) TranslateBatch(ctx context.Context, batch []srt.Subtitl
244244
}
245245
} else {
246246
// Non-streaming mode
247-
result, err := g.client.Models.GenerateContent(ctx, config.ModelName, contents, genContentConfig)
248-
if err != nil {
249-
return nil, fmt.Errorf("generation failed: %v", err)
247+
result, errGenerateContent := g.client.Models.GenerateContent(ctx, config.ModelName, contents, genContentConfig)
248+
if errGenerateContent != nil {
249+
return nil, fmt.Errorf("generation failed: %v", errGenerateContent)
250250
}
251251

252252
if len(result.Candidates) > 0 && result.Candidates[0].Content != nil {
@@ -260,7 +260,7 @@ func (g *GeminiProvider) TranslateBatch(ctx context.Context, batch []srt.Subtitl
260260

261261
// Parse response
262262
var translatedBatch []srt.SubtitleObject
263-
if err := json.Unmarshal([]byte(responseText), &translatedBatch); err != nil {
263+
if err = json.Unmarshal([]byte(responseText), &translatedBatch); err != nil {
264264
return nil, errors.NewTranslationError("failed to parse response", err).WithContext("response_text", responseText)
265265
}
266266

‎internal/translator/translator_test.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,14 @@ func TestTranslator_prepareSRTFile(t *testing.T) {
119119
if err != nil {
120120
t.Fatalf("Failed to create temp dir: %v", err)
121121
}
122-
defer os.RemoveAll(tempDir)
122+
defer func() {
123+
_ = os.RemoveAll(tempDir)
124+
}()
123125

124126
// Create a test SRT file
125127
srtPath := filepath.Join(tempDir, "test.srt")
126128
srtContent := "1\n00:00:01,000 --> 00:00:03,000\nTest subtitle\n\n"
127-
if err := os.WriteFile(srtPath, []byte(srtContent), 0644); err != nil {
129+
if err = os.WriteFile(srtPath, []byte(srtContent), 0644); err != nil {
128130
t.Fatalf("Failed to create test SRT file: %v", err)
129131
}
130132

@@ -156,9 +158,9 @@ func TestTranslator_prepareSRTFile(t *testing.T) {
156158
},
157159
}
158160

159-
result, err := translator.prepareSRTFile()
160-
if (err != nil) != tt.wantErr {
161-
t.Errorf("prepareSRTFile() error = %v, wantErr %v", err, tt.wantErr)
161+
result, errPrepareSRTFile := translator.prepareSRTFile()
162+
if (errPrepareSRTFile != nil) != tt.wantErr {
163+
t.Errorf("prepareSRTFile() error = %v, wantErr %v", errPrepareSRTFile, tt.wantErr)
162164
return
163165
}
164166

@@ -235,7 +237,7 @@ func TestProgressInfo_JSON(t *testing.T) {
235237

236238
// Test unmarshaling
237239
var unmarshaled ProgressInfo
238-
if err := json.Unmarshal(data, &unmarshaled); err != nil {
240+
if err = json.Unmarshal(data, &unmarshaled); err != nil {
239241
t.Fatalf("Failed to unmarshal ProgressInfo: %v", err)
240242
}
241243

‎internal/video/mkv.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ func (p *MKVParser) Parse() error {
8383
// Extract subtitle tracks (with deduplication)
8484
seenTracks := make(map[uint8]bool)
8585
for i := uint(0); i < numTracks; i++ {
86-
trackInfo, err := demuxer.GetTrackInfo(i)
87-
if err != nil {
86+
trackInfo, errGetTrackInfo := demuxer.GetTrackInfo(i)
87+
if errGetTrackInfo != nil {
8888
continue // Skip tracks we can't read
8989
}
9090

‎internal/video/mkv_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,9 @@ func TestExtractToSRT(t *testing.T) {
104104
if err != nil {
105105
t.Fatalf("Failed to create temp dir: %v", err)
106106
}
107-
defer os.RemoveAll(tempDir)
107+
defer func() {
108+
_ = os.RemoveAll(tempDir)
109+
}()
108110

109111
outputPath := filepath.Join(tempDir, "test.srt")
110112

‎pkg/config/config_test.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,14 @@ func TestNewConfig(t *testing.T) {
1010
originalAPIKey := os.Getenv("GEMINI_API_KEY")
1111
defer func() {
1212
if originalAPIKey != "" {
13-
os.Setenv("GEMINI_API_KEY", originalAPIKey)
13+
_ = os.Setenv("GEMINI_API_KEY", originalAPIKey)
1414
} else {
15-
os.Unsetenv("GEMINI_API_KEY")
15+
_ = os.Unsetenv("GEMINI_API_KEY")
1616
}
1717
}()
1818

1919
// Test with no API key
20-
os.Unsetenv("GEMINI_API_KEY")
20+
_ = os.Unsetenv("GEMINI_API_KEY")
2121
cfg := NewConfig()
2222

2323
if len(cfg.APIKeys) != 0 {
@@ -83,11 +83,13 @@ func TestParseAPIKeys(t *testing.T) {
8383
t.Run(tt.name, func(t *testing.T) {
8484
// Set environment variable
8585
if tt.envValue != "" {
86-
os.Setenv("TEST_API_KEY", tt.envValue)
86+
_ = os.Setenv("TEST_API_KEY", tt.envValue)
8787
} else {
88-
os.Unsetenv("TEST_API_KEY")
88+
_ = os.Unsetenv("TEST_API_KEY")
8989
}
90-
defer os.Unsetenv("TEST_API_KEY")
90+
defer func() {
91+
_ = os.Unsetenv("TEST_API_KEY")
92+
}()
9193

9294
result := parseAPIKeys("TEST_API_KEY")
9395

‎pkg/errors/errors_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func TestTranslatorError_Error(t *testing.T) {
4141

4242
func TestTranslatorError_WithContext(t *testing.T) {
4343
err := NewValidationError("test error", nil)
44-
err.WithContext("key1", "value1").WithContext("key2", 42)
44+
_ = err.WithContext("key1", "value1").WithContext("key2", 42)
4545

4646
if err.Context["key1"] != "value1" {
4747
t.Errorf("Expected context key1 to be 'value1', got %v", err.Context["key1"])
@@ -61,7 +61,7 @@ func TestNewValidationError(t *testing.T) {
6161
if err.Message != "validation failed" {
6262
t.Errorf("Expected message 'validation failed', got %v", err.Message)
6363
}
64-
if err.Cause != cause {
64+
if !errors.Is(cause, err.Cause) {
6565
t.Errorf("Expected cause to be %v, got %v", cause, err.Cause)
6666
}
6767
}
@@ -99,4 +99,4 @@ func TestNewNetworkError(t *testing.T) {
9999
if err.Type != ErrorTypeNetwork {
100100
t.Errorf("Expected type %v, got %v", ErrorTypeNetwork, err.Type)
101101
}
102-
}
102+
}

0 commit comments

Comments
 (0)