Skip to content

fix(cli): record a timed-out mocha test's failure when mocha reports it (SDK-7843) [v8] - #273

Open
AakashHotchandani wants to merge 8 commits into
v8from
fix/SDK-7843-record-timeout-at-source-v8
Open

AakashHotchandani wants to merge 8 commits into
v8from
fix/SDK-7843-record-timeout-at-source-v8

Conversation

@AakashHotchandani

@AakashHotchandani AakashHotchandani commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

What is this about?

v8 port of #272. On the CLI flow, a mocha test that hits mocha's timeout has its session marked passed on the Automate / App Automate dashboard.

Why: wdio runs afterTest inside the test's runnable. When the test times out, mocha fails it (and emits fail to reporters) while the body and its afterTest are still pending. With bail, or on the worker's last test, wdio runs after() first. after() emits EXECUTE/POST, and AutomateModule.onAfterExecute marks the session from results that only afterTest supplies. There is none yet, so the session is marked passed. The classic flow reads after(result), mocha's own failure count, and was correct.

Same fix as #272: service.ts and reporter.ts only feed the existing trackEvent calls, and AutomateModule is unchanged.

  • Reporter onTestFail (CLI, mocha): sends LOG_REPORT/POST + TEST/POST through framework.trackEvent, with mocha's own result.
  • WdioMochaTestFramework tracks each attempt from TEST/PRE:
    • it reports the finish once, against the instance the attempt started on;
    • it keeps mocha's failure when a late afterTest says passed;
    • settleTestFinishes() waits for finishes wdio doesn't await.
  • service.ts: after() calls settleTestFinishes() before EXECUTE/POST.

afterTest's events are unchanged, including the existing suiteTite payload key.

Difference from main: v8 has no deferred TEST/POST, no bail cascade and no uuid pin, so its Test Reporting results were already right. On v8 this fixes the session status. It also closes a late afterTest against its own test run instead of whichever test holds the tracked slot.

Also in this PR — the resolveInstance ERROR (same change as #265): console output from wdio's before hook (the customer's [SelfHealer] Installed …) reaches trackEvent(LOG, POST) before mocha's first hook, when no test or hook instance exists. resolveInstance then printed resolveInstance: unable to resolve/create instance for TestFrameworkState.LOG HookState.POST on every worker. WdioMochaTestFramework.trackEvent now drops a LOG with no tracked instance at debug level, as the classic flow does. New test: tests/cli/wdioMochaTestFramework.preTestLog.test.ts (3 cases). Verified on real sessions under #265: stock 8.53.1 build smosjpekx4… prints the ERROR, patched build djju5rlyfb… is clean.

Review follow-ups:

Verification

Real WDIO 8 + mocha sessions on Automate (chrome/Win11), mochaOpts: { timeout: 10000, bail: true }. The timed-out test waits 40 s for a missing element.

build SDK session status of the timed-out test Test Reporting
gceel2cz0bnksiupe9w90rzx7ttnf06w4dgaqj8n stock 8.53.1 3eeef5f73f174165fb02f17f6616041f43656360 passed ❌ (CLIENT_STOPPED_SESSION) passed 1, failed 1
niszt4uhggs0bb2nnia3ngxookl2oemckau8wrmc this PR 0c835059b01822990f4e38cb092af6a9a934d438 failed, reason = mocha timeout passed 1, failed 1

Timing on both runs: reporter onTestFail (05:53:48.831) → after(result=1) (.840). On stock, the session status is set before the failed result exists.

Re-verified after ede6baa: build tghdkxxgbkri0bdjqljxq4usngfp8ysbyj29dx6s, session 4281fcd7949b32d55966848f94ef95d99dd48643 is failed with the mocha timeout reason. Test Reporting shows passed 1, failed 1. The late afterTest logs was already reported, dropping.

Tests: the same files as #272, minus the bail-cascade file (v8 has no cascade).

  • tests/cli/wdioMochaTestFramework.timedOutTest.test.ts (7).
  • tests/reporter.onTestFail.test.ts (4).
  • tests/service.timedOutTestFinish.test.ts (2): after() settles before EXECUTE/POST.

Full suite: 1133/1133 passed (51/51 files) vs 1109 on v8. tsc -p tsconfig.prod.json --noEmit is clean. The v8 branch has no eslint config.

Related Jira task/s

SDK-7843

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 mocha tests that fail by exceeding their timeout being marked "passed" on the Automate / App Automate dashboard. Such sessions are now marked failed, with the timeout error as the reason.
  • Removed the resolveInstance: unable to resolve/create instance ... LOG POST error printed when a wdio before hook writes to the console.

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

Checklist

  • Ready to review
  • Has it been tested locally?

PR Validations

Run Tests: Comment RUN_TESTS to trigger sanity tests.

🤖 Generated with Claude Code

…it (SDK-7843)

v8 port of the main-line fix. When a mocha test hits its timeout, mocha
fails it while the body and wdio's afterTest are still pending. With
`bail`, or on the worker's last test, wdio runs after() first, so on the
CLI flow AutomateModule.onAfterExecute marks the session `passed`: its
results only come from afterTest. The classic flow reads after(result)
and was correct.

The service now registers a finisher per mocha test in beforeTest
(cli/earlyTestFinish.ts). The reporter's onTestFail reports the failure
through it when mocha fails the test. after() waits for those finishes
before EXECUTE/POST, and the late afterTest stands down for a test that
was already reported. v8 sends TEST/POST inline (no deferred finish), so
its Test Reporting results were already right; this fixes the session
status.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AakashHotchandani
AakashHotchandani requested a review from a team as a code owner October 7, 2026 05:55
@AakashHotchandani
AakashHotchandani requested review from harshit-browserstack and kamal-kaur04 and removed request for a team October 7, 2026 05:55
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Central YAML (base), Organization UI (inherited), Workspace UI (inherited)
  • Review profile: ASSERTIVE
  • Plan: Enterprise
  • Run ID: 3bdbbdcc-5b09-4652-b215-cf92a5991046

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

🔴 SDK PR Review gate is red. Pending:

  • The SDK PR Review Agent has not been run on the current head commit yet — run the SDK PR Review Agent (its verdict is advisory; this gate only requires that it ran on the latest commit).

It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge.

…ogging an ERROR (SDK-7843)

Console output from wdio's `before` hook reaches trackEvent(LOG, POST) before mocha's first
hook, when no test or hook instance exists, so resolveInstance printed
"resolveInstance: unable to resolve/create instance ... LOG POST" on every worker. Drop it at
debug level, as the classic path does. Same change as #265.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

🔴 SDK PR Review gate is red. Pending:

  • The SDK PR Review Agent has not been run on the current head commit yet — run the SDK PR Review Agent (its verdict is advisory; this gate only requires that it ran on the latest commit).

It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge.

github-actions Bot and others added 2 commits October 7, 2026 10:31
- Finish a test mocha already failed from after() when no reporter claimed it, using mocha's
  own runnable, so the fix holds with Test Reporting, Accessibility and Percy all off (the
  reporter is only registered when Test Hub events are on).
- Key the finish hand-off per retry attempt, so a retried attempt's late afterTest closes that
  attempt and not the next one.
- Drop the try/catch around awaitCliTestFinishesOnFailure(); every finish catches its own error.
- Tests: no reporter, retries. Same changes as #272.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

🔴 SDK PR Review gate is red. Pending:

  • The SDK PR Review Agent has not been run on the current head commit yet — run the SDK PR Review Agent (its verdict is advisory; this gate only requires that it ran on the latest commit).

It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge.

With no reporter, a timed-out test whose body finishes late gets an afterTest saying only whether
the body threw. If mocha already failed that attempt, claimCliTestFinish now reports mocha's
failure and afterTest stands down, instead of reporting it passed. Same change as #272.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

🔴 SDK PR Review gate is red. Pending:

  • The SDK PR Review Agent has not been run on the current head commit yet — run the SDK PR Review Agent (its verdict is advisory; this gate only requires that it ran on the latest commit).

It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge.

…K-7843)

