Skip to content

feat(local): forward connection_settings.binaryPath to browserstack-local [SDK-7284] - #1176

Open
anish353 wants to merge 1 commit into
masterfrom
feat/sdk-7284-local-binary-path
Open

feat(local): forward connection_settings.binaryPath to browserstack-local [SDK-7284]#1176
anish353 wants to merge 1 commit into
masterfrom
feat/sdk-7284-local-binary-path

Conversation

@anish353

@anish353 anish353 commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

A customer's network blocks the BrowserStackLocal binary download, so they asked for a way to point the CLI at a binary they had already placed on disk. browserstack-local has supported exactly that for a long time — a binarypath option that short-circuits LocalBinary entirely — but the CLI never forwarded one.

setLocalArgs forwards five connection_settings keys today (local_identifier, local_config_file, proxyHost, proxyPort, useCaCertificate). binaryPath appears nowhere in bin/, so setting it in browserstack.json is silently dropped and the download path is always taken. That silent drop is what made this hard to diagnose from the outside: there is no log line saying the key was ignored.

Reported and root-caused on SDK-7284.

What lands here

File Change
bin/helpers/utils.js setLocalArgs forwards a configured binary path as binarypath. Both binaryPath and binarypath are accepted in browserstack.json.
test/unit/bin/helpers/utils.js Three specs: binaryPath is forwarded as the lowercase key, the lowercase config key is accepted too, and no key is emitted when none is configured. Plus a require-order fix, below.

The forwarded key is deliberately all-lowercase. browserstack-local only special-cases binarypath in addArgs; any other casing falls through to its default: branch and is handed to the binary as an unknown CLI flag — while the download still happens:

binaryPath  -> this.binaryPath = undefined | leaks to the binary as ["--binaryPath","<path>"]
binarypath  -> this.binaryPath = <path>    | leaks: []
BinaryPath  -> this.binaryPath = undefined | leaks to the binary as ["--BinaryPath","<path>"]

Accepting both spellings in browserstack.json is for the same reason — a user copying binarypath from the browserstack-local README should not land back in the silent-drop case this PR exists to remove.

Verification

Against the published browserstack-cypress-cli@1.36.18 with its real browserstack-local@1.5.13, using a stub binary that reports {"state":"connected"} so the assertion is which binary got spawned:

local_args from setLocalArgs Binary spawned
1.36.18 as published {key, localIdentifier, daemon, enable-logging-for-api, source} — no binarypath downloaded one; the supplied path is ignored
this branch {... , binarypath: "<path>"} the supplied binary, zero download attempts

On the fixed run browserstack-local prints BINARY PATH IS DEFINED and start returns no error. The argv it hands the binary matches the shape in the customer's error line, including the duplicate --daemon / --source the two layers each add.

Test plan

  • Three new specs on setLocalArgs (forwarded key, lowercase alias, absent key).
  • test/unit/bin/helpers/utils.js: 398 passing / 3 failing on this branch vs 393 passing / 5 failing on a clean master. Net +5 passing, zero regressions. The 3 remaining failures are pre-existing getVideoConfig specs, red on master too.
  • End-to-end check against the published tarball plus a stub binary (table above).

Why two pre-existing failures go green

bin/helpers/usageReporting.js requires ./utils (line 9) and then reassigns module.exports (line 21). Requiring usageReporting before utils therefore leaves utils holding the empty exports object captured mid-cycle, while the test holds the real one — so usageReporting.cli_version_and_path was undefined inside setLocalArgs, nothing stubbed in the test was visible to the code under test, and the existing spec threw. Requiring utils first makes it capture the finished exports.

This is a test-harness fix only; no production code depends on the ordering, since the CLI's entry points already load utils first. The stale-reference hazard in usageReporting.js itself is left alone as out of scope for this ticket.

…ocal [SDK-7284]

`browserstack-local` accepts a `binarypath` option that skips the binary
download entirely, but the CLI never forwarded one, so a customer whose network
blocks the download had no way to point the run at a binary they had already
placed on disk. `setLocalArgs` now passes it through.

The key it forwards is deliberately all-lowercase: `browserstack-local` only
special-cases `binarypath`, and any other casing falls through its `addArgs`
default branch and is handed to the binary as an unknown CLI flag while the
download happens anyway. Both `binaryPath` and `binarypath` are accepted in
`browserstack.json` for the same reason — a silently ignored config key is the
failure mode this change exists to remove.

Also reorders two requires in the test file. `usageReporting` reassigns
`module.exports` after its own circular `require('./utils')`, so requiring it
before `utils` left `utils` holding a stale, empty exports object: nothing
stubbed in the test was visible to the code under test, and `setLocalArgs`
threw. That is why the existing `setLocalArgs` spec was already failing on
master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@anish353
anish353 requested a review from a team as a code owner August 18, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant