fix(android-tests): replace flaky UIAutomator harness with headless logcat driver#473
Conversation
…ogcat driver
The Android instrumented-test job has never passed on CI. BaseReactNativeTest
drove a React Native <ScrollView> of ~35 test buttons with blind UIAutomator
swipe gestures (RN ScrollView exposes no scroll semantics to UIAutomator), which
is non-deterministic on Firebase Test Lab's slow ARM emulators: button-not-found,
stale-element clicks, and false "did not complete in time".
Replace it with a headless driver (no UI navigation):
- test/HeadlessTestApp.js: new Android-only component that runs the whole shared
suite once on mount and emits one single-line logcat sentinel per result
(SFTESTRESULT::{json}) plus SFTESTDONE::. Per-suite JS timeouts (each timer is
cleared once the race settles so a stale timeout can't corrupt a later test);
fail-closed so a crash/hang never masquerades as a pass.
- androidTests/index.js: mount HeadlessTestApp as the primary component; keep the
interactive TestApp as SalesforceReactTestAppInteractive for local debugging.
iOS is untouched (iosTests/index.js still mounts TestApp; XCUITest stays green).
- BaseReactNativeTest.kt: delete all UIAutomator; launch once, stream logcat
(tag ReactNativeJS) via uiAutomation.executeShellCommand, parse sentinels into a
process-static map released by a CountDownLatch on the DONE sentinel, and assert
per @test. One launch for all 35 tests collapses the ~70min runtime to a single
cold start while every @test still reports independently in the JUnit XML.
- reusable-android-workflow.yaml: num-uniform-shards 2 -> 1, since the single
run-all executes the mutating Net/SmartStore/MobileSync suites and two shards
would race the same org.
Validated locally on an x86_64 emulator against a scratch org: 34/35 pass
deterministically (old harness: 11/35). The remaining failure, MobileSync
testCleanResyncGhosts, is a genuine test-level timeout (iOS passes it) tracked
separately.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015CGbVaRKrx7DeurX81NWdQ
…swallowed rejections
testCleanResyncGhosts timed out (240s cap) on Android whenever the org's Id
lottery went against it: its smart query had no ORDER BY, so rows came back in
SQLite Id-index order, and Salesforce assigns Ids from pools, not in creation
order. The order-sensitive assert.deepEqual then threw inside a .catch-less
promise chain; Hermes drops unhandled rejections silently in --dev false
bundles, so the failure masqueraded as a timeout. iOS "passing" and the
isolated-repro "passing" were both luck of the Id draw, not platform behavior.
- mobilesync.test.js: add `order by {contacts:FirstName}` to the querySpec,
matching the idiom testReSync already uses.
- HeadlessTestApp.js: install Hermes's promise-rejection tracker; emit an
SFTESTRESULT-style `SFTESTUNHANDLED::<msg>` logcat sentinel and fold the last
rejection seen into the timeout error message. Attribution only - a stale
rejection from an earlier test must not fail the current one (same lesson as
the leaked-timer bug).
Validated locally: MobileSync suite 3x green, full suite 35/35 in 72s.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015CGbVaRKrx7DeurX81NWdQ
…s; harden diagnostics
Follow-ups from a review of the headless harness. All test-infra only; no
shipped SDK code changes.
testRunner.js — the single module-level resolver had no ownership token, so a
timed-out test whose promise chain later resumed could resolve the *next* test's
resolver (false pass), and a timeout during suite setUp could let the abandoned
runTest clobber the active test and run its body concurrently against the shared
store/sync manager. Add a generation counter: runTest claims a generation, the
harness advances it on timeout (abandonActiveTest), and a superseded run neither
installs a resolver nor runs its body. This closes the dangerous cases (setUp
clobber, concurrent execution). One residual is documented in-code: because tests
call the module-level testDone() rather than a per-test handle, a resumed timed-out
chain can still misattribute one verdict within an already-failed run — fully
closing that needs a per-test done() across the shared suite (iOS too).
HeadlessTestApp.js — toMessage() now JSON-stringifies non-Error rejection values
instead of rendering them "[object Object]", so the SFTESTUNHANDLED sentinel and
the timeout message carry the real bridge error (this is the same attribution the
tracker was added to provide). Timeout path now calls abandonActiveTest().
net.test.js — testPublicApiCall's tolerance catch asserted a string ('The
requested resource failed') that no platform's error actually contains, so any
ipify hiccup threw inside the catch and stalled the test to its 120s cap. Make it
tolerant: reaching the catch already proves the unauthenticated request path ran.
assert.js — sameDeepMembers used a JSON.stringify array replacer, which whitelists
key names at every depth and silently drops nested keys from both sides (a false
pass on nested objects). Canonicalize recursively instead.
Validation: on a local x86_64 emulator the new testRunner ran all 35 tests with
correct verdicts for every offline test and correct error attribution for the rest;
a fully clean 35/35 run is pending stable device/CI (Firebase ARM) confirmation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015CGbVaRKrx7DeurX81NWdQ
…d creds
A review of the collector found its only exit was the DONE latch, so any failure
that leaves the app alive-but-silent before SFTESTDONE:: (missing/`{}` creds, a JS
bundle load failure, a release variant that finish()es) blocked the whole
maxRunMs (45 min default) and then failed all 35 tests with a misleading
"did not emit DONE" — never pointing at the real cause.
- BaseReactNativeTest.kt: add a BEGIN watchdog. HeadlessTestApp emits SFTESTBEGIN::
on mount; if it doesn't arrive within beginTimeoutMs (default 3 min, overridable),
fail fast with a clear cause instead of blocking on DONE. Also add AndroidRuntime:E
to the logcat filter and capture a FATAL EXCEPTION for our process, folded into
the failure message (best-effort — a crash that kills the shared instrumentation
process is already reported quickly by `am instrument` as "Process crashed"; this
covers the alive-but-silent / cross-process case and enriches the message). Guard
parseResult so one malformed line can't kill the reader thread (the sole DONE
consumer).
- prepareandroid.js: exit non-zero when shared/test/test_credentials.json is missing
instead of writing an empty '{}' asset that yields a green build which crashes in
TestAuthenticationActivity.onCreate at runtime.
Validated on a local x86_64 emulator: happy path 35/35 in ~35 s (BEGIN watchdog
does not regress normal runs); with `-e beginTimeoutMs 1` the run fails in ~5 s
(not 45 min) with "did not emit BEGIN within 1ms — app launched but HeadlessTestApp
never mounted (check test_credentials.json and the JS bundle)".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015CGbVaRKrx7DeurX81NWdQ
|
Hi @james-enperso — thank you for this, it's a substantial and well-thought-out contribution. We validated it locally against a live org: Android (API 36 emulator): 35/35 passed in ~40 s — including iOS (iPhone 16 simulator): 35/35 passed — the shared suite changes ( Approving. |
wmathurin
left a comment
There was a problem hiding this comment.
Validated locally — 35/35 Android and 35/35 iOS. LGTM.
|
||||||||||||||
|
CI failure root cause — `prepareandroid.js` missing test overlay The `android-pr / test-android` job was failing at Install Dependencies with: ``` Why: `prepareandroid.js` runs `yarn install`, which installs `react-native-force` from the `dev` branch (the version pinned in `androidTests/package.json`). That published copy doesn't have `HeadlessTestApp.js` yet — it only exists in this PR branch. So Metro fails to resolve the import. Fix: After `yarn install`, overlay the local repo's `test/` directory onto the installed package's `test/` folder. This is the same thing you need to do locally and what we'd expect any branch that adds new test files to require. I've added this fix in commit 3fead89 on our tracking branch. We'll include it when we merge this PR to `dev`. |
Problem
The Android instrumented-test job (
androidTests/) has never passed on CI.BaseReactNativeTestdrove a React Native<ScrollView>of ~35 test buttons using blind UIAutomator swipe gestures — RN's ScrollView exposes no scroll semantics to UIAutomator, so navigation was fixed-distance swipes. On Firebase Test Lab's slow ARM emulators this is non-deterministic:Button not found, stale-elementUiObjectNotFoundException, and falsedid not complete in time. The most recent nightly reported 24/35 failures, and the job has been red on every run since the harness was introduced.iOS uses the same shared
test/suite via XCUITest and stays green, because XCUITest has element-targeted scrolling that UIAutomator lacks for RN ScrollViews.Fix — headless driver (no UI navigation)
test/HeadlessTestApp.js(new, Android-only): runs the whole shared suite once on mount and emits one single-line logcat sentinel per result (SFTESTRESULT::{json}) plusSFTESTDONE::. Per-suite JS timeouts, each cleared once the race settles (so a stale timer can't fire later and corrupt an unrelated test); fail-closed so a crash/hang can't masquerade as a pass. Also installs Hermes's promise-rejection tracker: an unhandled rejection during a test emitsSFTESTUNHANDLED::<msg>and is folded into the timeout error message, so a swallowed assertion failure can never masquerade as a bare timeout again (attribution only — a stale rejection from an earlier test cannot fail the current one).androidTests/index.js: mountHeadlessTestAppas the primary component; keep the interactiveTestAppasSalesforceReactTestAppInteractivefor local debugging. iOS is untouched —iosTests/index.jsstill mountsTestApp, so XCUITest stays green.BaseReactNativeTest.kt: delete all UIAutomator. Launch once, stream logcat (tagReactNativeJS) viauiAutomation.executeShellCommand, parse sentinels into a process-static map released by aCountDownLatchon theDONEsentinel, and assert per@Test. One launch for all 35 tests collapses the ~70-min runtime to a single cold start, while every@Teststill reports independently in the JUnit XML.reusable-android-workflow.yaml:--num-uniform-shards 2 → 1, since the single run-all executes the mutating Net/SmartStore/MobileSync suites and two shards would race the same org.test/mobilesync.test.js(second commit): fix the one test that was still red.testCleanResyncGhosts's "timeout" turned out to be a masked assertion failure, not a harness or bridge issue: its smart query had noORDER BY, so rows came back in SQLite Id-index order — and Salesforce assigns record Ids from pools, not in creation order. On an unlucky Id draw the order-sensitiveassert.deepEqualthrew inside a.catch-less promise chain; Hermes drops unhandled rejections silently in--dev falsebundles, so the test stalled to the suite cap. (iOS "passing" was the same Id lottery, not platform behavior.) Fixed by addingorder by {contacts:FirstName}, the same idiomtestReSyncalready uses.No changes to the shipping SDK (
android/,src/), no native module, no codegen, no new gradle dependency.Validation
Ran locally on an x86_64 emulator (API 37) against a scratch org:
MobileSync suite additionally validated 3× consecutive green after the
testCleanResyncGhostsfix.Follow-up hardening (third commit)
A review of the new harness surfaced a few more test-infra issues (no shipped SDK code):
testRunner.js— verdict integrity. The single module-level resolver had no ownership token, so a timed-out test whose promise chain later resumed could resolve the next test's resolver (false pass), and a timeout during suitesetUpcould let the abandonedrunTestclobber the active test and run its body concurrently against the shared store/sync manager. Added a generation counter (runTestclaims a generation; the harness advances it on timeout viaabandonActiveTest; a superseded run neither installs a resolver nor runs its body). This closes the dangerous cases. One residual is documented in-code: because tests call the module-leveltestDone()rather than a per-test handle, a resumed timed-out chain can still misattribute one verdict within an already-failed run — fully closing that needs a per-testdone()across the shared suite (iOS too), left as a follow-up.HeadlessTestApp.js—toMessage()now JSON-stringifies non-Errorrejection values instead of rendering them"[object Object]", so theSFTESTUNHANDLED::sentinel and the timeout message carry the real bridge error.net.test.js—testPublicApiCalltolerance catch asserted a string no platform's error contains, so any ipify hiccup threw inside the catch and stalled the test to its 120 s cap; made it actually tolerant.assert.js—sameDeepMembersused aJSON.stringifyarray replacer that drops nested keys from both sides (a false pass on nested objects); canonicalize recursively instead.Validation of this commit: 35/35 pass in 49 s on a local x86_64 emulator with the new
testRunnerin place (Firebase ARM confirmation still pending, as for the rest of the PR).Collector fast-fail (fourth commit)
A separate review of the Kotlin collector found its only exit was the DONE latch, so any failure that leaves the app alive but silent before
SFTESTDONE::(missing/{}creds, a JS bundle load failure, a release variant thatfinish()es) blocked the wholemaxRunMs(45 min default) and then failed all 35 tests with a misleading "did not emit DONE" — never pointing at the real cause.BaseReactNativeTest.kt— add a BEGIN watchdog:HeadlessTestAppemitsSFTESTBEGIN::on mount, and if it doesn't arrive withinbeginTimeoutMs(default 3 min, overridable via instrumentation arg) the run fails fast with a clear cause. AddAndroidRuntime:Eto the logcat filter and capture aFATAL EXCEPTIONfor our process into the failure message (best-effort — a crash that kills the shared instrumentation process is already reported quickly byam instrumentas "Process crashed"; this covers the alive-but-silent / cross-process case). GuardparseResultso one malformed line can't kill the reader thread (the sole DONE consumer).prepareandroid.js— exit non-zero whenshared/test/test_credentials.jsonis missing instead of writing an empty{}asset that yields a green build which crashes at runtime.Validated locally: happy path 35/35 in ~35 s (the watchdog doesn't regress normal runs); with
-e beginTimeoutMs 1the run fails in ~5 s (not 45 min) withHeadless run did not emit BEGIN within 1ms — app launched but HeadlessTestApp never mounted (check test_credentials.json and the JS bundle).Test plan
iosTests/index.jsunchanged).MediumPhone.arm) run to confirm the deterministic result on ARM infra.🤖 Generated with Claude Code
https://claude.ai/code/session_015CGbVaRKrx7DeurX81NWdQ