ADFA-4109: Harden e2e automation and add Gradle offline-mode toggle#1526
Conversation
![claude[bot]](https://cdn.github.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
📝 Walkthrough
WalkthroughThe instrumentation tests centralize privacy disclosure handling, refine permission assertions, add accessibility polling, configure optional offline builds, extend onboarding/build timeouts, and simplify home-screen project creation interactions. ChangesOnboarding automation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EndToEndTest
participant OnboardingHelpers
participant HomeScreen
participant BuildScenario
EndToEndTest->>OnboardingHelpers: configure permissions and privacy disclosure
EndToEndTest->>OnboardingHelpers: configureAutomationBuildPreferences()
EndToEndTest->>HomeScreen: create project from selected template
HomeScreen-->>EndToEndTest: project setup completed
EndToEndTest->>BuildScenario: initialize and run build
BuildScenario-->>EndToEndTest: build outcome or timeout
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt (1)
45-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNon-ASCII / decorative comment characters.
Same box-drawing dividers and em dash as flagged in
AutomationEndToEndTest.kt; see consolidated comment.Also applies to: 51-51, 66-66, 132-132, 142-142, 173-173
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt` at line 45, Replace the non-ASCII decorative divider and em dash characters in EndToEndTest with plain ASCII equivalents, including the affected comments at the referenced locations, while preserving the comments’ readability and meaning.Source: Coding guidelines
app/src/androidTest/kotlin/com/itsaky/androidide/AutomationEndToEndTest.kt (3)
49-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNon-ASCII / decorative comment characters.
These comments use box-drawing dividers (
── ──) and an em dash (—), both non-ASCII. As per coding guidelines,**/*.{java,kt,kts}should "Use ASCII in code and code comments where an equivalent ASCII character is available; prefer->,-, and straight quotes over non-ASCII equivalents," and should "not add separator or decorative comments such as banner bars." This finding recurs in other files in this PR; see consolidated comment.Also applies to: 56-56, 71-71, 137-137, 147-147, 213-213
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/com/itsaky/androidide/AutomationEndToEndTest.kt` at line 49, Remove the decorative separator comments in AutomationEndToEndTest, including the markers near the Launch section and the other referenced locations; do not replace them with alternative banner comments or non-ASCII characters.Source: Coding guidelines
49-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNon-ASCII characters and decorative banner comments across the changed test files. Several comments use box-drawing dividers (
── ──) and em dashes (—); the shared root cause is a comment-writing habit, and the fix is the same everywhere: swap to ASCII hyphens/plain comments. As per coding guidelines,**/*.{java,kt,kts}files must "Use ASCII in code and code comments where an equivalent ASCII character is available; prefer->,-, and straight quotes over non-ASCII equivalents," and must not "add separator or decorative comments such as banner bars."
app/src/androidTest/kotlin/com/itsaky/androidide/AutomationEndToEndTest.kt#L49-L213: replace the// ── ... ──dividers at lines 49, 56, 71, 147, 213 with plain ASCII comments (or remove them), and replace the em dash at line 137 with a hyphen.app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt#L45-L173: same fix for the dividers at lines 45, 51, 66, 142, 173 and the em dash at line 132.app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.kt#L188-L188: replace the em dash in theQUICK_RUN_TIMEOUT_MSrationale comment with a hyphen.app/src/androidTest/kotlin/com/itsaky/androidide/screens/HomeScreen.kt#L22-L22: replace the em dash in the accessibility-click rationale comment with a hyphen.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/com/itsaky/androidide/AutomationEndToEndTest.kt` around lines 49 - 213, Replace decorative box-drawing divider comments and non-ASCII em dashes with plain ASCII comments or remove the dividers. Apply this in AutomationEndToEndTest.kt (lines 49-213), EndToEndTest.kt (lines 45-173), InitializationProjectAndCancelingBuildScenario.kt (line 188), and HomeScreen.kt (line 22); preserve the surrounding test logic unchanged.Source: Coding guidelines
43-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftOnboarding flow (launch → welcome → privacy disclosure → permissions → wait-for-IDE → build-preferences) is duplicated almost verbatim between the two E2E test classes. Both files independently repeat launch, welcome-screen assertions,
handlePrivacyDisclosure(), the permission-item assertion block (including the identical!item.isGranted && !item.isOptionalconditional), grant/confirm steps, andconfigureAutomationBuildPreferences(). This is a real maintenance risk: any future UI/permission change requires updating this logic in two places in lockstep.
app/src/androidTest/kotlin/com/itsaky/androidide/AutomationEndToEndTest.kt#L43-L149: extract this block into a sharedTestContext<Unit>.runOnboardingFlow()helper (e.g., in thehelperpackage) and call it from here.app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt#L41-L144: call the same sharedrunOnboardingFlow()helper instead of re-implementing the identical steps.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/com/itsaky/androidide/AutomationEndToEndTest.kt` around lines 43 - 149, Extract the duplicated onboarding sequence into a shared TestContext<Unit>.runOnboardingFlow() helper, preserving launch, welcome assertions, privacy disclosure, permission validation/granting, completion checks, IDE setup wait, and configureAutomationBuildPreferences(). Replace the duplicated block in app/src/androidTest/kotlin/com/itsaky/androidide/AutomationEndToEndTest.kt lines 43-149 and app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt lines 41-144 with calls to this helper; both sites require the same change.Source: Coding guidelines
app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.kt (1)
180-193: 🚀 Performance & Scalability | 🔵 TrivialConsider the cumulative CI time budget from these higher ceilings.
PROJECT_INIT_TIMEOUT_MSandQUICK_RUN_TIMEOUT_MStogether allow up to 35 minutes per template iteration in the worst case. With the expanded template matrix inAutomationEndToEndTest.kt(up to 15 iterations), a single stuck run could consume many hours of CI time before failing. The rationale comments are solid; just worth confirming the CI job has an overall wall-clock timeout/kill-switch so a single hang doesn't block the pipeline for hours.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.kt` around lines 180 - 193, Confirm that the CI job running the template iterations has an overall wall-clock timeout or kill-switch that bounds cumulative execution across all iterations using PROJECT_INIT_TIMEOUT_MS and QUICK_RUN_TIMEOUT_MS. If no such limit exists, add one at the CI job level rather than changing these per-scenario timeout constants.app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt (4)
168-177: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid cumulative UI Automator timeouts in sequential selector searches.
Because
waitForExistsblocks for the full timeout duration if the selector is never met, this sequential fallback approach will accumulate massive dead-time delays. For example, if the actual UI node matches the third selector in a list, the test will needlessly sleep for the full timeout of the first two selectors before finding the third one.
To prevent slowing down the E2E suite, do a single fast-polling loop that checks all selectors on each iteration, rather than blocking sequentially.
app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt#L168-L177: refactor the 5000ms drawer navigation search to use a unified polling loop.app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt#L193-L202: refactor the 10,000ms close-project control search to avoid cumulative delays.app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt#L208-L215: refactor the 20,000ms close-without-saving control search to avoid cumulative delays.♻️ Proposed refactor for fast selector checking (example for drawer nav)
val navToClick = - listOf( - UiSelector().description(openDrawer), - UiSelector().description(closeDrawer), - UiSelector().descriptionContains(openDrawer), - UiSelector().descriptionContains(closeDrawer), - ).firstNotNullOfOrNull { sel -> - val o = d.findObject(sel) - if (o.waitForExists(5000) && o.exists()) o else null - } + run { + val selectors = listOf( + UiSelector().description(openDrawer), + UiSelector().description(closeDrawer), + UiSelector().descriptionContains(openDrawer), + UiSelector().descriptionContains(closeDrawer) + ) + + // Try a fast pass without blocking on each fallback + val deadline = System.currentTimeMillis() + 5000 + while (System.currentTimeMillis() < deadline) { + val found = selectors.firstNotNullOfOrNull { sel -> + val o = d.findObject(sel) + if (o.exists()) o else null + } + if (found != null) return@run found + Thread.sleep(250) + } + null + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt` around lines 168 - 177, The sequential selector searches in EnsureHomeScreenHelper.kt at lines 168-177, 193-202, and 208-215 cumulatively block on waitForExists timeouts. Replace each search with a unified fast-polling loop that checks every selector on each iteration and stops when a matching existing node is found, preserving the respective 5,000 ms, 10,000 ms, and 20,000 ms overall time limits.
193-202: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid cumulative UI Automator timeouts in sequential selector searches.
Similar to the drawer navigation search above, sequentially calling
waitForExists(10000)on multiple fallback selectors will introduce up to 30 seconds of dead wait time if the valid selector is the last one in the list. Consider using a single polling loop that quickly checks all selectors per iteration instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt` around lines 193 - 202, The close-project control lookup in EnsureHomeScreenHelper currently waits up to 10 seconds for each fallback selector sequentially. Replace the firstNotNullOfOrNull and per-selector waitForExists calls with a shared polling loop that quickly checks all four selectors on each iteration, returning the first existing control while preserving the existing error when none is found.
208-215: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid cumulative UI Automator timeouts in sequential selector searches.
Sequentially waiting up to 20,000ms per selector can introduce a 20-second unnecessary delay if the first selector is not the match. Please update this to use a fast-polling approach across all selectors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt` around lines 208 - 215, Update the withoutSaving selector search in EnsureHomeScreenHelper to avoid calling waitForExists(20000) separately for each selector; use a fast-polling approach that checks all selectors within a shared timeout so a failed first selector does not add another 20-second delay. Preserve the existing matched-object result and “Close without saving not found” error behavior.
168-177: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid cumulative UI Automator timeouts in sequential selector searches.
Because
waitForExistsblocks for the full timeout duration if the selector is never met, this sequential fallback approach will accumulate massive delays. If the actual navigation button matches the third selector, the test will needlessly sleep for 10 seconds (5000ms + 5000ms) waiting for the first two selectors to appear before finally finding the third one.To prevent significant dead time in tests, do a fast initial pass over all selectors before falling back to a longer wait, or use a single polling loop that quickly checks all selectors on each iteration.
♻️ Proposed refactor for fast selector checking
val navToClick = - listOf( - UiSelector().description(openDrawer), - UiSelector().description(closeDrawer), - UiSelector().descriptionContains(openDrawer), - UiSelector().descriptionContains(closeDrawer), - ).firstNotNullOfOrNull { sel -> - val o = d.findObject(sel) - if (o.waitForExists(5000) && o.exists()) o else null - } + run { + val selectors = listOf( + UiSelector().description(openDrawer), + UiSelector().description(closeDrawer), + UiSelector().descriptionContains(openDrawer), + UiSelector().descriptionContains(closeDrawer) + ) + + // Try a fast pass without blocking on each fallback + val deadline = System.currentTimeMillis() + 5000 + while (System.currentTimeMillis() < deadline) { + val found = selectors.firstNotNullOfOrNull { sel -> + val o = d.findObject(sel) + if (o.exists()) o else null + } + if (found != null) return@run found + Thread.sleep(250) + } + null + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt` around lines 168 - 177, Update the selector lookup in the navigation-button flow around navToClick to avoid sequential 5000ms waits for each UiSelector. First perform a non-blocking or very short existence check across all selectors, then use a bounded polling/fallback strategy that checks every selector on each iteration, preserving the existing selector priority and null result when none are found.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/HandlePrivacyDisclosureHelper.kt`:
- Line 3: Replace the android.util.Log import and its usages in
HandlePrivacyDisclosureHelper with the project’s LoggerFactory-based SLF4J
logger, following the existing app logging pattern while preserving the current
log messages and levels.
---
Nitpick comments:
In `@app/src/androidTest/kotlin/com/itsaky/androidide/AutomationEndToEndTest.kt`:
- Line 49: Remove the decorative separator comments in AutomationEndToEndTest,
including the markers near the Launch section and the other referenced
locations; do not replace them with alternative banner comments or non-ASCII
characters.
- Around line 49-213: Replace decorative box-drawing divider comments and
non-ASCII em dashes with plain ASCII comments or remove the dividers. Apply this
in AutomationEndToEndTest.kt (lines 49-213), EndToEndTest.kt (lines 45-173),
InitializationProjectAndCancelingBuildScenario.kt (line 188), and HomeScreen.kt
(line 22); preserve the surrounding test logic unchanged.
- Around line 43-149: Extract the duplicated onboarding sequence into a shared
TestContext<Unit>.runOnboardingFlow() helper, preserving launch, welcome
assertions, privacy disclosure, permission validation/granting, completion
checks, IDE setup wait, and configureAutomationBuildPreferences(). Replace the
duplicated block in
app/src/androidTest/kotlin/com/itsaky/androidide/AutomationEndToEndTest.kt lines
43-149 and app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt
lines 41-144 with calls to this helper; both sites require the same change.
In `@app/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.kt`:
- Line 45: Replace the non-ASCII decorative divider and em dash characters in
EndToEndTest with plain ASCII equivalents, including the affected comments at
the referenced locations, while preserving the comments’ readability and
meaning.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt`:
- Around line 168-177: The sequential selector searches in
EnsureHomeScreenHelper.kt at lines 168-177, 193-202, and 208-215 cumulatively
block on waitForExists timeouts. Replace each search with a unified fast-polling
loop that checks every selector on each iteration and stops when a matching
existing node is found, preserving the respective 5,000 ms, 10,000 ms, and
20,000 ms overall time limits.
- Around line 193-202: The close-project control lookup in
EnsureHomeScreenHelper currently waits up to 10 seconds for each fallback
selector sequentially. Replace the firstNotNullOfOrNull and per-selector
waitForExists calls with a shared polling loop that quickly checks all four
selectors on each iteration, returning the first existing control while
preserving the existing error when none is found.
- Around line 208-215: Update the withoutSaving selector search in
EnsureHomeScreenHelper to avoid calling waitForExists(20000) separately for each
selector; use a fast-polling approach that checks all selectors within a shared
timeout so a failed first selector does not add another 20-second delay.
Preserve the existing matched-object result and “Close without saving not found”
error behavior.
- Around line 168-177: Update the selector lookup in the navigation-button flow
around navToClick to avoid sequential 5000ms waits for each UiSelector. First
perform a non-blocking or very short existence check across all selectors, then
use a bounded polling/fallback strategy that checks every selector on each
iteration, preserving the existing selector priority and null result when none
are found.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.kt`:
- Around line 180-193: Confirm that the CI job running the template iterations
has an overall wall-clock timeout or kill-switch that bounds cumulative
execution across all iterations using PROJECT_INIT_TIMEOUT_MS and
QUICK_RUN_TIMEOUT_MS. If no such limit exists, add one at the CI job level
rather than changing these per-scenario timeout constants.
🪄 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: 01a0057d-6e7c-48e9-af1b-dce512644f19
📒 Files selected for processing (9)
app/src/androidTest/kotlin/com/itsaky/androidide/AutomationEndToEndTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/EndToEndTest.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/ConfigureAutomationBuildPreferencesHelper.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/HandlePrivacyDisclosureHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.ktapp/src/androidTest/kotlin/com/itsaky/androidide/screens/HomeScreen.kt
|
ADFA-4109 is an epic. What is the problem being solved here? I need more context. |
|
@hal-eisen-adfa The mods made to androidtest of CodeOnTheGo to facilitate E2E testing of almost all the artifacts generated with the sub tickets under the ADFA-4109 epic. Not sure were to hang it off of. |
3 participants
Summary
Test-only changes (no IDE-side code modified) that take the APK-based
AutomationTestSuitefrom failing at onboarding to fully green 15/15template runs, and add a deterministic offline-mode toggle for automation.
androidide.build.offlineinstrumentation arg(
true/false; omit to keep the IDE default) so farm runs force generatedproject builds onto the asset-backed offline repository.
privacy.disclosure.shownflag instead of a timing probe — hard-asserts thedialog on fresh installs, asserts its absence on reruns, and verifies the
flag persists after accepting.
poll, 10 s ceiling) instead of failing on a single scan; a transiently null
root window is retryable.
R.string.create_projectinstead of a hardcoded literal that no longermatched the UI.
clickable Allow buttons for ungranted required rows.
5 -> 15 min, quick-run 10 -> 20 min): emulator throughput varies ~5x run to
run, and each old ceiling was hit while work was demonstrably still healthy
(sync completed at 311 s; pip setup active; dex-merge writing when killed).
Fast-fail error detection is unchanged, so genuine failures still surface
early.
Validation
Fully green
AutomationTestSuiteruns (15/15 templates) on the API 36 arm64emulator via
test-farm/scripts/run-local-v8-debug-apks.sh, including one runof the exact final branch state. Durations are environment-dominated: 73 min
(healthy) to ~4 h (degraded emulator). Evidence in
test-farm/results/;diagnostic history in
wiki/testing/maintaining-tests.md(Issues 6, 9, 10 andthe July 14 updates).
Review notes
Three commits, reviewable independently:
spotlessCheckpasses