3.1.0: arm64 + proxy passthrough for binary download (LOC-6563)#57
Open
yashdsaraf wants to merge 2 commits into
Open
3.1.0: arm64 + proxy passthrough for binary download (LOC-6563)#57yashdsaraf wants to merge 2 commits into
yashdsaraf wants to merge 2 commits into
Conversation
- GetBinaryName made public and adds Linux arm64 branch via RuntimeInformation.OSArchitecture. arm64 wins over alpine to match the Node SDK ordering. - BrowserStackTunnel.SetProxy(host, port) + proxyHost/proxyPort fields. - fetchSourceUrl builds HttpClientHandler with Proxy = WebProxy(host, port) when proxy is set. - downloadBinary sets WebClient.Proxy = WebProxy(host, port) when set. (Keeping WebClient; proxy plumbing on it is one line.) - Local.addArgs captures proxyHost/proxyPort on local fields in addition to appending to argumentString. Local.start calls tunnel.SetProxy(...) before DownloadVerifyAndRunBinary. - Test fixtures derive binaryName from BrowserStackTunnel.GetBinaryName() so arm64 CI hosts pick the right binary. Tracks LOC-6563. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Local.cs addArgs: int.TryParse failure now logs to stderr instead of silently dropping the proxy on download with no signal to the user. - BrowserStackTunnel.cs: doc comment on proxyHost/proxyPort fields documents the required ordering (SetProxy before addBinaryPath). - BrowserStackTunnelTests.cs: 2 new unit tests covering GetBinaryName return-set and SetProxy no-throw contract. Adds the missing TunnelClass constructor that broke compile pre-this-PR. - LocalTests.cs: 3 new unit tests covering proxy-options-to-tunnel plumbing (with proxy, without proxy, with invalid port). Fixes pre-existing CS0854 errors on Mock.Verify expressions with optional args, and pre-existing missing-ctor errors on Mock<BrowserStackTunnel>(). Project now compiles for the first time in years; 5 new tests pass, 11 pre-existing tests still fail with pre-existing assertion bugs (unchanged behavior; CI does not run this project anyway). Code review findings #3, #4, #5 from LOC-6563 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yashdsaraf
requested review from
a team,
AdityaHirapara,
kamal-kaur04 and
rounak610
and removed request for
a team and
kamal-kaur04
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RuntimeInformation.OSArchitecture == Architecture.Arm64selectsBrowserStackLocal-linux-arm64. arm64 wins over alpine to match the Node SDK ordering.proxyHost/proxyPortfromLocal.start(options)are now also threaded into the endpoint POST and the binary GET.fetchSourceUrl(HttpClient) anddownloadBinary(WebClient) both honor the proxy now. Users behind enterprise proxies can finally download.GetBinaryName()made public — test fixtures (BrowserStackTunnelTests.cs,IntegrationTests.cs) now use it as a single source of truth instead of hardcodedbinaryNameternaries.CloudFront migration was already done in this binding (see
fetchSourceUrl— already prod). This PR closes the remaining LOC-6563 items: arm64 + proxy.Background
Tracks LOC-6563. Ruby parity PR ships in coordination — see browserstack-local-ruby#39.
Files
Test plan
Pre-existing — NOT addressed in this PR
The `BrowserStackLocal Unit Tests` project does not compile in HEAD:
CS0854)Neither `.github/workflows/ci.yml` nor `.github/workflows/cd.yml` build or run this project — both target `BrowserStackLocalIntegrationTests` only. Left alone per scope; happy to file a follow-up.
🤖 Generated with Claude Code