test: fix pre-existing unit-test failures (green the suite) - #100
Merged
kamal-kaur04 merged 1 commit intoAug 12, 2026
Merged
Conversation
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
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>
kamal-kaur04
approved these changes
Aug 12, 2026
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
Reviewers
2 participants
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 test reported 2 errors out of 14 tests:
Both threw a NullPointerException from ProcessBuilder.start() (via Local.runCommand) whenever BROWSERSTACK_ACCESS_KEY was 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, a null element lands in the command list and ProcessBuilder.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
testIsRunning and testMultipleBinary are integration tests that genuinely require credentials. Guarded each with a JUnit assumeNotNull(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_KEY set: testIsRunning runs 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, testMultipleBinary can still fail intermittently at its first start() with "Either another browserstack local client is running ... port 45691". This is a test-isolation issue — the tunnel from the preceding testIsRunning has 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 test