Skip to content

test(e2e): harden syncToggleReceivesRealTap with bounded tap retry - #287

Merged
ErikBjare merged 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/e2e-sync-toggle-tap-retry
Sep 17, 2026
Merged

ErikBjare merged 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/e2e-sync-toggle-tap-retry

Conversation

@TimeToBuildBob

Copy link
Copy Markdown
Contributor

Problem

NativeWindowInsetsTest.syncToggleReceivesRealTap continues to fail intermittently
in Test E2E (29) even after the UiAutomator-node fix (#266) added awaitSyncEnabled:

date run notes
2026-09-14 35185908412 no ANR; unrelated diff
2026-09-16 35080080197 ANR in com.android.systemui — tap eaten by dialog
2026-09-17 35196333754 no ANR; master post-merge
2026-09-17 35219470515 no ANR; #273 head

The ANR class is a known emulator flake. The no-ANR class has a different
mechanism: the tap lands while SyncSettingsActivity is still applying async
preference state, so the switch is laid out but not yet interactive — the click
is silently ignored and the 5 s poll expires unchanged.

Fix

Add tapUntilPrefChanges to replace the tapViewCenter + awaitSyncEnabled
pair:

  1. Pre-tap interactivity wait: polls (bounded 2 s) for isEnabled == true
    and stable bounds across two consecutive 50 ms samples before injecting the tap.
  2. Per-attempt poll: 2 s (100 ms intervals) for the preference to flip.
  3. Bounded retry: up to 3 attempts before the test fails. A genuinely broken
    toggle still fails — retries tolerate input timing, not a bad toggle.

Production code is unchanged. The now-unused awaitSyncEnabled helper is removed.

Related

Add tapUntilPrefChanges: wait for the switch to be enabled and bounds-stable
before each tap attempt, then poll up to 2s for the preference to flip, and
retry the whole sequence up to 3 times before failing.

Addresses two no-ANR flake classes observed across API-29 CI runs after the
UiAutomator-node fix (ActivityWatch#266):
 1. The tap lands before SyncSettingsActivity finishes applying async state —
    the switch is laid out but not yet interactive, so the click is ignored.
 2. The preference write completes after the 2s window on a loaded emulator;
    the next attempt observes the change immediately.

The assertion stays strict: a genuinely broken toggle still fails the test
after maxAttempts taps. Removes the now-unused awaitSyncEnabled helper.

Git-Session-Id: d850
@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with no outstanding correctness, security, or repository-rule issues identified.

Summary

This PR hardens the Android end-to-end sync-toggle test against emulator timing flakes.

  • Waits for the toggle to become enabled with stable bounds before tapping.
  • Retries within bounded readiness and preference-polling windows.
  • Avoids reversing a preference update that completes after an earlier polling deadline.
  • Fails clearly when no valid tap can be injected.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Start attempt] --> B{Retry attempt?}
    B -->|Yes| C[Wait for device idle]
    C --> D{Preference already expected?}
    D -->|Yes| Z[Return success]
    D -->|No| E[Poll enabled state and stable bounds]
    B -->|No| E
    E --> F{Ready within 2 seconds?}
    F -->|No| G[Skip tap and start next attempt]
    F -->|Yes| H[Tap view center]
    H --> I[Poll preference for bounded interval]
    I --> J{Preference changed?}
    J -->|Yes| Z
    J -->|No| G
    G --> K{Attempts remain?}
    K -->|Yes| A
    K -->|No, no taps| L[Fail with readiness message]
    K -->|No, tap occurred| M[Fail preference assertion]
Loading

Reviews (2) · Last reviewed commit: "fix(e2e): don't reverse late taps or tap..."

Address two correctness issues Greptile flagged on ActivityWatch#287's retry helper:

- Re-check prefs.isSyncEnabled() before each retry tap. A tap processed
  just after its poll window closed flips the preference late; retrying
  blindly reversed the successful transition and could fail a working
  toggle intermittently.
- Track readiness and skip the tap when the enabled + stable-bounds wait
  times out. Falling through to tapViewCenter could inject a premature or
  misplaced tap and consume an attempt. If no attempt ever reached
  readiness, fail with a message naming that condition rather than the
  tap assertion.

Verified: ./gradlew :mobile:compileStandardDebugAndroidTestKotlin builds.
Git-Session-Id: ece32b17-aee2-59d1-b14a-b6186e142390
@TimeToBuildBob

TimeToBuildBob commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI code review

Replaces the tapViewCenter + awaitSyncEnabled pair in NativeWindowInsetsTest.syncToggleReceivesRealTap with a new tapUntilPrefChanges helper that waits for the view to be enabled and bounds-stable, polls for the preference flip, and retries the tap up to maxAttempts times. Removes the old awaitSyncEnabled helper and updates the test to call the new helper for both toggle directions.

Safe to merge — no P0/P1 findings

Confidence 5/5

No findings. The diff looks correct to me on this pass.

Files changed (1) — the diff as I read it
  • mobile/src/androidTest/java/net/activitywatch/android/NativeWindowInsetsTest.kt — Adds tapUntilPrefChanges with bounded readiness wait, per-attempt poll, and retry; removes awaitSyncEnabled; updates syncToggleReceivesRealTap to use the new helper.
Previous review passes
commit score findings engine when
9fce0dd6a30b 3/5 1 llm 2026-09-17 13:24 UTC

Reviewed af7425ad7065 · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 41s · about this reviewer

Maintainer commands

@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

@greptileai review

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

@ErikBjare
ErikBjare merged commit d8beef9 into ActivityWatch:master Sep 17, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants