Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/pr-212.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@wdio/browserstack-service": patch
---

- Fixed accessibility command wrapping aborting on App Automate sessions, which logged `Error in onBeforeExecute` and left the remaining commands unwrapped for auto-scanning. Applies to both the CLI and the non-CLI flow.
- Accessibility results and summary failures now report the HTTP status, the response body and the identifiers the query used, instead of an empty result with no explanation.
Comment thread
kamal-kaur04 marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,11 @@ class _AccessibilityHandler {
.filter((command) => command.name && command.class)
.forEach((command) => {
const browser = this._browser as WebdriverIO.Browser
browser.overwriteCommand(command.name, this.commandWrapper.bind(this, command), command.class === 'Element')
try {
browser.overwriteCommand(command.name, this.commandWrapper.bind(this, command), command.class === 'Element')

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Warning — the service-flow guard ships without the regression test its CLI-flow twin got.

The guard itself is correct, to be clear: browser.overwriteCommand is synchronous and throws when the driver has no matching base command, so this try/catch confines a single bad accessibilityScripts.commandsToWrap entry to its own iteration, the forEach continues wrapping the rest, PerformanceTester.end(...) below is now reached, and the failure still surfaces via BStackLogger.debug with both the command name and the error.

What's missing is the evidence. This is the mirror of the change to src/cli/modules/accessibilityModule.ts in the same PR, and that side did get a regression test — "skips a command the driver did not register without aborting the wrap loop". The service flow has its own established suite at packages/browserstack-service/tests/accessibility-handler.test.ts, which this PR doesn't touch.

That asymmetry matters because of how this failure presents: the loop aborting on the first Selenium-shaped entry exits 0 and shows up only as silently unwrapped commands and zero accessibility scans. So the untested half is precisely the one that can regress unnoticed while the CLI half stays green in CI.

Suggested fix — add the symmetric case to tests/accessibility-handler.test.ts: stub accessibilityScripts.commandsToWrap with three entries, mock browser.overwriteCommand to throw on the first call, invoke the before path, then assert that (a) overwriteCommand was still called for the remaining two entries and (b) the method returns normally.

Capture and restore the original commandsToWrap in beforeEach/afterEach rather than inside the test body — the CLI test learned that one the hard way, so a failing assertion here can't leak the mutated singleton into later tests in the file.

} catch (error) {
BStackLogger.debug(`Exception in overwrite command ${command.name} - ${error}`)
}
})

PerformanceTester.end(PERFORMANCE_SDK_EVENTS.CONFIG_EVENTS.ACCESSIBILITY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,16 +139,22 @@ export default class AccessibilityModule extends BaseModule {
return
}

// Wrap commands if accessibility scripts are available
// Guard EACH overwriteCommand individually: a command the driver doesn't register just
// skips (logged) rather than aborting the whole wrap loop, so the commands appium DOES
// register (click, setValue, ...) still auto-scan on app.
if (this.scriptInstance.commandsToWrap && this.scriptInstance.commandsToWrap.length > 0) {
this.scriptInstance.commandsToWrap
.filter((command) => command.name && command.class)
.forEach((command) => {
browser.overwriteCommand(
command.name,
this.commandWrapper.bind(this, command),
command.class === 'Element'
)
try {
browser.overwriteCommand(
command.name,
this.commandWrapper.bind(this, command),
command.class === 'Element'
)
} catch (wrapError) {
this.logger.debug(`Skipping command wrap for ${command.name}: ${wrapError}`)
}
})
}

Expand Down
23 changes: 16 additions & 7 deletions packages/browserstack-service/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -660,8 +660,7 @@ export const getAppA11yResults = PerformanceTester.measureWrapper(PERFORMANCE_SD
BStackLogger.debug(`Polling Result: ${JSON.stringify(result)}`)
return result
} catch (error: any) {
BStackLogger.error('No accessibility summary was found.')
BStackLogger.debug(`getAppA11yResults Failed. Error: ${error}`)
BStackLogger.error(`No accessibility results were found for testRunUuid=${process.env.TEST_ANALYTICS_ID} sessionId=${sessionId}. Error: ${error?.message ?? util.inspect(error, { depth: 2 })}`)
return []
}
})
Expand All @@ -682,8 +681,8 @@ export const getAppA11yResultsSummary = PerformanceTester.measureWrapper(PERFORM
const result = apiRespone?.data?.data?.summary
BStackLogger.debug(`Polling Result: ${JSON.stringify(result)}`)
return result
} catch {
BStackLogger.error('No accessibility summary was found.')
} catch (error: any) {
BStackLogger.error(`No accessibility summary was found for testRunUuid=${process.env.TEST_ANALYTICS_ID} sessionId=${sessionId}. Error: ${error?.message ?? util.inspect(error, { depth: 2 })}`)
return {}
}
})
Expand Down Expand Up @@ -711,8 +710,8 @@ export const getA11yResultsSummary = PerformanceTester.measureWrapper(PERFORMANC
await performA11yScan(isAppAutomate, browser, isBrowserStackSession, isAccessibility)
const summaryResults: { [key: string]: any; } = await (browser as WebdriverIO.Browser).executeAsync(AccessibilityScripts.getResultsSummary as string)
return summaryResults
} catch {
BStackLogger.error('No accessibility summary was found.')
} catch (error: any) {
BStackLogger.error(`No accessibility summary was found. Error: ${error?.message ?? util.inspect(error, { depth: 2 })}`)
return {}
}
})
Expand Down Expand Up @@ -1785,10 +1784,20 @@ export async function pollApi(
await new Promise((resolve) => setTimeout(resolve, elapsedTime))
return pollApi(url, params, headers, upperLimit, startTime)
} else if (error.response) {
const statusCode = error.response.statusCode
Comment thread
kamal-kaur04 marked this conversation as resolved.
const body = typeof error.response.body === 'string' ? error.response.body : ''
let message: string | undefined
try {
message = body ? JSON.parse(body).message : undefined
} catch {
// non-JSON body; the raw-body message below carries it instead
}
throw {
data: {},
headers: {},
message: error.response.body ? JSON.parse(error.response.body).message : 'Unknown error',
statusCode,
body,
message: message ?? `HTTP ${statusCode}${body ? `: ${body.slice(0, 300)}` : ''}`,
}
} else {
BStackLogger.error(`Unexpected error occurred: ${error}`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import logger from '@wdio/logger'

import AccessibilityHandler from '../src/accessibility-handler.js'
import * as utils from '../src/util.js'
import accessibilityScripts from '../src/scripts/accessibility-scripts.js'
import type { Capabilities, Options } from '@wdio/types'
import * as bstackLogger from '../src/bstackLogger.js'
import type { BrowserstackConfig, BrowserstackOptions } from '../src/types.js'
Expand Down Expand Up @@ -154,6 +155,37 @@ describe('before', () => {
(browser as WebdriverIO.Browser).getAccessibilityResults()
expect(getA11yResultsSpy).toBeCalledTimes(1)
})

// The server-sent commandsToWrap list can name a command this driver never registered
// (Selenium-shaped entries meant for another SDK). overwriteCommand throws on those, and
// before() has no try/catch of its own, so an unguarded loop rejects the whole hook.
it('skips a command the driver did not register without aborting the wrap loop', async () => {
const originalCommandsToWrap = accessibilityScripts.commandsToWrap
try {
isBrowserstackSessionSpy.mockReturnValue(true)
isAccessibilityAutomationSessionSpy.mockReturnValue(true)
vi.spyOn(utils, 'validateCapsWithA11y').mockReturnValue(true)
accessibilityScripts.commandsToWrap = [
{ name: 'click', class: 'Element' },
{ name: 'startA11yScanning', class: 'HttpCommandExecutor' },
{ name: 'addValue', class: 'Element' }
] as any
const overwriteCommand = vi.fn((name: string) => {
if (name === 'startA11yScanning') {
throw new Error('overwriteCommand: no command to be overwritten: ' + name)
}
});
(browser as any).overwriteCommand = overwriteCommand

await accessibilityHandler.before('session123')

expect(overwriteCommand).toHaveBeenCalledTimes(3)
expect(overwriteCommand).toHaveBeenLastCalledWith('addValue', expect.any(Function), true)
} finally {
accessibilityScripts.commandsToWrap = originalCommandsToWrap
delete (browser as any).overwriteCommand
}
})
})

