f3abef6c44de0280ce0e58983848e940bec0a32a
8 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 SoftwareApp
|
4846454645 |
Ship a baseline profile, generated on the emulator because the phone cannot collect one
The profile is real: 13193 lines in app/src/main/baseline-prof.txt, 584 of them the app's own classes, bundled into the release APK as assets/dexopt/baseline.prof. It is produced by a new :baselineprofile test module that walks the first journey - roots screen, one volume opened - under the androidx.baselineprofile plugin, and :app now depends on androidx.profileinstaller so the bundled profile is applied at install time. Two device facts decided how it was made. Collection requires API 33+ (BaselineProfileRule refuses Android 12 outright), so it ran on the API 35 AVD that was already in ~/.android/avd; that emulator is software-rendered and needs androidx.benchmark.suppressErrors=EMULATOR for any macrobenchmark. And the release build needed profileable android:shell=true, which is why app/src/release/AndroidManifest.xml exists and says why: the same APK is both the thing measured and the thing shipped, so the profile's class names are the ones in the shipped dex. The effect is not measured, and PERFORMANCE.md says so: 7404 ms median without the profile against 9005 ms with it, run to run, because a TCG emulator varies by more than a profile can buy. What remains is one run on a physical API 33+ phone, with the two compile commands documented. The emulator did settle the other open question - FrameTimingMetric there reports frameDurationCpuMs and frameOverrunMs, the metrics this phone cannot produce at all. :baselineprofile is compiled by tools/verify.sh --release like :benchmark, so it cannot rot between device runs. |
||
|
SoftwareApp
|
1f94d0a38a |
Add a macrobenchmark module, and find out what the build was hiding
docs/PERFORMANCE.md has been honest that three of its four numbers do not exist. They exist now as measurements, if not yet as results: benchmark/ is a macrobenchmark module with four tests - cold start with StartupTimingMetric, then opening, scrolling and searching a folder of a thousand files with FrameTimingMetric - sharing a fixture at /sdcard/AABenchmark that is created once per run and deleted in @AfterClass. Frames rather than a named trace section on purpose: what a user calls slow is jank on screen, and the alternative puts benchmarking calls into production code for the sake of a number. CompilationMode.None() is the honest cold-install baseline; Partial() plus a baseline profile is the follow-up and needs the same module. The storage grant is applied with appops in setup, which is what lets the run be unattended - a permission dialog is the one thing a test cannot answer. Three build facts had to be discovered rather than designed, and each is now written where it will be read. The app under test cannot keep its own package id: it needs a debug signature to install on a test device, so it cannot replace a release install, and an un-suffixed build is exactly that. It also cannot use the obvious suffix: `.benchmark` is already the benchmark module's own APK id, and the test APK then fails to install with INSTALL_FAILED_VERSION_DOWNGRADE - which is what the first attempt did. Hence `.perf`. And `testBuildType = "benchmark"` cannot be set unconditionally, because it makes benchmark the variant that *unit* tests run against and :app:testDebugUnitTest - the task the entire gate is built on - stops existing; it is behind -PbenchmarkBuild instead. All three were measured, not guessed, and the third would have broken CI silently. Two smaller things came with it. tools/verify.sh --release now also compiles the app's benchmark variant and the test APK, because build wiring nobody runs is wiring that rots; no device is involved. And the code map grew an EXTRA_ROOTS section so it indexes benchmark/ too - a map that silently omits a module lies by omission, which is worse than the one line it costs. The numbers themselves are still missing, and for a device reason rather than a code one: installing the new package id on this MIUI phone fails with INSTALL_FAILED_USER_RESTRICTED (the "Install via USB" gate, which wants a human and answered too late). Both APKs are pushed to /sdcard/Download/ for a manual install, and the run is one command away: `adb shell am instrument -w -e class de.softwareapp_hb.filemanager.benchmark.BrowserBenchmark de.softwareapp_hb.filemanager.benchmark/androidx.test.runner.AndroidJUnitRunner`. Gate green with --release, including the new compile step; map current; 386 tests unchanged. |
||
|
SoftwareApp
|
0bbee25ab3 |
Test the interface as a user sees it, and let a debug build tell on itself
Fifteen rendering tests across four files are the first tests in this project that compose a screen, and they cover the claims that used to be prose only: the app bar's star exists where there is a folder to star and says Add or Remove according to the state, the selection bar offers Rename to several files and Details only to one, the batch dialog spells out the names it will produce and refuses a collision with the file named, the shared name dialog refuses a slash while it is typed, and a favourite that has gone says "Folder not found" rather than showing a path that is not there. They run on the JVM through Robolectric - the Compose test artifacts plus the test manifest - so they belong to the same verify.sh gate as everything else instead of needing a device: 386 tests in total. Writing them turned up one behaviour worth recording. The batch dialog continues the folder's series by default, so a collision cannot be shown unless the start number is forced back to 1 - which is precisely why the feature works in practice, and now also why two of these tests set the field explicitly. The debug build now enables StrictMode (disk reads and writes, network, leaked closables; logged, not fatal) and attaches LeakCanary. A file manager is the worst possible app for a blocking disk read on the main thread, and a debug build is the last place where noticing it is still cheap. Logging rather than a crash is deliberate: turning a slow screen into a crash on someone's phone is the worse trade. The flag is read from the manifest, not from BuildConfig, which this build does not generate. Performance has a number for the first time, and it immediately paid for itself. tools/measure-startup.sh takes cold start with `am start -W` - median of several runs, one warm-up discarded because the first launch after an install pays for dexopt - and docs/PERFORMANCE.md records 1814 ms over five runs, in the debug build, which is the slow case by construction. StrictMode's first run then produced five violations against that number: 492 ms resolving `app.cacheDir` inside the view-model factory, 397 ms reading free space with StatFs for every volume inside refresh(), 477 ms of the platform loading its own typefaces, and two 54 ms OEM font hooks. That is about 1.5 s of main-thread disk I/O in a 1.8 s start, of which ~0.9 s is ours and mechanical to move. Neither of ours is fixed here on purpose: the numbers and the two fixes are recorded in PERFORMANCE.md, and they are the first items of the speed work. What is not measured is stated in the same file rather than left to be assumed: folder open at 1 000 / 5 000 / 20 000 files, thumbnail paint while scrolling, and keystroke-to-result all need a `:benchmark` module and a profileable build under the release package id, and this device waits for a confirmation dialog on the first install of a new package id. The module is deliberately absent instead of half-configured: build wiring nobody can run is worse than a recorded gap. Gate green (map current, detekt, 386 tests, lint), and CHANGELOG, README, AGENTS.md, ROADMAP, PERFORMANCE.md, EDGE_CASE_TESTING and the regenerated map carry it. |
||
|
SoftwareApp
|
b1bc805af0 |
Make the gate one command, and add static analysis behind a baseline
Everything this repository promised was voluntary until now: 371 tests, lint and the generated code map ran only if whoever changed the code remembered to run them. tools/verify.sh is that gate as one command - the map check, detekt, the unit tests and lint, cheapest first so a stale document fails in a second rather than after a release build - and --release adds the R8 build. The ordering is not cosmetic: the two cheapest checks are also the two that catch drift nothing else can see. The same script runs in three places: CI on every push and pull request, a nightly --release job (R8 is the one check nothing else performs, and it has hidden a real defect here once), and a pre-push hook once the clone has been told to use .githooks. The nightly also uploads the R8 mapping, which is what turns a release stack trace back into a line of Kotlin and is only worth keeping if it belongs to the build that shipped. Detekt is adopted with a baseline, which is the entire adoption strategy: everything it finds today is frozen in config/detekt/baseline.xml, so the gate fails on a new finding rather than on a backlog nobody is going to clear this week. Entries are keyed by rule, file and enclosing declaration, so a new long function is still reported inside a file that already has one - proved by writing a TODO into a file, watching the build fail on that exact line, and taking it out again. What it found first agrees with the churn numbers from the last pass: ArchiveEngine 16 findings, BrowserViewModel 11, SelectionBar 9, 86 in total over 13 rules, the largest groups being the complexity ones this was added for (cyclomatic complexity 10, nested block depth 7, too many functions 4, long methods 3, two classes over 700 lines) plus 30 over-long lines. Three findings were fixed rather than parked, because they were real: dead `menuOpen` state left in BrowserScreen when the overflow menu moved into BrowserTopBar, a missing trailing newline, and two `db` parameters that are the platform's signature rather than an oversight (suppressed with that reason written next to them). Four rules were configured rather than obeyed: PackageNaming, because the applicationId's underscore is an identity this app cannot change; FunctionNaming, for @Composable (PascalCase by convention) and for backticked test names, which is the project's specification style; MagicNumber, because sizes and limits are named constants here; and MatchingDeclarationName, because a file is named after its concept rather than after a declaration inside it. Two tooling facts cost real time and are in neither tool's documentation. Detekt runs through its CLI rather than its Gradle plugin: the plugin performs the analysis inside the build's own JVM, and detekt 1.23 cannot parse JDK 25's version string - it fails with "25.0.4.1" as the entire error message - so the task drives detekt-cli on a 21 launcher, which is also the only way to pin the JVM the analyser runs on. And a baseline has to exist before it can be read, which is why it starts as an empty document rather than being generated into place. Verified: the fast gate is green in 17 seconds, --release adds a 36-second R8 build, 371 JVM tests still pass, lint is clean and the map is current. CHANGELOG, README, AGENTS.md, ROADMAP (the gate is now a recorded decision) and the regenerated map carry it. |
||
|
SoftwareApp
|
47e48f054e |
Give a video row a frame of the video, and fix what R8 was hiding
A video row showed a film-strip icon, and a phone holds more video than anything else, so a column of identical icons said nothing about which clip was which. Images and videos now take the same path - EntryIcon hands Coil the file, and for a video the request asks for a frame - which cost exactly one dependency: io.coil-kt.coil3:coil-video, at the same version as the Coil already in use, whose own POM adds nothing that Coil did not already bring (coil-core, kotlin-stdlib, core-ktx). No ExoPlayer, no media3, and no new permission: the decoder is the platform's own MediaMetadataRetriever. Two details are decisions rather than defaults. The frame is taken one second in, because the first frame of a recording is very often black or the middle of a fade-in, and a thumbnail of a black rectangle is no more useful than the icon it replaced. And a decode that fails - an unsupported codec, or a file only pretending to be a video - settles on the icon rather than leaving an empty square, which the row remembers so it is not retried on every recomposition. The dependency also nearly shipped a feature that did not work. coil-video registers its decoder through a META-INF/services entry, and in a debug APK that is enough: the file and the class it names are both there, and frames decoded. In the release APK, R8 removes both, because a class mentioned only from a resource looks unused to it. So the feature worked in every build that had been looked at and would have failed silently in the one users install - the worst shape a bug can have, since it needs no code change to "fix" and would have been reported as "video thumbnails don't work" against a build nobody had run. The factory is now registered explicitly against an ImageLoader in FileManagerApplication, which makes it a code reference R8 cannot drop, and the release APK was installed and driven to the Videos library to confirm the frames decode there too. That also closes the gap noted two changes ago, that no ImageLoader was registered at all; its cache sizes are left to Coil deliberately, since they are derived from the device's heap and a number written here would only be a guess. The same run found a smaller thing that no test could have: opening the search field from inside a library left the scope on "this folder" - the hint read "Search this folder" and a folder toggle sat beside it - while the search in fact ran everywhere, because the view model has always known that a library is not a folder and answered from the index instead. The behaviour was right and the screen was wrong, which is the same shape as the volume-list bug fixed earlier and the same root cause: the switch-on-open asked whether this was the root list rather than whether a folder was open. It now asks for a path, and the scope toggle appears only when there is a folder to choose about, because its two settings did the same thing otherwise. Every other use of isRootView was then checked for the same conflation and none was found. One test was added for the search behaviour; the scope mismatch itself is not testable, being a disagreement between two correct pieces. 324 JVM tests, lint clean. Verified on the phone in both builds: two clips made with ffmpeg (a testsrc2 pattern and SMPTE bars) drew their own patterns as thumbnails, beside a real 4.3 MB video from the device's own storage, and the demo library's three zero-filled stand-in .mp4s fell back to the icon - which exercised the failure path with material that happened to be lying around. The release APK is 2 294 021 bytes and still declares no INTERNET. Not done: the store screenshots still show the home screen from before the library views, and the release build running on a device is in nobody's checklist - which is exactly why the R8 problem survived until now, and is the first thing worth adding. |
||
|
SoftwareApp
|
ba92cf392d |
Support 7z extraction, and fix .xz extraction which would have crashed at runtime
Commons Compress declares its LZMA/LZMA2 support against org.tukaani:xz as an *optional* dependency, so nothing pulled it in transitively and nothing failed at build time. The app nevertheless advertised .xz as extractable, so the first user to open one would have got NoClassDefFoundError: org/tukaani/xz/FilterOptions. XZCompressorInputStream references org/tukaani/xz/XZInputStream directly. The library is now declared, and a round-trip test covers the path that was silently missing. That same library is what 7z needs, and Commons Compress already ships a pure-Java 7z reader, so 7z extraction costs no native code and no further dependency. The docs had assumed the format was off limits and said so in three places, including the customer-facing store copy. 7z is the only format here that cannot arrive through a stream: its header sits at the end of the file, so the reader has to seek. That is now a capability the backend advertises, StorageSource.openSeekable: the all-files backend answers with a real FileChannel, the SAF fallback with null, so folder mode reports needs-a-seekable-file instead of failing obscurely. Entry names go through the same ArchivePath.safeRelativePath zip-slip guard. 7z creation stays deferred. |
||
|
SoftwareApp
|
2c86f3dc67 | M4: browser UI (navigation, sort, selection, dialogs, permission prompts, platform delete) | ||
|
SoftwareApp
|
adfd95fc38 | M0: project scaffold (Gradle 9.3.1 / AGP 9.1.0, Compose shell, storage permissions) |