diff --git a/.changeset/pr-200.md b/.changeset/pr-200.md new file mode 100644 index 00000000..43cd3f4e --- /dev/null +++ b/.changeset/pr-200.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": minor +--- + +- End-of-build messages from BrowserStack — such as a notice that your SDK version is outdated or has a known issue — are now shown at the end of your test run, highlighted in yellow for a warning and red for an error, and written to the SDK log without colour so they stay searchable. diff --git a/packages/browserstack-service/src/cli/grpcClient.ts b/packages/browserstack-service/src/cli/grpcClient.ts index 51243188..2aabc1cd 100644 --- a/packages/browserstack-service/src/cli/grpcClient.ts +++ b/packages/browserstack-service/src/cli/grpcClient.ts @@ -47,6 +47,14 @@ import { BStackLogger } from './cliLogger.js' const GRPC_MESSAGE_LIMIT = 20 * 1024 * 1024 // 20 MB in bytes +// Explicit \x1b escapes so the ESC byte stays visible in source. Applied to the +// terminal copy of a summary entry only; the archived copy stays plain. +const SUMMARY_ANSI = { + reset: '\x1b[0m', + error: { base: '\x1b[31m', emphasis: '\x1b[1;31m' }, + warn: { base: '\x1b[33m', emphasis: '\x1b[1;33m' } +} + /** * GrpcClient - Singleton class for managing gRPC client connections * @@ -277,6 +285,7 @@ export class GrpcClient { try { const response = await stopBinSessionPromise(request) this.logger.info('StopBinSession successful') + this.renderCustomerVisibleSummary(response) PerformanceTester.end(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_CLI_ON_STOP) return response } catch (error: unknown) { @@ -291,6 +300,101 @@ export class GrpcClient { } } + /** + * Render end-of-build customer-visible summary entries. + * + * Per the binary proto contract (CustomerVisibleSummaryEntry in + * sdk-messages.proto): iterate by `severity` + `body`, write `body` verbatim, + * and pick the stream from `severity`. Never branches on `entryType`, so new + * entry types need no SDK change. + * @private + */ + private renderCustomerVisibleSummary(response: unknown) { + try { + const entries = (response as { entries?: Array<{ severity?: string, body?: string }> })?.entries + if (!entries?.length) { + return + } + + for (const entry of entries) { + // Scoped per entry, not around the loop: a stream that rejects one + // entry must not drop the entries after it. The proto allows many + // entry types, so this widens as more are added. + try { + const body = entry?.body || '' + if (!body) { + continue + } + + const severity = (entry?.severity || 'info').toLowerCase() + // warn/warning/error -> stderr, everything else (info AND unknown) -> + // stdout, so a malformed severity cannot false-alarm CI tooling + // watching stderr. + const isErrorStream = severity === 'warn' || severity === 'warning' || severity === 'error' + + // Archived copy is written FIRST — terminal scrollback is lost on + // CI runners that keep only the log directory, so the durable copy + // must not depend on the stream write succeeding. + // + // logToFile, NOT the info/warn/error helpers: those also call + // @wdio/logger, which writes to the console, so the customer + // would see the block twice (once raw below, once prefixed). + this.logger.logToFile(body, severity === 'error' ? 'error' : (isErrorStream ? 'warn' : 'info')) + + // Written directly rather than through the logger, whose per-line + // prefix would break the binary's box-border alignment. Colour is + // applied here only — the archived copy above stays plain. + ;(isErrorStream ? process.stderr : process.stdout) + .write(`${this.colouriseSummaryBody(body, severity)}\n`) + } catch (error: unknown) { + this.logger.debug(`StopBinSession entry forwarding failed: ${util.format(error)}`) + } + } + } catch (error: unknown) { + this.logger.debug(`StopBinSession entries forwarding failed: ${util.format(error)}`) + } + } + + /** + * Tint a summary block by severity — yellow for warn (an outdated SDK), red for + * error (a deprecated one), untouched otherwise. + * + * Each line is wrapped and reset on its own rather than the block as a whole, so + * a truncated or interleaved write cannot leave the customer's terminal stuck in + * colour. The first non-blank, non-border line is emphasised. + * @private + */ + private colouriseSummaryBody(body: string, severity: string): string { + try { + const palette = severity === 'error' + ? SUMMARY_ANSI.error + : ((severity === 'warn' || severity === 'warning') ? SUMMARY_ANSI.warn : null) + if (!palette) { + return body + } + + let emphasised = false + return body.split('\n').map((line) => { + const trimmed = line.trim() + if (!trimmed) { + return line + } + // A border is any line carrying no letters or digits, rather than a + // check for the binary's current U+2500 divider — so a change to the + // glyph cannot silently start emphasising the wrong line. + const isBorder = !/[A-Za-z0-9]/.test(trimmed) + if (!isBorder && !emphasised) { + emphasised = true + return `${palette.emphasis}${line}${SUMMARY_ANSI.reset}` + } + return `${palette.base}${line}${SUMMARY_ANSI.reset}` + }).join('\n') + } catch { + // Colour is cosmetic — never let it cost the customer the message. + return body + } + } + async testSessionEvent(data: Omit) { PerformanceTester.start(PERFORMANCE_SDK_EVENTS.DISPATCHER_EVENTS.TEST_SESSION) const workerId = this.getClientWorkerIdFromContext(data.executionContext) @@ -496,6 +600,11 @@ export class GrpcClient { message: log.message, timestamp: log.timestamp, level: log.level, + // Attachment entries carry no message — the binary streams the file + // from filePath when it drains its upload queue. + fileName: log.fileName, + fileSize: log.fileSize, + filePath: log.filePath, }) logEntries.push(logEntry) } diff --git a/packages/browserstack-service/src/proto/browserstack/sdk/v1/sdk-messages.proto b/packages/browserstack-service/src/proto/browserstack/sdk/v1/sdk-messages.proto index f6d840e5..b9dc77bd 100644 --- a/packages/browserstack-service/src/proto/browserstack/sdk/v1/sdk-messages.proto +++ b/packages/browserstack-service/src/proto/browserstack/sdk/v1/sdk-messages.proto @@ -157,6 +157,24 @@ message StopBinSessionResponse { optional string error = 2; optional string automate_buildlink = 3; optional string hashed_id = 4; + // End-of-build customer-visible summary entries. Populated on EVERY response + // shape — success, error, and clean-build alike (empty when nothing to + // surface). Iterate by `severity` + `body`; do not branch on `entry_type`, + // so new entry types need no SDK change. + repeated CustomerVisibleSummaryEntry entries = 5; +} + +// A single customer-visible summary entry surfaced at end of build via +// StopBinSessionResponse.entries. The binary owns the prose so all SDKs render +// consistent text; SDKs choose the output stream from `severity`. +message CustomerVisibleSummaryEntry { + // Stable machine-readable identifier (e.g. "network_restrictions"). + string entry_type = 1; + // One of: "info" | "warn" | "error". + string severity = 2; + // Pre-formatted block to display verbatim. May contain embedded newlines. + string body = 3; + optional string doc_link = 4; } message ConnectBinSessionRequest { diff --git a/packages/browserstack-service/tests/cli/grpcClient.test.ts b/packages/browserstack-service/tests/cli/grpcClient.test.ts index e63ab79f..94312c1c 100644 --- a/packages/browserstack-service/tests/cli/grpcClient.test.ts +++ b/packages/browserstack-service/tests/cli/grpcClient.test.ts @@ -1,9 +1,14 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' import { GrpcClient } from '../../src/cli/grpcClient.js' +import { BStackLogger } from '../../src/cli/cliLogger.js' vi.mock('../../src/grpc/index.js', () => ({ - StopBinSessionRequestConstructor: { create: (fields: Record) => ({ ...fields }) } + StopBinSessionRequestConstructor: { create: (fields: Record) => ({ ...fields }) }, + ExecutionContextConstructor: { create: (fields: Record) => ({ ...fields }) }, + LogCreatedEventRequestConstructor: { create: (fields: Record) => ({ ...fields }) }, + // eslint-disable-next-line camelcase + LogCreatedEventRequest_LogEntryConstructor: { create: (fields: Record) => ({ ...fields }) } })) vi.mock('../../src/cli/cliUtils.js', () => ({ @@ -11,7 +16,7 @@ vi.mock('../../src/cli/cliUtils.js', () => ({ })) vi.mock('../../src/cli/cliLogger.js', () => ({ - BStackLogger: { debug: vi.fn(), info: vi.fn(), error: vi.fn(), warn: vi.fn() } + BStackLogger: { debug: vi.fn(), info: vi.fn(), error: vi.fn(), warn: vi.fn(), logToFile: vi.fn() } })) vi.mock('../../src/instrumentation/performance/performance-tester.js', () => ({ @@ -51,3 +56,188 @@ describe('GrpcClient.stopBinSession', () => { expect(request.exitReason).toBeUndefined() }) }) + +describe('GrpcClient.stopBinSession customer-visible summary entries', () => { + let client: GrpcClient + let stdoutSpy: ReturnType + let stderrSpy: ReturnType + + const respondWith = (response: unknown) => { + client.client = { + stopBinSession: vi.fn((_req: unknown, cb: (err: unknown, res: unknown) => void) => cb(null, response)) + } as any + } + + beforeEach(() => { + client = new GrpcClient() + client.binSessionId = 'bin-1' + stdoutSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true) + stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) + }) + + afterEach(() => { + stdoutSpy.mockRestore() + stderrSpy.mockRestore() + vi.clearAllMocks() + }) + + it('writes the body verbatim to stdout for an info entry', async () => { + respondWith({ entries: [{ entryType: 'version_nudge', severity: 'info', body: 'line one\nline two' }] }) + await client.stopBinSession() + expect(stdoutSpy).toHaveBeenCalledWith('line one\nline two\n') + expect(stderrSpy).not.toHaveBeenCalled() + }) + + it('routes warn and error entries to stderr', async () => { + respondWith({ entries: [ + { entryType: 'version_nudge', severity: 'warn', body: 'outdated' }, + { entryType: 'version_nudge', severity: 'error', body: 'deprecated' } + ] }) + await client.stopBinSession() + expect(stderrSpy).toHaveBeenCalledWith('\x1b[1;33moutdated\x1b[0m\n') + expect(stderrSpy).toHaveBeenCalledWith('\x1b[1;31mdeprecated\x1b[0m\n') + expect(stdoutSpy).not.toHaveBeenCalled() + }) + + it('treats the server\'s "warning" spelling as an error stream', async () => { + respondWith({ entries: [{ entryType: 'version_nudge', severity: 'warning', body: 'outdated' }] }) + await client.stopBinSession() + expect(stderrSpy).toHaveBeenCalledWith('\x1b[1;33moutdated\x1b[0m\n') + }) + + it('tints a warn block yellow and an error block red, line by line', async () => { + const body = '────\n Title\n\n Detail\n────' + respondWith({ entries: [ + { entryType: 'version_nudge', severity: 'warn', body }, + { entryType: 'version_nudge', severity: 'error', body } + ] }) + await client.stopBinSession() + + // Borders take the base tint and the first line carrying text is emphasised. + // Blank lines are left alone, and every tinted line closes its own reset, so a + // truncated write cannot leave the terminal stuck in colour. + expect(stderrSpy).toHaveBeenCalledWith( + '\x1b[33m────\x1b[0m\n\x1b[1;33m Title\x1b[0m\n\n' + + '\x1b[33m Detail\x1b[0m\n\x1b[33m────\x1b[0m\n' + ) + expect(stderrSpy).toHaveBeenCalledWith( + '\x1b[31m────\x1b[0m\n\x1b[1;31m Title\x1b[0m\n\n' + + '\x1b[31m Detail\x1b[0m\n\x1b[31m────\x1b[0m\n' + ) + }) + + it('sends an unknown severity to stdout so CI stderr watchers are not tripped', async () => { + respondWith({ entries: [{ entryType: 'version_nudge', severity: 'bogus', body: 'body' }] }) + await client.stopBinSession() + expect(stdoutSpy).toHaveBeenCalledWith('body\n') + expect(stderrSpy).not.toHaveBeenCalled() + }) + + it('writes nothing when entries are absent, empty, or bodiless', async () => { + for (const response of [{ done: true }, { entries: [] }, { entries: [{ severity: 'warn', body: '' }] }]) { + respondWith(response) + await client.stopBinSession() + } + expect(stdoutSpy).not.toHaveBeenCalled() + expect(stderrSpy).not.toHaveBeenCalled() + }) + + it('archives via logToFile only, so the block is never printed twice', async () => { + respondWith({ entries: [ + { entryType: 'version_nudge', severity: 'warning', body: 'outdated' }, + { entryType: 'version_nudge', severity: 'error', body: 'deprecated' }, + { entryType: 'version_nudge', severity: 'info', body: 'notice' } + ] }) + await client.stopBinSession() + + // logToFile writes to the log file only. info/warn/error additionally + // call @wdio/logger, which writes to the console — using them here + // would duplicate the block the stream writes above already emitted. + expect(BStackLogger.logToFile).toHaveBeenCalledWith('outdated', 'warn') + expect(BStackLogger.logToFile).toHaveBeenCalledWith('deprecated', 'error') + expect(BStackLogger.logToFile).toHaveBeenCalledWith('notice', 'info') + // The console-writing helpers must never receive a body. (They are still + // used for unrelated lines such as "StopBinSession successful".) + for (const body of ['outdated', 'deprecated', 'notice']) { + expect(BStackLogger.warn).not.toHaveBeenCalledWith(body) + expect(BStackLogger.error).not.toHaveBeenCalledWith(body) + expect(BStackLogger.info).not.toHaveBeenCalledWith(body) + } + }) + + it('keeps rendering and archiving the entries after one whose write throws', async () => { + // The catch is scoped per entry, not around the loop: a stream that + // rejects entry one must not silently drop entries two and three. + stderrSpy.mockImplementation((chunk: any) => { + if (String(chunk).includes('first')) { + throw new Error('stream closed') + } + return true + }) + + respondWith({ entries: [ + { entryType: 'version_nudge', severity: 'warn', body: 'first' }, + { entryType: 'version_nudge', severity: 'warn', body: 'second' }, + { entryType: 'version_nudge', severity: 'info', body: 'third' } + ] }) + await client.stopBinSession() + + expect(stderrSpy).toHaveBeenCalledWith('\x1b[1;33msecond\x1b[0m\n') + expect(stdoutSpy).toHaveBeenCalledWith('third\n') + // Archival runs before the stream write, so even the entry whose write + // threw is still kept in the log directory. + expect(BStackLogger.logToFile).toHaveBeenCalledWith('first', 'warn') + expect(BStackLogger.logToFile).toHaveBeenCalledWith('second', 'warn') + expect(BStackLogger.logToFile).toHaveBeenCalledWith('third', 'info') + }) + + it('still returns the response when rendering throws', async () => { + stdoutSpy.mockImplementation(() => { + throw new Error('stream closed') + }) + respondWith({ entries: [{ severity: 'info', body: 'body' }], done: true }) + await expect(client.stopBinSession()).resolves.toMatchObject({ done: true }) + }) +}) + +describe('GrpcClient.logCreatedEvent', () => { + let client: GrpcClient + let logCreatedEvent: ReturnType + + beforeEach(() => { + logCreatedEvent = vi.fn((_req: unknown, cb: (err: unknown, res: unknown) => void) => cb(null, {})) + client = new GrpcClient() + client.binSessionId = 'bin-1' + client.client = { logCreatedEvent } as any + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + it('forwards the attachment fields on a log entry to the binary', async () => { + // Attachment entries carry no message — the binary streams the file from + // filePath, so dropping these three silently breaks attachment upload. + await client.logCreatedEvent({ + platformIndex: 0, + logs: [{ + uuid: 'log-1', + kind: 'TEST_ATTACHMENT', + timestamp: '2026-01-01T00:00:00Z', + level: 'info', + fileName: 'screenshot.png', + fileSize: 2048, + filePath: '/tmp/screenshot.png' + }], + executionContext: { processId: 1, threadId: 2, hash: 'h' } + } as any) + + expect(logCreatedEvent).toHaveBeenCalledTimes(1) + const sent = logCreatedEvent.mock.calls[0][0] as any + expect(sent.logs[0]).toMatchObject({ + fileName: 'screenshot.png', + fileSize: 2048, + filePath: '/tmp/screenshot.png' + }) + }) +})