ADFA-2738 connectedV8DebugAndroidTest Part 3 of 3: iterate over 1st three templates#1187
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 44 minutes and 6 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded accessibility-driven UIAutomator helpers and migrated several instrumentation test screens and scenarios from Espresso/Kakao to accessibility-based interactions; extended the end-to-end test to create and initialize projects for three templates, invoking project settings, naming, and cancelling builds. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Runner
participant Home as IDE Home UI
participant Template as Template List UI
participant Settings as Project Settings UI
participant Build as Build Process / Package Installer
participant Device as Android Device
Test->>Home: ensure on home screen
Home->>Test: home visible
Test->>Home: click "Create project" (accessibility)
Home->>Template: open template list
Test->>Template: select template (accessibility parent click)
Template->>Settings: open project settings
Test->>Settings: set project name (ACTION_SET_TEXT)
Test->>Settings: click "Create" / initialize
Settings->>Build: start project init & quick-run
Build->>Device: show package installer / permission UI
Test->>Device: pressBack to dismiss installer
Test->>Home: return to home (close project)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/src/androidTest/kotlin/com/itsaky/androidide/screens/ProjectSettingsScreen.kt (1)
116-124: Hardcoded "My Application" may break if default project name changes.The helper searches for text starting with "My Application" to locate the project name field. If the default project name changes or differs by locale, this will fail.
Consider using a resource ID-based lookup as a fallback, or documenting this assumption.
🤖 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/screens/ProjectSettingsScreen.kt` around lines 116 - 124, The locator in TestContext<Unit>.setProjectName uses a hardcoded UiSelector().textStartsWith("My Application") which is brittle; update setProjectName to first try a stable resource-id or content-description lookup (e.g., the view's resource id / accessibility label used by setAccessibilityEditText) and only fall back to textStartsWith when the id/description isn't found, or document the assumption if id isn't available; ensure you reference the existing helper setAccessibilityEditText and the UiSelector().textStartsWith("My Application") usage when making the change so the fallback strategy is implemented around those symbols.app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt (1)
94-98: Consider addingisEnabledandisVisibleToUserchecks for consistency.
clickFirstAccessibilityNodeByTextchecksisEnabledandisVisibleToUserbefore clicking, but this function only checksisClickable. This could lead to clicking disabled or invisible toolbar buttons.♻️ Suggested fix
val desc = node.contentDescription?.toString() ?: "" - if (!clicked && desc.contains(searchText, ignoreCase = true) && node.isClickable) { + if (!clicked && desc.contains(searchText, ignoreCase = true) + && node.isClickable && node.isEnabled && node.isVisibleToUser) { clicked = node.performAction(AccessibilityNodeInfo.ACTION_CLICK) }🤖 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 94 - 98, The loop that finds and clicks toolbar nodes only checks node.isClickable and may click disabled or invisible elements; update the logic in the same method (the loop iterating over nodes, using node.contentDescription, searchText, clicked, and node.performAction(AccessibilityNodeInfo.ACTION_CLICK)) to also verify node.isEnabled and node.isVisibleToUser before attempting performAction, mirroring the safeguards used in clickFirstAccessibilityNodeByText so you only click nodes that are clickable, enabled, and visible to the user.app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt (1)
220-232: Consider wrapping template operations in steps for better failure diagnostics.Currently only
clickCreateProjectHomeScreen()is inside astep(). IfselectProjectTemplate,setProjectName,clickCreateProjectProjectSettings, orinitializeProjectAndCancelBuildfail, the failure won't indicate which operation failed for each template.♻️ Proposed structure for better traceability
for ((index, config) in templates.withIndex()) { - step("Create+build template ${index + 1}/${templates.size}: ${config.label}") { - clickCreateProjectHomeScreen() - } - selectProjectTemplate("Select ${config.label} template", config.templateResId) - setProjectName(config.projectName) - clickCreateProjectProjectSettings() - initializeProjectAndCancelBuild() + step("Click create project for ${config.label}") { + clickCreateProjectHomeScreen() + } + step("Select ${config.label} template") { + selectProjectTemplate("Select ${config.label} template", config.templateResId) + } + step("Set project name for ${config.label}") { + setProjectName(config.projectName) + } + step("Click create project settings for ${config.label}") { + clickCreateProjectProjectSettings() + } + step("Initialize and cancel build for ${config.label}") { + initializeProjectAndCancelBuild() + } if (index < templates.lastIndex) { - ensureOnHomeScreenBeforeCreateProject() + step("Return to home screen after ${config.label}") { + ensureOnHomeScreenBeforeCreateProject() + } } }🤖 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 220 - 232, The current step("Create+build template ...") only wraps clickCreateProjectHomeScreen(), which loses per-operation context on failure; change the step around the whole template sequence so that selectProjectTemplate, setProjectName, clickCreateProjectProjectSettings, and initializeProjectAndCancelBuild are executed inside the same step block (keeping the step label using config.label and index), e.g. open the step before clickCreateProjectHomeScreen() and close it after initializeProjectAndCancelBuild so failures report the template and specific step, and keep ensureOnHomeScreenBeforeCreateProject() outside the step for the index < templates.lastIndex case.
🤖 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 125-135: The parent chain traversal in
DevicePermissionGrantUiHelper iterates using current: AccessibilityNodeInfo? =
node and sets current = current.parent but never recycles those parent
instances, leaking AccessibilityNodeInfo objects; fix by capturing
current.parent into a temporary variable (e.g., parent = current.parent), and
after using current (checking isClickable and performAction) call
current.recycle() before assigning current = parent so every node retrieved via
.parent is recycled, taking care not to recycle the original `node` twice (only
recycle it once at the end if not already recycled).
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.kt`:
- Around line 71-77: The code re-uses the existing UiObject saveBtn after
waitForExists returns false, but UiObject doesn't re-query the UI - recreate the
object after the back press and before checking existence again; specifically,
after d.pressBack() and d.waitForIdle(), reassign saveBtn =
d.findObject(UiSelector().text("Save files and close project")) (or create a new
local variable) and then call waitForExists(3_000) on that new UiObject to
ensure the UI tree is re-searched.
---
Nitpick comments:
In `@app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt`:
- Around line 220-232: The current step("Create+build template ...") only wraps
clickCreateProjectHomeScreen(), which loses per-operation context on failure;
change the step around the whole template sequence so that
selectProjectTemplate, setProjectName, clickCreateProjectProjectSettings, and
initializeProjectAndCancelBuild are executed inside the same step block (keeping
the step label using config.label and index), e.g. open the step before
clickCreateProjectHomeScreen() and close it after
initializeProjectAndCancelBuild so failures report the template and specific
step, and keep ensureOnHomeScreenBeforeCreateProject() outside the step for the
index < templates.lastIndex case.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt`:
- Around line 94-98: The loop that finds and clicks toolbar nodes only checks
node.isClickable and may click disabled or invisible elements; update the logic
in the same method (the loop iterating over nodes, using
node.contentDescription, searchText, clicked, and
node.performAction(AccessibilityNodeInfo.ACTION_CLICK)) to also verify
node.isEnabled and node.isVisibleToUser before attempting performAction,
mirroring the safeguards used in clickFirstAccessibilityNodeByText so you only
click nodes that are clickable, enabled, and visible to the user.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/screens/ProjectSettingsScreen.kt`:
- Around line 116-124: The locator in TestContext<Unit>.setProjectName uses a
hardcoded UiSelector().textStartsWith("My Application") which is brittle; update
setProjectName to first try a stable resource-id or content-description lookup
(e.g., the view's resource id / accessibility label used by
setAccessibilityEditText) and only fall back to textStartsWith when the
id/description isn't found, or document the assumption if id isn't available;
ensure you reference the existing helper setAccessibilityEditText and the
UiSelector().textStartsWith("My Application") usage when making the change so
the fallback strategy is implemented around those symbols.
🪄 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: f1016b59-e58a-41d7-b8ef-b41a7dc3e7f0
📒 Files selected for processing (6)
app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.ktapp/src/androidTest/kotlin/com/itsaky/androidide/screens/HomeScreen.ktapp/src/androidTest/kotlin/com/itsaky/androidide/screens/ProjectSettingsScreen.ktapp/src/androidTest/kotlin/com/itsaky/androidide/screens/TemplateScreen.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt (1)
136-147: Ensure root/node recycling is exception-safe.
rootis recycled only afterfindAccessibilityNodeInfosByTextsucceeds, andnode.recycle()is skipped ifaction(node)throws. Wrapping both operations intry/finallyprevents leaks in failure paths.♻️ Proposed fix
- val nodes = root.findAccessibilityNodeInfosByText(searchText) var success = false try { + val nodes = root.findAccessibilityNodeInfosByText(searchText) for (node in nodes) { - if (!success) { - success = action(node) + try { + if (!success) { + success = action(node) + } + } 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 136 - 147, The code currently calls root.findAccessibilityNodeInfosByText(...) and recycles root only in the outer finally, but if findAccessibilityNodeInfosByText or action(node) throws some nodes may never be recycled; change the structure so root and every node are always recycled: after obtaining nodes from root.findAccessibilityNodeInfosByText(searchText) wrap the node iteration in a try/finally where each node is recycled in an inner finally (e.g., for each node do try { success = success || action(node) } finally { node.recycle() }), and then in the outer finally call root.recycle(); ensure you reference the existing symbols root, nodes, action(node), node.recycle(), root.recycle(), and findAccessibilityNodeInfosByText(searchText) when applying the change.
🤖 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`:
- Line 60: The KDoc for the function in DevicePermissionGrantUiHelper that uses
check(success) is incorrect (it states AssertionError) — update the
documentation to state that the call will throw IllegalStateException when no
button is found (include context mentioning the check(success) call and the
errorLabel used in the message), or alternatively change the runtime check to
throw AssertionError if you prefer that contract; modify the KDoc and/or the
check usage so the documented exception type matches the actual thrown type.
---
Nitpick comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.kt`:
- Around line 136-147: The code currently calls
root.findAccessibilityNodeInfosByText(...) and recycles root only in the outer
finally, but if findAccessibilityNodeInfosByText or action(node) throws some
nodes may never be recycled; change the structure so root and every node are
always recycled: after obtaining nodes from
root.findAccessibilityNodeInfosByText(searchText) wrap the node iteration in a
try/finally where each node is recycled in an inner finally (e.g., for each node
do try { success = success || action(node) } finally { node.recycle() }), and
then in the outer finally call root.recycle(); ensure you reference the existing
symbols root, nodes, action(node), node.recycle(), root.recycle(), and
findAccessibilityNodeInfosByText(searchText) when applying the change.
🪄 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: a6c69f5e-4ac6-4274-ae71-f193ee5f9cb4
📒 Files selected for processing (2)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/DevicePermissionGrantUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.kt
Extend the end2end test to iterate over the 1st three project templates
It's tricky getting the tests to not get blocked by the universal tooltips
Added a template loop to EndToEndTest that iterates over three templates (No Activity, Empty Activity, Basic Activity), and for each one: clicks "Create project" on the home screen, selects the template from the list, sets a meaningful project name, clicks "Create project" on the settings page, then runs the InitializationProjectAndCancelingBuildScenario which dismisses the first-build notice, clicks "Sync project" to trigger Gradle initialization, waits for "Project initialized", clicks "Quick run" to build the APK, waits for the system package installer to appear (confirming the APK was built successfully), dismisses it, and closes the project back to the home screen before the next iteration. The key technical challenge was that the IDETooltipWebviewFragment creates a transparent WebView overlay that intercepts all coordinate-based clicks (both Espresso and UiAutomator), so had to use accessibility ACTION_CLICK via clickFirstAccessibilityNodeByText and related helpers throughout. Extracted three new shared helpers (clickFirstAccessibilityNodeByDescription, clickFirstAccessibilityNodeParentByText, and setAccessibilityEditText) into DevicePermissionGrantUiHelper to eliminate duplicated accessibility tree traversal code across the screen objects and scenario.