ADFA-2738 fix connected v8 debug android test#1133
Open
hal-eisen-adfa wants to merge 4 commits intostagefrom
Open
ADFA-2738 fix connected v8 debug android test#1133hal-eisen-adfa wants to merge 4 commits intostagefrom
hal-eisen-adfa wants to merge 4 commits intostagefrom
Conversation
Contributor
📝 WalkthroughRelease Notes - ADFA-2738: Fix Connected V8 Debug Android TestTest Infrastructure Improvements
Test Execution Enhancements
New Test Automation Helpers
Production Code Changes
Build Configuration Updates
|
| Cohort / File(s) | Summary |
|---|---|
Onboarding & Permission UI Helpers app/src/androidTest/kotlin/com/itsaky/androidide/helper/GrantRequiredPermissionsUiHelper.kt, app/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPrivacyHelper.kt, app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt |
Added TestContext helpers to accept privacy, advance onboarding slides, grant required permissions via onboarding UI, and ensure normalized home/editor state before project creation. |
Device Permission Extensions app/src/androidTest/kotlin/com/itsaky/androidide/helper/PermissionGrantDeviceExt.kt |
New Device extension functions to automate system permission flows (post-notifications, display-over-apps, storage, install-unknown-apps) with retries/fallbacks and final assertions. |
Navigate / Scenario / Test Restructure app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt, app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/EditorNewTemplateProjectScenario.kt, app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.kt, app/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithKtsGradle.kt, app/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithGroovyGradle.kt |
Removed inline system-permission flows, added EditorNewTemplateProjectScenario, deleted old initialization scenario, consolidated multiple template tests into a Kotlin smoke flow, and annotated tests with AndroidJUnit4 runner. |
Screens & UI matchers app/src/androidTest/kotlin/com/itsaky/androidide/screens/OnboardingScreen.kt, app/src/androidTest/kotlin/com/itsaky/androidide/screens/PermissionScreen.kt, app/src/androidTest/kotlin/com/itsaky/androidide/screens/HomeScreen.kt, app/src/androidTest/kotlin/com/itsaky/androidide/screens/ProjectSettingsScreen.kt |
Adjusted view matchers (nextButton → KImageView), added finishInstallationButton, changed create-project click flow to UiAutomator-first with Kakao fallback, and wrapped create click in flakySafely where applicable. |
First-build / Editor helpers & automation app/src/androidTest/kotlin/com/itsaky/androidide/helper/FirstBuildDialogHelper.kt, app/src/androidTest/kotlin/com/itsaky/androidide/helper/EditorTemplateProjectUiHelper.kt, app/src/androidTest/kotlin/com/itsaky/androidide/helper/ProjectBuildHelper.kt |
Added first-build dismissal helper, project-init and build-wait utilities, quick-run click helper, instrumentation logging, and invoked ensure-on-home-screen before creating projects. |
Permission-related test updates app/src/androidTest/kotlin/com/itsaky/androidide/PermissionsScreenTest.kt, app/src/androidTest/kotlin/com/itsaky/androidide/WelcomeScreenTest.kt, app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt |
Tests now derive required permissions dynamically, use onboarding helpers for granting, replace sleeps with flakySafely, and update permission-row assertions and finish-button click flow. |
Device/test-runner & build-config testing/android/src/main/java/com/itsaky/androidide/testing/android/TestInstrumentationRunner.kt, composite-builds/build-deps/javapoet/build.gradle.kts, composite-builds/build-logic/common/src/main/java/.../VersionUtils.kt, gradle.properties |
TestInstrumentationRunner disables global animations best-effort; javapoet compile target set to Java 17; VersionUtils no longer fails CI on metadata download; added kotlin.incremental.useClasspathSnapshot=false. |
Instrumentation probes & formatter app/src/main/java/com/itsaky/androidide/testing/InstrumentationStateProbe.kt, app/src/main/java/com/itsaky/androidide/testing/WaitProgressFormatter.kt, app/src/test/java/com/itsaky/androidide/testing/WaitProgressFormatterTest.kt |
Added InstrumentationStateProbe (projectInit/build states), WaitProgressFormatter and unit tests for its output formatting. |
Runtime/test-mode gating app/src/main/java/com/itsaky/androidide/activities/OnboardingActivity.kt, app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt, app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt |
Gated pulse animations and first-build dialog display when in test mode to reduce flaky UI animations/dialogs during tests. |
Sequence Diagram
sequenceDiagram
participant Test as Test Scenario
participant Onboarding as Onboarding UI
participant PermScreen as Permission Screen
participant Device as Device/System UI
participant Helper as Permission Helper
Test->>Onboarding: passPermissionsInfoSlideWithPrivacyDialog()
Onboarding->>Onboarding: accept privacy, click next
Test->>PermScreen: wait for permissions list
PermScreen-->>Test: list visible
Test->>Helper: grantAllRequiredPermissionsThroughOnboardingUi()
loop per-permission
Test->>PermScreen: click grant button (row N)
PermScreen->>Device: open system grant UI
Device->>Device: perform system consent flow
Device-->>Test: return to app
Test->>Helper: verify permission granted
end
Test->>PermScreen: click finishInstallationButton
Test->>Test: ensureOnHomeScreenBeforeCreateProject()
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- ADFA-2212: Implement native Support for Creating Code On the Go Plugins in Code On the Go #744 — modifies ProjectHandlerActivity; overlaps instrumentation/probe-related changes.
- ADFA-2784: add Gradle build tuner #973 — touches ProjectHandlerActivity initialization/build logic similar to this PR.
Suggested reviewers
- itsaky-adfa
- jatezzz
Poem
🐇
I hopped through tests with tiny paws,
Accepted privacy without a pause,
Clicked each grant in tidy rows,
Quieted animations as testing goes,
Java seventeen hums while the rabbit bows.
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately reflects a key objective of the PR—fixing the connected V8 debug Android test suite that had become stale. |
| Description check | ✅ Passed | The description directly addresses the problem: tests were stale and stuck waiting for buttons to become idle, and includes testing methodology and execution instructions. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
ADFA-2738-fix-connectedV8DebugAndroidTest
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 help to get the list of available commands and usage tips.
![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: 2
🧹 Nitpick comments (4)
composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/VersionUtils.kt (1)
75-81: Caching the fallback value may mask transient network failures.Setting
cachedVersion = LATEST_INTEGRATIONon failure means subsequent calls within the same build will return the fallback without retrying, even if the network becomes available. Additionally, the cached log at line 55 would misleadingly print "Found latest version... (cached)" for what was actually a fallback.Consider only caching successful lookups:
♻️ Proposed fix
} catch (err: Throwable) { // Sonatype or DNS can be unavailable; `latest.integration` still resolves from // declared repositories. Do not fail the build (CI included) on metadata fetch alone. println("Failed to download $moduleMetadata: ${err.message}") - cachedVersion = LATEST_INTEGRATION return LATEST_INTEGRATION }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/VersionUtils.kt` around lines 75 - 81, The catch block in VersionUtils.kt that catches Throwable while resolving moduleMetadata must not set cachedVersion = LATEST_INTEGRATION; remove the assignment so transient network/DNS failures don't poison the cache, and simply return LATEST_INTEGRATION on error. Keep the existing println/error message but ensure any "cached" messaging (the log that prints "Found latest version... (cached)") only runs when cachedVersion was actually set from a successful lookup, not from this fallback path.app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt (1)
352-354: Avoid restarting the pulse animation on repeated state emissions.Consider matching the
OnboardingActivitypattern by checking for an existing animation before starting a new one.Suggested tweak
- if (!isTestMode()) { + if (!isTestMode() && finishButton?.animation == null) { finishButton?.startAnimation(pulseAnimation) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt` around lines 352 - 354, The pulse animation is restarted on every state emission; change the block that currently calls finishButton?.startAnimation(pulseAnimation) (guarded by isTestMode()) to first check whether the view already has that animation running (e.g., finishButton?.animation is null or not the same instance) and only call startAnimation(pulseAnimation) if there is no existing pulseAnimation on finishButton; follow the same pattern used in OnboardingActivity to avoid restarting the animation on repeated emissions.gradle.properties (1)
17-18: Add explicit rollback criteria for the classpath-snapshot workaround.This is a valid stability fix, but it’s easy for this global perf trade-off to become permanent. Please add a TODO with an issue link/condition for re-enabling it.
Suggested small follow-up
# Gradle 8.10+ / Kotlin: avoid kapt stub tasks failing when shrunk-classpath-snapshot.bin is missing. +# TODO(ADFA-2738): Re-enable when kapt/classpath-snapshot issue is resolved upstream. kotlin.incremental.useClasspathSnapshot=false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gradle.properties` around lines 17 - 18, Add an inline TODO comment immediately above the kotlin.incremental.useClasspathSnapshot=false property documenting rollback criteria and a ticket/link: state the temporary nature, reference a tracking issue or upstream fix URL (or a condition such as "re-enable when Gradle/Kotlin releases X.Y that fixes shrunk-classpath-snapshot.bin" or "when kotlin.incremental.useClasspathSnapshot is safe by default"), and include an owner or timeline for follow-up so the global perf trade-off isn't left permanent.testing/android/src/main/java/com/itsaky/androidide/testing/android/TestInstrumentationRunner.kt (1)
25-29: Please narrow theThrowablecatches here.Swallowing
Throwablewill also hide assertion failures and VM-level errors, which makes runner/setup regressions look like random UI flakiness. Limiting this to the shell/IO/security failures you actually expect keeps the best-effort behavior without masking real bugs.Based on learnings, prefer narrow exception handling that catches only the specific exception type reported in crashes instead of a broad catch-all.
Also applies to: 41-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testing/android/src/main/java/com/itsaky/androidide/testing/android/TestInstrumentationRunner.kt` around lines 25 - 29, Replace the broad "catch (_: Throwable)" around calls to disableGlobalAnimations() with narrow exception handlers that only swallow expected platform/IO/security failures (e.g., catch SecurityException and IOException) so real test/VM errors still surface; update both the disableGlobalAnimations() try block and the similar block at lines 41-47 to catch SecurityException and IOException (or other specific exceptions surfaced by the underlying shell call) and leave any other exceptions unhandled so they fail the test runner.
🤖 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/EnsureHomeScreenHelper.kt`:
- Around line 37-68: The close loop in EnsureHomeScreenHelper's step "Close
restored project and return to home if editor is open" can exit silently after 4
attempts while editor_appBarLayout (editorAppBarId) still exists; modify the
code after the while loop to check
device.uiDevice.findObject(UiSelector().resourceId(editorAppBarId)).exists() (or
use the same appBar waitForExists check) and throw an exception (or call fail)
if it still exists so flakySafely treats the step as failed; keep existing retry
logic (closeAttempts, flakySafely, EditorScreen->closeProjectDialog flow) but
ensure a final existence assertion/failure is performed when
EditorScreen/closeProjectDialog didn't close the editor.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/helper/GrantRequiredPermissionsUiHelper.kt`:
- Around line 22-35: The test should skip rows whose permission is already
granted instead of always trying to click the grant button; update the loop in
required.forEachIndexed to first locate PermissionScreen.PermissionItem at index
and check the grantButton's enabled/clickable state (or presence) before calling
click inside flakySafely, so pre-granted/special-access rows are not clicked;
reference PermissionScreen, PermissionItem, grantButton and
PermissionsHelper.getRequiredPermissions to find the logic to change.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt`:
- Around line 352-354: The pulse animation is restarted on every state emission;
change the block that currently calls
finishButton?.startAnimation(pulseAnimation) (guarded by isTestMode()) to first
check whether the view already has that animation running (e.g.,
finishButton?.animation is null or not the same instance) and only call
startAnimation(pulseAnimation) if there is no existing pulseAnimation on
finishButton; follow the same pattern used in OnboardingActivity to avoid
restarting the animation on repeated emissions.
In
`@composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/VersionUtils.kt`:
- Around line 75-81: The catch block in VersionUtils.kt that catches Throwable
while resolving moduleMetadata must not set cachedVersion = LATEST_INTEGRATION;
remove the assignment so transient network/DNS failures don't poison the cache,
and simply return LATEST_INTEGRATION on error. Keep the existing println/error
message but ensure any "cached" messaging (the log that prints "Found latest
version... (cached)") only runs when cachedVersion was actually set from a
successful lookup, not from this fallback path.
In `@gradle.properties`:
- Around line 17-18: Add an inline TODO comment immediately above the
kotlin.incremental.useClasspathSnapshot=false property documenting rollback
criteria and a ticket/link: state the temporary nature, reference a tracking
issue or upstream fix URL (or a condition such as "re-enable when Gradle/Kotlin
releases X.Y that fixes shrunk-classpath-snapshot.bin" or "when
kotlin.incremental.useClasspathSnapshot is safe by default"), and include an
owner or timeline for follow-up so the global perf trade-off isn't left
permanent.
In
`@testing/android/src/main/java/com/itsaky/androidide/testing/android/TestInstrumentationRunner.kt`:
- Around line 25-29: Replace the broad "catch (_: Throwable)" around calls to
disableGlobalAnimations() with narrow exception handlers that only swallow
expected platform/IO/security failures (e.g., catch SecurityException and
IOException) so real test/VM errors still surface; update both the
disableGlobalAnimations() try block and the similar block at lines 41-47 to
catch SecurityException and IOException (or other specific exceptions surfaced
by the underlying shell call) and leave any other exceptions unhandled so they
fail the test runner.
🪄 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: 4cd10a2d-9730-4f07-9b8e-0a836db28afe
📒 Files selected for processing (17)
app/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/EnsureHomeScreenHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/GrantRequiredPermissionsUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPrivacyHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/PermissionGrantDeviceExt.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.ktapp/src/androidTest/kotlin/com/itsaky/androidide/screens/OnboardingScreen.ktapp/src/androidTest/kotlin/com/itsaky/androidide/screens/PermissionScreen.ktapp/src/main/java/com/itsaky/androidide/activities/OnboardingActivity.ktapp/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.ktcomposite-builds/build-deps/javapoet/build.gradle.ktscomposite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/VersionUtils.ktgradle.propertiestesting/android/src/main/java/com/itsaky/androidide/testing/android/TestInstrumentationRunner.kt
app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/kotlin/com/itsaky/androidide/helper/GrantRequiredPermissionsUiHelper.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/ProjectBuildHelper.kt (1)
12-12: Use appropriate log level — ERROR is for actual errors.
Log.eis for error conditions; this is an informational message. UsingLog.iorLog.dwould be more appropriate.🔧 Change log level from ERROR to INFO
fun TestContext<Unit>.navigateToMainScreen() { - Log.e(TAG_NAV_MAIN, "navigateToMainScreen() starting") + Log.i(TAG_NAV_MAIN, "navigateToMainScreen() starting") System.err.println("$TAG_NAV_MAIN: starting")🤖 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/ProjectBuildHelper.kt` at line 12, The log call using Log.e(TAG_NAV_MAIN, "navigateToMainScreen() starting") is using the ERROR level for an informational message; change it to an appropriate level such as Log.i or Log.d in the navigateToMainScreen() code in ProjectBuildHelper (keep TAG_NAV_MAIN as the tag) so the message is logged as info/debug rather than an error.app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt (1)
89-95: Clarify the purpose of the final deny dialog step.After granting all permissions through the onboarding UI, under what conditions would a runtime permission dialog still appear? If this is a safety net for edge cases, a brief comment would help maintainability.
📝 Optional: Add clarifying comment
+ // Safety net: dismiss any unexpected runtime permission prompts that may appear + // after onboarding (e.g., if system re-prompts or UI state is inconsistent). step("Decline runtime permission dialog if still shown") { runCatching { flakySafely(timeoutMs = 8000) { device.permissions.denyViaDialog() } } }🤖 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 89 - 95, Add a short clarifying comment above the step("Decline runtime permission dialog if still shown") block explaining why this safety-net exists: note that even after granting permissions via the onboarding UI the system runtime permission dialog can still appear in some OS/device configurations or flakily on CI/emulators, so we call flakySafely { device.permissions.denyViaDialog() } wrapped in runCatching to defensively dismiss it; reference the step text, flakySafely, runCatching and device.permissions.denyViaDialog() to make locating the code easy.app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/EditorNewTemplateProjectScenario.kt (1)
44-71: Consider adding a post-close verification for home screen state.The scenario closes the project but doesn't verify the app returned to the home screen. Tests in the loop pattern (
clickCreateProjectHomeScreen()after this) rely on this implicit behavior. Adding a brief check could catch navigation regressions early.💡 Optional: Add home screen verification after close
step("Close project from editor") { flakySafely(timeoutMs = 35_000) { d.waitForIdle(2000) d.pressBack() val first = runCatching { EditorScreen { closeProjectDialog { positiveButton { hasText(ctx.getString(R.string.save_and_close)) click() } } } } if (first.isFailure) { d.waitForIdle(2000) d.pressBack() EditorScreen { closeProjectDialog { positiveButton { hasText(ctx.getString(R.string.save_and_close)) click() } } } } + // Allow navigation to complete after dialog dismissal + d.waitForIdle(3000) } }🤖 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/EditorNewTemplateProjectScenario.kt` around lines 44 - 71, After closing the project in the EditorNewTemplateProjectScenario (inside the flakySafely block where EditorScreen { closeProjectDialog { positiveButton { ... } } } is used), add a short post-close verification that the app returned to the home screen before continuing (e.g., call HomeScreen { /* assert a known home element exists or call clickCreateProjectHomeScreen() */ } or wait for an element like the create-project button), retry once on failure consistent with the existing retry pattern, and ensure this check is placed after both the primary and fallback close attempts so subsequent steps relying on clickCreateProjectHomeScreen() see the expected home state.app/src/androidTest/kotlin/com/itsaky/androidide/screens/HomeScreen.kt (1)
88-96: Consider logging when UiAutomator click succeeds.The function returns
trueimmediately afterrunCatching { node.click() }without checking the click result or logging which selector matched. Adding a log would help debug test failures.💡 Optional: Add logging for successful click
for (sel in selectors) { val node = d.findObject(sel) if (node.waitForExists(8000) && node.exists() && node.isEnabled) { - runCatching { node.click() } + val clicked = runCatching { node.click() }.getOrDefault(false) + if (clicked) { + Log.i("HomeScreen", "Clicked create project via UiAutomator") + } d.waitForIdle(2000) return true } }🤖 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/HomeScreen.kt` around lines 88 - 96, The loop in HomeScreen that finds and clicks selectors uses runCatching { node.click() } and returns true immediately without verifying the click or logging which selector matched; update the code in the function that iterates over selectors (uses d.findObject(sel), node.waitForExists, node.click()) to capture the runCatching result, check that the click succeeded (no exception and returned true if applicable), log the successful selector (e.g., Log.i or the test logger with the selector string and node info) and only then return true; if the click failed, log the failure and continue to the next selector.
🤖 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/ProjectBuildHelper.kt`:
- Line 12: The log call using Log.e(TAG_NAV_MAIN, "navigateToMainScreen()
starting") is using the ERROR level for an informational message; change it to
an appropriate level such as Log.i or Log.d in the navigateToMainScreen() code
in ProjectBuildHelper (keep TAG_NAV_MAIN as the tag) so the message is logged as
info/debug rather than an error.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/EditorNewTemplateProjectScenario.kt`:
- Around line 44-71: After closing the project in the
EditorNewTemplateProjectScenario (inside the flakySafely block where
EditorScreen { closeProjectDialog { positiveButton { ... } } } is used), add a
short post-close verification that the app returned to the home screen before
continuing (e.g., call HomeScreen { /* assert a known home element exists or
call clickCreateProjectHomeScreen() */ } or wait for an element like the
create-project button), retry once on failure consistent with the existing retry
pattern, and ensure this check is placed after both the primary and fallback
close attempts so subsequent steps relying on clickCreateProjectHomeScreen() see
the expected home state.
In
`@app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt`:
- Around line 89-95: Add a short clarifying comment above the step("Decline
runtime permission dialog if still shown") block explaining why this safety-net
exists: note that even after granting permissions via the onboarding UI the
system runtime permission dialog can still appear in some OS/device
configurations or flakily on CI/emulators, so we call flakySafely {
device.permissions.denyViaDialog() } wrapped in runCatching to defensively
dismiss it; reference the step text, flakySafely, runCatching and
device.permissions.denyViaDialog() to make locating the code easy.
In `@app/src/androidTest/kotlin/com/itsaky/androidide/screens/HomeScreen.kt`:
- Around line 88-96: The loop in HomeScreen that finds and clicks selectors uses
runCatching { node.click() } and returns true immediately without verifying the
click or logging which selector matched; update the code in the function that
iterates over selectors (uses d.findObject(sel), node.waitForExists,
node.click()) to capture the runCatching result, check that the click succeeded
(no exception and returned true if applicable), log the successful selector
(e.g., Log.i or the test logger with the selector string and node info) and only
then return true; if the click failed, log the failure and continue to the next
selector.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48c98af1-24f1-47ea-900e-d0b2bc1ace13
📒 Files selected for processing (10)
app/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithKtsGradle.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/EditorTemplateProjectUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/FirstBuildDialogHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/ProjectBuildHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/EditorNewTemplateProjectScenario.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.ktapp/src/androidTest/kotlin/com/itsaky/androidide/screens/HomeScreen.ktapp/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt
💤 Files with no reviewable changes (1)
- app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/InitializationProjectAndCancelingBuildScenario.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithKtsGradle.kt
- app/src/androidTest/kotlin/com/itsaky/androidide/helper/EnsureHomeScreenHelper.kt
hal-eisen-adfa
requested review from
Daniel-ADFA,
dara-abijo-adfa,
itsaky-adfa and
jatezzz
…emplate iteration bug
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/EditorTemplateProjectUiHelper.kt (1)
208-237: Hardcoded max minutes doesn't match thetimeoutMsparameter.Lines 218 and 229 hardcode "5m max" and
maxMinutes = 5, but the actual timeout is passed viatimeoutMs. If timeout values change, the logged message will be inconsistent.♻️ Suggested fix to derive max minutes from timeout
private fun logMinuteProgress( waitName: String, timeoutMs: Long, deadlineMs: Long, lastMinuteReported: Long, ): Long { val now = System.currentTimeMillis() val waitedMs = (timeoutMs - (deadlineMs - now)).coerceAtLeast(0L) - val elapsedMinutes = ((timeoutMs - (deadlineMs - now)).coerceAtLeast(0L)) / 60_000L + val elapsedMinutes = waitedMs / 60_000L + val maxMinutes = timeoutMs / 60_000L if (lastMinuteReported == 0L && waitedMs >= 5_000L) { - val started = "waiting for $waitName (5m max): started" + val started = "waiting for $waitName (${maxMinutes}m max): started" Log.i(TAG, started) System.err.println("$TAG: $started") println("$TAG: $started") } if (elapsedMinutes >= 1 && elapsedMinutes > lastMinuteReported) { val msg = WaitProgressFormatter.formatMinutesElapsedMessage( waitName = waitName, elapsedMinutes = elapsedMinutes, - maxMinutes = 5, + maxMinutes = maxMinutes, ) Log.i(TAG, msg) System.err.println("$TAG: $msg") println("$TAG: $msg") sendInstrumentationProgress(msg) return elapsedMinutes } return lastMinuteReported }🤖 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/EditorTemplateProjectUiHelper.kt` around lines 208 - 237, In logMinuteProgress, the "5m max" string and maxMinutes = 5 are hardcoded; compute maxMinutes from the timeoutMs parameter (e.g. val maxMinutes = (timeoutMs + 59_999L) / 60_000L or another appropriate minute conversion) and use that value both in the started message and when calling WaitProgressFormatter.formatMinutesElapsedMessage; update the started log string from "waiting for $waitName (5m max): started" to use the computed maxMinutes and pass maxMinutes to formatMinutesElapsedMessage so logs always reflect the actual timeout.
🤖 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/EditorTemplateProjectUiHelper.kt`:
- Around line 208-237: In logMinuteProgress, the "5m max" string and maxMinutes
= 5 are hardcoded; compute maxMinutes from the timeoutMs parameter (e.g. val
maxMinutes = (timeoutMs + 59_999L) / 60_000L or another appropriate minute
conversion) and use that value both in the started message and when calling
WaitProgressFormatter.formatMinutesElapsedMessage; update the started log string
from "waiting for $waitName (5m max): started" to use the computed maxMinutes
and pass maxMinutes to formatMinutesElapsedMessage so logs always reflect the
actual timeout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8511058c-28af-434d-8028-82f952d58447
📒 Files selected for processing (9)
app/src/androidTest/kotlin/com/itsaky/androidide/ProjectBuildTestWithKtsGradle.ktapp/src/androidTest/kotlin/com/itsaky/androidide/helper/EditorTemplateProjectUiHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/EditorNewTemplateProjectScenario.ktapp/src/androidTest/kotlin/com/itsaky/androidide/screens/ProjectSettingsScreen.ktapp/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.ktapp/src/main/java/com/itsaky/androidide/testing/InstrumentationStateProbe.ktapp/src/main/java/com/itsaky/androidide/testing/WaitProgressFormatter.ktapp/src/test/java/com/itsaky/androidide/testing/WaitProgressFormatterTest.kt
✅ Files skipped from review due to trivial changes (1)
- app/src/test/java/com/itsaky/androidide/testing/WaitProgressFormatterTest.kt
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
1 participant
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.
The tests were quite stale, and were stuck waiting for the infinite throbbing buttons to become idle
I had to test this on a special emulator running AOSP and gave it 8gb of RAM
Run with:
./gradlew :app:connectedV8DebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=com.itsaky.androidide.ProjectBuildTestWithKtsGradle