v8 port of the #272 review rework: service.ts / reporter.ts only feed the
existing CLI events; product logic stays in the CLI layer.

- reporter onTestFail sends LOG_REPORT/POST + TEST/POST through
  framework.trackEvent with mocha's result; AutomateModule already marks the
  session from a failed TEST/POST, it just arrived after after().
- WdioMochaTestFramework tracks each mocha test attempt from TEST/PRE and
  reports its finish once, against the instance it started on, keeping
  mocha's failure when a late afterTest says passed.
- settleTestFinishes() (TestFramework, no-op by default); after() calls it
  before EXECUTE/POST.
- cli/earlyTestFinish.ts removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

🔴 SDK PR Review gate is red. Pending:

  • The SDK PR Review Agent has not been run on the current head commit yet — run the SDK PR Review Agent (its verdict is advisory; this gate only requires that it ran on the latest commit).

It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge.

…name (SDK-7843)

A finished attempt's key went into `finishedAttempts` and was never cleared.
The key is `${parent} - ${title}` with only the immediate parent, so a later
test with the same key in a worker (e.g. login > valid > works and
signup > valid > works) had every finish dropped: it stayed In Progress in
Test Reporting and its failure never reached the session status. Two
same-key attempts open at once (a timed-out test's late afterTest) also
overwrote each other's entry.

Track finished per attempt. Resolve wdio's afterTest by the test's body
(`fn`, on both the beforeTest and afterTest copies of the mocha test) plus
retry, and the reporter's `fail`, which has only title and parent, by the
latest attempt with that key, resolved before any await so it is the test
mocha just failed. Pin the resolved attempt to the event's `test` object so
the same source's TEST/POST cannot land on a later same-named test.

`file::fullTitle` (review suggestion) cannot key both sides: wdio's hook
copy `{ ...context.test }` drops mocha's prototype `fullTitle()`, and the
reporter's TestStats has no `file`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

🔴 SDK PR Review gate is red. Pending:

  • The SDK PR Review Agent has not been run on the current head commit yet — run the SDK PR Review Agent (its verdict is advisory; this gate only requires that it ran on the latest commit).

It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant