Skip to content

feat(app-a11y): scan the WDIO config-level hook window, with or without a parent uuid - #176

Open
kamal-kaur04 wants to merge 3 commits into
mainfrom
SDK-7422-scan-pre-test-window
Open

feat(app-a11y): scan the WDIO config-level hook window, with or without a parent uuid#176
kamal-kaur04 wants to merge 3 commits into
mainfrom
SDK-7422-scan-pre-test-window

Conversation

@kamal-kaur04

@kamal-kaur04 kamal-kaur04 commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

What is this about?

A driver command issued from a WDIO config-level before() or beforeSuite was never scanned for App Accessibility. The scan gate opened at the first test — and WDIO runs config-level hooks before any test or framework hook exists — so screens visited during setup went uncovered.

The gate now opens as soon as the session is known a11y-capable, and the scan is sent whether or not a parent uuid exists: in that window there is no test and no framework hook, so the payload carries neither thTestRunUuid nor thHookRunUuid.

Every other validation still applies

Only the include/exclude tag filter is skipped, and only inside the window — it matches on suite and test titles, and neither exists yet. Everything else the per-test gate checks is unchanged: an a11y-capable session (caps-validated), autoScanning, a supported framework (mocha + cucumber; jasmine untouched, where App Accessibility is not supported), a real session id, and non-multiremote. onBeforeTest / beforeScenario re-computes the per-test gate — tags included — so the window governs nothing beyond itself.

No test run uuid for a window scan

TEST_ANALYTICS_ID in that window holds a uuid the framework minted at instance creation — a test that has not started. Sending it attributed the scan to a test it did not come from.

Verification

Bench: test-samples/app_automate-wdio_mocha-android/, phase boundaries annotated into the session log via browserstack_executor so each scan is attributed by position. Verdicts read from the App Automate session-logs API (work/SDK-7422-app-a11y-fixes/uuid-verdict.py).

flow conf baseline (main) this branch new window scans
mocha (CLI) repro.conf.ts chnsj2hcevtndlsq4q4fghp8b0zvnvo3ajx0zded · 119316d3e3689f4921b9256ed9bb9b5a948704ef — 6 scans, 0 in the window bfhusrbbjfyxlf4gbh0crdudvwsaup7wy5qi2gau · 4fda1f2fe573fd326b09ec560c148588fe62aa33 — 13 scans 7execute, executeScript, click ×3, execute, executeScript, each with no test uuid and no hook uuid
cucumber repro-cucumber.conf.ts zrlxhmjdu5z2diuvqeyqtizwvclttuc4uoefa8jt · c2a250be4c351e5ddc63b57ead8ba752e3a00fe4 — 3 scans edsvapa0evg18zt2ilbbjmd3t4l3s2fhuhhuccih · 86407fe669f17674685c03293866de9711cf6544 — 5 scans 2, likewise parentless
mocha (CLI) repro-inline-hooks.conf.tsframework hooks only 5xslabfrn225yzhdlaivg1z2fqubny7lcdsntrbj · ccd994cd941375698e449cc18398d73def6b66e8 — 6 scans, all carrying their test uuid bqez3voc1pj3kgqefduycmvjis1ksttcajmfojy7 · 4a1e06127c5d9c9d39610ac63d599dec1a8b0897 — 6 scans, all carrying their test uuid 0 — structurally identical to baseline. The control: this conf's before() touches no scannable command, so its only scans come from framework hooks

Per-phase on the mocha hook-command conf, this branch: 7 scans in config-level before() where the baseline had zero, then mocha before all 2, before each #1 1, test 1 body 3 — every one of those four framework-hook and test-body scans carrying test uuid db285069, exactly as the baseline does.

Framework hooks are deliberately untouched

The window closes the moment the framework signals a hook (onHookStart on the CLI path, beforeHook on the classic one), not at the first test. Closing at the first test would sweep in mocha's before all and before each #1 — they run inside that span — and strip their test run uuid, which is a behaviour change to framework hooks that this feature has no business making. The third row above is the control: a conf whose only scans come from framework hooks is structurally identical to baseline.

Unit: the params rule (window ⇒ no test uuid, hook uuid preserved; non-window ⇒ uuid sent), the CLI gate (opens at driver creation, respects autoScanning, closes at the first test), and the classic gate (mocha, cucumber, jasmine unchanged, multiremote skipped, closes at the first test). Full suite 1231 passed, 0 failed; npm run lint clean.

