diff --git a/AGENTS.md b/AGENTS.md index 6b9ad3e..2de9dfe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,6 +27,7 @@ feature name once you know what you are changing. GRADLE_FLAGS=--offline ./tools/verify.sh # when the network is unavailable ./gradlew :app:assembleDebug # then: adb install -r app/build/outputs/apk/debug/app-debug.apk +./tools/measure-startup.sh 5 # cold start, median of five, on the installed device ./gradlew :app:detektBaseline # re-freezes the findings that exist now — think before running it python3 tools/make-code-map.py # regenerate docs/CODE_MAP.md after touching source ``` @@ -73,8 +74,9 @@ comment. `app/src/test/java/…/ProjectInvariantsTest.kt` holds the source-level - **Comments explain why, not what.** The house style is a KDoc paragraph per file and type that says what it is for and what was rejected — decisions and limits, not restatements of the code. - **A test name is a sentence about behaviour** (`a batch rename is a single undo step`), because - the names are read as the spec. Logic gets behaviour tests; rules about the shape of the project - get source-level tests. + the names are read as the spec. Logic gets behaviour tests, rules about the shape of the project + get source-level tests, and **what a user sees gets a Compose rendering test** — those run on the + JVM through Robolectric, so they are part of the same gate rather than needing a device. - **The docs are part of the change.** `CHANGELOG.md` (Added/Changed/Fixed, in the house voice), the `ROADMAP.md` milestone table, the README if a claim changed, and `docs/EDGE_CASE_TESTING.md` for anything only a device could confirm. @@ -112,6 +114,7 @@ comment. `app/src/test/java/…/ProjectInvariantsTest.kt` holds the source-level | `CHANGELOG.md` | The reasoning behind each change. Grep it; do not read it end to end | | `docs/ROADMAP.md` | Milestones with their evidence, the key decisions, the deferrals | | `docs/EDGE_CASE_TESTING.md` | What was verified, on which device or AVD, and what was not | +| `docs/PERFORMANCE.md` | The numbers that exist, and the ones that deliberately do not yet | | `docs/PLAY_RELEASE_CHECKLIST.md` | Release only — work it in order | | `docs/COMPETITIVE_ANALYSIS.md`, `docs/MONETISATION.md`, `docs/STORE_LISTING.md` | Product context, pricing, and length-checked listing copy | | `PRIVACY.md` | The policy text the store listing points at | diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dc8c63..0060550 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,6 +158,37 @@ upload needs a **higher `versionCode`** than every previous upload. place. **371 JVM tests** still pass, lint is clean, the map is current, and the gate is green in 17 seconds. +- **The user interface is now tested as a user sees it, and a debug build tells on itself.** Fifteen + rendering tests across four files are the first tests in this project that compose a screen: + the app bar's star exists only where there is a folder to star and says *Add* or *Remove* according + to the state, the selection bar offers Rename for several files and Details only for one, the batch + rename dialog spells out the names it will produce and refuses a collision with the file named, the + shared name dialog refuses a slash as it is typed, and a favourite that has gone says *Folder not + found* instead of 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 `tools/verify.sh` gate as + everything else instead of needing a device; **386 JVM tests** in total. Writing them turned up a + behaviour worth recording: the batch dialog's numbering continues the folder's series, so a + collision can only be *shown* by forcing the start number back, which is exactly why the feature + works in practice. + + The debug build now enables **StrictMode** (disk reads and writes, network, leaked closables, logged + rather than fatal) and attaches **LeakCanary**. A file manager is the worst possible app for a + blocking disk read on the main thread — every listing, `stat` and thumbnail is one — 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 device is a worse trade than a line in logcat. The + flag is read from the manifest rather than from `BuildConfig`, which this build does not generate. + + **Performance has a number for the first time.** `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](docs/PERFORMANCE.md) records the result: **1 814 + ms** median over five runs on the phone, in the debug build, which is the slow case by construction + (StrictMode, LeakCanary, no R8). The same file states what is *not* measured — folder open at 1 000 / + 5 000 / 20 000 files, thumbnail paint while scrolling, and keystroke-to-result — with the method each + would use and the honest reason they are missing: they 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 rather than half-configured: build + wiring nobody can run is worse than a recorded gap. + ### Changed - **File thumbnails are bigger.** The leading visual in a list row went from 28 dp to 48 dp and diff --git a/README.md b/README.md index 193ec22..a0f2054 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,9 @@ and Android Lint, cheapest first; `--release` adds an R8 build. CI because R8 is the one check that has caught a defect nothing else could. To run it before pushing: `git config core.hooksPath .githooks`. +The numbers that do exist — and the ones deliberately still missing — are in +[docs/PERFORMANCE.md](docs/PERFORMANCE.md). + Release signing is optional and driven by a git-ignored `keystore.properties` (see `keystore.properties.example`); without it the release build still runs R8/minification but produces an unsigned artefact. diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 4086988..669f63c 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -201,6 +201,15 @@ dependencies { testImplementation(libs.junit) // Runs the real Android framework on the JVM, so the data layer is testable without a device. testImplementation(libs.robolectric) + // Compose rendering tests: the only shape of test here that asserts what a user actually *sees*. + // They run on the JVM through Robolectric too, so they belong to the same gate as everything else + // rather than needing a device. The manifest artifact supplies the activity they are hosted in. + testImplementation(platform(libs.androidx.compose.bom)) + testImplementation(libs.androidx.compose.ui.test.junit4) + debugImplementation(libs.androidx.compose.ui.test.manifest) + // Leak detection, debug builds only. It installs itself; there is no code to add, and nothing of + // it reaches a release build (the dependency is on the debug variant alone). + debugImplementation(libs.leakcanary.android) // Lets a ViewModel test drive `viewModelScope` on a deterministic dispatcher. testImplementation(libs.kotlinx.coroutines.test) } diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/FileManagerApplication.kt b/app/src/main/java/de/softwareapp_hb/filemanager/FileManagerApplication.kt index 483c871..23b675a 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/FileManagerApplication.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/FileManagerApplication.kt @@ -2,6 +2,8 @@ package de.softwareapp_hb.filemanager import android.app.Application import android.content.Context +import android.content.pm.ApplicationInfo +import android.os.StrictMode import coil3.ImageLoader import coil3.SingletonImageLoader import coil3.video.VideoFrameDecoder @@ -64,6 +66,33 @@ class FileManagerApplication : Application(), SingletonImageLoader.Factory { override fun onCreate() { super.onCreate() + /* + * StrictMode, in a debug build only. + * + * A file manager is the worst possible app for a disk read on the main thread — every + * listing, `stat` and thumbnail is one — and a debug build is the only place where noticing + * it is still cheap. `penaltyLog` and nothing stronger on purpose: the point is to see the + * violation in logcat while using the app, not to turn a slow screen into a crash on a + * device someone is relying on. Checked through the manifest flag rather than BuildConfig, + * which this build does not generate. + */ + if (applicationInfo.flags and ApplicationInfo.FLAG_DEBUGGABLE != 0) { + StrictMode.setThreadPolicy( + StrictMode.ThreadPolicy.Builder() + .detectDiskReads() + .detectDiskWrites() + .detectNetwork() + .penaltyLog() + .build(), + ) + StrictMode.setVmPolicy( + StrictMode.VmPolicy.Builder() + .detectLeakedClosableObjects() + .penaltyLog() + .build(), + ) + } + storage = StorageProvider(this) notifier = ServiceOperationNotifier(this) deleteRequests = MediaStoreDeleter(this).factory() diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserDialogsTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserDialogsTest.kt new file mode 100644 index 0000000..942b6eb --- /dev/null +++ b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserDialogsTest.kt @@ -0,0 +1,135 @@ +package de.softwareapp_hb.filemanager.ui.browser + +import androidx.compose.ui.test.assertIsEnabled +import androidx.compose.ui.test.assertIsNotEnabled +import androidx.compose.ui.test.hasSetTextAction +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performTextClearance +import androidx.compose.ui.test.performTextInput +import de.softwareapp_hb.filemanager.core.FileEntry +import de.softwareapp_hb.filemanager.core.StoragePath +import de.softwareapp_hb.filemanager.ui.theme.FileManagerTheme +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +/** + * What the two name dialogs say before anything is pressed. + * + * The claim these protect is the one the project keeps making in prose: a rename that cannot be done + * is *refused with the reason* rather than discovered as a failure afterwards, and a name that is not + * one path segment never reaches the file operations at all. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class BrowserDialogsTest { + + @get:Rule + val compose = createComposeRule() + + private val downloads = StoragePath.of("/storage/emulated/0/Download") + + private fun entry(name: String) = + FileEntry(downloads.child(name), isDirectory = false, sizeBytes = 4L, lastModifiedMillis = 0L) + + private fun batchRename( + entries: List, + namesInFolder: List, + /** Forced start number: the dialog otherwise carries the folder's series on, which is the point. */ + start: String? = null, + ) { + compose.setContent { + FileManagerTheme { + BatchRenameDialog( + entries = entries, + namesInFolder = namesInFolder, + onDismiss = {}, + onConfirm = { _, _ -> }, + ) + } + } + // The name field is the first of the two; the start number is the second. + compose.onAllNodes(hasSetTextAction())[0].performTextInput("Holiday") + if (start != null) { + compose.onAllNodes(hasSetTextAction())[1].performTextClearance() + compose.onAllNodes(hasSetTextAction())[1].performTextInput(start) + } + } + + @Test + fun `the batch dialog spells the names out before anything moves`() { + batchRename( + entries = listOf(entry("a.txt"), entry("b.txt")), + namesInFolder = listOf("a.txt", "b.txt"), + ) + + compose.onNodeWithText("Will be renamed to Holiday (1).txt, Holiday (2).txt").assertExists() + compose.onNodeWithText("Rename").assertIsEnabled() + } + + @Test + fun `a name the folder already holds is named, and the button is off`() { + // The start is forced back to 1 on purpose: left to itself the dialog continues the folder's + // series (from "Holiday (1)"), which is exactly what makes a collision avoidable in practice. + batchRename( + entries = listOf(entry("a.txt"), entry("b.txt")), + namesInFolder = listOf("a.txt", "b.txt", "Holiday (1).txt"), + start = "1", + ) + + compose.onNodeWithText("Holiday (1).txt is already in this folder").assertExists() + compose.onNodeWithText("Rename").assertIsNotEnabled() + } + + @Test + fun `files that already hold the names are said to be done`() { + batchRename( + entries = listOf(entry("Holiday (1).txt"), entry("Holiday (2).txt")), + namesInFolder = listOf("Holiday (1).txt", "Holiday (2).txt"), + start = "1", + ) + + compose.onNodeWithText("Every selected file already has that name").assertExists() + } + + @Test + fun `a name with a slash is refused while it is typed`() { + compose.setContent { + FileManagerTheme { + NameDialog( + title = "New folder", + confirmLabel = "Create", + initialValue = "notes/final.txt", + onDismiss = {}, + onConfirm = {}, + ) + } + } + + compose.onNodeWithText("A name cannot contain a slash").assertExists() + compose.onNodeWithText("Create").assertIsNotEnabled() + } + + @Test + fun `a plain name is accepted`() { + compose.setContent { + FileManagerTheme { + NameDialog( + title = "New folder", + confirmLabel = "Create", + initialValue = "final.txt", + onDismiss = {}, + onConfirm = {}, + ) + } + } + + compose.onNodeWithText("A name cannot contain a slash").assertDoesNotExist() + compose.onNodeWithText("Create").assertIsEnabled() + } +} diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserRootsTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserRootsTest.kt new file mode 100644 index 0000000..79cc918 --- /dev/null +++ b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserRootsTest.kt @@ -0,0 +1,91 @@ +package de.softwareapp_hb.filemanager.ui.browser + +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithText +import de.softwareapp_hb.filemanager.core.FileCategory +import de.softwareapp_hb.filemanager.core.StoragePath +import de.softwareapp_hb.filemanager.ui.theme.FileManagerTheme +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +/** + * The two kinds of row the home screen puts above a volume: a starred folder and a library. + * + * Both carry a claim that is easy to lose in a refactor — a favourite shows *where* it is, and says + * so when it is not there any more; a library shows a count only when the index has one, because a + * zero would claim the device holds none. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class BrowserRootsTest { + + @get:Rule + val compose = createComposeRule() + + private val volume = StoragePath.of("/storage/emulated/0") + + @Test + fun `a starred folder shows its label, where it is, and how to remove it`() { + compose.setContent { + FileManagerTheme { + BookmarkRow( + label = "Internal storage", + bookmark = Bookmark(volume), + onClick = {}, + onRemove = {}, + ) + } + } + + compose.onNodeWithText("Internal storage").assertExists() + compose.onNodeWithText("/storage/emulated/0").assertExists() + compose.onNodeWithContentDescription("Remove from favourites").assertExists() + } + + @Test + fun `a favourite that has gone says so instead of its path`() { + compose.setContent { + FileManagerTheme { + BookmarkRow( + label = "Holiday", + bookmark = Bookmark(volume.child("Holiday"), exists = false), + onClick = {}, + onRemove = {}, + ) + } + } + + compose.onNodeWithText("Folder not found").assertExists() + compose.onNodeWithText("/storage/emulated/0/Holiday").assertDoesNotExist() + } + + @Test + fun `a library the index knows nothing about shows no count`() { + compose.setContent { + FileManagerTheme { + LibraryRow(category = FileCategory.IMAGES, count = null, onClick = {}) + } + } + + compose.onNodeWithText("Images").assertExists() + // A zero would be a claim about the device; "no count" is the honest version of "no answer". + compose.onNodeWithText("item", substring = true).assertDoesNotExist() + } + + @Test + fun `a library with an index behind it shows what it holds`() { + compose.setContent { + FileManagerTheme { + LibraryRow(category = FileCategory.IMAGES, count = 3, onClick = {}) + } + } + + compose.onNodeWithText("3 items").assertExists() + } +} diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserTopBarTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserTopBarTest.kt new file mode 100644 index 0000000..aa843ab --- /dev/null +++ b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserTopBarTest.kt @@ -0,0 +1,93 @@ +package de.softwareapp_hb.filemanager.ui.browser + +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithText +import de.softwareapp_hb.filemanager.ui.theme.FileManagerTheme +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +/** + * What the app bar offers, as a user sees it. + * + * The star is the one action whose *presence* is a rule rather than a layout choice — it exists only + * where there is a folder to come back to — and it is drawn from two booleans that the view model + * supplies, so getting them across is worth a test that renders the bar rather than one that reads + * the state. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class BrowserTopBarTest { + + @get:Rule + val compose = createComposeRule() + + private fun topBar( + canBookmark: Boolean = true, + bookmarked: Boolean = false, + searchOpen: Boolean = false, + ) = compose.setContent { + FileManagerTheme { + BrowserTopBar( + title = "Download", + canNavigateUp = true, + canBookmark = canBookmark, + bookmarked = bookmarked, + query = "", + searchOpen = searchOpen, + searchScope = SearchScope.FOLDER, + scopeChoosable = true, + canPaste = false, + onUp = {}, + onQuery = {}, + onSearchOpen = {}, + onSearchScope = {}, + onSort = {}, + onPaste = {}, + onToggleBookmark = {}, + onRefresh = {}, + onUsage = {}, + onDuplicates = {}, + onTrash = {}, + ) + } + } + + @Test + fun `inside a folder the star invites the folder to be starred`() { + topBar(canBookmark = true, bookmarked = false) + + compose.onNodeWithContentDescription("Add to favourites").assertIsDisplayed() + compose.onNodeWithText("Download").assertIsDisplayed() + } + + @Test + fun `a starred folder says so instead`() { + topBar(canBookmark = true, bookmarked = true) + + compose.onNodeWithContentDescription("Remove from favourites").assertIsDisplayed() + } + + @Test + fun `a library or the root list has nothing to star`() { + topBar(canBookmark = false) + + compose.onNodeWithContentDescription("Add to favourites").assertDoesNotExist() + compose.onNodeWithContentDescription("Remove from favourites").assertDoesNotExist() + } + + @Test + fun `the star gives way to the search field`() { + // The field takes the whole bar while it is open; a star squeezed in beside it would be a + // button with nowhere to go. + topBar(canBookmark = true, searchOpen = true) + + compose.onNodeWithContentDescription("Add to favourites").assertDoesNotExist() + } +} diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/SelectionBarTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/SelectionBarTest.kt new file mode 100644 index 0000000..2a57f32 --- /dev/null +++ b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/SelectionBarTest.kt @@ -0,0 +1,79 @@ +package de.softwareapp_hb.filemanager.ui.browser + +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithText +import de.softwareapp_hb.filemanager.core.FileEntry +import de.softwareapp_hb.filemanager.core.StoragePath +import de.softwareapp_hb.filemanager.ui.theme.FileManagerTheme +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +/** + * Which actions a selection is offered — the rule that Rename means one thing for a single file and + * another for several, and that "the details" of twelve files is not a screen. + * + * Presence rather than position: the bar scrolls horizontally, so an action can be off-screen at a + * given width and still correctly offered. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class SelectionBarTest { + + @get:Rule + val compose = createComposeRule() + + private val downloads = StoragePath.of("/storage/emulated/0/Download") + + private fun entry(name: String) = + FileEntry(downloads.child(name), isDirectory = false, sizeBytes = 4L, lastModifiedMillis = 0L) + + private fun bar(selected: Int) { + val entries = listOf(entry("a.txt"), entry("b.txt"), entry("c.txt")).take(selected) + compose.setContent { + FileManagerTheme { + SelectionBar( + state = BrowserUiState( + path = downloads, + entries = entries, + selection = entries.map { it.path }.toSet(), + ), + onCopy = {}, + onCut = {}, + onDelete = {}, + onDetails = {}, + onRename = {}, + onOpenWith = {}, + onShare = {}, + onCompress = {}, + onExtract = {}, + onSelectAll = {}, + onClear = {}, + ) + } + } + } + + @Test + fun `several files are offered a rename and not the details`() { + bar(selected = 3) + + compose.onNodeWithContentDescription("Rename").assertExists() + compose.onNodeWithContentDescription("Details").assertDoesNotExist() + compose.onNodeWithText("3 items").assertExists() + } + + @Test + fun `a single file keeps both`() { + bar(selected = 1) + + compose.onNodeWithContentDescription("Rename").assertExists() + compose.onNodeWithContentDescription("Details").assertExists() + compose.onNodeWithText("1 item").assertExists() + } +} diff --git a/docs/CODE_MAP.md b/docs/CODE_MAP.md index 6293cee..7c609cd 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. -76 source files, 12463 lines - 29 test files, 371 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. +76 source files, 12492 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`. @@ -96,17 +96,17 @@ The browser screen: view model, state, list and grid, dialogs, selection bar | `BrowserComponents.kt` | 301 | The thumbnail an entry's leading visual gets in a list row. ListThumbnailSize, GridThumbnailSize, GridCellMinSize, EntryIcon(), FileEntry(), FileEntry(), VIDEO_FRAME_MILLIS, iconFor(), iconFor(), EntryRow(), EntryGridCell(), FileEntry() | - | | `BrowserContent.kt` | 463 | Everything below the app bar: the permission prompt, the roots, or a directory. WIDE_LAYOUT_MIN_WIDTH, MasterPaneWidth, BrowserContent(), LazyListScope(), LIBRARY_SECTION_KEY, LazyListScope(), BOOKMARK_SECTION_KEY, DirectoryView(), FileEntry(), IndexStatusRow(), FileEntry() | - | | `BrowserDetailPane.kt` | 131 | The second pane of the wide layout: what is inside the folder the user picked, or a preview and the details of the file they picked. DetailPane(), FolderPreview(), ImagePreview() | - | -| `BrowserDialogs.kt` | 279 | Renaming several files at once, with the numbering worked out before anything moves. BatchRenameDialog(), PREVIEW_NAMES, batchRenameNote(), NameDialog(), ConfirmDialog(), ConflictDialog() | - | -| `BrowserRoots.kt` | 270 | A row on the root screen: a volume, with its free space when the platform reports it. RootRow(), itemCountLabel(), EmptyState(), LibrarySectionHeader(), BookmarkSectionHeader(), SectionHeader(), BookmarkRow(), LibraryRow(), Breadcrumbs() | - | +| `BrowserDialogs.kt` | 279 | Renaming several files at once, with the numbering worked out before anything moves. BatchRenameDialog(), PREVIEW_NAMES, batchRenameNote(), NameDialog(), ConfirmDialog(), ConflictDialog() | `BrowserDialogsTest` (5) | +| `BrowserRoots.kt` | 270 | A row on the root screen: a volume, with its free space when the platform reports it. RootRow(), itemCountLabel(), EmptyState(), LibrarySectionHeader(), BookmarkSectionHeader(), SectionHeader(), BookmarkRow(), LibraryRow(), Breadcrumbs() | `BrowserRootsTest` (4) | | `BrowserScreen.kt` | 591 | BrowserScreen() | - | -| `BrowserTopBar.kt` | 238 | The browser's app bar: title, up, the favourite star, and the search / sort / overflow actions. BrowserTopBar(), SEARCH_FIELD_WIDTH | - | +| `BrowserTopBar.kt` | 238 | The browser's app bar: title, up, the favourite star, and the search / sort / overflow actions. BrowserTopBar(), SEARCH_FIELD_WIDTH | `BrowserTopBarTest` (4) | | `BrowserUiState.kt` | 311 | One immutable description of everything the browser shows. ViewMode, SearchScope, Clipboard, UiMessage, ConflictPrompt, RootItem, Bookmark, DetailFocus, Undo, UndoMove, Overlay, BrowserUiState | - | | `BrowserViewModel.kt` | 2100 | Drives the browser screen. BrowserViewModel, ArchivePreview | `BrowserViewModelTest` (89) | | `DetailsSheet.kt` | 109 | Name, location, size, type and modified date for one entry. DetailsSheet(), DetailLines(), DetailLine() | - | | `EntryIntents.kt` | 58 | Wraps an entry's path in a content:// URI another app may read. contentUriFor(), viewIntent(), shareIntent() | - | | `IncomingBar.kt` | 60 | The bar shown while another app is waiting on this one. IncomingBar() | - | | `IncomingRequest.kt` | 46 | What another app asked this one to do. IncomingRequest | - | -| `SelectionBar.kt` | 176 | The contextual action bar shown while entries are selected. StorageAccessPrompt(), SelectionBar() | - | +| `SelectionBar.kt` | 176 | The contextual action bar shown while entries are selected. StorageAccessPrompt(), SelectionBar() | `SelectionBarTest` (2) | | `SortSheet.kt` | 128 | Sort key, direction, hidden files and list/grid — all in one sheet. sortKeyLabel(), SortSheet() | - | ## ui/viewer/ @@ -161,7 +161,7 @@ Compose theme, colours, type | file | lines | what it holds | tests | |---|---|---|---| -| `FileManagerApplication.kt` | 97 | Application entry point and the app's tiny object graph. FileManagerApplication | - | +| `FileManagerApplication.kt` | 126 | Application entry point and the app's tiny object graph. FileManagerApplication | - | | `MainActivity.kt` | 65 | MainActivity | - | ## Behaviour index @@ -534,6 +534,28 @@ Every test name in the suite, by file: the fastest way to see whether something - a determinate operation shows the counter - starting the service puts it in the foreground with that title +### `ui/browser/BrowserDialogsTest.kt` (5) + +- the batch dialog spells the names out before anything moves +- a name the folder already holds is named, and the button is off +- files that already hold the names are said to be done +- a name with a slash is refused while it is typed +- a plain name is accepted + +### `ui/browser/BrowserRootsTest.kt` (4) + +- a starred folder shows its label, where it is, and how to remove it +- a favourite that has gone says so instead of its path +- a library the index knows nothing about shows no count +- a library with an index behind it shows what it holds + +### `ui/browser/BrowserTopBarTest.kt` (4) + +- inside a folder the star invites the folder to be starred +- a starred folder says so instead +- a library or the root list has nothing to star +- the star gives way to the search field + ### `ui/browser/BrowserViewModelTest.kt` (89) - starts on the root list @@ -626,6 +648,11 @@ Every test name in the suite, by file: the fastest way to see whether something - while providing, tapping a file chooses it instead of opening it - while providing, a folder still navigates so the file can actually be reached +### `ui/browser/SelectionBarTest.kt` (2) + +- several files are offered a rename and not the details +- a single file keeps both + ### Test files with no source file of the same name - `ProjectInvariantsTest.kt` (4 tests) - ProjectInvariants, ProjectInvariants diff --git a/docs/EDGE_CASE_TESTING.md b/docs/EDGE_CASE_TESTING.md index 8270248..ec91dfe 100644 --- a/docs/EDGE_CASE_TESTING.md +++ b/docs/EDGE_CASE_TESTING.md @@ -264,6 +264,9 @@ row's wording and the message are the same strings those tests assert. opens the row instead of selecting it. Driving it manually on the API 35 AVD then paid for the caution: the compress/extract pass below found the selection bar clipping its own actions. The archive engine itself is covered by end-to-end JVM tests, including a zip-slip archive. +- **Folder open, thumbnail paint and search timings.** Only cold start has been measured — see + [PERFORMANCE](PERFORMANCE.md), which also records why the rest needs a benchmark module and a + profileable build, and why that module does not exist yet. - **The SAF folder-picker round trip.** Driving the system picker and granting a tree was not attempted; the code path is unit-tested against a stand-in `DocumentsProvider`. - **Image viewer and thumbnails.** Verified on the AVD in M4/M5; not repeated on the phone. diff --git a/docs/PERFORMANCE.md b/docs/PERFORMANCE.md new file mode 100644 index 0000000..4b44e61 --- /dev/null +++ b/docs/PERFORMANCE.md @@ -0,0 +1,68 @@ +# Performance + +What has been measured, on what, and — explicitly — what has **not** been measured at all. The rule +this file exists to enforce comes from the competitive survey: the market leader's top complaint is +thumbnails that load late, its own support reply to a speed complaint was *"your SD card is slow"*, +and the lesson recorded there was to measure what was measured and never close a performance +complaint by blaming the hardware. + +Until this file existed, the app had **no performance number of any kind**. + +## Cold start + +| build | device | runs (ms) | median | +|---|---|---|---| +| debug, `1.0.0-debug` | Xiaomi phone, Android 15 | 1 758 · 1 773 · 1 814 · 1 926 · 2 043 | **1 814 ms** | + +Taken with `./tools/measure-startup.sh 5`: `am start -W`, and one warm-up start discarded because the +first launch after an install pays for dexopt and folding that into a median is how a baseline gets +written that nothing can meet. + +The debug build is deliberately the *slow* case — StrictMode on, LeakCanary attached, no R8 — so this +is an upper bound rather than what a user sees. **The release number is the one worth publishing, and +it has not been taken yet.** Until it is, no speed claim belongs in the README or the store listing. + +### What StrictMode found on the first cold start + +The same debug build now logs StrictMode violations, and the first cold start produced five, against a +1 814 ms start: + +| duration | where | ours? | +|---|---|---| +| 492 ms | `BrowserViewModel.Factory` → `app.cacheDir` | **yes** — `Context.getCacheDir()` checks and creates the private cache directory while the view model is being created, on the main thread | +| 397 ms | `BrowserViewModel.rootsOf` → `StorageProvider.space` → `VolumeRepository.stat` | **yes** — free space for every volume is read with `StatFs` inside `refresh()`, which the screen calls from a `LaunchedEffect` | +| 477 ms | `FileManagerTheme` → `typography = FileManagerTypography` | no — the platform loading its own typefaces on first composition | +| 54 ms × 2 | `miui.util.font.FontSettings` via `MainActivity.onCreate` | no — the OEM's font hook doing `File.exists` while the activity is created | + +That is **~1.5 s of main-thread disk I/O inside a 1.8 s start**, of which **~0.9 s is ours**. Both of +ours are mechanical to move and neither is fixed yet: the cache directory is only needed when a preview +copies something, and the free-space line on the roots screen can arrive a frame later. They are the +first items of the speed work. + +## Not measured yet, and why + +| number | how it would be taken | why it is not here | +|---|---|---| +| Folder open, 1 000 / 5 000 / 20 000 files | macrobenchmark `StartupTimingMetric` over a fixture volume, or `TraceSectionMetric` around the listing | needs a `:benchmark` module and a profileable build installed under the **release package id**; on this MIUI phone the first install of a new package id waits for a confirmation dialog, so it cannot run unattended | +| Thumbnail paint while scrolling a photo grid | `FrameTimingMetric` | same | +| Keystroke → search result | `TraceSectionMetric` around the query and the index read | same | + +The module does not exist yet, deliberately: half-configured build wiring that nobody can run is worse +than an honest gap. It is the next measurement step, and it belongs with the baseline-profile work, +which needs the same module. + +## What is measured instead, meanwhile + +The gate measures things that are not latency but do not rot: the test count, lint, detekt against its +baseline, and whether `docs/CODE_MAP.md` still matches the tree. Those are the numbers that can be +kept true by automation; the three above need a device and a decision about which device. + +## Two rules + +- **No number that was not taken from a real build on a real device.** Not from a debug build + pretending to be release, and not from reading the code. +- **No performance change judged by reading the code.** The bigger-thumbnail change (28 → 48 dp in a + row, 40 → 96 dp in a grid) is the standing example: it may be exactly right, and nobody measured + either side of it. `docs/COMPETITIVE_ANALYSIS.md` had already written "a thumbnail that is bigger and + slower is strictly worse than one that is smaller and instant" — which is a hypothesis until it is a + number. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index aa31f75..6a053b7 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -34,6 +34,7 @@ Legend: ✅ done · 🚧 in progress · ⏳ not started · 💡 candidate (propo | Storage access | `MANAGE_EXTERNAL_STORAGE` first, SAF fallback | Google Play explicitly permits "All files access" for file managers; SAF keeps the app usable if the user declines | | Connectivity | Local-first (no `INTERNET` in v1) | Preserves the "nothing leaves the device" claim and keeps the Data Safety form trivial | | File sharing | Receives `SEND`/`SEND_MULTIPLE`, serves `GET_CONTENT`/`OPEN_DOCUMENT`, and deliberately does **not** register `ACTION_VIEW` | A file manager should be able to receive a file and to supply one, and neither costs a permission. Registering as a viewer as well would put Plain Files in the "open with" chooser the app itself uses, offering it back to itself | +| Performance | Measured, not assumed: [PERFORMANCE](PERFORMANCE.md) holds the numbers and `tools/measure-startup.sh` takes one of them | The survey's top complaint is latency, so a speed claim has to be a number — and a number nobody can reproduce is worse than none | | Quality gate | `tools/verify.sh` — map check, detekt, tests, lint; `--release` adds R8 — run by CI, by the pre-push hook and by hand | One command that means the same thing in all three places. The detekt baseline is what makes adopting static analysis possible without a backlog-clearing week, while still failing on anything new | | Monetisation | One-time paid (€4.49; €2.29 launch price), set in the Console | No Billing client, no `BILLING` permission, no purchase code — which is what keeps "nothing leaves the device" true in the shipped APK. Free → paid is impossible after the first upload, so the price must be set beforehand. See [MONETISATION](MONETISATION.md) | diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index cdf9464..182dbae 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -38,6 +38,9 @@ xz = "1.10" # Testing junit = "4.13.2" robolectric = "4.17" +# Debug-only leak detection: a Compose screen that outlives its screen is invisible in tests and +# obvious in a heap dump. Never shipped — `debugImplementation` below. +leakcanary = "2.14" [libraries] androidx-core-ktx = { group = "androidx.core", name = "core-ktx", version.ref = "coreKtx" } @@ -73,6 +76,10 @@ xz = { group = "org.tukaani", name = "xz", version.ref = "xz" } junit = { group = "junit", name = "junit", version.ref = "junit" } robolectric = { group = "org.robolectric", name = "robolectric", version.ref = "robolectric" } +# Compose rendering tests, run on the JVM through Robolectric. Versions come from the Compose BOM. +androidx-compose-ui-test-junit4 = { group = "androidx.compose.ui", name = "ui-test-junit4" } +androidx-compose-ui-test-manifest = { group = "androidx.compose.ui", name = "ui-test-manifest" } +leakcanary-android = { group = "com.squareup.leakcanary", name = "leakcanary-android", version.ref = "leakcanary" } kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "coroutines" } # Static analysis, run by the :app:detekt task from its own configuration — never an app dependency. diff --git a/tools/measure-startup.sh b/tools/measure-startup.sh new file mode 100755 index 0000000..a0dc16f --- /dev/null +++ b/tools/measure-startup.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# +# Cold-start baseline: installs nothing, measures the app that is already on the device. +# +# ./tools/measure-startup.sh # three runs +# ./tools/measure-startup.sh 5 # five runs +# +# Why `am start -W` rather than a benchmark library: it needs no new module and no instrumented +# test APK, and "time from tap to first frame" is exactly what it reports. What it does *not* do is +# measure anything inside the app — folder open, thumbnail paint and search latency need a +# macrobenchmark module (see docs/PERFORMANCE.md for what is still missing and why). +set -euo pipefail + +runs=${1:-3} +package=de.softwareapp_hb.filemanager.debug +component="$package/de.softwareapp_hb.filemanager.MainActivity" + +if ! adb shell pm list packages | grep -qx "package:$package"; then + echo "measure-startup: $package is not installed." >&2 + echo " ./gradlew :app:assembleDebug && adb install -r app/build/outputs/apk/debug/app-debug.apk" >&2 + exit 2 +fi + +version=$(adb shell dumpsys package "$package" | sed -n 's/.*versionName=\([^ ]*\).*/\1/p' | head -1) +echo "package $package ($version), $runs cold starts" + +# One start that is thrown away. The first launch after an install pays for dexopt and JIT, and +# folding that into a median is how a baseline gets written that nothing can meet. +adb shell am force-stop "$package" +sleep 1 +adb shell am start -W -n "$component" >/dev/null +sleep 2 + +times=$( + for run in $(seq 1 "$runs"); do + adb shell am force-stop "$package" + sleep 1 + adb shell am start -W -n "$component" | sed -n 's/^TotalTime: //p' | tr -d '\r' + done | sort -n +) + +echo "runs (ms): $(echo "$times" | tr '\n' ' ')" +# Median of an odd count; for an even count this takes the lower of the two middle runs, which is +# the honest side to err on when the point is to notice a regression. +echo "$times" | awk ' + { value[NR] = $1 } + END { + printf "median: %d ms (min %d, max %d)\n", value[int((NR + 1) / 2)], value[1], value[NR] + } +'