test: fix pre-existing unit-test failures (green the suite) - #62
Draft
07souravkunda wants to merge 1 commit into
Draft
test: fix pre-existing unit-test failures (green the suite)#6207souravkunda wants to merge 1 commit into
07souravkunda wants to merge 1 commit into
Conversation
The unit suite had 11 pre-existing failures on a clean checkout, all due to test rot / a network dependency rather than product bugs: - LocalTests (6): the addBinaryPath(...) mock verifications hardcoded the access-key argument as "" while the binding correctly forwards the real access key (needed as auth_token for the endpoint lookup). Updated the expected argument to match current behavior. - BrowserStackTunnelTests (5): the binary-path / fallback tests use a real tunnel subclass, so addBinaryPath's first call made a live HTTP POST to the endpoint API and failed with 401 in any offline / no-credential environment. Added a test-only override of the network fetch so these tests exercise the real path-resolution and fallback logic hermetically. The only production change is making that fetch method protected virtual to create the seam; behavior is unchanged. Suite is now green (23/23) and runs with no network access. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
What this does
Greens the pre-existing unit-test baseline. On a clean
mastercheckout the suite had 11 failing tests (12 passing / 23 total), all caused by test rot or a test-side network dependency — no product bugs. This lets future contributors trust the suite as a clean baseline.What was failing & why
LocalTests— 6 failures (stale assertion / test rot)TestWorksForBinaryPath,TestWorksWithBooleanOptions,TestWorksWithValueOptions,TestWorksWithCustomOptions,TestCallsFallbackOnFailure,TestKillsTunnel.Each verified
addBinaryPath("<path>", "", ...)— hardcoding the access-key argument as"". The binding actually forwards the real access key ("dummyKey"in these tests), because it is required as theauth_tokenfor the binary-endpoint lookup. The production call is correct; the assertion was stale. Updated the expected argument to"dummyKey".BrowserStackTunnelTests— 5 failures (network dependency)TestBinaryPathIsSet,TestBinaryPathOnNull,TestBinaryPathOnEmpty,TestBinaryPathOnFallback,TestBinaryPathOnNoMoreFallback.These use a real tunnel subclass to test binary-path resolution and fallback ordering. The first
addBinaryPath(...)call eagerly performs a live HTTP POST to the endpoint API, which returns401 Unauthorizedwith no/empty credentials — so the tests failed in any offline or credential-less environment (e.g. CI). Added a test-only override of the network-fetch method in the testTunnelClassso the tests exercise the real path-resolution and fallback logic hermetically (no network).Fixes applied (scoped)
BrowserStackLocal Unit Tests/LocalTests.cs— corrected the access-key argument in 6 mock verifications.BrowserStackLocal Unit Tests/BrowserStackTunnelTests.cs— added a test-only override that stubs the network fetch.BrowserStackLocal/BrowserStackTunnel.cs— the only production change: made the network-fetch methodprotected virtualto create the test seam. No behavior change (no production subclass overrides it).Left red
None — no test was masking a real product bug, so nothing was left failing.
How to run the suite
Result after this PR: Passed: 23, Failed: 0 (runs with no network access).
🤖 Generated with Claude Code