Two existing tests asserted the last argument of the scan helpers, so appending a parameter made them silently follow it. Both now assert by position.

Known limits

  • A command in the hook's first tick can still escape. Command wrapping completes a few ms after session creation, so a driver call in that gap is unscanned. Not closable from inside a service — no driver object exists to wrap earlier.
  • The window's scans have no parent on the dashboard — by design here. Reporting the window as a hook run is feat(app-a11y): scan the WDIO config-level before() window, and report it as a hook #168's scope, not this PR's.

Related Jira task/s

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

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)

  • Accessibility scans now run for driver commands issued from your WDIO config's before() and beforeSuite hooks, so screens visited during setup are covered.

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

  • The App-A11y scan gate now opens at driver creation instead of at the first test, so config-level WDIO hooks (which run before any test or framework hook exists) are covered on both the CLI and classic flows. Scoped to mocha + cucumber, non-multiremote; jasmine deliberately unchanged.
  • Scans fired in that window send no thTestRunUuidTEST_ANALYTICS_ID there holds a uuid minted at instance creation for a test that has not started. Every validation except the include/exclude tag filter still applies; tags cannot be evaluated without a suite/test title.

Checklist

  • Ready to review
  • Has it been tested locally?

PR Validations

Run Tests: Comment RUN_TESTS to trigger sanity tests.

A driver command issued from a config-level before()/beforeSuite was never scanned. The scan gate
opened at the first test, and WDIO runs those hooks before any test or framework hook exists, so
setup screens went uncovered. The gate now opens as soon as the session is known a11y-capable.

Every validation the per-test gate applies still applies here — an a11y-capable session,
autoScanning, a supported framework (mocha, cucumber; jasmine untouched), a real session id, and
non-multiremote. The include/exclude tag filter is the one exception: it matches on suite and test
titles, and in this window neither exists yet. onBeforeTest/beforeScenario re-computes the per-test
gate, tags included, so the window governs nothing beyond itself.

Scans from the window carry no test run uuid. TEST_ANALYTICS_ID there holds a uuid the framework
minted at instance creation — a test that has not started — so sending it attributed the scan to a
test it did not come from.

Two existing tests pinned the exact argument list of the scan helpers, so appending a parameter made
them silently follow it; both now assert by position.

SDK-7422
@kamal-kaur04
kamal-kaur04 requested a review from a team as a code owner August 31, 2026 09:36
@coderabbitai

coderabbitai Bot commented Aug 31, 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: Pro Plus

Run ID: b68f82a1-2c26-45de-8925-4b08dc964936

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

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

github-actions Bot and others added 2 commits August 31, 2026 09:36
…he first test

The window targets WDIO's own config hooks. Closing it at the first test swept in mocha's
before all / before each #1, which run inside that span, and stripped their test run uuid —
baseline carried it, the first cut showed none. That is a behaviour change to framework hooks,
which are not this feature's business.

It now closes the moment the framework signals a hook: onHookStart on the CLI path, beforeHook on
the classic one. The CLI clear sits ahead of that method's own early returns — if a framework hook
started at all, the config-level window is over, whether or not that hook goes on to scan. The
existing clears at the first test/scenario stay as a backstop for a spec with no framework hooks.

Four tests cover it, two per flow: a framework hook closes the window, and a framework-hook scan
still carries its test run uuid. Reverting either clear fails exactly those four.

Verified on device — repro-inline-hooks (framework hooks only) is now structurally identical to
baseline, every scan carrying its test uuid, while repro.conf.ts still gains its 7 parentless
config-level scans.

SDK-7422
@kamal-kaur04

Copy link
Copy Markdown
Collaborator Author

Correction pushed (2a872db): framework hooks must not change

The first cut defined the window as driver creation → first test. Mocha's before all and before each #1 run inside that span, so they got swept in and lost their test run uuid — baseline carried 61047812, the first cut showed none. That is a behaviour change to framework hooks, which this feature has no business making.

The window now closes the moment the framework signals a hook — onHookStart on the CLI path, beforeHook on the classic one. The CLI clear sits ahead of that method's own early returns: if a framework hook started at all, the config-level window is over, whether or not that hook goes on to scan. The clears at the first test/scenario stay as a backstop for a spec with no framework hooks.

Re-verified on device, all three confs

