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. 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 e64a312..a38363e 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, @@ -45,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' @@ -790,13 +792,34 @@ 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) { - await this.listener.onScreenshot([{ - test_run_uuid: testMeta.uuid, - timestamp: new Date().toISOString(), - message: result.value, - kind: 'TEST_SCREENSHOT' - }]) + // `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) { + // 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. 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, + 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/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/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') + }) +}) diff --git a/packages/browserstack-service/tests/insights-handler.test.ts b/packages/browserstack-service/tests/insights-handler.test.ts index a56b869..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,33 @@ 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' + 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,