Skip to content

fix(automate): name the session created by a mid-test reload (SDK-7270) - #163

Open
harshit-browserstack wants to merge 2 commits into
mainfrom
fix/sdk-7270-mid-test-reload-session-name
Open

fix(automate): name the session created by a mid-test reload (SDK-7270)#163
harshit-browserstack wants to merge 2 commits into
mainfrom
fix/sdk-7270-mid-test-reload-session-name

Conversation

@harshit-browserstack

Copy link
Copy Markdown
Collaborator

What is this about?

Follow-up to #148 (shipped in 9.34.0), which moved App Automate / Automate session naming to fire per test from automateModule.onBeforeTest. A residual case remained: a session replaced during a test is still never named.

@wdio/mocha-framework binds beforeTest to the test function itself (wrapGlobalTestMethod), so onBeforeTest is the single per-test naming opportunity — and it has already passed by the time the test body calls browser.reloadSession(). The replacement session is never registered in sessionMap, so flushSessionName() returns early on !sessionData and the onAfterExecute teardown sweep — which iterates that same map — misses it too. service.onReload learns the new session id but the only rename it issues targets the outgoing session. Net effect: the new session keeps its creation-time sessionName capability. A reload between tests is unaffected, because that runs before beforeTest.

onAfterTest now re-resolves the live session id and adopts it into sessionMap before flushing the name, while that session is still open.

Two deliberate details:

  • The naming block is gated on skipSessionName, not skipSessionStatus, and sits above the status gate. Gating a name repair behind a status flag would silently skip it for setSessionStatus: false + setSessionName: true users.
  • Adoption is skipped when skipSessionName is set. Those users' sessionMap is empty today, so onAfterExecute issues them no status calls at all; adopting unconditionally would have started sending one per session.

Steady state costs nothing extra: with no mid-test reload the session id is unchanged and appliedName de-dupes the flush away.

Also fixes a second effect on the same path — the post-reload session's TestResult was dropped by the old if (sessionData) guard, so onAfterExecute never marked its status either. It now carries its own pass/fail result.

Verification (App Automate, iOS, WDIO 9 + Mocha, mid-test reloadSession()):

  • Before, on shipped 9.34.0 from npm — 5/5 runs, the post-reload session's stored name is the static capability:
    automation_session.name: "STATIC-CAP-NAME-should-be-overwritten"
  • After, same project with this build linked in — build SDK-7270-after2-094653, both sessions carry the test title:
    automation_session.name: "SDK-7270 mid-test session reload - SUBJECT reloads the session mid-test"
  • Call census on the after run: 3 name PUTs + 2 status PUTs for 2 tests + 1 reload — one per session, no redundant sends.
  • automateModule.test.ts 35/35, including 4 new tests: mid-test-reload naming, steady-state de-dupe (asserts zero extra calls), guard independence under skipSessionStatus, and no new status traffic under skipSessionName.
  • Full package suite unchanged against a clean tree: 75 failed / 1123 passed before, 75 failed / 1127 passed after (+4 = the new tests). No regressions.

Known residual, out of scope here — and correctly so. A worker's initial session, when the suite discards it with a reloadSession() before the first test runs, hosts no test at all. There is no title to apply, and sessionMap is reachable only from onBeforeTest/onAfterTest — both of which require a test — so no naming path can or should touch it. On the reported build this takes statically-named sessions from 5 to 2, not to 0. That residual class is a suite-structure artifact rather than an SDK defect: in the same run, on the same build, the two workers that did not discard their initial session had it named normally. (The trigger for the discard sits in spec/helper code not present in the captured archive, so it is not attributed further here.)

Cucumber is not reached by this path (beforeScenario/afterScenario never emit TestFrameworkState.TEST) — a pre-existing gap, neither helped nor harmed. Jasmine is covered.

Related Jira task/s

SDK-7270

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 App Automate and Automate session names staying on the static sessionName capability when a test reloads the session part-way through — for example a deep-link or app-relaunch step. Such sessions now show the test title, and also carry their own pass/fail result.

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

  • automateModule.onAfterTest now re-resolves the live session id and adopts it into sessionMap before flushing the name, covering sessions created by a reloadSession() inside the test body — onBeforeTest (bound per test function by @wdio/mocha-framework) has already run by then, so those sessions were never registered and neither the per-test flush nor the onAfterExecute sweep could reach them.
  • The naming block is gated on skipSessionName and moved above the skipSessionStatus early return, so a setSessionStatus: false user still gets the name repair and a setSessionName: false user is not pulled into sessionMap (which would have added a status PUT per session where they previously had none).
  • Adoption also records the test result, so the post-reload session gets its status marked at teardown instead of being dropped by the old if (sessionData) guard.
  • Zero additional API calls in the steady state — appliedName de-dupes the second flush when the session did not change.