conf baseline (main) corrected branch reading
repro.conf.ts chnsj2hc… — 6 scans, 0 in window bfhusrbb… · 4fda1f2fe573fd326b09ec560c148588fe62aa33 — 13 7 new parentless window scans; before all + before each #1 restored to test uuid db285069 + hook uuid
repro-cucumber.conf.ts zrlxhmjd… — 3 edsvapa0… · 86407fe669f17674685c03293866de9711cf6544 — 5 2 new parentless; steps keep 86bbd66b
repro-inline-hooks.conf.ts 5xslabfr… — 6, all with test uuid bqez3voc… · 4a1e06127c5d9c9d39610ac63d599dec1a8b0897 — 6, all with test uuid structurally identical to baseline — the control, since its only scans come from framework hooks

Tests

Four added, two per flow: a framework hook closes the window, and a framework-hook scan still carries its test run uuid. They are real regression tests — reverting src/ to the previous commit fails exactly those four (4 failed, 92 passed). Full suite 1231 passed, 0 failed; tsc and lint clean.

The PR description is updated: new build ids, and the "before each #1 loses its test uuid" known limit is deleted rather than reworded — it no longer happens.

@kamal-kaur04
kamal-kaur04 requested review from 07souravkunda and removed request for pri-gadhiya August 31, 2026 13:34
// validation still applies — an a11y-capable session (returned above), autoScanning, a
// supported framework, a real session id. The include/exclude tag filter is the one
// exception: it matches on suite and test titles, and neither exists in this window.
if (this._autoScanning && this.supportsPreTestWindow() && sessionId) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gate has no app-accessibility check, so it opens on web (Automate) a11y sessions too — supportsPreTestWindow() checks framework and multiremote only, and the !this._accessibility return above is satisfied by validateCapsWithA11y as well as the App variant.

On the web path the isGlobalHook correction has no reach: performA11yScan takes the non-app branch and calls executeAccessibilityScript(browser, performScan, { method: commandName })isGlobalHook is never forwarded. The web payload carries no test identity at all; attribution happens later, when sendTestStopEvent flushes accumulated results via saveTestResults(dataForExtension) with thTestRunUuid = process.env.TEST_ANALYTICS_ID.

Evidence: so every scan fired in the config-level window is flushed into the first test's results, inflating its issue count with setup screens — the same misattribution this PR removes on the app path, reintroduced where the fix cannot apply. The PR title, the changeset, and all three verification rows are app_automate; no web run appears in the table.

Fix:

if (this._autoScanning
    && isAppAccessibilityAutomationSession(this._accessibility, this.isAppAutomate)
    && this.supportsPreTestWindow() && sessionId) {  }

Question: was a web a11y session bench-tested? If web coverage is intended, what attributes a window scan there, given saveTestResults is the only thing that assigns a test uuid?

