diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 5b2797f..7a18e29 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -96,22 +96,31 @@ android { ) } /* - * What macrobenchmark measures: release rules (R8 on, not debuggable) but signed with the - * debug key so it can be installed on a test device. The manifest in src/benchmark adds the - * one thing a benchmark cannot work without — ``. + * What macrobenchmark measures: release rules (R8 on, not debuggable) but measurable. * - * The suffix is not cosmetic, and it is `.perf` rather than `.benchmark` on purpose: the - * benchmark module's own APK is `de.softwareapp_hb.filemanager.benchmark`, so an app under - * test with that id collides with the test APK and the install fails with - * INSTALL_FAILED_VERSION_DOWNGRADE. Without a suffix at all, this build would have the app's - * own package id and a debug signature — which cannot be installed beside a release build, - * and would replace a release install if the keys happened to match. A benchmark must never be - * able to take over the shipped app. + * **It keeps the app's own package id on purpose.** A separate id (`.perf`) was the first + * design, and it is the tidier one — a benchmark can never take over a release install — but + * on this MIUI device a *new* package id cannot be installed unattended at all: `adb install` + * is refused with INSTALL_FAILED_USER_RESTRICTED, and installing by tapping the APK left the + * package listed by `pm list packages` yet unknown to `pm enable` and invisible to + * PackageManager, which is exactly what macrobenchmark reports as "Unable to find target + * package". Replacing the installed app is the only path that works without a human in the + * loop for every run. + * + * So it is signed with the release key when that key is configured: same package, same + * signature, so it *updates* the installed app and keeps its data. Without `keystore.properties` + * (CI, a fresh clone) it falls back to the debug key and can then only be installed on a device + * that has no release build of the app. + * + * **This build replaces a release install.** Reinstalling the release APK puts it back. */ create("benchmark") { initWith(getByName("release")) - applicationIdSuffix = ".perf" - signingConfig = signingConfigs.getByName("debug") + signingConfig = if (hasReleaseSigning) { + signingConfigs.getByName("release") + } else { + signingConfigs.getByName("debug") + } matchingFallbacks += listOf("release") isDebuggable = false } @@ -131,8 +140,12 @@ android { * `testBenchmarkUnitTest`. * * ./gradlew -PbenchmarkBuild :benchmark:connectedDebugAndroidTest + * + * `hasProperty` rather than `providers.gradleProperty`: the latter is a Provider, and inside this + * block it read as absent even when the flag was passed — which showed up as the benchmark + * installing the *debug* APK and failing on it. */ - if (providers.gradleProperty("benchmarkBuild").isPresent) { + if (project.hasProperty("benchmarkBuild")) { testBuildType = "benchmark" } diff --git a/benchmark/src/main/AndroidManifest.xml b/benchmark/src/main/AndroidManifest.xml index 56fbfe6..41de1b1 100644 --- a/benchmark/src/main/AndroidManifest.xml +++ b/benchmark/src/main/AndroidManifest.xml @@ -17,6 +17,6 @@ android:name="android.permission.QUERY_ALL_PACKAGES" tools:ignore="QueryAllPackagesPermission" /> - + diff --git a/benchmark/src/main/java/de/softwareapp_hb/filemanager/benchmark/BrowserBenchmark.kt b/benchmark/src/main/java/de/softwareapp_hb/filemanager/benchmark/BrowserBenchmark.kt index a1f8df2..59d9917 100644 --- a/benchmark/src/main/java/de/softwareapp_hb/filemanager/benchmark/BrowserBenchmark.kt +++ b/benchmark/src/main/java/de/softwareapp_hb/filemanager/benchmark/BrowserBenchmark.kt @@ -43,10 +43,11 @@ class BrowserBenchmark { val benchmark = MacrobenchmarkRule() /** - * The app under test: the `benchmark` build type, which carries its own package id (`.perf`) so it - * can never collide with — or replace — a release install on the device, or with the test APK. + * The app under test: the `benchmark` build type of the app itself — same package id, signed with + * whatever key the release build uses, so it updates an installed release build rather than + * needing an install of its own. See `:app`'s build file for why. */ - private val app = "de.softwareapp_hb.filemanager.perf" + private val app = "de.softwareapp_hb.filemanager" // No setup by shell command here on purpose. UiAutomation's executeShellCommand runs with *this // app's* uid, not the shell user's, so it cannot create files in /sdcard and cannot grant itself diff --git a/docs/CODE_MAP.md b/docs/CODE_MAP.md index 7e4fe53..8137d8a 100644 --- a/docs/CODE_MAP.md +++ b/docs/CODE_MAP.md @@ -2,7 +2,7 @@ Generated from the source by `tools/make-code-map.py` - do not edit by hand. Regenerate it after a change (`python3 tools/make-code-map.py`); `--check` exits non-zero when this file is stale. -77 source files, 12659 lines - 33 test files, 386 tests. The behaviour index at the end lists every test name, and in this project a test name is a sentence about what the app does, so it doubles as the specification. +77 source files, 12660 lines - 33 test files, 386 tests. The behaviour index at the end lists every test name, and in this project a test name is a sentence about what the app does, so it doubles as the specification. Start at `AGENTS.md`. What is done, and why: `docs/ROADMAP.md` and `CHANGELOG.md`. What was verified by hand rather than by a test: `docs/EDGE_CASE_TESTING.md`. @@ -170,7 +170,7 @@ The macrobenchmark test APK: four measurements over a fixture folder, run on a d | file | lines | what it holds | tests | |---|---|---|---| -| `BrowserBenchmark.kt` | 167 | The four numbers docs/PERFORMANCE.md promises, measured on a benchmark build of the app. BrowserBenchmark, Fixture | - | +| `BrowserBenchmark.kt` | 168 | The four numbers docs/PERFORMANCE.md promises, measured on a benchmark build of the app. BrowserBenchmark, Fixture | - | ## Behaviour index diff --git a/docs/PERFORMANCE.md b/docs/PERFORMANCE.md index deb92dc..32c138c 100644 --- a/docs/PERFORMANCE.md +++ b/docs/PERFORMANCE.md @@ -51,9 +51,14 @@ first items of the speed work. ## Measured on demand: the `:benchmark` module `benchmark/` holds four macrobenchmark tests that measure the interaction numbers above on a benchmark -build of the app: release rules (R8 on, not debuggable), debug-signed, and profileable — under its own -package id, `…filemanager.perf`, so it can never collide with the test APK or take over a release -install on the device. +build of the app: release rules (R8 on, not debuggable) and profileable, **under the app's own package id +and signed with the release key**, so installing it *updates* an installed release build and keeps its +data. A separate package id (`.perf`) was the first design and is the tidier one, but on this MIUI phone a +new package id cannot be installed without a human: `adb install` is refused with +`INSTALL_FAILED_USER_RESTRICTED`, and installing by tapping the APK left the package visible to +`pm list packages` yet invisible to `PackageManager`, which is what macrobenchmark reports as *"Unable to +find target package"*. The trade-off is written in `:app`'s build file: **this build replaces a release +install**, and reinstalling the release APK puts it back. | benchmark | metric | what it answers | |---|---|---| @@ -69,30 +74,34 @@ baseline. `CompilationMode.Partial()` plus a baseline profile is the follow-up, module. **Two of these have produced a number** (`coldStart`, above); the three interaction ones have not, and -the reason is the device rather than the code. Getting them to run at all took four fixes, each of which -is written next to the code it belongs to: the app under test needs its own package id; it needs to be -installed by hand once, because MIUI refuses `adb install` of a new package id without a human -(`INSTALL_FAILED_USER_RESTRICTED`); the test APK needs package-visibility for the app it measures, or -macrobenchmark reports `NameNotFoundException` for an app that `pm list packages` shows installed; and -the fixture has to be created through adb, because `UiAutomation.executeShellCommand` runs with the -*test app's* uid and cannot write to `/sdcard`. +the reason is a device policy rather than the code. Getting this far took five fixes, each written next +to what it constrains: the app under test needs the app's own package id and release key (a new id cannot +be installed here); the test APK needs package visibility for it; the fixture has to be created through +adb, because `UiAutomation.executeShellCommand` runs with the *test app's* uid and cannot write `/sdcard`; +`testBuildType` has to sit behind `-PbenchmarkBuild`, or `:app:testDebugUnitTest` stops existing; and +`hasProperty` is needed to read that flag, because the Provider form read as absent and the runner then +installed the *debug* APK and failed on it. -The workflow that avoids all of it after the first install: +The workflow: ```bash -# once, by hand on the phone (Files → Download → tap), because MIUI gates adb installs: -# /sdcard/Download/PlainFiles-perf.apk (app under test, package …filemanager.perf) -# /sdcard/Download/PlainFiles-benchmark-test.apk - -./tools/benchmark-fixture.sh # create the fixture and grant storage access -adb shell am instrument -w \ - -e class de.softwareapp_hb.filemanager.benchmark.BrowserBenchmark \ - de.softwareapp_hb.filemanager.benchmark/androidx.test.runner.AndroidJUnitRunner +./tools/benchmark-fixture.sh # the fixture, and the storage grant the benchmark build lacks +./gradlew -PbenchmarkBuild :benchmark:connectedDebugAndroidTest ``` -`am instrument` is preferred over `./gradlew -PbenchmarkBuild :benchmark:connectedDebugAndroidTest` on a -device with an install gate: the Gradle task installs *and uninstalls* the APKs around the run, so every -run needs a human again — and an uninstall between runs is what made the app under test disappear once. +**The Gradle runner is required, not preferred.** Macrobenchmark's rule uninstalls the app at the start +of a run and reinstalls it before measuring. On this device *that reinstall* is what MIUI refuses, so the +run leaves the app uninstalled and then fails with `NameNotFoundException` for an app that was there a +second earlier — seen twice through `am instrument` and once through `connectedDebugAndroidTest`, with +`Benchmark: Uninstalling de.softwareapp_hb.filemanager` in logcat and the package gone from +`pm list packages` afterwards. + +The fix is a device setting rather than code: *Developer options → Install via USB* — and on MIUI usually +*MIUI optimization off* as well — so that an install raised by the benchmark is allowed without a prompt. +Until that is on, **the interaction benchmarks cannot run on this phone**, and the cold-start number above +stands alone. `tools/benchmark-fixture.sh` is still needed either way: the benchmark build has no storage +grant of its own, and `UiAutomation.executeShellCommand` (the only shell a test has) runs with the test +app's uid and cannot write `/sdcard`. ## Not measured yet, and why diff --git a/tools/benchmark-fixture.sh b/tools/benchmark-fixture.sh index 10da97d..9306152 100755 --- a/tools/benchmark-fixture.sh +++ b/tools/benchmark-fixture.sh @@ -15,7 +15,7 @@ # benchmark finds its row without scrolling. set -euo pipefail -package=de.softwareapp_hb.filemanager.perf +package=de.softwareapp_hb.filemanager directory=/sdcard/AABenchmark count=1000