Repository navigation
fix(cli): honour BROWSERSTACK_PROJECT_NAME / BROWSERSTACK_BUILD_NAME in session caps (SDK-7075) - #282
Conversation
The service read projectName/buildName only from the config file, so an
exported BROWSERSTACK_PROJECT_NAME / BROWSERSTACK_BUILD_NAME never reached
the session and the build was created under the config-file name.
Every other SDK already maps these (node-agent capsMapping.js, python
constants.py, javaagent SeleniumPatchUtils.java, csharp utils.cs) and so
does the binary's own env->config map (core/config/env.js). The binary
resolves them correctly here too, but the service consumes only hubUrl,
apis, credentials, accessibilityOptions and testContextOptions from that
resolved config, so the values were dropped before reaching the caps.
Write them into the capability at both shapes, mirroring the existing
localIdentifier handling, so the session carries the env value.
Verified against a wdio_cucumber sample in binary flow (SDK_CLI_BIN_PATH
pinned, cold cache). Session capabilities, config file vs env differing:
before platforms[0].projectName = 'repro_project'
platforms[0].buildName = 'repro_build'
after platforms[0].projectName = 'repro_project_env_vars'
platforms[0].buildName = 'repro_build_env_vars'
Refs SDK-7075. v9 counterpart of the same fix on v8.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…7075) Adds three constructor tests: the env vars win over the caps for both the array and the multiremote capability shapes, and the caps values survive when the env vars are unset. The two "prefer env" tests fail without the launcher change. Folding BROWSERSTACK_BUILD_NAME into the caps also means a buildName is now present whenever that var is exported, so _handleBuildIdentifier no longer treats it as missing and keeps the identifier instead of dropping it. Updates that test to the corrected contract and refreshes the SDK-4748 comments that asserted this service never reads the var as a buildName source. "buildName absent" now means absent from both the caps and the environment, which the BUILD_RUN_IDENTIFIER test already covers. 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 configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
v8 backport: #283 — same change, same tests, verified the same way. Please review the two together. |
|
🔴 SDK PR Review gate is red. Pending:
It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge. |
|
🔴 SDK PR Review gate is red. Pending:
It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge. |
|
@shivam5643 Please test a scenario where you set different project names and build names using environment variables and in the config file. Then check whether the O11y build and the Automate/App Automate build contain the names set using the environment variables. |
…(SDK-7075) The env override only ran when the caps already carried bstack:options. Caps the service builds bstack:options for (extension caps such as goog:chromeOptions) and legacy caps (top-level build/project keys) still ignored BROWSERSTACK_PROJECT_NAME / BROWSERSTACK_BUILD_NAME, so the build start payload and _handleBuildIdentifier kept seeing the config name. Fold the override into one _applyEnvBuildNames helper called from all four capability arms: it writes into bstack:options when present, into the top-level project/build keys of legacy caps on a BrowserStack session, and never touches the caps of a non-BrowserStack session. Blank env values are ignored. The tests now clean the env vars in afterEach and cover the service-created, legacy, multiremote and non-BrowserStack shapes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…orting (SDK-7075) - Align top-level project/build keys that sit next to bstack:options, so a capability never carries two different names. (Live probe: the hub prefers bstack:options, so this is defensive, not a behaviour change.) - Return no names for a non-BrowserStack session: its caps are left untouched, so the Test Reporting fields keep the caps values instead of disagreeing with the session. - Tests: restore the isBStackSession spy in finally; cover padded env values, a build-only env override and the mixed bstack:options + top-level shape. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…plies (SDK-7075) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Yes working, have verified. |
|
🔴 SDK PR Review gate is red. Pending:
It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge. |
|
✅ Good to go Per-File Confidence
Change map (generated deterministically from the diff)graph LR
subgraph nwdio_service["wdio-service"]
npackages_browserstack_service_src_launcher_ts["⚠ launcher.ts<br/>~113 lines"]
npackages_browserstack_service_tests_launcher_test_ts["⚠ launcher.test.ts<br/>~113 lines"]
n_changeset_pr_282_md["pr-282.md<br/>~5 lines"]
end
↻ 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 |
|
🟢 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. |
1 similar comment
|
🟢 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. |
|
RUN_TESTS |
|
[SDK Wdio Test] TRA build state: failed | Stability 98% — verdict: success. Passed: 51, Failed: 1, Aggregate: 52. TRA: https://observability.browserstack.com/builds/sktqkkdampcskw3nnggpiuetif2qqf1m87wdwfvs |
|
🔴 SDK PR Review gate is red. Pending:
It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge. |
What is this about?
BROWSERSTACK_PROJECT_NAMEandBROWSERSTACK_BUILD_NAMEwere ignored by this service. The launcher readprojectName/buildNameonly out of the capabilities, so a value exported in the environment never reached the session — the build landed on the dashboard under whatever the config file said.Every other SDK, and the binary's own env→config map, already give the environment precedence over the config file. The service was the one place that did not, so a user who overrides the build name per CI run (the standard pattern) silently got the config name instead.
The fix
A private helper,
_applyEnvBuildNames(capability), readsBROWSERSTACK_PROJECT_NAME/BROWSERSTACK_BUILD_NAME(trimmed; blank values ignored) and writes them back into the capabilities, so the session itself carries them andthis._buildName/this._projectName(Test Reporting) read the same values. It is called from all four capability arms in the constructor - array and multiremote, with and withoutbstack:options.Knock-on:
_handleBuildIdentifier— please readBecause the env var is now folded into the caps, a
buildNameis present wheneverBROWSERSTACK_BUILD_NAMEis exported (on a BrowserStack session, or whenever the caps carrybstack:options)._handleBuildIdentifierdrops the identifier when!this._buildName, so that branch no longer fires in this case and the identifier is appended to the env-sourced name instead of being discarded.This is the direction already taken and approved in this repo, not a new one:
f1ebafc,079f4b8, merged) removed|| process.env.BROWSERSTACK_BUILD_NAMEfrom that guard, so the env var stopped suppressing the identifier.Together they give the coherent result: the env names the build, and the identifier is appended to it. The previously open #192 (SDK-4165) asked for the same end state and was superseded by SDK-4748.
The one test that encoded the old edge case — env var set, caps carrying no
buildName— asserted the identifier was dropped. Its premise is now self-contradictory, since the env var is abuildNamesource, so it is rewritten to assert the corrected contract. AbuildNamecounts as absent only when neither the caps nor the environment supply one, whichshould not set buildIdentifier from env when buildName is absentalready covers and which still passes unchanged. The SDK-4748 comments that asserted "this service never reads that var as a buildName source" are updated, since that is no longer true.Scope
bstack:optionsbstack:options.projectName/buildName; any top-levelproject/buildthe caps already carry are aligned to the same value (never added there)bstack:options, extension caps (e.g.goog:chromeOptions), BrowserStack sessionbstack:optionsthe service createsbstack:options, no:keys), BrowserStack sessionproject/buildkeysbstack:options_buildName/_projectNamekeep the caps valuesTop-level
project/buildare only ever added where the caps have no:extension keys -webdriverrejects a capability mixing:extension keys with non-W3C unprefixed keys, and any capability with a:key getsbstack:optionsinstead.testObservabilityOptions.projectName/buildName, when set, still take precedence for Test Reporting naming (util.ts,cli/cliUtils.ts) - unchanged by this PR. The environment always wins for the session itself.Verification
End to end, against a real session. The config file and the environment deliberately disagreed:
sdk7075repro_projectsdk7075v9_project_env_varssdk7075repro_buildsdk7075v9_build_env_vars v9_identifier_reproRun via the
automate/wdio_cucumberBStackAutomation sample (binary/CLI flow) —Spec Files: 1 passed, 1 total, session statuspassed. Both env values won; before the fix both came through as the config values. The identifier is appended, as described above.Capability shapes checked directly against the hub - raw W3C
POST /sessiontohub-cloud.browserstack.com, thenGET api.browserstack.com/automate/sessions/<id>.json:alwaysMatch{browserName, build, project}only - nobstack:optionsbuild/projectvalues: the hub honours legacy keys in a W3C requestbstack:options {buildName, projectName}and top-levelbuild/projectwith different valuesbstack:optionsvalues:bstack:optionswins, so the top-level alignment above is defensiveUnit tests. 11 constructor tests in a dedicated
describewithafterEachenv cleanup: prefer env, keep caps when unset, blank values ignored, padded values trimmed, build-only override, service-createdbstack:options, legacy caps, aligned top-level keys, multiremote with and withoutbstack:options, and an untouched non-BrowserStack session; plus the rewritten_handleBuildIdentifiertest. Against the baselauncher.ts8 of the 11 fail, so they are not vacuous; the other 3 (env unset, blank env, non-BrowserStack session) are no-regression guards that pass either way.launcher.test.ts: 143 passed.Full suite on this branch: 61 files, 1394 tests passed (the 3 unhandled errors from
uploadLogsArchive.test.tsare pre-existing - identical on the base commit),tscandeslintclean.Related Jira task/s
SDK-7075
Release (mandatory for every PR — required for the
ready-for-reviewlabel)Version bump: (required — tick exactly one)
Release notes type:
Release notes (customer-facing):
BROWSERSTACK_PROJECT_NAMEandBROWSERSTACK_BUILD_NAMEbeing ignored — the environment now takes precedence over the project and build names in your config file, matching the other BrowserStack SDKs.Release notes (internal):
_applyEnvBuildNameshelper, called from all four capability arms of the launcher constructor, foldsBROWSERSTACK_PROJECT_NAME/BROWSERSTACK_BUILD_NAMEinto the caps ahead of thethis._buildName/this._projectNameassignment, so the session and Test Reporting both carry the env value.buildNameis now present wheneverBROWSERSTACK_BUILD_NAMEis set, so_handleBuildIdentifierkeeps the identifier rather than dropping it. Completes the direction SDK-4748 started; the test encoding the old edge case is rewritten and the stale comments corrected.bstack:options(incl. the one the service creates for extension caps), and top-levelproject/buildfor legacy caps on BrowserStack sessions. Non-BrowserStack sessions are left untouched.Checklist
PR Validations
Run Tests: Comment RUN_TESTS to trigger sanity tests.
🤖 Generated with Claude Code