fix(SDK-4165): keep buildIdentifier when BROWSERSTACK_BUILD_NAME is set - #192
fix(SDK-4165): keep buildIdentifier when BROWSERSTACK_BUILD_NAME is set#192harshit-browserstack wants to merge 2 commits into
Conversation
_handleBuildIdentifier skipped buildIdentifier resolution whenever
BROWSERSTACK_BUILD_NAME was set, even though a buildName was available.
The skip also deleted buildIdentifier from the capabilities only — this
._buildIdentifier kept the unresolved '#${BUILD_NUMBER}' template, which
onPrepare forwards to TestHub as build_identifier.
Consequence: successive runs sharing a build name were never
disambiguated, so their sessions collapsed into a single build.
The guard now keys solely on the absence of a buildName, which is what
its own warning always claimed and what the binary's handleBuildIdentifier
already does (its BROWSERSTACK_BUILD_NAME skip is commented out). The
skip path additionally clears _buildIdentifier so the raw template is
never reported as a value.
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 |
|
Additional regression check — the binary path (wdio + mocha, where This is the path the BStackAutomation wdio_mocha wrappers use, and it is the one behaviourally changed by removing the env disjunct: Post-fix, Two distinct builds, one identifier each — no double application, and the spurious |
1 participant
What is this about?
Launcher._handleBuildIdentifierskipped buildIdentifier resolution wheneverBROWSERSTACK_BUILD_NAMEwas set — even though abuildNamewas available in that case. Its own warning (Skipping buildIdentifier as buildName is not passed.) contradicted the condition it fired under.The skip was also partial. It deleted
buildIdentifierfrom the capabilities but leftthis._buildIdentifierholding the unresolved#${BUILD_NUMBER}template, andonPrepareforwards that field to TestHub asbuild_identifier(src/launcher.ts:445).Net effect: successive runs sharing a build name were never disambiguated, so their sessions collapsed into a single build instead of
name #1,name #2.The guard now keys solely on the absence of a
buildName— which is exactly what its warning always claimed, and what the binary'shandleBuildIdentifieralready does (the equivalentBROWSERSTACK_BUILD_NAMEskip is commented out there,core/config/index.js:1203-1211). The skip path additionally clears_buildIdentifierso a raw template is never reported as a value.Scope of impact.
CLIUtils.CLISupportedFrameworks = ['mocha'], and the CLI is also disabled for multiremote. For every other configuration — WDIO + jasmine, WDIO + cucumber, and any multiremote run — the classiclaunchTestSessionpath is live and this defect applies.Related Jira task/s
SDK-4165 —
[Framework- WDIO v8 v9] Builds are getting merged for GRR user for AA session in o11y dashboardRelease (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)
buildIdentifierbeing ignored when theBROWSERSTACK_BUILD_NAMEenvironment variable is set. Successive runs that share a build name are now reported as separate builds (my-build #1,my-build #2) instead of merging into one.Release notes (internal): (required — engineer-facing; what actually changed / why)
_handleBuildIdentifierno longer treats a setBROWSERSTACK_BUILD_NAMEas a reason to skip resolution; the guard keys only on a missingbuildName, matching the binary'shandleBuildIdentifierand the warning's own wording.this._buildIdentifier, so the unresolved#${BUILD_NUMBER}/${DATE_TIME}template is no longer sent to TestHub asbuild_identifier.Checklist
How this was tested
App Automate (Android, Samsung Galaxy S22 Ultra),
buildIdentifier: '#${BUILD_NUMBER}',testObservability: true,BROWSERSTACK_BUILD_NAMEexported, same build name across each pair. Frameworkjasmineso the run takes the classic path. A temporary log line at thelaunchTestSessioncall site captured the value actually sent, and was removed before commit.BROWSERSTACK_BUILD_NAMEbuild_identifiersent"#${BUILD_NUMBER}"sdkFourOneSixFiveMergeDemo"#${BUILD_NUMBER}"08e21249…, 2 sessions"#1"sdkFourOneSixFiveCtrlDemo 1"#2"sdkFourOneSixFiveCtrlDemo 2"#1"sdkFourOneSixFiveFixedDemo 1"#2"sdkFourOneSixFiveFixedDemo 2GET /app-automate/builds/08e21249…/sessions.jsonreturnssession_count = 2(19:05:12Z and 19:06:19Z) — the two D runs merged. Each control and each fixed-state build returnssession_count = 1.Re-verified on WDIO v9 (
webdriverio@9.31.5): runs G1/G2 under the same conditions producedsdkFourOneSixFiveVNineFixed 1and2, and the spuriousSkipping buildIdentifierwarning is gone.Unit tests: all 10
_handleBuildIdentifiercases pass, including the pre-existingshould delete buildIdentifier if buildName is not present in caps. Two_uploadAppfailures intests/launcher.test.tsare pre-existing — confirmed by re-running that block on an unmodified checkout.npx eslintclean on both changed files.Note on the existing env-var test
should delete buildIdentifier if BROWSERSTACK_BUILD_NAME is defined as env varpassed caps with nobuildName, so the!this._buildNamebranch already covered it and the assertion is unchanged. Only its title was corrected, since the env var was never the operative cause. Two tests were added: one locking resolution when abuildNameis present alongside the env var, one asserting the unresolved template is not retained.PR Validations
Run Tests: Comment RUN_TESTS to trigger sanity tests.
🤖 Generated with Claude Code