fix(nav): keep NavKey classes so screen-root test tags survive R8 - #1375
Merged
Conversation
`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.
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.
…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.
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
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.
Screen-root test tags come out wrong on a minified build, so the Maestro smoke suite can only run unminified.
annotatedEntryderives each screen's root test tag fromT::class.simpleName(NavMetadata.screenRootTag).getSimpleName()on a nested class reads theInnerClassesattribute, 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, soAppRoute.Main.ScannerreportedAppRoute$Main$Scannerand its tag came out asapp_route$main$scanner_screen. The global-keepattributes InnerClassesthat Retrofit's bundled rules pull in does not change that.They also never matched
com.flipcash.app.core.onboarding.OnboardingStep, which reachesNavKeythroughFlowSteprather thanAppRoute. Those classes were obfuscated outright, so the seed screen reportedn69_screen. That case is also why reading the binary name in Kotlin instead would not have worked.Matching on the
NavKeysupertype 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 leavesUI_TESTABLEtrue), auiautomator dumpreportswallet_screenandscanner_screen, both nested underAppRoute.Main. In the same APK's dex,OnboardingStep$SeedInputcarriesInnerClass name="SeedInput"— the membergetSimpleName()reads.Only builds with
BuildConfig.UI_TESTABLEtrue 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.ymlnever setDEBUG_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 totrue, which is what the nightly run gets. Aminifydispatch input takestrue,false, orboth;bothexpands 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-evalue,inputText: ${SEED_PHRASE}typed nothing, and the run failed onwallet_screen is visibleinside 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: theMAESTRO_*secrets it maps do not exist, so each expands to"".run.shnow checks the credentials before touching the device — a missingSEED_PHRASEorLOGIN_DEEPLINKstops the run, the flow-specific ones warn and name the flows that read them, and neither prints a value. It also fixesseed_contactdying silently whenpipefailturns a no-matchgrepinto a failed assignment, and adds theLOGIN_USERNAMEmapping the workflow was missing.Creating the secrets is separate from this PR; the nightly stays red until they exist.