describe('beforeScenario', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import { HookState } from '../../../src/cli/states/hookState.js'
import { TestFrameworkState } from '../../../src/cli/states/testFrameworkState.js'
import { BrowserstackCLI } from '../../../src/cli/index.js'
import { shouldScanTestForAccessibility, validateCapsWithA11y, validateCapsWithAppA11y } from '../../../src/util.js'
import accessibilityScripts from '../../../src/scripts/accessibility-scripts.js'

describe('AccessibilityModule', () => {
let accessibilityModule: AccessibilityModule
Expand Down Expand Up @@ -265,6 +266,42 @@ describe('AccessibilityModule', () => {

expect(loggerWarnSpy).toHaveBeenCalledWith('Accessibility scanning cannot be stopped from outside the test')
})

// SDK-7452: the server-sent commandsToWrap list ends with Selenium-shaped entries
// (startA11yScanning/stopA11yScanning/performScan, class HttpCommandExecutor) that a
// WebdriverIO driver never registers. overwriteCommand throws on those names, and without a
// per-command guard the throw escaped the forEach and aborted onBeforeExecute.
it('skips a command the driver did not register without aborting the wrap loop', async () => {
const loggerErrorSpy = vi.spyOn(accessibilityModule.logger, 'error')
accessibilityModule.accessibility = true
accessibilityModule.isAppAccessibility = true
vi.mocked(validateCapsWithA11y).mockReturnValue(true)
vi.mocked(validateCapsWithAppA11y).mockReturnValue(true)

const originalCommandsToWrap = accessibilityScripts.commandsToWrap
try {
accessibilityScripts.commandsToWrap = [
{ name: 'click', class: 'Element' },
{ name: 'startA11yScanning', class: 'HttpCommandExecutor' },
{ name: 'addValue', class: 'Element' }
] as any
mockBrowser.overwriteCommand = vi.fn((name: string) => {
if (name === 'startA11yScanning') {
throw new Error('overwriteCommand: no command to be overwritten: ' + name)
}
})

await accessibilityModule.onBeforeExecute()

expect(mockBrowser.overwriteCommand).toHaveBeenCalledTimes(3)
expect(mockBrowser.overwriteCommand).toHaveBeenLastCalledWith('addValue', expect.any(Function), true)
expect(loggerErrorSpy).not.toHaveBeenCalledWith(
expect.stringContaining('Error in onBeforeExecute')
)
} finally {
accessibilityScripts.commandsToWrap = originalCommandsToWrap
}
})
})

describe('onBeforeTest', () => {
Expand Down
Loading