Fix executor_test.go: ErrorIs arg order, wrong target, and data races

- TestNewParallelExecutorFailed: fix assert.ErrorIs argument order
- TestNewParallelExecutorCanceled: check for context.Canceled (not the
  executor error) since NewParallelExecutor returns ctx.Err() when
  context is cancelled; use atomic counter to fix data race
- TestNewParallelExecutor: use atomic counters to fix data race with
  concurrent goroutines

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
silverwind
2026-02-24 08:06:05 +01:00
parent 8d702e75e7
commit 4fdf9ab904

View File

@@ -3,6 +3,7 @@ package common
import ( import (
"context" "context"
"errors" "errors"
"sync/atomic"
"testing" "testing"
"time" "time"
@@ -80,37 +81,40 @@ func TestNewParallelExecutor(t *testing.T) {
ctx := context.Background() ctx := context.Background()
count := 0 var count atomic.Int32
activeCount := 0 var activeCount atomic.Int32
maxCount := 0 var maxCount atomic.Int32
emptyWorkflow := NewPipelineExecutor(func(_ context.Context) error { emptyWorkflow := NewPipelineExecutor(func(_ context.Context) error {
count++ count.Add(1)
activeCount++ cur := activeCount.Add(1)
if activeCount > maxCount { for {
maxCount = activeCount old := maxCount.Load()
if cur <= old || maxCount.CompareAndSwap(old, cur) {
break
}
} }
time.Sleep(2 * time.Second) time.Sleep(2 * time.Second)
activeCount-- activeCount.Add(-1)
return nil return nil
}) })
err := NewParallelExecutor(2, emptyWorkflow, emptyWorkflow, emptyWorkflow)(ctx) err := NewParallelExecutor(2, emptyWorkflow, emptyWorkflow, emptyWorkflow)(ctx)
assert.Equal(3, count, "should run all 3 executors") assert.Equal(int32(3), count.Load(), "should run all 3 executors")
assert.Equal(2, maxCount, "should run at most 2 executors in parallel") assert.Equal(int32(2), maxCount.Load(), "should run at most 2 executors in parallel")
require.NoError(t, err) require.NoError(t, err)
// Reset to test running the executor with 0 parallelism // Reset to test running the executor with 0 parallelism
count = 0 count.Store(0)
activeCount = 0 activeCount.Store(0)
maxCount = 0 maxCount.Store(0)
errSingle := NewParallelExecutor(0, emptyWorkflow, emptyWorkflow, emptyWorkflow)(ctx) errSingle := NewParallelExecutor(0, emptyWorkflow, emptyWorkflow, emptyWorkflow)(ctx)
assert.Equal(3, count, "should run all 3 executors") assert.Equal(int32(3), count.Load(), "should run all 3 executors")
assert.Equal(1, maxCount, "should run at most 1 executors in parallel") assert.Equal(int32(1), maxCount.Load(), "should run at most 1 executors in parallel")
require.NoError(t, errSingle) require.NoError(t, errSingle)
} }
@@ -127,7 +131,7 @@ func TestNewParallelExecutorFailed(t *testing.T) {
}) })
err := NewParallelExecutor(1, errorWorkflow)(ctx) err := NewParallelExecutor(1, errorWorkflow)(ctx)
assert.Equal(1, count) assert.Equal(1, count)
assert.ErrorIs(context.Canceled, err) assert.ErrorIs(err, context.Canceled)
} }
func TestNewParallelExecutorCanceled(t *testing.T) { func TestNewParallelExecutorCanceled(t *testing.T) {
@@ -136,18 +140,16 @@ func TestNewParallelExecutorCanceled(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
cancel() cancel()
errExpected := errors.New("fake error") var count atomic.Int32
count := 0
successWorkflow := NewPipelineExecutor(func(_ context.Context) error { successWorkflow := NewPipelineExecutor(func(_ context.Context) error {
count++ count.Add(1)
return nil return nil
}) })
errorWorkflow := NewPipelineExecutor(func(_ context.Context) error { errorWorkflow := NewPipelineExecutor(func(_ context.Context) error {
count++ count.Add(1)
return errExpected return errors.New("fake error")
}) })
err := NewParallelExecutor(3, errorWorkflow, successWorkflow, successWorkflow)(ctx) err := NewParallelExecutor(3, errorWorkflow, successWorkflow, successWorkflow)(ctx)
assert.Equal(3, count) assert.Equal(int32(3), count.Load())
assert.ErrorIs(errExpected, err) assert.ErrorIs(err, context.Canceled)
} }