Checklist

  • Ready to review
  • Has it been tested locally?

PR Validations

Run Tests: Comment RUN_TESTS to trigger sanity tests.

Follow-up to #148. @wdio/mocha-framework binds beforeTest to the test function
itself (wrapGlobalTestMethod), so onBeforeTest is the single per-test naming
opportunity and it has already passed by the time the test body calls
browser.reloadSession(). The replacement session is never registered in
sessionMap, so flushSessionName() returns early on !sessionData and the
onAfterExecute sweep -- which iterates the same map -- misses it too.
service.onReload learns the new session id but only renames the outgoing one,
leaving the live session on its creation-time sessionName capability.

onAfterTest now re-resolves the live session id and adopts it into sessionMap
before flushing the name, while that session is still open. The naming block is
gated on skipSessionName rather than skipSessionStatus and sits above the status
gate: gating a name repair behind a status flag would skip it for
setSessionStatus:false users, and adopting unconditionally would pull
setSessionName:false users into sessionMap and start issuing them a status PUT
per session where they previously had none.

Adoption also records the test result, so the post-reload session's status is
marked at teardown instead of being dropped by the old `if (sessionData)` guard.
Steady state costs nothing extra -- appliedName de-dupes the flush when the
session did not change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@harshit-browserstack

Copy link
Copy Markdown
Collaborator Author

✅ Automated review — no blocking findings

Scope: the 2-file diff on this branch (automateModule.ts, automateModule.test.ts). Each item below was checked against the code rather than assumed, because an earlier revision of this change failed three of them and the fix was reworked as a result.

# Concern Verdict Evidence
1 Session naming must not be gated behind the status flag naming block at automateModule.ts:192; the skipSessionStatus early-return is at :199, below it. A setSessionStatus: false + setSessionName: true user still gets the repair.
2 setSessionName: false users must not be newly pulled into sessionMap (which would start issuing them one markSessionStatus PUT per session where they previously had none) adoption condition carries !testContextOptions.skipSessionName (:192); their map stays empty, so the onAfterExecute loop body never executes. Pinned by a test.
3 Falsy sessionId must never become a map key sessionId && leads the condition (:192). markSessionStatus in the teardown sweep has no such guard, so this matters.
4 Steady state must cost zero extra API calls appliedName de-dupe in flushSessionName (:124-127); pinned by a test asserting no fetch, and observed live as 3 name PUTs for 2 tests + 1 reload.
5 Can name computed in onAfterTest diverge from onBeforeTest's? the two blocks are mechanically identical (whitespace-normalised comparison). Independently, the existing-session path deliberately does not rewrite lastTestName, so even a hypothetical divergence cannot trigger a spurious rename.
6 Cross-framework impact TestFrameworkState.TEST is emitted only from service.ts:528 / :574 and skipReporter.ts:58 / :60. Cucumber's beforeScenario (service.ts:825) never emits it, so cucumber is untouched by this change — a pre-existing gap, neither helped nor harmed. Jasmine shares the mocha hook path and is covered. Multiremote's single-KEY_FRAMEWORK_SESSION_ID behaviour on reload is unchanged.

Bail-skipped tests — checked, unaffected

skipReporter also drives TEST PRE/POST (for tests mocha drops after bail), so it is a second caller into the changed method. It adds no extra calls under this change: PRE registers the session, POST finds sessionMap.has(sessionId) true and the flush de-dupes out. skipReporter.ts is not in this diff.

Worth separating out, though: on main today a bail-skipped test already renames the live session to that skipped test's title, via the per-test flush introduced in #148. Pre-existing, out of scope here, but it means a bailing run can finish with sessions named after tests that never executed. Happy to raise it separately.

Verification backing this

  • Reproduced 5/5 on shipped 9.34.0 from npm: the post-reload session's stored name stays "STATIC-CAP-NAME-should-be-overwritten".
  • With this branch: both sessions carry the test title; 3 name PUTs + 2 status PUTs for 2 tests + 1 reload.
  • automateModule.test.ts 35/35 (4 new tests). Full package suite against a clean tree: 75 failed / 1123 passed before, 75 failed / 1127 passed after — the +4 are the new tests, and the 75 are pre-existing/environmental.

Two limits on this verdict

  1. This is an automated review, not a human approval — it does not substitute for a reviewer sign-off.
  2. CI sanity tests have not been run on this branch yet (RUN_TESTS not triggered). The numbers above are local.

@harshit-browserstack
harshit-browserstack requested review from anish353 and shivam5643 and removed request for kamal-kaur04 and pri-gadhiya August 24, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant