Skip to content

fix: make sure android path is valid #108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
gmegidish merged 2 commits into from
Nov 26, 2025
Merged

fix: make sure android path is valid #108

gmegidish merged 2 commits into from
Nov 26, 2025

Conversation

@gmegidish
Copy link
Member

@gmegidish gmegidish commented Nov 23, 2025 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • Bug Fixes
    • Improved Android SDK path detection to validate path existence before use, preventing issues with misconfigured environment variables
    • Enhanced device boot error messages to provide correct command syntax with the --device flag for both Android and simulator devices

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 23, 2025 •
edited
Loading

Walkthrough

Two device handler files are updated: Android SDK path resolution now validates path existence before returning it, and error messages in both Android and simulator device startup failures are corrected to include the --device flag in the boot command instructions.

Changes

Cohort / File(s) Summary
Android SDK path validation
devices/android.go		 getAndroidSdkPath now checks if the ANDROID_HOME path exists before returning it, allowing fallback to other locations when the path is invalid
Device boot error messages
devices/android.go, devices/simulator.go		 Updated offline/shutdown device error messages to instruct users to use device boot --device <udid> instead of the previous format without the explicit --device flag

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Specific areas for attention:
    • Verify the path existence check in getAndroidSdkPath uses the correct file system operation and doesn't introduce performance concerns
    • Confirm error message consistency across both files and that the --device flag format matches the actual CLI interface

Possibly related PRs

Poem

🐰 A rabbit hops through SDK paths true,
Checking if they really do exist in view,
Error messages now guide with care,
"Boot with --device," we declare!
Hop along, devices, all is well! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title focuses on the Android path validation fix in getAndroidSdkPath, which is a significant part of the changes, but the PR also includes unrelated updates to device boot command messages in both Android and simulator code. Consider clarifying the scope of the PR: either focus the title on 'fix: make sure android path is valid' if the device boot message changes are secondary, or update the title to reflect both changes like 'fix: validate android path and update device boot messages'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-make-sure-android-path-is-valid

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Nov 23, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
devices/android.go (1)

62-64: LGTM! Robust path validation added.

The path existence check prevents using an invalid ANDROID_HOME and allows fallback to default SDK locations. This is a solid defensive improvement.

Optionally, consider adding verbose logging when ANDROID_HOME is set but the path doesn't exist, to help users debug configuration issues:

 func getAndroidSdkPath() string {
 	sdkPath := os.Getenv("ANDROID_HOME")
 	if sdkPath != "" {
 		if _, err := os.Stat(sdkPath); err == nil {
 			return sdkPath
+		} else {
+			utils.Verbose("ANDROID_HOME is set to '%s' but path does not exist, trying default locations", sdkPath)
 		}
 	}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12528fd and 8cf3bfc.

📒 Files selected for processing (2)
  • devices/android.go (2 hunks)
  • devices/simulator.go (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: lint
🔇 Additional comments (2)
devices/simulator.go (1)

463-463: LGTM! Clear user guidance improvement.

The updated error message correctly instructs users to include the --device flag when booting an offline simulator, which aligns with the corresponding change in android.go.

devices/android.go (1)

392-392: LGTM! Improved user guidance.

The error message now correctly includes the --device flag in the boot command instruction, consistent with the simulator.go change.

@gmegidish gmegidish merged commit 3470934 into main Nov 26, 2025
15 checks passed
@gmegidish gmegidish deleted the fix-make-sure-android-path-is-valid branch November 26, 2025 12:12
gmegidish added a commit that referenced this pull request Nov 26, 2025
@gmegidish
fix: make sure ANDROID_HOME path is valid (#108)
3b2884c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gmegidish