Skip to content

fix(nav): keep NavKey classes so screen-root test tags survive R8 - #1375

Merged
bmc08gt merged 3 commits into
code/cashfrom
fix/nav-test-tags-r8
Aug 31, 2026
Merged

fix(nav): keep NavKey classes so screen-root test tags survive R8#1375
bmc08gt merged 3 commits into
code/cashfrom
fix/nav-test-tags-r8

Conversation

@bmc08gt

@bmc08gt bmc08gt commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Screen-root test tags come out wrong on a minified build, so the Maestro smoke suite can only run unminified.

annotatedEntry derives each screen's root test tag from T::class.simpleName (NavMetadata.screenRootTag). getSimpleName() on a nested class reads the InnerClasses attribute, and R8 emits that attribute only for classes matched by a full -keep. The two rules this replaces were -keepnames, which preserves the binary name but not the attribute, so AppRoute.Main.Scanner reported AppRoute$Main$Scanner and its tag came out as app_route$main$scanner_screen. The global -keepattributes InnerClasses that Retrofit's bundled rules pull in does not change that.

They also never matched com.flipcash.app.core.onboarding.OnboardingStep, which reaches NavKey through FlowStep rather than AppRoute. Those classes were obfuscated outright, so the seed screen reported n69_screen. That case is also why reading the binary name in Kotlin instead would not have worked.

-keep class * implements androidx.navigation3.runtime.NavKey

Matching on the NavKey supertype covers both hierarchies and any future one. Class-only, with no member wildcard, so members stay shrinkable and renameable. It costs +137 classes retained and +16 KB of APK against the current baseline; for scale, #1370 removed 568 classes and 25,542 live methods.

On a minified debug build (DEBUG_MINIFY=true, which leaves UI_TESTABLE true), a uiautomator dump reports wallet_screen and scanner_screen, both nested under AppRoute.Main. In the same APK's dex, OnboardingStep$SeedInput carries InnerClass name="SeedInput" — the member getSimpleName() reads.

Only builds with BuildConfig.UI_TESTABLE true expose testTags as resource-ids, so a stock release never showed these tags and nothing user-facing changes. What this buys is Maestro running against a minified build.

The second commit makes CI do that. .github/workflows/maestro.yml never set DEBUG_MINIFY, so the suite only ever ran unminified and a keep-rule regression would have gone green. It now comes from a matrix entry defaulting to true, which is what the nightly run gets. A minify dispatch input takes true, false, or both; both expands to a two-entry matrix through a small setup job, since a matrix cannot be built from an input inline. max-parallel: 1, because every flow signs into the same shared test account, and the report artifact is named per variant.

A third commit makes the failure name itself. An unset credential was not an error anywhere — cred() returned an empty string, Maestro got an empty -e value, inputText: ${SEED_PHRASE} typed nothing, and the run failed on wallet_screen is visible inside a login subflow that names none of the cause. That is why the nightly job has failed on all 25 runs since it was added: the MAESTRO_* secrets it maps do not exist, so each expands to "". run.sh now checks the credentials before touching the device — a missing SEED_PHRASE or LOGIN_DEEPLINK stops the run, the flow-specific ones warn and name the flows that read them, and neither prints a value. It also fixes seed_contact dying silently when pipefail turns a no-match grep into a failed assignment, and adds the LOGIN_USERNAME mapping the workflow was missing.

Creating the secrets is separate from this PR; the nightly stays red until they exist.

`annotatedEntry` derives each screen's root test tag from `T::class.simpleName`
(NavMetadata.screenRootTag). `getSimpleName()` on a nested class reads the
`InnerClasses` attribute, and R8 emits that attribute only for classes matched by a
full -keep. Under the two `-keepnames class com.flipcash.app.core.AppRoute...` rules
the binary name survived but the attribute did not, so `AppRoute.Main.Scanner`
reported `AppRoute$Main$Scanner` and its tag came out as
`app_route$main$scanner_screen`. The global `-keepattributes InnerClasses`, which
Retrofit's bundled rules already pull in, does not change that.

The rules also never matched `com.flipcash.app.core.onboarding.OnboardingStep`, which
reaches `NavKey` through `FlowStep` rather than `AppRoute`. Those classes were
obfuscated outright, so the seed screen reported `n69_screen`. That second case is
also why reading the binary name in Kotlin would not have been a fix.

Matching on the `NavKey` supertype covers both hierarchies and any future one.
Class-only, with no member wildcard, so members stay shrinkable and renameable. Cost
against the current baseline is +137 classes retained and +16 KB of APK; for scale,
#1370 removed 568 classes and 25,542 live methods.

On a minified debug build (DEBUG_MINIFY=true, which leaves UI_TESTABLE true),
`uiautomator dump` now reports `wallet_screen` and `scanner_screen` — both nested
under `AppRoute.Main`. In the same APK's dex, `OnboardingStep$SeedInput` carries
`InnerClass name="SeedInput"`, the member `getSimpleName()` reads.

Only builds with `BuildConfig.UI_TESTABLE` true expose testTags as resource-ids, so a
stock release never showed these tags. The gain is that Maestro can run against a
minified build.
@bmc08gt bmc08gt self-assigned this Aug 31, 2026
@github-actions github-actions Bot added the type: fix Bug fix label Aug 31, 2026
Nothing in the workflow set DEBUG_MINIFY, so Maestro only ever ran against an
unminified debug build. A keep rule that stops protecting the screen-root test
tags would pass CI and only show up when someone ran the suite locally on a
minified build.

Set DEBUG_MINIFY from a matrix entry, defaulting to true, so the nightly run is
the one that exercises R8. A `minify` dispatch input takes true, false, or both;
`both` expands to a two-entry matrix through a small setup job, since a matrix
cannot be built from an input inline. max-parallel is 1 because every flow signs
into the same shared test account. Reports are uploaded per variant.
@bmc08gt
bmc08gt requested a review from jeffyanta as a code owner August 31, 2026 14:23
…sertion

An unset credential was not an error anywhere: cred() returned an empty string,
run.sh passed it to Maestro as an empty -e value, `inputText: ${SEED_PHRASE}`
typed nothing, and the run failed twenty minutes later on `wallet_screen is
visible` — an assertion inside a login subflow that names none of the cause.
The nightly CI job has failed that way on every run since it was added, because
the MAESTRO_* repo secrets it maps do not exist and each expanded to "".

Check the credentials before touching the device. A missing SEED_PHRASE or
LOGIN_DEEPLINK stops the run, since between them they gate every flow; the
flow-specific ones warn and name the flows that read them, because a local run
of one flow has no reason to set the rest. Names only, never values.

Also fix seed_contact dying silently: pipefail turns a no-match grep into a
failed assignment, which set -e acts on before the empty-rid guard below it can
report anything. It fires only when the raw_contacts query comes back empty, so
a real emulator does not hit it.

LOGIN_USERNAME had no mapping in the workflow, so the vanity flows would have
warned on every CI run. Added it.
@bmc08gt
bmc08gt merged commit 6e13a14 into code/cash Aug 31, 2026
3 checks passed
@bmc08gt
bmc08gt deleted the fix/nav-test-tags-r8 branch August 31, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant