From 928b735cac63db4e13e3adb69736013a4bd0f0f4 Mon Sep 17 00:00:00 2001 From: Zihan Dai <99155080+PDGGK@users.noreply.github.com> Date: Wed, 22 Jul 2026 04:03:28 +1000 Subject: [PATCH] Fix executeConcurrent successCount when stopping on error executeConcurrent reported successCount as `items.length - errors.length`. When stopOnError trips, the remaining items are never attempted, yet this formula counts every skipped item as a success (e.g. 10 items where the first fails reports successCount 9 with 0 operations actually completed). Use completedCount, which is incremented only after an operation resolves successfully, and add a regression test. Signed-off-by: Zihan Dai <99155080+PDGGK@users.noreply.github.com> --- src/utils/ConcurrentExecutor.ts | 2 +- tests/unit/ConcurrentExecutor.test.ts | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/utils/ConcurrentExecutor.ts b/src/utils/ConcurrentExecutor.ts index 70cfa8e..4a51b0d 100644 --- a/src/utils/ConcurrentExecutor.ts +++ b/src/utils/ConcurrentExecutor.ts @@ -159,7 +159,7 @@ export async function executeConcurrent( return { results, durationMs, - successCount: items.length - errors.length, + successCount: completedCount, failureCount: errors.length, errors, }; diff --git a/tests/unit/ConcurrentExecutor.test.ts b/tests/unit/ConcurrentExecutor.test.ts index 5715cb9..cbde7b8 100644 --- a/tests/unit/ConcurrentExecutor.test.ts +++ b/tests/unit/ConcurrentExecutor.test.ts @@ -105,6 +105,29 @@ describe('ConcurrentExecutor', () => { expect(processedCount).toBeLessThanOrEqual(items.length); }); + it('should not count skipped items as successes when stopping on error', async () => { + const items = Array.from({ length: 10 }, (_, i) => i); + let processedCount = 0; + + const result = await executeConcurrent( + items, + async (item) => { + processedCount++; + // The very first item fails, so nothing else is attempted. + throw new Error(`fail ${item}`); + }, + { concurrency: 1, stopOnError: true } + ); + + // Only one item was ever attempted (and it failed); the remaining 9 + // were skipped and are neither successes nor failures. + expect(processedCount).toBe(1); + expect(result.failureCount).toBe(1); + // successCount must reflect actually-completed operations (0), not + // items.length - errors.length (which would wrongly report 9). + expect(result.successCount).toBe(0); + }); + it('should limit concurrency', async () => { const maxConcurrent = { current: 0, max: 0 }; const items = Array.from({ length: 10 }, (_, i) => i);