diff --git a/.changeset/pr-212.md b/.changeset/pr-212.md new file mode 100644 index 00000000..15d058f5 --- /dev/null +++ b/.changeset/pr-212.md @@ -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. diff --git a/.changeset/pr-215.md b/.changeset/pr-215.md new file mode 100644 index 00000000..b7406483 --- /dev/null +++ b/.changeset/pr-215.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": patch +--- + +- Fixed test tags not being reported to Test Observability for Mocha and Jasmine. Tags written as `@tag` tokens in suite or test titles are now sent with each test. diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 9da71d1d..fd7041f1 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -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') + } catch (error) { + BStackLogger.debug(`Exception in overwrite command ${command.name} - ${error}`) + } }) PerformanceTester.end(PERFORMANCE_SDK_EVENTS.CONFIG_EVENTS.ACCESSIBILITY) diff --git a/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts b/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts index 4c384b1f..b5706ee2 100644 --- a/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts +++ b/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts @@ -10,7 +10,7 @@ import TrackedInstance from '../instances/trackedInstance.js' import { TestFrameworkConstants } from './constants/testFrameworkConstants.js' import { BStackLogger as logger } from '../cliLogger.js' import type { Frameworks } from '@wdio/types' -import { getGitMetaData, getMochaTestHierarchy, getUniqueIdentifier, isUndefined, removeAnsiColors } from '../../util.js' +import { getGitMetaData, getMochaTestHierarchy, getTestTags, getUniqueIdentifier, isUndefined, removeAnsiColors } from '../../util.js' import { TEST_ANALYTICS_ID } from '../../constants.js' export default class WdioMochaTestFramework extends TestFramework { @@ -192,6 +192,8 @@ export default class WdioMochaTestFramework extends TestFramework { const gitConfig = await getGitMetaData() const filename = test.file // || this._suiteFile + const scopes = getMochaTestHierarchy(test) + const testData: Record = { [TestFrameworkConstants.KEY_TEST_ID]: getUniqueIdentifier(test, framework), [TestFrameworkConstants.KEY_TEST_NAME]: test.title || test.description, @@ -199,7 +201,8 @@ export default class WdioMochaTestFramework extends TestFramework { [TestFrameworkConstants.KEY_TEST_FILE_PATH]: (gitConfig?.root && filename) ? path.relative(gitConfig.root, filename) : undefined, [TestFrameworkConstants.KEY_TEST_LOCATION]: filename ? path.relative(process.cwd(), filename) : undefined, [TestFrameworkConstants.KEY_TEST_SCOPE]: fullTitle, - [TestFrameworkConstants.KEY_TEST_SCOPES]: getMochaTestHierarchy(test), + [TestFrameworkConstants.KEY_TEST_SCOPES]: scopes, + [TestFrameworkConstants.KEY_TEST_TAGS]: getTestTags(test, scopes), } return testData diff --git a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts index e568f14d..1a12b179 100644 --- a/packages/browserstack-service/src/cli/modules/accessibilityModule.ts +++ b/packages/browserstack-service/src/cli/modules/accessibilityModule.ts @@ -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}`) + } }) } diff --git a/packages/browserstack-service/src/insights-handler.ts b/packages/browserstack-service/src/insights-handler.ts index 767b4d8e..ee2a23ae 100644 --- a/packages/browserstack-service/src/insights-handler.ts +++ b/packages/browserstack-service/src/insights-handler.ts @@ -15,6 +15,7 @@ import { getGitMetaData, getHookType, getPlatformVersion, getScenarioExamples, + getTestTags, getUniqueIdentifier, getUniqueIdentifierForCucumber, isBrowserstackSession, @@ -725,6 +726,8 @@ class _InsightsHandler { InsightsHandler.currentTest.name = test.title || test.description } + const scopes = this.getHierarchy(test) + const testData: TestData = { uuid: testMetaData.uuid, type: test.type || 'test', @@ -734,7 +737,8 @@ class _InsightsHandler { code: test.body }, scope: fullTitle, - scopes: this.getHierarchy(test), + scopes, + tags: getTestTags(test, scopes), identifier: fullTitle, file_name: filename ? path.relative(process.cwd(), filename) : undefined, location: filename ? path.relative(process.cwd(), filename) : undefined, diff --git a/packages/browserstack-service/src/reporter.ts b/packages/browserstack-service/src/reporter.ts index 88608e03..d9449827 100644 --- a/packages/browserstack-service/src/reporter.ts +++ b/packages/browserstack-service/src/reporter.ts @@ -17,7 +17,8 @@ import { getGitMetaData, removeAnsiColors, getHookType, - getPlatformVersion + getPlatformVersion, + getTestTags } from './util.js' import { BStackLogger } from './bstackLogger.js' import type { Capabilities } from '@wdio/types' @@ -270,6 +271,7 @@ class _TestReporter extends WDIOReporter { }, scope: scope, scopes: scopes, + tags: getTestTags(testStats as unknown as Frameworks.Test, scopes), identifier: identifier, file_name: suiteFileName ? path.relative(process.cwd(), suiteFileName) : undefined, location: suiteFileName ? path.relative(process.cwd(), suiteFileName) : undefined, diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 8a14f3be..82e09623 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -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 [] } }) @@ -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 {} } }) @@ -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 {} } }) @@ -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 + 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}`) @@ -1937,6 +1946,29 @@ export function getMochaTestHierarchy(test: Frameworks.Test) { return value.reverse() } +// The lookbehind is load-bearing: without it every `@` starts a match, so an address +// like `user@example.com` in a title yields a bogus `@example` tag. +const TEST_TAG_PATTERN = /(? { (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', () => { diff --git a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts index e0e36768..0cd78797 100644 --- a/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts +++ b/packages/browserstack-service/tests/cli/modules/accessibilityModule.test.ts @@ -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 @@ -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', () => { diff --git a/packages/browserstack-service/tests/util.test.ts b/packages/browserstack-service/tests/util.test.ts index 5414eab0..74388334 100644 --- a/packages/browserstack-service/tests/util.test.ts +++ b/packages/browserstack-service/tests/util.test.ts @@ -2282,3 +2282,56 @@ describe('getCentralUser', () => { expect(utils.getCentralUser()).toEqual({}) }) }) + +describe('getTestTags', () => { + const tagsFor = (title: string, scopes: string[] = []) => + utils.getTestTags({ title } as any, scopes) + + it('picks up a tag in the test title', () => { + expect(tagsFor('logs in @smoke')).toEqual(['@smoke']) + }) + + it('picks up a tag from the describe scope', () => { + expect(tagsFor('logs in', ['auth @regression'])).toEqual(['@regression']) + }) + + it('merges scope and title tags, deduped', () => { + expect(tagsFor('logs in @smoke', ['auth @smoke', 'nested @regression'])) + .toEqual(['@smoke', '@regression']) + }) + + it('returns an empty array when nothing is tagged', () => { + expect(tagsFor('logs in', ['auth'])).toEqual([]) + }) + + it('picks up multiple tags from one title', () => { + expect(tagsFor('logs in @smoke @p1')).toEqual(['@smoke', '@p1']) + }) + + it('keeps hyphens in a tag', () => { + expect(tagsFor('logs in @smoke-test')).toEqual(['@smoke-test']) + }) + + it('falls back to the Jasmine description when there is no title', () => { + expect(utils.getTestTags({ description: 'logs in @jasmine' } as any, [])).toEqual(['@jasmine']) + }) + + it('ignores an @ embedded in a larger token', () => { + expect(tagsFor('sends the invite to user@example.com')).toEqual([]) + expect(tagsFor('installs pkg@1.2.3')).toEqual([]) + }) + + it('derives scopes from the mocha hierarchy when none are supplied', () => { + const test = { + title: 'logs in @smoke', + ctx: { test: {} }, + parent: { title: 'auth @regression', parent: { title: '' } } + } + expect(utils.getTestTags(test as any)).toEqual(['@regression', '@smoke']) + }) + + it('derives scopes from the jasmine hierarchy when none are supplied', () => { + const test = { description: 'logs in @smoke', fullName: 'auth @regression logs in @smoke' } + expect(utils.getTestTags(test as any)).toEqual(['@regression', '@smoke']) + }) +})