From 5218642ebe4c56048233623c1252c8e9ed087d5e Mon Sep 17 00:00:00 2001 From: scnerd Date: Fri, 13 Mar 2026 22:39:32 -0700 Subject: [PATCH 1/5] test(queue): add unit tests for QUEUE_WORKERS env var behavior Add tests verifying that getOptimalWorkerCount() respects the QUEUE_WORKERS environment variable and that NewTaskQueue() should allow QUEUE_WORKERS to override the hardcoded legacy worker count. Includes a failing test (TestNewTaskQueue_EnvOverridesLegacy) that reproduces the bug where QUEUE_WORKERS is always overridden by the hardcoded legacyWorkers parameter. Ref: rishikanthc/Scriberr#379 Co-Authored-By: Claude Opus 4.6 --- internal/queue/queue_test.go | 137 +++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 internal/queue/queue_test.go diff --git a/internal/queue/queue_test.go b/internal/queue/queue_test.go new file mode 100644 index 00000000..61ffd4f6 --- /dev/null +++ b/internal/queue/queue_test.go @@ -0,0 +1,137 @@ +package queue + +import ( + "os" + "testing" +) + +func TestGetOptimalWorkerCount_DefaultBehavior(t *testing.T) { + os.Unsetenv("QUEUE_WORKERS") + + min, max := getOptimalWorkerCount() + + // Without QUEUE_WORKERS, should return CPU-based values (non-zero) + if min <= 0 || max <= 0 { + t.Errorf("expected positive worker counts, got min=%d, max=%d", min, max) + } + if min > max { + t.Errorf("min (%d) should be <= max (%d)", min, max) + } +} + +func TestGetOptimalWorkerCount_RespectsEnvVar(t *testing.T) { + tests := []struct { + name string + envValue string + wantMin int + wantMax int + }{ + {"single worker", "1", 1, 1}, + {"four workers", "4", 4, 4}, + {"ten workers", "10", 10, 10}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("QUEUE_WORKERS", tt.envValue) + defer os.Unsetenv("QUEUE_WORKERS") + + min, max := getOptimalWorkerCount() + if min != tt.wantMin || max != tt.wantMax { + t.Errorf("QUEUE_WORKERS=%s: got min=%d, max=%d; want min=%d, max=%d", + tt.envValue, min, max, tt.wantMin, tt.wantMax) + } + }) + } +} + +func TestGetOptimalWorkerCount_IgnoresInvalidEnvVar(t *testing.T) { + tests := []struct { + name string + envValue string + }{ + {"non-numeric", "abc"}, + {"zero", "0"}, + {"negative", "-1"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("QUEUE_WORKERS", tt.envValue) + defer os.Unsetenv("QUEUE_WORKERS") + + min, max := getOptimalWorkerCount() + // Should fall back to CPU-based defaults + if min <= 0 || max <= 0 { + t.Errorf("QUEUE_WORKERS=%s: expected positive defaults, got min=%d, max=%d", + tt.envValue, min, max) + } + }) + } +} + +// TestNewTaskQueue_DefaultWorkerCount verifies that without QUEUE_WORKERS, +// the legacy parameter (2) is used as default - preserving existing behavior. +func TestNewTaskQueue_DefaultWorkerCount(t *testing.T) { + os.Unsetenv("QUEUE_WORKERS") + + tq := NewTaskQueue(2, nil, nil) + defer tq.cancel() + + if tq.minWorkers != 2 { + t.Errorf("without QUEUE_WORKERS, expected minWorkers=2, got %d", tq.minWorkers) + } + if tq.maxWorkers != 2 { + t.Errorf("without QUEUE_WORKERS, expected maxWorkers=2, got %d", tq.maxWorkers) + } +} + +// TestNewTaskQueue_EnvOverridesLegacy verifies that QUEUE_WORKERS takes +// precedence over the hardcoded legacy parameter. +// This is the core bug test - currently QUEUE_WORKERS is ignored. +func TestNewTaskQueue_EnvOverridesLegacy(t *testing.T) { + tests := []struct { + name string + envValue string + legacyWorkers int + wantMin int + wantMax int + }{ + {"env=1 overrides legacy=2", "1", 2, 1, 1}, + {"env=4 overrides legacy=2", "4", 2, 4, 4}, + {"env=3 overrides legacy=2", "3", 2, 3, 3}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("QUEUE_WORKERS", tt.envValue) + defer os.Unsetenv("QUEUE_WORKERS") + + tq := NewTaskQueue(tt.legacyWorkers, nil, nil) + defer tq.cancel() + + if tq.minWorkers != tt.wantMin { + t.Errorf("QUEUE_WORKERS=%s with legacy=%d: got minWorkers=%d, want %d", + tt.envValue, tt.legacyWorkers, tq.minWorkers, tt.wantMin) + } + if tq.maxWorkers != tt.wantMax { + t.Errorf("QUEUE_WORKERS=%s with legacy=%d: got maxWorkers=%d, want %d", + tt.envValue, tt.legacyWorkers, tq.maxWorkers, tt.wantMax) + } + }) + } +} + +// TestNewTaskQueue_AutoScaleDisabledWithFixedWorkers verifies that +// auto-scaling is disabled when QUEUE_WORKERS sets a fixed count. +func TestNewTaskQueue_AutoScaleDisabledWithFixedWorkers(t *testing.T) { + os.Setenv("QUEUE_WORKERS", "3") + defer os.Unsetenv("QUEUE_WORKERS") + + tq := NewTaskQueue(2, nil, nil) + defer tq.cancel() + + if tq.autoScale { + t.Error("auto-scaling should be disabled when QUEUE_WORKERS sets fixed worker count") + } +} From f72fcbe28a0d22e36be6c8f62b34766c4f6a5e54 Mon Sep 17 00:00:00 2001 From: scnerd Date: Fri, 13 Mar 2026 22:39:58 -0700 Subject: [PATCH 2/5] fix(queue): allow QUEUE_WORKERS env var to override hardcoded worker count The QUEUE_WORKERS environment variable was defined and read in getOptimalWorkerCount(), but NewTaskQueue() unconditionally overwrote the result with the hardcoded legacyWorkers parameter (always 2). This made QUEUE_WORKERS effectively dead code. Now legacyWorkers is only used as a fallback when QUEUE_WORKERS is not set, preserving the default of 2 workers while allowing users to control concurrency via the environment variable. Set QUEUE_WORKERS=1 to serialize all transcription jobs and prevent system overload during bulk uploads. Fixes: rishikanthc/Scriberr#379 Co-Authored-By: Claude Opus 4.6 --- internal/queue/queue.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/queue/queue.go b/internal/queue/queue.go index 4c87a87c..f9d6c4f0 100644 --- a/internal/queue/queue.go +++ b/internal/queue/queue.go @@ -84,7 +84,8 @@ func NewTaskQueue(legacyWorkers int, processor JobProcessor, jobRepo repository. // Calculate optimal worker counts, fallback to legacy parameter min, max := getOptimalWorkerCount() - if legacyWorkers > 0 { + // Only use legacy parameter as fallback when QUEUE_WORKERS env var is not set + if os.Getenv("QUEUE_WORKERS") == "" && legacyWorkers > 0 { min = legacyWorkers max = legacyWorkers } From fcbd8069226b6d37a2d68336fecb5ac5ae313fba Mon Sep 17 00:00:00 2001 From: scnerd Date: Fri, 13 Mar 2026 22:47:01 -0700 Subject: [PATCH 3/5] refactor(queue): use testify assertions and t.Setenv in queue tests Switch from raw t.Errorf to testify/assert for consistency with the rest of the codebase. Use t.Setenv() instead of manual os.Setenv/defer os.Unsetenv for automatic cleanup. Simplify table structs where min and max are always equal. Co-Authored-By: Claude Opus 4.6 --- internal/queue/queue_test.go | 87 +++++++++++++----------------------- 1 file changed, 31 insertions(+), 56 deletions(-) diff --git a/internal/queue/queue_test.go b/internal/queue/queue_test.go index 61ffd4f6..086e130f 100644 --- a/internal/queue/queue_test.go +++ b/internal/queue/queue_test.go @@ -1,46 +1,40 @@ package queue import ( - "os" "testing" + + "github.com/stretchr/testify/assert" ) func TestGetOptimalWorkerCount_DefaultBehavior(t *testing.T) { - os.Unsetenv("QUEUE_WORKERS") + t.Setenv("QUEUE_WORKERS", "") min, max := getOptimalWorkerCount() - // Without QUEUE_WORKERS, should return CPU-based values (non-zero) - if min <= 0 || max <= 0 { - t.Errorf("expected positive worker counts, got min=%d, max=%d", min, max) - } - if min > max { - t.Errorf("min (%d) should be <= max (%d)", min, max) - } + assert.Positive(t, min, "min workers should be positive") + assert.Positive(t, max, "max workers should be positive") + assert.LessOrEqual(t, min, max, "min should be <= max") } func TestGetOptimalWorkerCount_RespectsEnvVar(t *testing.T) { tests := []struct { name string envValue string - wantMin int - wantMax int + want int }{ - {"single worker", "1", 1, 1}, - {"four workers", "4", 4, 4}, - {"ten workers", "10", 10, 10}, + {"single worker", "1", 1}, + {"four workers", "4", 4}, + {"ten workers", "10", 10}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - os.Setenv("QUEUE_WORKERS", tt.envValue) - defer os.Unsetenv("QUEUE_WORKERS") + t.Setenv("QUEUE_WORKERS", tt.envValue) min, max := getOptimalWorkerCount() - if min != tt.wantMin || max != tt.wantMax { - t.Errorf("QUEUE_WORKERS=%s: got min=%d, max=%d; want min=%d, max=%d", - tt.envValue, min, max, tt.wantMin, tt.wantMax) - } + + assert.Equal(t, tt.want, min, "min workers") + assert.Equal(t, tt.want, max, "max workers") }) } } @@ -57,67 +51,51 @@ func TestGetOptimalWorkerCount_IgnoresInvalidEnvVar(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - os.Setenv("QUEUE_WORKERS", tt.envValue) - defer os.Unsetenv("QUEUE_WORKERS") + t.Setenv("QUEUE_WORKERS", tt.envValue) min, max := getOptimalWorkerCount() - // Should fall back to CPU-based defaults - if min <= 0 || max <= 0 { - t.Errorf("QUEUE_WORKERS=%s: expected positive defaults, got min=%d, max=%d", - tt.envValue, min, max) - } + + assert.Positive(t, min, "should fall back to positive CPU-based default") + assert.Positive(t, max, "should fall back to positive CPU-based default") }) } } // TestNewTaskQueue_DefaultWorkerCount verifies that without QUEUE_WORKERS, -// the legacy parameter (2) is used as default - preserving existing behavior. +// the legacy parameter (2) is used as default, preserving existing behavior. func TestNewTaskQueue_DefaultWorkerCount(t *testing.T) { - os.Unsetenv("QUEUE_WORKERS") + t.Setenv("QUEUE_WORKERS", "") tq := NewTaskQueue(2, nil, nil) defer tq.cancel() - if tq.minWorkers != 2 { - t.Errorf("without QUEUE_WORKERS, expected minWorkers=2, got %d", tq.minWorkers) - } - if tq.maxWorkers != 2 { - t.Errorf("without QUEUE_WORKERS, expected maxWorkers=2, got %d", tq.maxWorkers) - } + assert.Equal(t, 2, tq.minWorkers, "default minWorkers should match legacy parameter") + assert.Equal(t, 2, tq.maxWorkers, "default maxWorkers should match legacy parameter") } // TestNewTaskQueue_EnvOverridesLegacy verifies that QUEUE_WORKERS takes // precedence over the hardcoded legacy parameter. -// This is the core bug test - currently QUEUE_WORKERS is ignored. func TestNewTaskQueue_EnvOverridesLegacy(t *testing.T) { tests := []struct { name string envValue string legacyWorkers int - wantMin int - wantMax int + want int }{ - {"env=1 overrides legacy=2", "1", 2, 1, 1}, - {"env=4 overrides legacy=2", "4", 2, 4, 4}, - {"env=3 overrides legacy=2", "3", 2, 3, 3}, + {"env=1 overrides legacy=2", "1", 2, 1}, + {"env=4 overrides legacy=2", "4", 2, 4}, + {"env=3 overrides legacy=2", "3", 2, 3}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - os.Setenv("QUEUE_WORKERS", tt.envValue) - defer os.Unsetenv("QUEUE_WORKERS") + t.Setenv("QUEUE_WORKERS", tt.envValue) tq := NewTaskQueue(tt.legacyWorkers, nil, nil) defer tq.cancel() - if tq.minWorkers != tt.wantMin { - t.Errorf("QUEUE_WORKERS=%s with legacy=%d: got minWorkers=%d, want %d", - tt.envValue, tt.legacyWorkers, tq.minWorkers, tt.wantMin) - } - if tq.maxWorkers != tt.wantMax { - t.Errorf("QUEUE_WORKERS=%s with legacy=%d: got maxWorkers=%d, want %d", - tt.envValue, tt.legacyWorkers, tq.maxWorkers, tt.wantMax) - } + assert.Equal(t, tt.want, tq.minWorkers, "QUEUE_WORKERS should override legacy minWorkers") + assert.Equal(t, tt.want, tq.maxWorkers, "QUEUE_WORKERS should override legacy maxWorkers") }) } } @@ -125,13 +103,10 @@ func TestNewTaskQueue_EnvOverridesLegacy(t *testing.T) { // TestNewTaskQueue_AutoScaleDisabledWithFixedWorkers verifies that // auto-scaling is disabled when QUEUE_WORKERS sets a fixed count. func TestNewTaskQueue_AutoScaleDisabledWithFixedWorkers(t *testing.T) { - os.Setenv("QUEUE_WORKERS", "3") - defer os.Unsetenv("QUEUE_WORKERS") + t.Setenv("QUEUE_WORKERS", "3") tq := NewTaskQueue(2, nil, nil) defer tq.cancel() - if tq.autoScale { - t.Error("auto-scaling should be disabled when QUEUE_WORKERS sets fixed worker count") - } + assert.False(t, tq.autoScale, "auto-scaling should be disabled when QUEUE_WORKERS sets fixed count") } From e690d0459e3d80d8182f9fae71224a2d139d02f4 Mon Sep 17 00:00:00 2001 From: scnerd Date: Sat, 14 Mar 2026 06:04:26 -0700 Subject: [PATCH 4/5] docs(queue): add comments to test functions for consistency All six test functions now have // TestFoo verifies... comments matching the project's existing convention. Co-Authored-By: Claude Opus 4.6 --- internal/queue/queue_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/queue/queue_test.go b/internal/queue/queue_test.go index 086e130f..d8f0719f 100644 --- a/internal/queue/queue_test.go +++ b/internal/queue/queue_test.go @@ -6,6 +6,8 @@ import ( "github.com/stretchr/testify/assert" ) +// TestGetOptimalWorkerCount_DefaultBehavior verifies that without QUEUE_WORKERS, +// the function returns positive CPU-based defaults with min <= max. func TestGetOptimalWorkerCount_DefaultBehavior(t *testing.T) { t.Setenv("QUEUE_WORKERS", "") @@ -16,6 +18,8 @@ func TestGetOptimalWorkerCount_DefaultBehavior(t *testing.T) { assert.LessOrEqual(t, min, max, "min should be <= max") } +// TestGetOptimalWorkerCount_RespectsEnvVar verifies that QUEUE_WORKERS env var +// pins both min and max to the exact value specified. func TestGetOptimalWorkerCount_RespectsEnvVar(t *testing.T) { tests := []struct { name string @@ -39,6 +43,9 @@ func TestGetOptimalWorkerCount_RespectsEnvVar(t *testing.T) { } } +// TestGetOptimalWorkerCount_IgnoresInvalidEnvVar verifies that non-numeric, +// zero, and negative QUEUE_WORKERS values are ignored, falling back to +// CPU-based defaults rather than crashing or using bad values. func TestGetOptimalWorkerCount_IgnoresInvalidEnvVar(t *testing.T) { tests := []struct { name string From 9251875525a9ec96ae66ee14add4923b9e6ba266 Mon Sep 17 00:00:00 2001 From: scnerd Date: Sat, 14 Mar 2026 06:12:31 -0700 Subject: [PATCH 5/5] Add TODO comment to remember that this fix is really about technical debt and we should just remove the legacy workers argument entirely. --- internal/queue/queue.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/queue/queue.go b/internal/queue/queue.go index f9d6c4f0..6de48bf5 100644 --- a/internal/queue/queue.go +++ b/internal/queue/queue.go @@ -85,6 +85,7 @@ func NewTaskQueue(legacyWorkers int, processor JobProcessor, jobRepo repository. // Calculate optimal worker counts, fallback to legacy parameter min, max := getOptimalWorkerCount() // Only use legacy parameter as fallback when QUEUE_WORKERS env var is not set + // TODO: Deprecate `legacyWorkers` and rely on `getOptimalWorkerCount` instead. if os.Getenv("QUEUE_WORKERS") == "" && legacyWorkers > 0 { min = legacyWorkers max = legacyWorkers