test: fix pre-existing unit-test failures (green the suite) - #100
Draft
07souravkunda wants to merge 1 commit into
Draft
test: fix pre-existing unit-test failures (green the suite)#10007souravkunda wants to merge 1 commit into
07souravkunda wants to merge 1 commit into
Conversation
The two live-tunnel tests (testIsRunning, testMultipleBinary) failed on a clean checkout with a NullPointerException whenever BROWSERSTACK_ACCESS_KEY was not set: the null key was appended to the process command and ProcessBuilder.start() rejected it. These are integration tests that start a real BrowserStack Local tunnel and genuinely require credentials. Guard them with a JUnit assumeNotNull on the access key so they skip gracefully when no key is present (local/fork/CI without secrets) while still running the full assertions whenever a key is available. No assertion is weakened or removed. 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 was failing
On a clean checkout of
master,mvn -q -B testreported 2 errors out of 14 tests:testIsRunningtestMultipleBinaryBoth threw a
NullPointerExceptionfromProcessBuilder.start()(viaLocal.runCommand) wheneverBROWSERSTACK_ACCESS_KEYwas not set in the environment. These two tests start a real BrowserStack Local tunnel, so the access key is read from the environment and appended to the binary command. With no key, anullelement lands in the command list andProcessBuilder.start()rejects it — the failure happens before any assertion runs. The other 12 tests only build the command string (onlyCommand=true) and pass regardless.This means every clean run without credentials (local dev, forks, CI without secrets) starts red, forcing anyone touching the repo to first confirm the failures pre-date their change.
What this fixes
testIsRunningandtestMultipleBinaryare integration tests that genuinely require credentials. Guarded each with a JUnitassumeNotNull(System.getenv("BROWSERSTACK_ACCESS_KEY"))so they skip gracefully when no key is present, while still running the full assertions whenever a key is available (e.g. CI with secrets). No assertion is weakened, disabled, or removed. Scope is limited to the test file.Verified with
BROWSERSTACK_ACCESS_KEYset:testIsRunningruns and passes end-to-end (real tunnel up →isRunning()true).What is left red
Nothing masks a product bug.
One note for maintainers: when a valid key is present,
testMultipleBinarycan still fail intermittently at its firststart()with"Either another browserstack local client is running ... port 45691". This is a test-isolation issue — the tunnel from the precedingtestIsRunninghas not released the fixed default port before this test opens its own on the same port. It is a harness-ordering problem, not a product defect (the binary correctly refuses a duplicate tunnel), and it is out of scope for this credential-less baseline fix. A follow-up could isolate the live tests with unique local identifiers / port cleanup.How to run the suite
mvn -q -B testBROWSERSTACK_ACCESS_KEY: 12 tests run, the 2 live tunnel tests are skipped → green.BROWSERSTACK_ACCESS_KEYexported: the live tests execute against a real tunnel.