feat(browserstack-service): render end-of-build summary entries (SDK-7358) - #200
Conversation
…7358) The binary returns CustomerVisibleSummaryEntry items on StopBinSessionResponse for end-of-build messages such as the SDK version nudge. The service had neither the proto field nor a renderer, so those messages were dropped for every wdio customer running through the CLI path. Adds the proto field and a renderer that writes `body` verbatim, picks the stream from `severity` (warn/warning/error -> stderr, everything else including unknown -> stdout so a malformed severity cannot trip CI stderr watchers), and archives a copy to the log file for runners that keep only the log directory. Deliberately does not branch on `entry_type`, so future entry types need no further service change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited), Workspace UI (inherited) Review profile: ASSERTIVE Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
…e (SDK-7358) Two changes to the end-of-build summary rendering added in this PR. Colour: a warn entry (an outdated SDK) is tinted yellow and an error entry (a deprecated one) red, with the block's first line of text emphasised. Lines are wrapped and reset individually rather than the block as a whole, so a truncated or interleaved write cannot leave the customer's terminal stuck in colour. Applied to the stream copy only — the archived copy stays plain, because escape codes reach a log file as literal bytes and break anchored searches over it. Archiving: switch from the info/warn/error helpers to logToFile. Those helpers also call @wdio/logger, which writes to the console, so the customer saw the block twice — once raw from the stream write, once prefixed by the logger. logToFile writes to the log file only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🔴 Blocking findings — fix required See the SDK PR Review Agent's report from your local run. Change map (generated deterministically from the diff)graph LR
subgraph nwdio_service["wdio-service"]
npackages_browserstack_service_src_cli_grpcClient_ts["⚠ grpcClient.ts<br/>~151 lines"]
npackages_browserstack_service_tests_cli_grpcClient_test_ts["⚠ grpcClient.test.ts<br/>~151 lines"]
npackages_browserstack_service_src_proto_browserstack_sdk_v1_sdk_messages_proto["⚠ sdk-messages.proto<br/>~18 lines"]
n_changeset_pr_200_md["pr-200.md<br/>~5 lines"]
end
nsvc_every_SDK___binary(["every SDK + binary"])
npackages_browserstack_service_src_proto_browserstack_sdk_v1_sdk_messages_proto --> nsvc_every_SDK___binary
↻ This verdict comment is the review anchor — it's updated in place on each run (the gate posts its status separately). — SDK PR Review Agent |
…over attachment fields Review follow-up on SDK-7358. 1. renderCustomerVisibleSummary wrapped the whole `for (const entry of entries)` loop in one try/catch, so a stream that rejected entry N aborted the loop: entries N+1.. were neither written nor archived. Only one entry exists today (the version nudge), but the proto is explicitly built for more entry types, so the gap widens as they are added. The catch now sits inside the loop body, and the archived copy is written before the stream write so archival no longer depends on the write succeeding. The outer catch stays — stopBinSession rethrows, so a throw escaping this method would cost the caller its response. This is the same defect and fix as the v8 port (#201). 2. Adds the missing test for the fileName/fileSize/filePath passthrough in logCreatedEvent. Those fields are unrelated to the summary-entry feature and arrived in this PR undocumented and uncovered; this locks their behaviour rather than leaving an untested drive-by. Whether the change belongs in its own PR is still worth a call. Both new tests fail without their respective fix. Package suite on Node 25: 56 files, 1259 tests, 0 failures. tsc --noEmit exit 0, eslint exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge. |
What is this about?
Renders end-of-build customer-visible summary entries (SDK-7358).
The binary returns
CustomerVisibleSummaryEntryitems onStopBinSessionResponseforend-of-build messages such as the SDK version nudge. The service had neither the proto
field nor a renderer, so those messages arrived and were silently dropped for every wdio
customer running through the CLI path.
Adds the proto field and a renderer that writes
body, picks the stream fromseverity,and archives a copy to the log file. Deliberately does not branch on
entryType, sofuture entry types need no further service change — matching node, java, python and ruby.
Stream choice:
warn/warning/error-> stderr, everything else including unknown ->stdout, so a malformed severity cannot false-alarm CI tooling watching stderr.
Colour. A
warnentry (an outdated SDK) is tinted yellow and anerrorentry (adeprecated one) red, with the block's first line of text emphasised. Lines are wrapped and
reset individually rather than the block as a whole, so a truncated or interleaved write
cannot leave the customer's terminal stuck in colour. Applied to the stream copy only — the
archived copy stays plain, because escape codes reach a log file as literal bytes and break
anchored searches over it. Border lines are detected as "carries no letters or digits"
rather than by matching U+2500, so a change to the binary's divider glyph cannot silently
start emphasising the wrong line.
Archive once. The archived copy now goes through
BStackLogger.logToFilerather thanthe
info/warn/errorhelpers. Those helpers also call@wdio/logger, which writes tothe console, so the customer saw the block twice — once raw from the stream write, once
prefixed by the logger.
logToFilewrites to the log file only. (The v8 branch alreadydid this; v9 did not.)
Worth noting for reviewers: only wdio-mocha (non-multiremote) reaches this path today, since
CLISupportedFrameworksis['mocha']and the classic path bypasses the binary entirely.Related Jira task/s
Release (mandatory for every PR — required for the
ready-for-reviewlabel)Version bump: (required — tick exactly one)
Release notes type: (optional)
Release notes (customer-facing): (optional but encouraged)
Release notes (internal): (required — engineer-facing; what actually changed / why)
entries+CustomerVisibleSummaryEntrytosdk-messages.proto, matching the binary's canonical definition (field 5).GrpcClient.renderCustomerVisibleSummarywritesbodyto the stream chosen byseverity, not through the logger, whose per-line prefix would break the binary's box-border alignment.GrpcClient.colouriseSummaryBodytints the stream copy by severity (yellow/warn, red/error, untouched otherwise), line by line with an explicit reset per line. Escapes are written as\x1bso the ESC byte stays visible in source.info/warn/errorhelpers tologToFile. The helpers also call@wdio/logger, which writes to the console — the block was being printed twice.stopBinSessionstill returns its response and the customer still gets the plain message.Checklist
PR Validations
Run Tests: Comment RUN_TESTS to trigger sanity tests.
Testing
stopBinSessionpath; file passes 10/10. Covers exact escape sequences per line for both tiers, plain archiving, and no-colour for info and unknown severities.tsc -p tsconfig.prod.json --noEmitandeslintboth exit 0.v8branch (feat(browserstack-service): render end-of-build summary entries (SDK-7358) [v8] #201).🤖 Generated with Claude Code