fix(testHub): retry deferred last-test-finish flush so it isn't dropped (SDK-7265)

[rounak610]

What is this about?

Hardens delivery of the deferred last-test TestRunFinished in the CLI/binary flow so a passing WebdriverIO (Mocha) build is no longer mis-reported as timeout.

In the binary flow each Mocha TestRunFinished (TEST/POST) is deferred past the afterEach window and flushed at the next test's boundary. The worker's last test has no next boundary, so it relies on the single flush from service.after() → flushPendingTestFinishEvent(). That flush went through sendTestFrameworkEvent, which swallowed send errors with no retry — so a transient gRPC failure on that one send silently dropped the finish. The test then stays "in progress" and Test Hub reaps it at its ~60-min per-test timeout (TEST_TIMED_OUT_WITH_BUILD_SUCCESS), which stamps the whole (passing) build timeout.

Fix: flushPendingTestFinishEvent now retries the send up to 3× with backoff, sendTestFrameworkEvent returns success/failure instead of silently swallowing, and on total failure the finish is re-stashed so a later flush/teardown can retry rather than dropping it. Mirrors the build-stop retry from SDK-7061, applied to the test-finish path. Adds a deterministic reproduction test (testHubModule.deferredFinish.test.ts).

Observed on the fast-upgrader cohort (9.32.1 / 9.33.0 / 9.33.1): representative group 4941460 had ~178 timeout builds vs 3 healthy over 10 days — 98.3% sent a build-stop and carried exactly one dangling-test reap per build (build-stop ~4 min in, the lone TEST_TIMED_OUT_WITH_BUILD_SUCCESS at ~64 min).

Related Jira task/s

https://browserstack.atlassian.net/browse/SDK-7265

Release (mandatory for every PR — required for the ready-for-review label)

Version bump: (required — tick exactly one)

• minor (backwards-compatible feature)
• patch (bug fix or other small change)

Release notes type: (optional)

• New Feature
• Bug Fix
• Other Improvement

Release notes (customer-facing): (optional but encouraged)

• Fixed WebdriverIO (Mocha) builds occasionally being reported as timed out even though the test run finished successfully.

Release notes (internal): (required — engineer-facing; what actually changed / why)

• The deferred last-test TestRunFinished flush (flushPendingTestFinishEvent) had no retry and swallowed send failures, so a single transient gRPC hiccup on the worker's last test dropped the finish → the test was reaped by Test Hub's ~60-min per-test timeout and the passing build was stamped timeout.
• flushPendingTestFinishEvent now retries 3× with backoff; sendTestFrameworkEvent returns a boolean success; on total failure the pending finish is re-stashed for a later flush instead of being dropped. Mirrors SDK-7061's build-stop retry, applied to the test-finish path.

Checklist

• Ready to review
• Has it been tested locally?

PR Validations

Run Tests: Comment RUN_TESTS to trigger sanity tests.

🤖 Generated with Claude Code

[rounak610]

🔴 Blocking findings — fix required

See the SDK PR Review Agent's report from your local run.

Change map (generated deterministically from the diff)

graph LR
  subgraph nwdio_browserstack_service["wdio-browserstack-service"]
    npackages_browserstack_service_tests_cli_modules_testHubModule_deferredFinish_test_ts["⚠ testHubModule.deferredFinish.test.ts<br/>~144 lines"]
    npackages_browserstack_service_src_cli_modules_testHubModule_ts["testHubModule.ts<br/>~31 lines"]
    n_changeset_pr_129_md["pr-129.md<br/>~5 lines"]
  end

Loading

↻ This verdict comment is the review anchor — it's updated in place on each run (the gate posts its status separately).

— SDK PR Review Agent

[rounak610]

Review findings addressed — review passing ✅

All 4 blocking findings from the automated review are resolved in 001340f.

#1 (Concurrency — re-stash race, Critical): Removed the re-stash-on-exhaustion entirely. It gave no reliable benefit — nothing re-flushes after service.after(), which is the last-test path — while racing the two fire-and-forget flushPendingTestFinishEvent call sites. args is now captured locally and the shared pendingTestFinish slot is only cleared, never written back, so concurrent flushes each retry their own event and cannot clobber or drop one another. The retry-on-transient-failure (the actual SDK-7265 fix) is unchanged.

#2 (re-stash-on-exhaustion branch never exercised): No longer applicable — that branch is gone. Replaced by an explicit exhaustion test.

#3 & #4 (retry test only asserted call count): Strengthened.

• The retry test now asserts every attempt re-sends the same finish (uuid + TEST/POST), not just the count — so a regressed single-shot send can't pass.
• New test drives the full 3-attempt budget to exhaustion (asserts 3 attempts, same finish each, no re-stash, error logged).
• New concurrency test proves an older retrying flush never drops a newer test's finish.

Verification (Node 20): testHubModule.deferredFinish.test.ts 6/6, testHubModule.test.ts 16/16, full package suite 826 passed. (The 8 failing files are pre-existing — un-generated gRPC protobuf stubs in a fresh clone — identical on the base branch, unrelated to this change.)

🤖 Generated with Claude Code