From 0a619b4e37b854df725e13c5b9025350ffd8937e Mon Sep 17 00:00:00 2001 From: Anish Sinha Date: Wed, 9 Sep 2026 21:22:40 +0530 Subject: [PATCH 1/5] fix(service): report screenshot logs on the binary flow (SDK-4177) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `InsightsHandler.browserCommand` is the only producer of TEST_SCREENSHOT logs, and its two call sites both sat behind `!BrowserstackCLI.isRunning()`. Since `CLISupportedFrameworks = ['mocha']`, every Mocha run took the binary flow and lost screenshots entirely — Observability received a test run with no TEST_LOG artifact, so no screenshots manifest. Jasmine and Cucumber, which stay on the Direct flow, were unaffected. Subscribe to the result event on the binary path as well. `command` (beforeCommand) is deliberately left unsubscribed: it only fills the map the HTTP-log half of `browserCommand` reads, and that half emits on the JS listener pipeline the binary owns here — so the screenshot upload, which rides its own JWT-authenticated endpoint, stays the single effect. Also honour an explicit denial: `allow_screenshots` is an optional *string* on the wire, so a denial arrives as `'false'`, which `Boolean()` read as permission granted. Same defect class `shouldProcessEventForTesthub` already guards against with `isTrue`. Co-Authored-By: Claude Opus 5 (1M context) --- .../sdk-4177-cli-flow-screenshot-log.md | 5 ++++ .../src/insights-handler.ts | 8 ++++-- packages/browserstack-service/src/service.ts | 19 +++++++++++++ .../tests/insights-handler.test.ts | 9 ++++++ .../tests/service.test.ts | 28 +++++++++++++++++++ 5 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 .changeset/sdk-4177-cli-flow-screenshot-log.md diff --git a/.changeset/sdk-4177-cli-flow-screenshot-log.md b/.changeset/sdk-4177-cli-flow-screenshot-log.md new file mode 100644 index 0000000..20ebcb0 --- /dev/null +++ b/.changeset/sdk-4177-cli-flow-screenshot-log.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": patch +--- + +- Fixed screenshots taken during a Mocha test never appearing in Test Reporting's consolidated logs. Mocha runs through the BrowserStack binary, and on that path the WebDriver result event the screenshot log is built from was never subscribed to, so the screenshot was captured by the browser but never reported. A screenshot denial from the server is now honoured too, where previously it was read as approval. diff --git a/packages/browserstack-service/src/insights-handler.ts b/packages/browserstack-service/src/insights-handler.ts index e64a312..6be5d74 100644 --- a/packages/browserstack-service/src/insights-handler.ts +++ b/packages/browserstack-service/src/insights-handler.ts @@ -27,7 +27,8 @@ import { removeAnsiColors, getObservabilityProduct, generateHashCodeFromFields, - isTrue + isTrue, + isFalse } from './util.js' import type { TestData, @@ -790,7 +791,10 @@ class _InsightsHandler { // log screenshot const body = 'body' in args ? args.body : undefined const result = 'result' in args ? args.result as { value: string } : undefined - if (Boolean(process.env[TESTOPS_SCREENSHOT_ENV]) && isScreenshotCommand(args) && result?.value) { + // `allow_screenshots` is an optional *string* on the wire, so a denial arrives as the + // string 'false' — which Boolean() reads as permission granted. Honour it explicitly. + const allowScreenshots = process.env[TESTOPS_SCREENSHOT_ENV] + if (Boolean(allowScreenshots) && !isFalse(allowScreenshots) && isScreenshotCommand(args) && result?.value) { await this.listener.onScreenshot([{ test_run_uuid: testMeta.uuid, timestamp: new Date().toISOString(), diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index eef859c..e4c37ea 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -314,6 +314,25 @@ export default class BrowserstackService implements Services.ServiceInstance { BStackLogger.info(`CLI is running, tracking insights event for before: ${sessionId}`) await BrowserstackCLI.getInstance().getAutomationFramework()!.trackEvent(AutomationFrameworkState.CREATE, HookState.POST, { browser: this._browser, hubUrl: this._config.hostname }) this._insightsHandler.setGitConfigPath() + /** + * `browserCommand` is the only producer of TEST_SCREENSHOT logs — the binary + * has no screenshot producer of its own — so the result event has to be + * registered on this path too, or a screenshot taken mid-test never reaches + * Observability (SDK-4177). The `command` (beforeCommand) event is + * deliberately NOT registered: it only fills the map that browserCommand's + * HTTP-log half reads, and that half emits on the JS listener pipeline the + * binary owns here. Leaving it unregistered keeps the screenshot upload — + * which rides its own JWT-authenticated endpoint — as the single effect. + */ + this._browser.on('result', (result) => { + if (shouldProcessEventForTesthub('')) { + this._insightsHandler?.browserCommand( + 'client:afterCommand', + Object.assign(result, { sessionId }), + this._currentTest + ) + } + }) PerformanceTester.end(PERFORMANCE_SDK_EVENTS.DRIVER_EVENT.PRE_INITIALIZE) return } diff --git a/packages/browserstack-service/tests/insights-handler.test.ts b/packages/browserstack-service/tests/insights-handler.test.ts index a56b869..4314e46 100644 --- a/packages/browserstack-service/tests/insights-handler.test.ts +++ b/packages/browserstack-service/tests/insights-handler.test.ts @@ -697,6 +697,15 @@ describe('browserCommand', () => { delete process.env[TESTOPS_SCREENSHOT_ENV] }) + it('client:afterCommand - screenshot not uploaded when screenshots are denied', () => { + // arrives as the string 'false' off the wire, which Boolean() would read as granted + process.env[TESTOPS_SCREENSHOT_ENV] = 'false' + commandSpy.mockImplementation(() => { return true }) + insightsHandler.browserCommand('client:afterCommand', { sessionId: 's', method: 'm', endpoint: 'e', result: { value: 'random' } } as any, {} as any) + expect(uploadEventDataSpy).toBeCalledTimes(0) + delete process.env[TESTOPS_SCREENSHOT_ENV] + }) + it('return if test not in _tests', () => { insightsHandler.browserCommand('client:afterCommand', { sessionId: 's', method: 'm', endpoint: 'e', result: { value: 'random' } } as any, {} as any) insightsHandler['_tests'] = { 'test title not there': { 'uuid': 'uuid' } } diff --git a/packages/browserstack-service/tests/service.test.ts b/packages/browserstack-service/tests/service.test.ts index bf544f9..1ebbadf 100644 --- a/packages/browserstack-service/tests/service.test.ts +++ b/packages/browserstack-service/tests/service.test.ts @@ -580,6 +580,34 @@ describe('before', () => { expect(service['_sessionBaseUrl']).toEqual(sessionBaseUrl) }) + it('registers the result event on the CLI path, and only that event', async () => { + // browserCommand is the only producer of TEST_SCREENSHOT logs, so the binary flow has to + // register the result event too (SDK-4177). `command` must stay unregistered — it only + // feeds browserCommand's HTTP-log half, which the binary owns on this path. + process.env.BROWSERSTACK_OBSERVABILITY = 'true' + const getInstanceSpy = vi.spyOn(BrowserstackCLI, 'getInstance').mockReturnValue({ + isRunning: () => true, + getTestFramework: () => null, + getAutomationFramework: () => ({ + trackEvent: vi.fn().mockResolvedValue(undefined) + }) + } as any) + const service = new BrowserstackService({} as any, [{}] as any, { + user: 'foo', + key: 'bar', + capabilities: {} + }) + + await service.before(service['_config'] as any, [], browser) + + const events = vi.mocked(browser.on).mock.calls.map(([event]) => event) + expect(events).toContain('result') + expect(events).not.toContain('command') + + getInstanceSpy.mockRestore() + delete process.env.BROWSERSTACK_OBSERVABILITY + }) + it('should initialize correctly for multiremote', () => { const service = new BrowserstackService( {} as any, From 709f108d2ea3f7ac070b9ea2707e51c3d9642d33 Mon Sep 17 00:00:00 2001 From: Anish Sinha Date: Wed, 9 Sep 2026 21:23:42 +0530 Subject: [PATCH 2/5] chore: drop manual changeset, PR template generates it `.github/PULL_REQUEST_TEMPLATE.md` states the changeset is generated from the PR's Release section as `.changeset/pr-.md`, so a hand-written one is redundant here. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/sdk-4177-cli-flow-screenshot-log.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/sdk-4177-cli-flow-screenshot-log.md diff --git a/.changeset/sdk-4177-cli-flow-screenshot-log.md b/.changeset/sdk-4177-cli-flow-screenshot-log.md deleted file mode 100644 index 20ebcb0..0000000 --- a/.changeset/sdk-4177-cli-flow-screenshot-log.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@wdio/browserstack-service": patch ---- - -- Fixed screenshots taken during a Mocha test never appearing in Test Reporting's consolidated logs. Mocha runs through the BrowserStack binary, and on that path the WebDriver result event the screenshot log is built from was never subscribed to, so the screenshot was captured by the browser but never reported. A screenshot denial from the server is now honoured too, where previously it was read as approval. From 93cde24c28b7cd592b610587239cea3eeff6fdb6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 15:55:31 +0000 Subject: [PATCH 3/5] chore(changeset): auto-generate from PR template (patch) --- .changeset/pr-190.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/pr-190.md diff --git a/.changeset/pr-190.md b/.changeset/pr-190.md new file mode 100644 index 0000000..49c33f7 --- /dev/null +++ b/.changeset/pr-190.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": patch +--- + +- Fixed screenshots taken during a Mocha test not appearing in Test Reporting's consolidated logs. From c8e28357136cca67a99144626fe836d5936c0e5f Mon Sep 17 00:00:00 2001 From: Anish Sinha Date: Wed, 9 Sep 2026 21:55:30 +0530 Subject: [PATCH 4/5] fix(observability): carry screenshot logs over gRPC on the binary flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the previous commit, which subscribed the result event on the binary path but stopped one step short — verified end to end and the screenshot still never reached Observability. Two further links were broken: 1. `browserCommand` uploaded via `listener.onScreenshot`, whose endpoint answers 401 to the binary's JWT (`[screenshot_upload] Failed ... status: 401`). On this path the binary owns reporting, so the entry now rides the same LOG rail `appendTestItemLog` uses: the CLI stamps the test uuid and forwards it over gRPC. 2. `loadLogEntries` hardcoded `logRecord.kind = KIND_LOG`, destructuring only `{level, message, timestamp}` and discarding the producer's kind — so the screenshot arrived labelled as a console log and no screenshots manifest was built. It now keeps an explicit kind and falls back to KIND_LOG, which is what the previously-unreferenced KIND_SCREENSHOT constant was for. Console logs are unchanged: `StdLog.kind` is already 'TEST_LOG'. Verified on a real build: two `"kind":"TEST_SCREENSHOT"` entries on the gRPC wire, `retries[].logs == ['TEST_LOG']` on both leaves, and BStackAutomation's `validate_o11y_screenshot` returning True for both test ids (0/2 failures). Co-Authored-By: Claude Opus 5 (1M context) --- .../cli/frameworks/wdioMochaTestFramework.ts | 6 +- .../src/insights-handler.ts | 24 +++++-- .../wdioMochaTestFramework.logKind.test.ts | 69 +++++++++++++++++++ 3 files changed, 91 insertions(+), 8 deletions(-) create mode 100644 packages/browserstack-service/tests/cli/wdioMochaTestFramework.logKind.test.ts diff --git a/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts b/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts index 9065858..3c7df71 100644 --- a/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts +++ b/packages/browserstack-service/src/cli/frameworks/wdioMochaTestFramework.ts @@ -266,12 +266,14 @@ export default class WdioMochaTestFramework extends TestFramework { */ loadLogEntries(instance: TestFrameworkInstance, testFrameworkState: State, hookState: State, logEntry: Record) { const logRecord: Record = {} - const { level, message, timestamp } = logEntry + const { level, message, timestamp, kind } = logEntry if (CLIUtils.matchHookRegex(instance.getCurrentTestState().toString().split('.')[1])) { logRecord[TestFrameworkConstants.KEY_HOOK_ID] = TestFramework.getState(instance, TestFrameworkConstants.KEY_HOOK_ID) } - logRecord.kind = TestFrameworkConstants.KIND_LOG + // Console logs carry no kind and stay KIND_LOG; a producer that sets one (a screenshot, + // say) keeps it, or the entry would reach Observability labelled as a console log. + logRecord.kind = kind ?? TestFrameworkConstants.KIND_LOG logRecord.message = Buffer.from(message as string) logRecord.level = level logRecord.timestamp = timestamp diff --git a/packages/browserstack-service/src/insights-handler.ts b/packages/browserstack-service/src/insights-handler.ts index 6be5d74..e0dc750 100644 --- a/packages/browserstack-service/src/insights-handler.ts +++ b/packages/browserstack-service/src/insights-handler.ts @@ -46,6 +46,7 @@ import { TESTOPS_SCREENSHOT_ENV } from './constants.js' import { BrowserstackCLI } from './cli/index.js' import { TestFrameworkState } from './cli/states/testFrameworkState.js' import { HookState } from './cli/states/hookState.js' +import { TestFrameworkConstants } from './cli/frameworks/constants/testFrameworkConstants.js' import PerformanceTester from './instrumentation/performance/performance-tester.js' import * as PERFORMANCE_SDK_EVENTS from './instrumentation/performance/constants.js' import CustomTagsHandler from './custom-tags-handler.js' @@ -795,12 +796,23 @@ class _InsightsHandler { // string 'false' — which Boolean() reads as permission granted. Honour it explicitly. const allowScreenshots = process.env[TESTOPS_SCREENSHOT_ENV] if (Boolean(allowScreenshots) && !isFalse(allowScreenshots) && isScreenshotCommand(args) && result?.value) { - await this.listener.onScreenshot([{ - test_run_uuid: testMeta.uuid, - timestamp: new Date().toISOString(), - message: result.value, - kind: 'TEST_SCREENSHOT' - }]) + // On the binary path the direct screenshot endpoint answers 401 to the binary's JWT, + // so ride the same LOG rail appendTestItemLog uses: the CLI stamps the test uuid and + // forwards the entry over gRPC, where the binary owns reporting. + await (BrowserstackCLI.getInstance().isRunning() + ? BrowserstackCLI.getInstance().getTestFramework()!.trackEvent(TestFrameworkState.LOG, HookState.POST, { + logEntry: { + kind: TestFrameworkConstants.KIND_SCREENSHOT, + message: result.value, + timestamp: new Date().toISOString() + } + }) + : this.listener.onScreenshot([{ + test_run_uuid: testMeta.uuid, + timestamp: new Date().toISOString(), + message: result.value, + kind: 'TEST_SCREENSHOT' + }])) } const requestData = this._commands[dataKey] diff --git a/packages/browserstack-service/tests/cli/wdioMochaTestFramework.logKind.test.ts b/packages/browserstack-service/tests/cli/wdioMochaTestFramework.logKind.test.ts new file mode 100644 index 0000000..3053e89 --- /dev/null +++ b/packages/browserstack-service/tests/cli/wdioMochaTestFramework.logKind.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' +import * as bstackLogger from '../../src/bstackLogger.js' + +import WdioMochaTestFramework from '../../src/cli/frameworks/wdioMochaTestFramework.js' +import TestFramework from '../../src/cli/frameworks/testFramework.js' +import { TestFrameworkConstants } from '../../src/cli/frameworks/constants/testFrameworkConstants.js' +import { TestFrameworkState } from '../../src/cli/states/testFrameworkState.js' +import { HookState } from '../../src/cli/states/hookState.js' + +vi.spyOn(bstackLogger.BStackLogger, 'logToFile').mockImplementation(() => {}) + +describe('SDK-4177 — loadLogEntries must not relabel a log that carries its own kind', () => { + let entries: unknown[] + let instance: any + + beforeEach(() => { + entries = [] + instance = { + getCurrentTestState: () => TestFrameworkState.TEST, + updateMultipleEntries: vi.fn(), + } + vi.spyOn(TestFramework, 'getState').mockReturnValue(entries) + vi.spyOn(WdioMochaTestFramework, 'lastActiveHook').mockReturnValue(null) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + const load = (logEntry: Record) => { + WdioMochaTestFramework.prototype.loadLogEntries.call( + WdioMochaTestFramework.prototype, + instance, + TestFrameworkState.LOG, + HookState.POST, + logEntry + ) + return entries[0] as Record + } + + it('keeps TEST_SCREENSHOT, which the kind was previously hardcoded over', () => { + // Hardcoding KIND_LOG here meant a screenshot reached Observability labelled as a + // console log, so no screenshots manifest was ever built for the test run. + const record = load({ + kind: TestFrameworkConstants.KIND_SCREENSHOT, + message: 'aBase64Screenshot', + timestamp: '2020-01-01T00:00:00.000Z', + }) + + expect(record.kind).toBe('TEST_SCREENSHOT') + expect(record.message).toEqual(Buffer.from('aBase64Screenshot')) + }) + + it('still labels a console log TEST_LOG', () => { + const record = load({ + kind: TestFrameworkConstants.KIND_LOG, + message: 'hello', + level: 'info', + timestamp: '2020-01-01T00:00:00.000Z', + }) + + expect(record.kind).toBe('TEST_LOG') + expect(record.level).toBe('info') + }) + + it('falls back to TEST_LOG for an entry with no kind', () => { + expect(load({ message: 'hello', timestamp: '2020-01-01T00:00:00.000Z' }).kind).toBe('TEST_LOG') + }) +}) From 0f825d04c8c613dfeb530ec8a2b75757be03f7d8 Mon Sep 17 00:00:00 2001 From: Anish Sinha Date: Thu, 10 Sep 2026 17:44:23 +0530 Subject: [PATCH 5/5] fix(observability): resolve the CLI test framework instead of asserting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding 1. `getTestFramework()` can be undefined while `isRunning()` is true — the dev-env short-circuit returns true before `setupTestFramework()` has run, and that only assigns for webdriverio-mocha — so the non-null assertion could throw. `o11yClassErrorHandler` wraps every InsightsHandler method and catches async rejections, so it could not break the customer's test, but the screenshot was thrown away silently. Resolve the framework and fall back to the direct upload when it is absent, so an untracked framework still gets its one chance at reporting the screenshot. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/insights-handler.ts | 13 ++++++++++--- .../tests/insights-handler.test.ts | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/browserstack-service/src/insights-handler.ts b/packages/browserstack-service/src/insights-handler.ts index e0dc750..a38363e 100644 --- a/packages/browserstack-service/src/insights-handler.ts +++ b/packages/browserstack-service/src/insights-handler.ts @@ -798,9 +798,16 @@ class _InsightsHandler { if (Boolean(allowScreenshots) && !isFalse(allowScreenshots) && isScreenshotCommand(args) && result?.value) { // On the binary path the direct screenshot endpoint answers 401 to the binary's JWT, // so ride the same LOG rail appendTestItemLog uses: the CLI stamps the test uuid and - // forwards the entry over gRPC, where the binary owns reporting. - await (BrowserstackCLI.getInstance().isRunning() - ? BrowserstackCLI.getInstance().getTestFramework()!.trackEvent(TestFrameworkState.LOG, HookState.POST, { + // forwards the entry over gRPC, where the binary owns reporting. The framework can be + // unset while isRunning() is true — the dev-env short-circuit returns true before + // setupTestFramework() has run, and it only assigns for webdriverio-mocha — so resolve + // it rather than assert, and let an untracked framework fall through to the direct + // upload instead of throwing the screenshot away. + const cliTestFramework = BrowserstackCLI.getInstance().isRunning() + ? BrowserstackCLI.getInstance().getTestFramework() + : undefined + await (cliTestFramework + ? cliTestFramework.trackEvent(TestFrameworkState.LOG, HookState.POST, { logEntry: { kind: TestFrameworkConstants.KIND_SCREENSHOT, message: result.value, diff --git a/packages/browserstack-service/tests/insights-handler.test.ts b/packages/browserstack-service/tests/insights-handler.test.ts index 4314e46..7034c97 100644 --- a/packages/browserstack-service/tests/insights-handler.test.ts +++ b/packages/browserstack-service/tests/insights-handler.test.ts @@ -9,6 +9,7 @@ import InsightsHandler from '../src/insights-handler.js' import * as utils from '../src/util.js' import * as bstackLogger from '../src/bstackLogger.js' import { TESTOPS_SCREENSHOT_ENV } from '../src/constants.js' +import { BrowserstackCLI } from '../src/cli/index.js' const log = logger('test') let insightsHandler: InsightsHandler @@ -697,6 +698,24 @@ describe('browserCommand', () => { delete process.env[TESTOPS_SCREENSHOT_ENV] }) + it('client:afterCommand - falls back to the direct upload when the CLI has no test framework', async () => { + // isRunning() can be true before setupTestFramework() has assigned one (dev-env + // short-circuit, or a framework the CLI doesn't track). Asserting non-null there threw the + // screenshot away; resolving it keeps the direct upload as the fallback. + process.env[TESTOPS_SCREENSHOT_ENV] = 'true' + commandSpy.mockImplementation(() => { return true }) + const getInstanceSpy = vi.spyOn(BrowserstackCLI, 'getInstance').mockReturnValue({ + isRunning: () => true, + getTestFramework: () => undefined + } as any) + + await insightsHandler.browserCommand('client:afterCommand', { sessionId: 's', method: 'm', endpoint: 'e', result: { value: 'random' } } as any, {} as any) + + expect(uploadEventDataSpy).toBeCalled() + getInstanceSpy.mockRestore() + delete process.env[TESTOPS_SCREENSHOT_ENV] + }) + it('client:afterCommand - screenshot not uploaded when screenshots are denied', () => { // arrives as the string 'false' off the wire, which Boolean() would read as granted process.env[TESTOPS_SCREENSHOT_ENV] = 'false'