// exception: it matches on suite and test titles, and neither exists in this window.
if (this._autoScanning && this.supportsPreTestWindow() && sessionId) {
AccessibilityHandler._a11yScanSessionMap[sessionId] = true
this._preTestWindowActive = true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR body and the changeset both promise coverage for config-level before() and beforeSuite, but only the first beforeSuite of a session falls inside this window.

Evidence: beforeSuite fires per suite (service.ts:825 sets _suiteTitle on each one). The window closes at the first framework hook, with the first test as backstop — so from the second top-level describe onward the window is long gone, and commands issued in that beforeSuite fall back to the previous test's state: _a11yScanSessionMap still holds the last test's shouldScanTest, and TEST_ANALYTICS_ID still holds the last test's uuid. Those scans are attributed to a test that has already finished — the same misattribution this PR exists to remove, shifted to the second suite onward. A spec file with two top-level describes reproduces it.

Fix: either reopen the window on beforeSuite (a suite start with no active test), or narrow the claim in the PR body and changeset to before() plus the first beforeSuite.

// on suite and test titles, and in this window neither exists yet. onBeforeTest
// re-computes the per-test gate, tags included, so this only affects the window.
const preTestSessionId = this.currentSessionId()
if (this.autoScanning && preTestSessionId !== undefined && preTestSessionId !== null) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern as on the classic gate (accessibility-handler.ts): this gate has no app-accessibility check, so it opens on web (Automate) a11y sessions too — the !this.accessibility return above is satisfied by validateCapsWithA11y as well as validateCapsWithAppA11y.

performScanCli only forwards isGlobalHook on the isAppAccessibility branch; the web branch is executeAsync(performScan, { method: commandName }) and drops it. Attribution on that path happens in sendTestStopEventsaveTestResults(dataForExtension), so window scans land in whichever test saves next — the first one.

Fix:

if (this.autoScanning && this.isAppAccessibility
    && preTestSessionId !== undefined && preTestSessionId !== null) {  }

Question: is web coverage intended here, or should both gates be scoped to app accessibility to match the PR's title and verification?

const preTestSessionId = this.currentSessionId()
if (this.autoScanning && preTestSessionId !== undefined && preTestSessionId !== null) {
this.accessibilityMap.set(preTestSessionId, true)
this.preTestWindowActive = true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gate carries neither of the two guards its classic twin has: no framework allowlist (PRE_TEST_SCAN_FRAMEWORKS = ['mocha','cucumber'], jasmine deliberately excluded) and no multiremote exclusion. Nothing upstream supplies them — AccessibilityModule is constructed purely on startBinResponse.accessibility?.success (cli/index.ts:190), and _isCliAccessibilityFlow() is isBrowserstackSession && isRunning() (service.ts:904). Neither is framework-aware.

Evidence: the window's two close paths are onBeforeTest (TestFrameworkState.TEST PRE) and onHookStart (BEFORE_ALL/BEFORE_EACH/…) — both registered on TestFramework, and both only ever emitted by WdioMochaTestFramework, which setupTestFramework() instantiates only for webdriverio-mocha (cli/index.ts:549). service.ts:437 already documents getTestFramework() being null for non-mocha. Cucumber never reaches beforeTest at all — service.beforeScenario calls the classic handler only.

So on CLI + cucumber the window opens at driver creation and never closes for the life of the session: accessibilityMap stays true and every scan in every scenario goes out with isGlobalHook = true, dropping thTestRunUuid for the whole run. Jasmine — "untouched" per the PR body — is likewise unguarded on this flow. The cucumber verification row is the one without a "(CLI)" annotation, so this cell looks untested.

Fix: mirror the classic guard here (ideally reading the same PRE_TEST_SCAN_FRAMEWORKS constant plus the multiremote check), and add a close on the cucumber scenario path — or restrict the CLI window to mocha, where a close signal provably exists.

Question: is the CLI/binary flow reachable for cucumber and jasmine today? If it is, what closes preTestWindowActive there?

await accessibilityModule.onBeforeExecute()
await accessibilityModule.onHookStart({ instance: mockTestInstance })

await (accessibilityModule as any).performScanCli(mockBrowser, 'click', 'hook-uuid-1', accessibilityModule.preTestWindowActive)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This asserts the helper, not the wiring. The flag is hand-fed as the fourth argument, so the test says nothing about what the production call site passes.

The real call site is commandWrapperperformScanCli(browser, command.name, this.currentHookRunUuid, this.preTestWindowActive) (accessibilityModule.ts:308). Change that fourth argument to a literal true and this test still passes — the regression it is meant to catch slips through.

Evidence: the classic-flow twin gets this right. tests/accessibility-handler.test.ts → "leaves a framework-hook scan carrying its test run uuid" drives handler['commandWrapper'](…) and asserts lastCall[7], so the wiring is genuinely pinned.

Fix: go through the wrapper and assert what the spy observed:

const orig = vi.fn().mockResolvedValue('ok')
await (accessibilityModule as any).commandWrapper({ name: 'click', class: 'Element' }, orig, 'arg')
expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, 'hook-uuid-1', false)

expect(_getParamsForAppAccessibility).toHaveBeenCalledWith('click', undefined, undefined, undefined)
})
})
}) No newline at end of file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has no trailing newline (\ No newline at end of file in the diff). Pre-existing on main rather than introduced here, but the PR already edits this file — worth closing while it is open.

@07souravkunda 07souravkunda left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mechanism is minimal and well-reasoned, and the 2a872db correction — closing the window at the first framework signal rather than the first test, with the framework-hooks-only conf as a control — is the right call and properly covered. Blocking on scope rather than on design: neither gate is conditioned on app accessibility, so web (Automate) a11y sessions are silently in scope on a path where isGlobalHook is never forwarded and attribution happens at saveTestResults — window scans land in the first test's results. And the CLI gate carries neither of the classic gate's guards (framework allowlist, multiremote), while its only close signals are emitted by WdioMochaTestFramework, which is wired for webdriverio-mocha alone — so on CLI + cucumber the window appears to open at driver creation and never close. Two smaller items: beforeSuite is covered only for the first suite in a session, and the CLI test for the uuid-preservation rule hand-feeds the flag instead of going through commandWrapper, so the :308 wiring is unpinned. Details in the inline threads.

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.

2 participants