ADFA-2738 connectedV8DebugAndroidTest Part 2 of 3: onboarding repairs#1176
Merged
hal-eisen-adfa merged 9 commits intostagefrom Apr 15, 2026
Merged
Conversation
Contributor
📝 WalkthroughRelease Notes - ADFA-2738 Onboarding Test Improvements (Part 2 of 3)Key Improvements
Test Suite Changes
Risks & Best Practice Considerations
Technical Implementation Details
WalkthroughReorders and consolidates instrumentation tests into an ordered suite, removes several isolated tests, adds an end-to-end Kaspresso test and a PermissionsInfo screen, centralizes accessibility-based helpers and appops/pm grant flows, and adds test-mode guards in onboarding UI. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Test Runner
participant App as OnboardingActivity / App UI
participant UiAutomator as UIAutomation (Accessibility)
participant Device as Android Device (appops/pm)
TestRunner->>App: Launch SplashActivity
App-->>TestRunner: Welcome screen visible
TestRunner->>UiAutomator: find "NEXT"/accept/Grant nodes
UiAutomator-->>TestRunner: node refs
TestRunner->>UiAutomator: perform ACTION_CLICK (NEXT/accept/Grant)
UiAutomator-->>App: UI event (click)
App-->>TestRunner: show PermissionsInfo screen (recycler)
loop per permission
TestRunner->>UiAutomator: click grant button
alt clicked via UI
UiAutomator-->>App: permission granted
else fallback
TestRunner->>Device: run `appops set` / `pm` shell to grant
Device-->>App: permission state changed
end
end
TestRunner->>UiAutomator: click "Finish installation"
UiAutomator-->>App: finish event
App-->>TestRunner: Main editor/home UI appears
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/AdvancePastWelcomeScreenHelper.kt (1)
28-60: Please extract this accessibility click logic into a shared helper.This tree walk now exists here and again in
app/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.kt:21-58, and the two versions have already drifted in matching behavior. Centralizing it will keep future onboarding click fixes from splitting across helpers.🤖 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/AdvancePastWelcomeScreenHelper.kt` around lines 28 - 60, Extract the duplicated tree-walk click logic from accessibilityClickByDescription and accessibilityClickByText into a single shared helper (e.g., a new object/class like AccessibilityClickHelper with a single method clickByPredicate or two thin wrappers clickByDescription/clickByText) and update both AdvancePastWelcomeScreenHelper and OnboardingPermissionsInfoHelper to call that shared helper; ensure the shared implementation centralizes the root retrieval (InstrumentationRegistry.getInstrumentation().uiAutomation.rootInActiveWindow), node iteration, matching predicate (contentDescription/text contains ignoreCase), performAction call, and proper recycle of node and root so behavior remains identical across callers (use the existing function names accessibilityClickByDescription/accessibilityClickByText as thin delegators if you prefer to keep public API).app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt (1)
40-48: Consider consuming the shell command output stream.
executeShellCommand()returns aParcelFileDescriptorwhose output should be read and closed to ensure the command completes before proceeding. WhilewaitForIdle()may provide sufficient synchronization in practice, explicitly draining the stream is more robust.♻️ Suggested improvement
private fun Device.grantViaAppOpsAndBack(appOp: String) { val pkg = InstrumentationRegistry.getInstrumentation().targetContext.packageName - InstrumentationRegistry.getInstrumentation().uiAutomation + val pfd = InstrumentationRegistry.getInstrumentation().uiAutomation .executeShellCommand("appops set $pkg $appOp allow") + pfd.close() // Ensures command completes val d = uiDevice d.waitForIdle() d.pressBack() d.waitForIdle() }🤖 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 40 - 48, The executeShellCommand call in Device.grantViaAppOpsAndBack doesn't consume or close the returned stream, so read and close the ParcelFileDescriptor/stream returned by InstrumentationRegistry.getInstrumentation().uiAutomation.executeShellCommand("appops set $pkg $appOp allow") to ensure the command completes before continuing; specifically, after calling executeShellCommand, obtain its FileDescriptor/InputStream, fully drain/read it (or copy to a sink), and close the stream/descriptor in a finally/try-with-resources block before calling uiDevice.waitForIdle()/pressBack to guarantee proper synchronization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/GrantRequiredPermissionsUiHelper.kt`:
- Around line 73-87: The loop over nodes in GrantRequiredPermissionsUiHelper.kt
leaks AccessibilityNodeInfo objects when breaking early; ensure all nodes are
recycled even after a successful click by replacing the current loop with logic
that still iterates remaining items (or uses a finally/try block) to call
node.recycle() for every element in the nodes list; locate the variable nodes
and the loop that sets clicked = true and performs node.performAction(...) and
guarantee node.recycle() is executed for every node in nodes (not just the ones
before the break).
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.kt`:
- Around line 39-58: The current flakySafely block in
OnboardingPermissionsInfoHelper.kt only clicks nodes whose contentDescription
contains "NEXT", but the search used
root.findAccessibilityNodeInfosByText("NEXT") may return nodes that expose
"NEXT" as text; update the loop that iterates over nodes to accept either
node.contentDescription or node.text (e.g., check
node.contentDescription?.toString() or node.text?.toString() for a
case-insensitive contains("NEXT")) before calling
node.performAction(AccessibilityNodeInfo.ACTION_CLICK), still recycling nodes
and root as done now; mirror the same desc/text fallback logic used in
AdvancePastWelcomeScreenHelper.kt to ensure both properties are considered.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/PermissionsInfoScreenTest.kt`:
- Around line 103-130: The test test_privacyDialog_notShownOnSecondVisit
currently only verifies the dialog is dismissed in the same session; either
rename it to test_privacyDialog_dismissedAfterAccept or actually simulate a
second visit by restarting the activity and re-checking the dialog: after
accepting (inside the "Accept privacy disclosure" step) call
activityRule.scenario.recreate() (or re-launch the app), wait for idle
(device.uiDevice.waitForIdle()), call advancePastWelcomeScreen() again, then
assert the dialog title does not appear using
device.uiDevice.findObject(UiSelector().text(dialogTitle)).waitForExists(timeout)
(or assertFalse on its existence) in the "Verify dialog not shown again" step so
the behavior is validated across a restart.
- Around line 51-66: The accessibilityClickByText function leaks
AccessibilityNodeInfo objects when returning early after finding a matching
node; change the logic so you perform the click on the matched node but do not
return immediately—store the boolean result, continue or finish iterating (or
explicitly recycle all remaining nodes in the nodes collection), then recycle
root and return the stored result; update references in accessibilityClickByText
to ensure every node in nodes is recycled and root.recycle() is called exactly
once before returning.
In `@app/src/main/java/com/itsaky/androidide/activities/OnboardingActivity.kt`:
- Around line 212-218: The test-mode guard currently skips calling
reloadJdkDistInfo() in onResume(), causing installedDistributions to remain
empty during async ToolsManager.init() and blocking navigation; remove the
isTestMode() conditional around the lifecycleScope.launch in onResume() so
reloadJdkDistInfo { tryNavigateToMainIfSetupIsCompleted() } always runs, and
keep or add the isTestMode() check only around the animation callback
(onPageSelected()) as suggested so tests still skip animations but retain the
distribution reload fallback.
---
Nitpick comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/AdvancePastWelcomeScreenHelper.kt`:
- Around line 28-60: Extract the duplicated tree-walk click logic from
accessibilityClickByDescription and accessibilityClickByText into a single
shared helper (e.g., a new object/class like AccessibilityClickHelper with a
single method clickByPredicate or two thin wrappers
clickByDescription/clickByText) and update both AdvancePastWelcomeScreenHelper
and OnboardingPermissionsInfoHelper to call that shared helper; ensure the
shared implementation centralizes the root retrieval
(InstrumentationRegistry.getInstrumentation().uiAutomation.rootInActiveWindow),
node iteration, matching predicate (contentDescription/text contains
ignoreCase), performAction call, and proper recycle of node and root so behavior
remains identical across callers (use the existing function names
accessibilityClickByDescription/accessibilityClickByText as thin delegators if
you prefer to keep public API).
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt`:
- Around line 40-48: The executeShellCommand call in
Device.grantViaAppOpsAndBack doesn't consume or close the returned stream, so
read and close the ParcelFileDescriptor/stream returned by
InstrumentationRegistry.getInstrumentation().uiAutomation.executeShellCommand("appops
set $pkg $appOp allow") to ensure the command completes before continuing;
specifically, after calling executeShellCommand, obtain its
FileDescriptor/InputStream, fully drain/read it (or copy to a sink), and close
the stream/descriptor in a finally/try-with-resources block before calling
uiDevice.waitForIdle()/pressBack to guarantee proper synchronization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c80761e-f8f3-481d-9297-e45565d3b1c5
📒 Files selected for processing (11)
app/src/androidTest/kotlin/com/itsaky/androidide/OrderedTestSuite.ktapp/src/androidTest/kotlin/com/itsaky/androidide/PermissionsInfoScreenTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/PermissionsScreenTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/AdvancePastWelcomeScreenHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.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/PermissionsInfoScreen.ktapp/src/main/java/com/itsaky/androidide/activities/OnboardingActivity.ktapp/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.kt (1)
43-45:⚠️ Potential issue | 🟠 MajorThe desc/text mismatch is still present in the wait gate.
Line 43 only waits on
descriptionContains("NEXT"), but the click path below accepts eithercontentDescriptionortext. A text-only NEXT button will still fail at Line 44 before the fallback logic ever runs.🤖 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/OnboardingPermissionsInfoHelper.kt` around lines 43 - 45, The wait gate in OnboardingPermissionsInfoHelper.kt uses nextObj = d.findObject(UiSelector().descriptionContains("NEXT")) and then waitForExists(...), which misses cases where the NEXT button is text-only; change the wait to look for either a description or text match (e.g., use UiSelector().descriptionContains("NEXT").or(UiSelector().textContains("NEXT")) or perform two finds and wait on either) so the existence check matches the same acceptance logic used later in the click path and avoid failing before the fallback runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/GrantRequiredPermissionsUiHelper.kt`:
- Around line 77-84: The code in GrantRequiredPermissionsUiHelper sets clicked =
true regardless of the result of node.performAction(...) which hides failed
click attempts from flakySafely; change the logic to check the boolean return of
node.performAction(AccessibilityNodeInfo.ACTION_CLICK) and only set clicked =
true when it returns true, and if it returns false throw or propagate an
exception (e.g., RuntimeException with context including grantText and node
info) so the caller/flakySafely can detect the failure and retry; update the
block around node.performAction(...) accordingly.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.kt`:
- Around line 24-33: The privacy-accept loop in OnboardingPermissionsInfoHelper
currently calls node.performAction(...) without checking its return value;
mirror the NEXT button helper by introducing a boolean clicked, iterate all
nodes returned by root.findAccessibilityNodeInfosByText(accept), for each node
attempt node.performAction(AccessibilityNodeInfo.ACTION_CLICK) and set clicked =
true only on success (then recycle), continue recycling all nodes, and after the
loop if clicked is still false throw or log an explicit failure/error to avoid
silent failures; use the same variable names and recycling pattern as the NEXT
handler to locate and replace the existing loop.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/PermissionsInfoScreenTest.kt`:
- Around line 46-48: The ParcelFileDescriptor returned by
InstrumentationRegistry.getInstrumentation().uiAutomation.executeShellCommand(...)
in cleanUp() is leaked; wrap that call in a use{} block (or otherwise close the
ParcelFileDescriptor) and consume its stream (e.g., via FileInputStream on the
descriptor) before returning so the descriptor is closed; update the cleanUp()
function in PermissionsInfoScreenTest.kt to mirror the pattern used in
CleanupTest.kt (wrap executeShellCommand(...) result in .use and drain the
stream) to avoid the leak.
---
Duplicate comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.kt`:
- Around line 43-45: The wait gate in OnboardingPermissionsInfoHelper.kt uses
nextObj = d.findObject(UiSelector().descriptionContains("NEXT")) and then
waitForExists(...), which misses cases where the NEXT button is text-only;
change the wait to look for either a description or text match (e.g., use
UiSelector().descriptionContains("NEXT").or(UiSelector().textContains("NEXT"))
or perform two finds and wait on either) so the existence check matches the same
acceptance logic used later in the click path and avoid failing before the
fallback runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d4863e2-8888-44b7-9c78-27bca68bbc32
📒 Files selected for processing (3)
app/src/androidTest/kotlin/com/itsaky/androidide/PermissionsInfoScreenTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/GrantRequiredPermissionsUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.kt
jatezzz
requested changes
Apr 14, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.kt (1)
28-30:⚠️ Potential issue | 🟠 MajorWait gate should match both
contentDescriptionandtextfor NEXT.Line 28 waits only on
descriptionContains("NEXT"), but Lines 33-40 allow either description or text. If NEXT is text-only, this throws before the click fallback runs.🔧 Suggested fix
- val nextObj = d.findObject(UiSelector().descriptionContains("NEXT")) - if (!nextObj.waitForExists(3_000)) { + val nextByDesc = d.findObject(UiSelector().descriptionContains("NEXT")) + val nextByText = d.findObject(UiSelector().textContains("NEXT")) + if (!nextByDesc.waitForExists(3_000) && !nextByText.exists()) { throw AssertionError("NEXT button not found on permissions info slide") }#!/bin/bash # Verify whether NEXT wait is description-only while click supports description/text fallback. fd -i OnboardingPermissionsInfoHelper.kt --exec sh -c ' echo "== $1 =="; cat -n "$1" | sed -n "20,60p" ' sh {} # Compare with the other onboarding helper to keep behavior aligned. fd -i AdvancePastWelcomeScreenHelper.kt --exec sh -c ' echo "== $1 =="; rg -n -C3 "NEXT|descriptionContains|textContains|findAccessibilityNodeInfosByText" "$1" ' sh {}Expected result: this file shows a description-only pre-wait and a desc/text click fallback; align both phases to the same matching rule.
🤖 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/OnboardingPermissionsInfoHelper.kt` around lines 28 - 30, The pre-wait in OnboardingPermissionsInfoHelper currently uses d.findObject(UiSelector().descriptionContains("NEXT")) and nextObj.waitForExists(...), which only matches contentDescription; update the wait to accept the same description-or-text rule used later during the click fallback (i.e., mirror the matching used in the click logic such as UiSelector().descriptionContains("NEXT") OR UiSelector().textContains("NEXT") / findAccessibilityNodeInfosByText) so the wait doesn't fail when NEXT is text-only—locate the wait call (nextObj.waitForExists) and replace the selector/matching logic to check both description and text before proceeding to the click fallback.
🧹 Nitpick comments (1)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt (1)
63-71: Recycle each node in a per-nodefinallyblock.
matchByis caller-supplied. If it throws for one candidate, the current loop exits before recycling thatAccessibilityNodeInfo, even though this helper is supposed to own cleanup.Suggested fix
try { for (node in nodes) { - if (!clicked && matchBy(node)) { - clicked = node.performAction(AccessibilityNodeInfo.ACTION_CLICK) + try { + if (!clicked && matchBy(node)) { + clicked = node.performAction(AccessibilityNodeInfo.ACTION_CLICK) + } + } finally { + node.recycle() } - node.recycle() } } finally { root.recycle() }🤖 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 63 - 71, The loop over AccessibilityNodeInfo `nodes` currently recycles nodes only after `matchBy` and `performAction` succeed, so if the caller-supplied `matchBy` throws the node is not recycled; change the loop in DevicePermissionGrantUiHelper so each `node` is handled inside its own try/finally: call `matchBy(node)` and `node.performAction(AccessibilityNodeInfo.ACTION_CLICK)` inside the per-node try and always call `node.recycle()` in that finally, while keeping the outer finally that recycles `root`; ensure you don't access `node` after its per-node finally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt`:
- Around line 79-87: The executeShellCommand call in
Device.grantViaAppOpsAndBack currently drops the ParcelFileDescriptor returned
by
InstrumentationRegistry.getInstrumentation().uiAutomation.executeShellCommand(...);
capture that returned descriptor, read/consume the stream as needed and
explicitly close the ParcelFileDescriptor (or its FileDescriptor/input stream)
in a finally/try-with-resources to avoid leaking file descriptors; update
Device.grantViaAppOpsAndBack to store the result of executeShellCommand, close
it after use, and ensure any streams are closed even on exceptions.
---
Duplicate comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.kt`:
- Around line 28-30: The pre-wait in OnboardingPermissionsInfoHelper currently
uses d.findObject(UiSelector().descriptionContains("NEXT")) and
nextObj.waitForExists(...), which only matches contentDescription; update the
wait to accept the same description-or-text rule used later during the click
fallback (i.e., mirror the matching used in the click logic such as
UiSelector().descriptionContains("NEXT") OR UiSelector().textContains("NEXT") /
findAccessibilityNodeInfosByText) so the wait doesn't fail when NEXT is
text-only—locate the wait call (nextObj.waitForExists) and replace the
selector/matching logic to check both description and text before proceeding to
the click fallback.
---
Nitpick comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt`:
- Around line 63-71: The loop over AccessibilityNodeInfo `nodes` currently
recycles nodes only after `matchBy` and `performAction` succeed, so if the
caller-supplied `matchBy` throws the node is not recycled; change the loop in
DevicePermissionGrantUiHelper so each `node` is handled inside its own
try/finally: call `matchBy(node)` and
`node.performAction(AccessibilityNodeInfo.ACTION_CLICK)` inside the per-node try
and always call `node.recycle()` in that finally, while keeping the outer
finally that recycles `root`; ensure you don't access `node` after its per-node
finally.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2ccfc86e-86d5-4055-be88-f3ff7480e738
📒 Files selected for processing (4)
app/src/androidTest/kotlin/com/itsaky/androidide/PermissionsInfoScreenTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/GrantRequiredPermissionsUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.kt
✅ Files skipped from review due to trivial changes (1)
- app/src/androidTest/kotlin/com/itsaky/androidide/PermissionsInfoScreenTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/androidTest/kotlin/com/itsaky/androidide/helper/GrantRequiredPermissionsUiHelper.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt (2)
70-73: Consider replacingThread.sleepwithwaitForIdle.Hardcoded sleeps can cause flakiness if the timing varies. Consider using
device.uiDevice.waitForIdle()or checking for a specific UI element to become visible.Suggested alternative
step("Launch app") { ActivityScenario.launch(SplashActivity::class.java) - Thread.sleep(1000) + device.uiDevice.waitForIdle() }🤖 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/EndToEndTest.kt` around lines 70 - 73, Replace the hardcoded Thread.sleep in the Launch app step with a deterministic idle/wait strategy: remove the Thread.sleep(1000) after ActivityScenario.launch(SplashActivity::class.java) and instead wait for UI idle or a specific UI element (e.g., use device.uiDevice.waitForIdle() or an Espresso onView(...).check(matches(isDisplayed()))/IdlingResource) so the test proceeds only when SplashActivity is ready; update the step that references Thread.sleep to call the chosen wait mechanism referencing ActivityScenario.launch and SplashActivity.
47-63: Consider consolidating withclickFirstAccessibilityNodeByText.This local
accessibilityClickByTextfunction has similar logic toclickFirstAccessibilityNodeByTextinDevicePermissionGrantUiHelper.kt. The difference is this returns a boolean while the helper throws on failure. Consider:
- Using the existing helper with a try-catch, or
- Adding a
tryClickFirstAccessibilityNodeByTextvariant that returns boolean🤖 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/EndToEndTest.kt` around lines 47 - 63, The local function accessibilityClickByText duplicates logic from DevicePermissionGrantUiHelper.clickFirstAccessibilityNodeByText and should be consolidated: either replace calls to accessibilityClickByText with a try/catch around clickFirstAccessibilityNodeByText (catch the exception and return false) or add a new helper tryClickFirstAccessibilityNodeByText in DevicePermissionGrantUiHelper that returns a Boolean and use that here; update EndToEndTest.kt to call the shared helper (accessibilityClickByText removed) so the click logic lives only in DevicePermissionGrantUiHelper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt`:
- Line 5: Remove the unused Espresso import: delete the import statement for
androidx.test.espresso.matcher.ViewMatchers.isNotEnabled in EndToEndTest.kt; the
test uses Kaspresso's KView.isNotEnabled() assertion (see usages around the
KView call at line ~196), so removing this unused import will clean up the file
and avoid confusion.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt`:
- Line 96: The log message passed to logOnboardingNavigation contains a typo
"unganted" — update the string argument in the call to
logOnboardingNavigation("Permission list visible with ungranted permissions —
granting") so it reads "ungranted" instead of "unganted" (locate the
logOnboardingNavigation invocation in NavigateToMainScreenScenario.kt and adjust
the message).
---
Nitpick comments:
In `@app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt`:
- Around line 70-73: Replace the hardcoded Thread.sleep in the Launch app step
with a deterministic idle/wait strategy: remove the Thread.sleep(1000) after
ActivityScenario.launch(SplashActivity::class.java) and instead wait for UI idle
or a specific UI element (e.g., use device.uiDevice.waitForIdle() or an Espresso
onView(...).check(matches(isDisplayed()))/IdlingResource) so the test proceeds
only when SplashActivity is ready; update the step that references Thread.sleep
to call the chosen wait mechanism referencing ActivityScenario.launch and
SplashActivity.
- Around line 47-63: The local function accessibilityClickByText duplicates
logic from DevicePermissionGrantUiHelper.clickFirstAccessibilityNodeByText and
should be consolidated: either replace calls to accessibilityClickByText with a
try/catch around clickFirstAccessibilityNodeByText (catch the exception and
return false) or add a new helper tryClickFirstAccessibilityNodeByText in
DevicePermissionGrantUiHelper that returns a Boolean and use that here; update
EndToEndTest.kt to call the shared helper (accessibilityClickByText removed) so
the click logic lives only in DevicePermissionGrantUiHelper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8429c458-a0c2-478e-9b07-099410288ff6
📒 Files selected for processing (9)
app/build.gradle.ktsapp/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/OrderedTestSuite.ktapp/src/androidTest/kotlin/com/itsaky/androidide/PermissionsScreenTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithGroovyGradle.ktapp/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithKtsGradle.ktapp/src/androidTest/kotlin/com/itsaky/androidide/WelcomeScreenTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt
💤 Files with no reviewable changes (4)
- app/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithGroovyGradle.kt
- app/src/androidTest/kotlin/com/itsaky/androidide/WelcomeScreenTest.kt
- app/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithKtsGradle.kt
- app/src/androidTest/kotlin/com/itsaky/androidide/PermissionsScreenTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/androidTest/kotlin/com/itsaky/androidide/OrderedTestSuite.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt (1)
52-55: Wait on UI state instead of sleeping.The fixed 1s delay here is still timing-based flakiness: slow emulators can take longer, while fast ones just pay the extra second every run. Waiting for the first onboarding signal would make the launch step deterministic.
🤖 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/EndToEndTest.kt` around lines 52 - 55, Replace the fixed Thread.sleep(1000) in the step("Launch app") block with a deterministic wait for the first onboarding UI signal instead of a timing sleep: after ActivityScenario.launch(SplashActivity::class.java) wait for a specific onboarding view to become visible (e.g. using Espresso.onView(withId(R.id.<onboarding_root_or_first_view>)).check(matches(isDisplayed())) or an IdlingResource that waits for the onboarding state), targeting the onboarding element that SplashActivity displays; remove Thread.sleep and use this UI assertion/wait so the launch step is deterministic and not emulator-speed dependent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt`:
- Around line 92-98: The function resetAppPermissionsAndClear incorrectly calls
"pm reset-permissions $pkg" which is unsupported (reset-permissions is global);
update resetAppPermissionsAndClear to revoke permissions per-app instead: after
running appops loop over APPOPS_PERMISSIONS via ua.executeShellCommand("appops
set $pkg $op default"), iterate the runtime permissions you need to clear and
call ua.executeShellCommand("pm revoke $pkg <PERMISSION>") for each, then keep
the existing "pm clear $pkg" call; reference the function name
resetAppPermissionsAndClear, the APPOPS_PERMISSIONS constant, and the
ua.executeShellCommand invocations when making the change.
- Around line 47-76: The current pre-filter using
root.findAccessibilityNodeInfosByText(searchText) excludes nodes that only have
contentDescription, so change clickFirstAccessibilityNodeByText to walk the
accessibility tree from root (BFS/DFS over root's children) and apply the
provided matchBy predicate to every descendant (checking node.text and
node.contentDescription as callers expect) instead of pre-filtering by text;
ensure you properly recycle each visited AccessibilityNodeInfo and the root, set
clicked when node.performAction(ACTION_CLICK) succeeds, and keep throwing the
same AssertionError using errorLabel if nothing was clicked.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt`:
- Around line 25-38: The early-exit currently uses return@step inside the "Check
if already on main screen" step which only exits that single step and allows
later teardown/final steps to run; change it to return from the overall scenario
lambda so the scenario truly stops when onboarding is already complete (use
return@steps or name the outer lambda with a label and return to that label).
Locate the override val steps: TestContext<Unit>.() -> Unit = { ... } block and
the step("Check if already on main screen") { ... return@step } and replace the
return@step with a return that targets the outer steps lambda (e.g. return@steps
or a labeled outer lambda) so the entire scenario returns immediately.
---
Nitpick comments:
In `@app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt`:
- Around line 52-55: Replace the fixed Thread.sleep(1000) in the step("Launch
app") block with a deterministic wait for the first onboarding UI signal instead
of a timing sleep: after ActivityScenario.launch(SplashActivity::class.java)
wait for a specific onboarding view to become visible (e.g. using
Espresso.onView(withId(R.id.<onboarding_root_or_first_view>)).check(matches(isDisplayed()))
or an IdlingResource that waits for the onboarding state), targeting the
onboarding element that SplashActivity displays; remove Thread.sleep and use
this UI assertion/wait so the launch step is deterministic and not
emulator-speed dependent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e9f405f-3db4-43e8-a92d-1f8127f7aef8
📒 Files selected for processing (10)
app/build.gradle.ktsapp/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/OrderedTestSuite.ktapp/src/androidTest/kotlin/com/itsaky/androidide/PermissionsScreenTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithGroovyGradle.ktapp/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithKtsGradle.ktapp/src/androidTest/kotlin/com/itsaky/androidide/WelcomeScreenTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.ktapp/src/main/java/com/itsaky/androidide/activities/OnboardingActivity.kt
💤 Files with no reviewable changes (4)
- app/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithGroovyGradle.kt
- app/src/androidTest/kotlin/com/itsaky/androidide/WelcomeScreenTest.kt
- app/src/androidTest/kotlin/com/itsaky/androidide/PermissionsScreenTest.kt
- app/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithKtsGradle.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/itsaky/androidide/activities/OnboardingActivity.kt
- app/src/androidTest/kotlin/com/itsaky/androidide/OrderedTestSuite.kt
- app/build.gradle.kts
jatezzz
approved these changes
Apr 15, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/build.gradle.kts (1)
343-343: Prefer removing the commented orchestrator dependency line.This is inactive now and can be deleted to keep the test dependency surface unambiguous.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/build.gradle.kts` at line 343, Remove the inactive commented dependency line "androidTestUtil(libs.tests.orchestrator)" from the build script; locate the commented line in app/build.gradle.kts and delete it so the test dependency declarations no longer include that inactive comment.
🤖 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/build.gradle.kts`:
- Line 343: Remove the inactive commented dependency line
"androidTestUtil(libs.tests.orchestrator)" from the build script; locate the
commented line in app/build.gradle.kts and delete it so the test dependency
declarations no longer include that inactive comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed47dd38-326f-4942-ac11-2e8a435ce783
📒 Files selected for processing (2)
app/build.gradle.ktsapp/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt
hal-eisen-adfa
deleted the
ADFA-2738-connectedV8DebugAndroidTest-part2-projects
branch
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.
Stability and reliability of the onboarding and permissions UI tests. Remove flakiness caused by system gesture exclusion zones by replacing coordinate-based Espresso and UI Automator clicks with accessibility-based actions. Refactor how system permissions are granted during tests, shifting away from fragile UI toggling in favor of direct shell commands and runtime permission APIs.