From 8b18de0ca78f1a9ae901c8a1edc5efa4aedc23f9 Mon Sep 17 00:00:00 2001 From: rounak bhatia Date: Wed, 12 Aug 2026 13:17:47 +0530 Subject: [PATCH 1/7] fix(testHub): retry deferred last-test-finish flush so it isn't dropped (SDK-7265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The binary flow defers each mocha TestRunFinished past the afterEach window and flushes it at the next test's boundary; the worker's last test relies on the single flush from service.after(). sendTestFrameworkEvent swallowed send errors with no retry, so a transient gRPC failure on that flush dropped the finish — orphaning one test that Test Hub reaps at its ~60-min per-test timeout, which stamps the whole (passing) build `timeout`. Retry the flush up to 3x with backoff, surface send success/failure, and re-stash on total failure so a later flush/teardown can retry rather than dropping it. Mirrors SDK-7061's build-stop retry, applied to the test-finish path. Adds a deterministic reproduction test. Co-Authored-By: Claude Opus 4.8 --- .../src/cli/modules/testHubModule.ts | 31 +++- .../testHubModule.deferredFinish.test.ts | 144 ++++++++++++++++++ 2 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 packages/browserstack-service/tests/cli/modules/testHubModule.deferredFinish.test.ts diff --git a/packages/browserstack-service/src/cli/modules/testHubModule.ts b/packages/browserstack-service/src/cli/modules/testHubModule.ts index 1d8d33d..7869c2b 100644 --- a/packages/browserstack-service/src/cli/modules/testHubModule.ts +++ b/packages/browserstack-service/src/cli/modules/testHubModule.ts @@ -139,17 +139,38 @@ export default class TestHubModule extends BaseModule { * instance so late custom-tag merges are included. Called from onAllTestEvents at the * next test's boundary and from service.after() at worker end. */ - flushPendingTestFinishEvent(): Promise | undefined { + async flushPendingTestFinishEvent(): Promise { if (!this.pendingTestFinish) { - return undefined + return } const { args } = this.pendingTestFinish this.pendingTestFinish = null this.logger.debug('flushPendingTestFinishEvent: sending deferred TEST/POST event') - return this.sendTestFrameworkEvent(args, { testFrameworkState: 'TEST', testHookState: 'POST' }) + // SDK-7265: this is the ONLY guaranteed send of a mocha test's TestRunFinished — the + // worker's last test has no next-test boundary and relies on the single flush from + // service.after(). A swallowed, un-retried failure orphans that test; Test Hub then reaps + // it at its ~60-min per-test timeout (TEST_TIMED_OUT_WITH_BUILD_SUCCESS) and stamps the whole + // build `timeout` even though the run passed. Retry with backoff, and on total failure keep + // it pending so a later flush / teardown can try again rather than dropping it outright. + const maxAttempts = 3 + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + const sent = await this.sendTestFrameworkEvent(args, { testFrameworkState: 'TEST', testHookState: 'POST' }) + if (sent) { + return + } + this.logger.debug(`flushPendingTestFinishEvent: attempt ${attempt}/${maxAttempts} failed`) + if (attempt < maxAttempts) { + await new Promise((resolve) => setTimeout(resolve, 200 * attempt)) + } + } + // Re-stash only if nothing newer took the slot, so a subsequent flush can retry. + if (!this.pendingTestFinish) { + this.pendingTestFinish = { args } + } + this.logger.error('flushPendingTestFinishEvent: deferred TEST/POST send failed after retries; left pending for re-flush') } - async sendTestFrameworkEvent(args: Record, stateOverride?: { testFrameworkState: string, testHookState: string }) { + async sendTestFrameworkEvent(args: Record, stateOverride?: { testFrameworkState: string, testHookState: string }): Promise { try { const testArgs = args as { test: Frameworks.Test, instance: TestFrameworkInstance } const instance = testArgs.instance as TestFrameworkInstance @@ -185,8 +206,10 @@ export default class TestHubModule extends BaseModule { this.logger.debug(`sendTestFrameworkEvent payload: ${JSON.stringify(payload)}`) await GrpcClient.getInstance().testFrameworkEvent(payload) this.logger.debug(`sendTestFrameworkEvent complete for testState: ${testFrameworkState} hookState: ${testHookState}`) + return true } catch (error) { this.logger.error(`Error in sendTestFrameworkEvent: ${util.format(error)}`) + return false } } diff --git a/packages/browserstack-service/tests/cli/modules/testHubModule.deferredFinish.test.ts b/packages/browserstack-service/tests/cli/modules/testHubModule.deferredFinish.test.ts new file mode 100644 index 0000000..5fa1de3 --- /dev/null +++ b/packages/browserstack-service/tests/cli/modules/testHubModule.deferredFinish.test.ts @@ -0,0 +1,144 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import TestHubModule from '../../../src/cli/modules/testHubModule.js' +import TestFramework from '../../../src/cli/frameworks/testFramework.js' +import { TestFrameworkState } from '../../../src/cli/states/testFrameworkState.js' +import { HookState } from '../../../src/cli/states/hookState.js' +import { GrpcClient } from '../../../src/cli/grpcClient.js' +import { TestFrameworkConstants } from '../../../src/cli/frameworks/constants/testFrameworkConstants.js' +import type { Frameworks } from '@wdio/types' + +vi.mock('../../../src/cli/frameworks/testFramework.js', () => ({ + default: { + registerObserver: vi.fn(), + getTrackedInstance: vi.fn(), + getState: vi.fn(), + setState: vi.fn(), + hasState: vi.fn() + } +})) + +vi.mock('../../../src/cli/frameworks/automationFramework.js', () => ({ + default: { getTrackedInstance: vi.fn(), getState: vi.fn(), getDriver: vi.fn() } +})) + +vi.mock('../../../src/cli/grpcClient.js', () => ({ + GrpcClient: { getInstance: vi.fn() } +})) + +vi.mock('../../../src/cli/frameworks/wdioMochaTestFramework.js', () => ({ + default: { getLogEntries: vi.fn(), clearLogs: vi.fn() } +})) + +vi.mock('../../../src/cli/cliLogger.js', () => ({ + BStackLogger: { debug: vi.fn(), info: vi.fn(), error: vi.fn(), warn: vi.fn() } +})) + +// Build a mock TestFrameworkInstance sitting at mocha TEST/POST (a finished test whose +// TestRunFinished the module defers past the after-each window). +function makeMochaTestInstance(uuid: string) { + return { + __uuid: uuid, + getContext: () => ({ + getId: () => 'ctx', + getThreadId: () => 'thread-1', + getProcessId: () => 'proc-1' + }), + getAllData: () => new Map([ + [TestFrameworkConstants.KEY_TEST_FRAMEWORK_NAME, 'WebdriverIO-mocha'], + [TestFrameworkConstants.KEY_TEST_FRAMEWORK_VERSION, '9.33.1'], + [TestFrameworkConstants.KEY_TEST_STARTED_AT, '2026-08-10T20:53:00Z'], + [TestFrameworkConstants.KEY_TEST_ENDED_AT, '2026-08-10T20:53:02Z'] + ]), + getRef: () => `ref-${uuid}`, + getCurrentTestState: () => TestFrameworkState.TEST, + getCurrentHookState: () => HookState.POST + } +} + +describe('TestHubModule — deferred last-test-finish delivery (SDK-7265)', () => { + let testHubModule: TestHubModule + let mockGrpcClient: { testFrameworkEvent: ReturnType } + + beforeEach(() => { + vi.clearAllMocks() + process.env.WDIO_WORKER_ID = '0-1' + + mockGrpcClient = { testFrameworkEvent: vi.fn().mockResolvedValue({ success: true }) } + vi.mocked(GrpcClient.getInstance).mockReturnValue(mockGrpcClient as never) + + // KEY_TEST_RESULT_AT present (so onAllTestEvents does not take the "no results" path); + // framework name resolves to mocha; uuid echoes the instance. + vi.mocked(TestFramework.hasState).mockReturnValue(true) + vi.mocked(TestFramework.getState).mockImplementation((instance: any, key: unknown) => { + if (key === TestFrameworkConstants.KEY_TEST_FRAMEWORK_NAME) { + return 'WebdriverIO-mocha' + } + if (key === TestFrameworkConstants.KEY_TEST_DEFERRED) { + return false + } + if (key === TestFrameworkConstants.KEY_TEST_UUID) { + return instance?.__uuid + } + return '' + }) + + testHubModule = new TestHubModule({ enabled: true, hubUrl: 'https://hub.browserstack.com' }) + Object.defineProperty(testHubModule, 'config', { + value: { hubUrl: 'https://hub.browserstack.com' }, + writable: true + }) + }) + + afterEach(() => { + vi.resetAllMocks() + delete process.env.WDIO_WORKER_ID + }) + + it('defers a mocha TEST/POST instead of sending it immediately', () => { + const inst = makeMochaTestInstance('t1') + testHubModule.onAllTestEvents({ instance: inst, test: { title: 't1' } as Frameworks.Test }) + // Held for the after-each window — not yet on the wire. + expect(mockGrpcClient.testFrameworkEvent).not.toHaveBeenCalled() + }) + + it('delivers the deferred finish when the flush send succeeds', async () => { + const inst = makeMochaTestInstance('t1') + testHubModule.onAllTestEvents({ instance: inst, test: { title: 't1' } as Frameworks.Test }) + + await testHubModule.flushPendingTestFinishEvent() + + expect(mockGrpcClient.testFrameworkEvent).toHaveBeenCalledTimes(1) + expect(mockGrpcClient.testFrameworkEvent).toHaveBeenCalledWith( + expect.objectContaining({ testFrameworkState: 'TEST', testHookState: 'POST', uuid: 't1' }) + ) + }) + + // REPRODUCTION: the worker's last test relies on this single best-effort flush (service.after()). + // A transient gRPC failure is swallowed with no retry, so the TestRunFinished never reaches the + // binary/backend. The test then stays "in progress" and is reaped by Test Hub's ~60-min per-test + // timeout (TEST_TIMED_OUT_WITH_BUILD_SUCCESS), which stamps the whole build `timeout`. + it('does not drop the last-test finish on a transient send failure (retries until delivered)', async () => { + const inst = makeMochaTestInstance('last') + testHubModule.onAllTestEvents({ instance: inst, test: { title: 'last' } as Frameworks.Test }) + + // First attempt fails transiently, second succeeds. + mockGrpcClient.testFrameworkEvent + .mockRejectedValueOnce(new Error('transient gRPC failure')) + .mockResolvedValueOnce({ success: true }) + + await testHubModule.flushPendingTestFinishEvent() + + // Must be retried and ultimately delivered — otherwise the test is orphaned. + expect(mockGrpcClient.testFrameworkEvent).toHaveBeenCalledTimes(2) + }) + + it('clears the pending finish after a flush so it is never sent twice', async () => { + const inst = makeMochaTestInstance('t1') + testHubModule.onAllTestEvents({ instance: inst, test: { title: 't1' } as Frameworks.Test }) + + await testHubModule.flushPendingTestFinishEvent() + await testHubModule.flushPendingTestFinishEvent() // second flush is a no-op + + expect(mockGrpcClient.testFrameworkEvent).toHaveBeenCalledTimes(1) + }) +}) From 6a9d81ff4f76e155548c8baa5f9acddc562ac0ee Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 12 Aug 2026 07:49:37 +0000 Subject: [PATCH 2/7] chore(changeset): auto-generate from PR template (patch) --- .changeset/pr-129.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/pr-129.md diff --git a/.changeset/pr-129.md b/.changeset/pr-129.md new file mode 100644 index 0000000..778b45a --- /dev/null +++ b/.changeset/pr-129.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": patch +--- + +- Fixed WebdriverIO (Mocha) builds occasionally being reported as timed out even though the test run finished successfully. From 06b9951d3806c353701a2c6ea31465982d3b0432 Mon Sep 17 00:00:00 2001 From: Rounak Bhatia Date: Wed, 12 Aug 2026 13:43:31 +0530 Subject: [PATCH 3/7] Delete .changeset/pr-129.md --- .changeset/pr-129.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/pr-129.md diff --git a/.changeset/pr-129.md b/.changeset/pr-129.md deleted file mode 100644 index 778b45a..0000000 --- a/.changeset/pr-129.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@wdio/browserstack-service": patch ---- - -- Fixed WebdriverIO (Mocha) builds occasionally being reported as timed out even though the test run finished successfully. From f75b180f4d748ee0417fb54c4f60d2776ef2c002 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 12 Aug 2026 08:13:43 +0000 Subject: [PATCH 4/7] chore(changeset): auto-generate from PR template (patch) --- .changeset/pr-129.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/pr-129.md diff --git a/.changeset/pr-129.md b/.changeset/pr-129.md new file mode 100644 index 0000000..778b45a --- /dev/null +++ b/.changeset/pr-129.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": patch +--- + +- Fixed WebdriverIO (Mocha) builds occasionally being reported as timed out even though the test run finished successfully. From 001340f2b6a4bb5f824a930bce8d97a4c0a71aa0 Mon Sep 17 00:00:00 2001 From: rounak bhatia Date: Wed, 12 Aug 2026 15:01:57 +0530 Subject: [PATCH 5/7] =?UTF-8?q?fix(testHub):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20drop=20racy=20re-stash,=20keep=20retry;=20strengthe?= =?UTF-8?q?n=20tests=20(SDK-7265)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the re-stash-on-exhaustion guard races the two fire-and-forget flushPendingTestFinishEvent call sites: because the retry loop keeps an invocation in flight, a newer test can take the single pendingTestFinish slot while an older invocation is still retrying, and on exhaustion the guard either re-stashes a stale, already-failed event or drops silently — reintroducing the SDK-7265 symptom under a narrower window. Remove the re-stash entirely (it gave no reliable benefit: nothing re-flushes after service.after(), the last-test path). args is captured locally and the shared slot is only cleared, never written back, so concurrent flushes each retry their own event without clobbering one another. Tests: assert every retry re-sends the same finish (not just call count); drive the retry budget to full exhaustion and assert no re-stash; add a concurrency guard proving an older retrying flush never drops a newer test's finish. Co-Authored-By: Claude Opus 4.8 --- .../src/cli/modules/testHubModule.ts | 20 +++---- .../testHubModule.deferredFinish.test.ts | 57 ++++++++++++++++++- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/packages/browserstack-service/src/cli/modules/testHubModule.ts b/packages/browserstack-service/src/cli/modules/testHubModule.ts index 7869c2b..02f6865 100644 --- a/packages/browserstack-service/src/cli/modules/testHubModule.ts +++ b/packages/browserstack-service/src/cli/modules/testHubModule.ts @@ -146,12 +146,16 @@ export default class TestHubModule extends BaseModule { const { args } = this.pendingTestFinish this.pendingTestFinish = null this.logger.debug('flushPendingTestFinishEvent: sending deferred TEST/POST event') - // SDK-7265: this is the ONLY guaranteed send of a mocha test's TestRunFinished — the + // SDK-7265: the deferred TEST/POST is the ONLY send of a mocha test's TestRunFinished — the // worker's last test has no next-test boundary and relies on the single flush from - // service.after(). A swallowed, un-retried failure orphans that test; Test Hub then reaps - // it at its ~60-min per-test timeout (TEST_TIMED_OUT_WITH_BUILD_SUCCESS) and stamps the whole - // build `timeout` even though the run passed. Retry with backoff, and on total failure keep - // it pending so a later flush / teardown can try again rather than dropping it outright. + // service.after(). Previously a swallowed, un-retried failure orphaned that test; Test Hub + // then reaped it at its ~60-min per-test timeout (TEST_TIMED_OUT_WITH_BUILD_SUCCESS) and + // stamped the whole (passing) build `timeout`. Retry with backoff so a transient gRPC + // failure does not drop the finish. `args` is captured locally and the shared + // `pendingTestFinish` slot is only cleared here (never written back), so concurrent + // fire-and-forget flushes of other tests each retry their own event and cannot clobber or + // drop one another; re-stashing an exhausted event would race those call sites for no gain + // (nothing re-flushes after service.after(), the last-test path). const maxAttempts = 3 for (let attempt = 1; attempt <= maxAttempts; attempt++) { const sent = await this.sendTestFrameworkEvent(args, { testFrameworkState: 'TEST', testHookState: 'POST' }) @@ -163,11 +167,7 @@ export default class TestHubModule extends BaseModule { await new Promise((resolve) => setTimeout(resolve, 200 * attempt)) } } - // Re-stash only if nothing newer took the slot, so a subsequent flush can retry. - if (!this.pendingTestFinish) { - this.pendingTestFinish = { args } - } - this.logger.error('flushPendingTestFinishEvent: deferred TEST/POST send failed after retries; left pending for re-flush') + this.logger.error('flushPendingTestFinishEvent: deferred TEST/POST send failed after all retries') } async sendTestFrameworkEvent(args: Record, stateOverride?: { testFrameworkState: string, testHookState: string }): Promise { diff --git a/packages/browserstack-service/tests/cli/modules/testHubModule.deferredFinish.test.ts b/packages/browserstack-service/tests/cli/modules/testHubModule.deferredFinish.test.ts index 5fa1de3..0a8a551 100644 --- a/packages/browserstack-service/tests/cli/modules/testHubModule.deferredFinish.test.ts +++ b/packages/browserstack-service/tests/cli/modules/testHubModule.deferredFinish.test.ts @@ -114,10 +114,10 @@ describe('TestHubModule — deferred last-test-finish delivery (SDK-7265)', () = }) // REPRODUCTION: the worker's last test relies on this single best-effort flush (service.after()). - // A transient gRPC failure is swallowed with no retry, so the TestRunFinished never reaches the + // A transient gRPC failure was swallowed with no retry, so the TestRunFinished never reached the // binary/backend. The test then stays "in progress" and is reaped by Test Hub's ~60-min per-test // timeout (TEST_TIMED_OUT_WITH_BUILD_SUCCESS), which stamps the whole build `timeout`. - it('does not drop the last-test finish on a transient send failure (retries until delivered)', async () => { + it('does not drop the last-test finish on a transient send failure — retries the SAME finish until delivered', async () => { const inst = makeMochaTestInstance('last') testHubModule.onAllTestEvents({ instance: inst, test: { title: 'last' } as Frameworks.Test }) @@ -128,8 +128,59 @@ describe('TestHubModule — deferred last-test-finish delivery (SDK-7265)', () = await testHubModule.flushPendingTestFinishEvent() - // Must be retried and ultimately delivered — otherwise the test is orphaned. + // Retried (not a single fixed attempt) AND every attempt re-sends the same test's finish — + // distinguishes a real retry-until-delivered from a regressed single-shot send. expect(mockGrpcClient.testFrameworkEvent).toHaveBeenCalledTimes(2) + for (const call of mockGrpcClient.testFrameworkEvent.mock.calls) { + expect(call[0]).toMatchObject({ uuid: 'last', testFrameworkState: 'TEST', testHookState: 'POST' }) + } + }) + + it('drives the retry budget to exhaustion on sustained failure, then gives up cleanly without re-stashing', async () => { + const inst = makeMochaTestInstance('exhaust') + testHubModule.onAllTestEvents({ instance: inst, test: { title: 'exhaust' } as Frameworks.Test }) + + mockGrpcClient.testFrameworkEvent.mockRejectedValue(new Error('sustained gRPC outage')) + + await expect(testHubModule.flushPendingTestFinishEvent()).resolves.toBeUndefined() + + // All three attempts ran, each re-sending the same finish. + expect(mockGrpcClient.testFrameworkEvent).toHaveBeenCalledTimes(3) + for (const call of mockGrpcClient.testFrameworkEvent.mock.calls) { + expect(call[0]).toMatchObject({ uuid: 'exhaust' }) + } + // An exhausted event must NOT be re-stashed into the shared slot — re-stashing races the + // fire-and-forget flush call sites and can drop a newer test's finish (SDK-7265 review #1). + expect((testHubModule as unknown as { pendingTestFinish: unknown }).pendingTestFinish).toBeNull() + expect(testHubModule.logger.error).toHaveBeenCalledWith( + expect.stringContaining('failed after all retries') + ) + }) + + it('concurrent flushes each deliver their own finish — a retrying older flush never drops a newer test', async () => { + let aAttempts = 0 + mockGrpcClient.testFrameworkEvent.mockImplementation((payload: { uuid: string }) => { + if (payload.uuid === 'A') { + aAttempts += 1 + if (aAttempts === 1) { + return Promise.reject(new Error('transient on A')) + } + } + return Promise.resolve({ success: true }) + }) + + // Test A finishes, is deferred, then flushed fire-and-forget (as the next-test boundary does). + testHubModule.onAllTestEvents({ instance: makeMochaTestInstance('A'), test: { title: 'A' } as Frameworks.Test }) + const flushA = testHubModule.flushPendingTestFinishEvent() // not awaited — A is retrying + + // While A retries, test B finishes, is deferred and flushed. + testHubModule.onAllTestEvents({ instance: makeMochaTestInstance('B'), test: { title: 'B' } as Frameworks.Test }) + await testHubModule.flushPendingTestFinishEvent() // B + await flushA + + const sent = mockGrpcClient.testFrameworkEvent.mock.calls.map((c: unknown[]) => (c[0] as { uuid: string }).uuid) + expect(sent.filter((u) => u === 'A').length).toBe(2) // 1 transient fail + 1 retry success + expect(sent.filter((u) => u === 'B').length).toBe(1) // delivered once, never dropped }) it('clears the pending finish after a flush so it is never sent twice', async () => { From 1cec5d37c8331458163aa7a3421b755067b056e0 Mon Sep 17 00:00:00 2001 From: rounak bhatia Date: Wed, 12 Aug 2026 19:10:10 +0530 Subject: [PATCH 6/7] refactor(testHub): keep flushPendingTestFinishEvent non-async (promise chain) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No behavior change — same retry-with-backoff and exhaustion handling, expressed as a recursive promise chain instead of an async/await loop, restoring the original Promise | undefined signature. sendTestFrameworkEvent keeps its boolean-success return (so the un-awaited caller at line 130 is unaffected). Co-Authored-By: Claude Opus 4.8 --- .../src/cli/modules/testHubModule.ts | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/browserstack-service/src/cli/modules/testHubModule.ts b/packages/browserstack-service/src/cli/modules/testHubModule.ts index 02f6865..ec42d32 100644 --- a/packages/browserstack-service/src/cli/modules/testHubModule.ts +++ b/packages/browserstack-service/src/cli/modules/testHubModule.ts @@ -139,9 +139,9 @@ export default class TestHubModule extends BaseModule { * instance so late custom-tag merges are included. Called from onAllTestEvents at the * next test's boundary and from service.after() at worker end. */ - async flushPendingTestFinishEvent(): Promise { + flushPendingTestFinishEvent(): Promise | undefined { if (!this.pendingTestFinish) { - return + return undefined } const { args } = this.pendingTestFinish this.pendingTestFinish = null @@ -156,18 +156,23 @@ export default class TestHubModule extends BaseModule { // fire-and-forget flushes of other tests each retry their own event and cannot clobber or // drop one another; re-stashing an exhausted event would race those call sites for no gain // (nothing re-flushes after service.after(), the last-test path). + // Retry as a promise chain (kept non-async): each attempt sends and, on a transient failure, + // waits 200*n ms before the next, so the returned promise resolves only once the send lands + // or the budget is exhausted — which is what service.after() awaits. const maxAttempts = 3 - for (let attempt = 1; attempt <= maxAttempts; attempt++) { - const sent = await this.sendTestFrameworkEvent(args, { testFrameworkState: 'TEST', testHookState: 'POST' }) - if (sent) { - return - } - this.logger.debug(`flushPendingTestFinishEvent: attempt ${attempt}/${maxAttempts} failed`) - if (attempt < maxAttempts) { - await new Promise((resolve) => setTimeout(resolve, 200 * attempt)) - } - } - this.logger.error('flushPendingTestFinishEvent: deferred TEST/POST send failed after all retries') + const attempt = (n: number): Promise => + this.sendTestFrameworkEvent(args, { testFrameworkState: 'TEST', testHookState: 'POST' }).then((sent) => { + if (sent) { + return + } + this.logger.debug(`flushPendingTestFinishEvent: attempt ${n}/${maxAttempts} failed`) + if (n >= maxAttempts) { + this.logger.error('flushPendingTestFinishEvent: deferred TEST/POST send failed after all retries') + return + } + return new Promise((resolve) => setTimeout(resolve, 200 * n)).then(() => attempt(n + 1)) + }) + return attempt(1) } async sendTestFrameworkEvent(args: Record, stateOverride?: { testFrameworkState: string, testHookState: string }): Promise { From f8127aa532595668162d8a708914a3fa17a6d02b Mon Sep 17 00:00:00 2001 From: rounak bhatia Date: Wed, 12 Aug 2026 19:42:05 +0530 Subject: [PATCH 7/7] docs(testHub): trim flushPendingTestFinishEvent comment to the load-bearing why Co-Authored-By: Claude Opus 4.8 --- .../src/cli/modules/testHubModule.ts | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/packages/browserstack-service/src/cli/modules/testHubModule.ts b/packages/browserstack-service/src/cli/modules/testHubModule.ts index ec42d32..859b6da 100644 --- a/packages/browserstack-service/src/cli/modules/testHubModule.ts +++ b/packages/browserstack-service/src/cli/modules/testHubModule.ts @@ -146,19 +146,11 @@ export default class TestHubModule extends BaseModule { const { args } = this.pendingTestFinish this.pendingTestFinish = null this.logger.debug('flushPendingTestFinishEvent: sending deferred TEST/POST event') - // SDK-7265: the deferred TEST/POST is the ONLY send of a mocha test's TestRunFinished — the - // worker's last test has no next-test boundary and relies on the single flush from - // service.after(). Previously a swallowed, un-retried failure orphaned that test; Test Hub - // then reaped it at its ~60-min per-test timeout (TEST_TIMED_OUT_WITH_BUILD_SUCCESS) and - // stamped the whole (passing) build `timeout`. Retry with backoff so a transient gRPC - // failure does not drop the finish. `args` is captured locally and the shared - // `pendingTestFinish` slot is only cleared here (never written back), so concurrent - // fire-and-forget flushes of other tests each retry their own event and cannot clobber or - // drop one another; re-stashing an exhausted event would race those call sites for no gain - // (nothing re-flushes after service.after(), the last-test path). - // Retry as a promise chain (kept non-async): each attempt sends and, on a transient failure, - // waits 200*n ms before the next, so the returned promise resolves only once the send lands - // or the budget is exhausted — which is what service.after() awaits. + // SDK-7265: this is the only send of a mocha test's TestRunFinished, and the worker's last + // test relies on this single flush from service.after(). A dropped send orphans the test → + // Test Hub reaps it at its ~60-min idle timeout → the passing build is stamped `timeout`. + // Retry with backoff. `args` is captured locally and the shared slot is only cleared (never + // written back), so concurrent flushes can't clobber one another. const maxAttempts = 3 const attempt = (n: number): Promise => this.sendTestFrameworkEvent(args, { testFrameworkState: 'TEST', testHookState: 'POST' }).then((sent) => {