From b18f45def76ee6ba1f404732978a0eec0ddeb936 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Mon, 24 Aug 2026 20:15:33 -0500 Subject: [PATCH] Give the system file picker something to pick Nothing in either source set drives SAF as a picker. The only SAF coverage is the publish side, in OutputPublisherPublishTest, against hand-written ContentProvider fakes -- so the launcher wiring in ConverterScreen, the MIME filter it passes, and the grant that comes back have never been executed by a test. Driving the real picker needs three things this repo did not have. UiAutomator, because DocumentsUI is another process. Compose's matchers stop at this process's composition and Espresso's stop at its view hierarchy; neither can see or tap a window belonging to another package. It FLOATS, at "2.+", which is the same argument the catalog already makes for work and lifecycle rather than a new one: androidx.test.uiautomator is inside floatedGroupPrefixes, so the componentSelection guard makes "+" mean "newest RELEASED", and that is load-bearing here -- this library publishes 2.4.0-alphas above its stable, so without the guard the float would be a pin to a prerelease. Resolved to 2.4.0 (released) on debugAndroidTestRuntimeClasspath, checked rather than assumed. It is deliberately NOT pinned alongside ktlint/detekt/JaCoCo/Robolectric: those are pinned because a new rule or a new runtime changes the verdict on files nobody touched. UiAutomator has no verdict -- it taps what a selector names, and a selector that stops matching is this repo's test to fix, in a diff that explains itself. The "2." rather than a bare "+" is the one thing held back: a major is where the selector API would be free to change under exactly that assumption. A DocumentsProvider, because DocumentsUI does not browse a filesystem -- it lists what providers offer it. Writing a file into Downloads would have worked and tested less: the fixture root declares Root.COLUMN_MIME_TYPES, and DocumentsUI filters the drawer by it, which is what gives the MIME filter a mutation with a shape rather than "one file among the hundreds in Downloads was not listed". Its contents are also exactly one file, where a shared directory accumulates whatever earlier runs left behind. And the first AndroidManifest.xml this source set has ever had, to declare it -- a ContentProvider is instantiated by the system and cannot be registered from test code. In androidTest rather than src/debug so it is installed by the instrumentation APK only, and never appears in a developer's own file picker. Two things worth knowing before editing either file. XML comments cannot contain "--", which the manifest's first draft failed the build on; and "*/" inside a KDoc closes the comment, which the provider's did. Both are silent in review and loud in the build. No test yet, and no new test tag: TestTags.Converter.CHOOSE_FILE and FILE_CARD_NAME already name both ends of the round trip. Co-Authored-By: Claude Opus 5 (1M context) --- app/build.gradle.kts | 6 + app/src/androidTest/AndroidManifest.xml | 47 +++++ .../saf/FixtureDocumentsProvider.kt | 197 ++++++++++++++++++ gradle/libs.versions.toml | 17 ++ 4 files changed, 267 insertions(+) create mode 100644 app/src/androidTest/AndroidManifest.xml create mode 100644 app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 6ed9c72..7287b11 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -340,5 +340,11 @@ dependencies { androidTestImplementation(libs.androidx.espresso.core) androidTestImplementation(libs.compose.ui.test.junit4) androidTestImplementation(libs.androidx.work.testing) + // androidTest only, and it has to be: UiAutomator drives the whole device, including + // windows belonging to other packages. The system file picker is one -- DocumentsUI runs + // in its own process, so Compose's matchers cannot see it and Espresso's cannot either + // (both are scoped to this process's view hierarchy). Nothing on the JVM has a device to + // drive, so there is no unit-test counterpart to add it to. + androidTestImplementation(libs.androidx.uiautomator) debugImplementation(libs.compose.ui.test.manifest) } diff --git a/app/src/androidTest/AndroidManifest.xml b/app/src/androidTest/AndroidManifest.xml new file mode 100644 index 0000000..2ed15ba --- /dev/null +++ b/app/src/androidTest/AndroidManifest.xml @@ -0,0 +1,47 @@ + + + + + + + + + + + + + diff --git a/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.kt b/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.kt new file mode 100644 index 0000000..3b20033 --- /dev/null +++ b/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.kt @@ -0,0 +1,197 @@ +package org.libremediaconverter.saf + +import android.database.Cursor +import android.database.MatrixCursor +import android.os.CancellationSignal +import android.os.ParcelFileDescriptor +import android.provider.DocumentsContract.Document +import android.provider.DocumentsContract.Root +import android.provider.DocumentsProvider +import java.io.File + +/** + * One file, offered to the system file picker, so that picking one can be tested at all. + * + * DocumentsUI does not browse the filesystem: it lists what [DocumentsProvider]s hand it. So a + * test that drives the real picker has to supply the thing being picked, and it has to supply it + * as a manifest-declared component — a `ContentProvider` is instantiated by the system, never + * registered from test code. `app/src/androidTest/AndroidManifest.xml` is that declaration and + * says why each of its attributes is load-bearing. + * + * ### Why not just write a file to Downloads + * + * That would work and it would test less. Two properties of a provider are what + * [SafPickerRoundTripTest] actually needs: + * + * - **The root declares [Root.COLUMN_MIME_TYPES], and DocumentsUI filters the drawer by it.** + * That is what gives the MIME mutation a bite with a shape: ask for a type this root does not + * offer and the root itself is not in the picker, so the failure is "the fixture root is not + * there" rather than "one file among the hundreds in Downloads was not listed". + * - **The contents are exactly this and nothing else.** A shared directory accumulates whatever + * earlier runs and other tests left in it, and a picker test that finds the wrong file passes. + * + * ### It runs in its own process, so it seeds itself + * + * The instrumentation's own code executes inside the *app's* process; this provider is a + * component of the instrumentation *package*, so the system starts a separate process under + * `org.libremediaconverter.test` for it. Nothing the test sets up in a field is visible here. + * The bytes therefore come from this APK's own assets, on demand, and the test and the provider + * agree only on the constants below — which are compile-time, so they cannot drift. + * + * The descriptor is opened on a real file rather than served through a pipe, deliberately. + * `InputQuery.sizeOf` falls back to `ParcelFileDescriptor.statSize` when a provider omits + * `OpenableColumns.SIZE`, and a pipe's `statSize` is `-1` — an unknown size, which is a + * different case with its own screen. This fixture is meant to be an ordinary, fully described + * file, so the one thing under test is the round trip. + */ +class FixtureDocumentsProvider : DocumentsProvider() { + + override fun onCreate(): Boolean = true + + /** + * The single root, named [ROOT_TITLE] so a UiAutomator text selector is unambiguous. + * + * [Root.COLUMN_MIME_TYPES] is the important column. Left null it would mean "this root + * supports everything", the picker would list it whatever was asked for, and the MIME + * mutation would have nothing to bite on. + */ + override fun queryRoots(projection: Array?): Cursor = + MatrixCursor(copyOf(projection, DEFAULT_ROOT_PROJECTION)).apply { + newRow() + .add(Root.COLUMN_ROOT_ID, ROOT_ID) + .add(Root.COLUMN_DOCUMENT_ID, ROOT_DOCUMENT_ID) + .add(Root.COLUMN_TITLE, ROOT_TITLE) + .add(Root.COLUMN_SUMMARY, "Instrumentation fixture") + .add(Root.COLUMN_MIME_TYPES, FIXTURE_MIME_TYPE) + .add(Root.COLUMN_FLAGS, Root.FLAG_LOCAL_ONLY) + .add(Root.COLUMN_ICON, android.R.drawable.ic_menu_gallery) + } + + override fun queryDocument(documentId: String?, projection: Array?): Cursor = + MatrixCursor(copyOf(projection, DEFAULT_DOCUMENT_PROJECTION)).apply { + when (documentId) { + ROOT_DOCUMENT_ID -> addDirectoryRow() + FIXTURE_DOCUMENT_ID -> addFixtureRow() + else -> throw java.io.FileNotFoundException("no such document: $documentId") + } + } + + override fun queryChildDocuments( + parentDocumentId: String?, + projection: Array?, + sortOrder: String?, + ): Cursor = MatrixCursor(copyOf(projection, DEFAULT_DOCUMENT_PROJECTION)).apply { + if (parentDocumentId == ROOT_DOCUMENT_ID) addFixtureRow() + } + + override fun openDocument(documentId: String?, mode: String?, signal: CancellationSignal?): ParcelFileDescriptor { + if (documentId != FIXTURE_DOCUMENT_ID) { + throw java.io.FileNotFoundException("no such document: $documentId") + } + return ParcelFileDescriptor.open(fixtureFile(), ParcelFileDescriptor.MODE_READ_ONLY) + } + + private fun MatrixCursor.addDirectoryRow() { + newRow() + .add(Document.COLUMN_DOCUMENT_ID, ROOT_DOCUMENT_ID) + .add(Document.COLUMN_DISPLAY_NAME, ROOT_TITLE) + .add(Document.COLUMN_MIME_TYPE, Document.MIME_TYPE_DIR) + .add(Document.COLUMN_FLAGS, 0) + .add(Document.COLUMN_SIZE, null) + } + + private fun MatrixCursor.addFixtureRow() { + newRow() + .add(Document.COLUMN_DOCUMENT_ID, FIXTURE_DOCUMENT_ID) + .add(Document.COLUMN_DISPLAY_NAME, FIXTURE_DISPLAY_NAME) + .add(Document.COLUMN_MIME_TYPE, FIXTURE_MIME_TYPE) + .add(Document.COLUMN_FLAGS, 0) + .add(Document.COLUMN_SIZE, fixtureFile().length()) + .add(Document.COLUMN_LAST_MODIFIED, fixtureFile().lastModified()) + } + + /** + * The fixture on disk, unpacked from this APK's assets the first time anything asks. + * + * Idempotent rather than seeded once at [onCreate], because this process is started by + * whoever queries the provider and can be killed between two queries of the same test. + */ + private fun fixtureFile(): File { + val context = requireNotNull(context) { "provider used before onCreate" } + val file = File(context.filesDir, FIXTURE_DISPLAY_NAME) + if (file.length() == 0L) { + context.assets.open(FIXTURE_ASSET).use { source -> + file.outputStream().use(source::copyTo) + } + } + return file + } + + /** + * [projection] as a `MatrixCursor` will take it, or [fallback] when the caller asked for + * everything. + * + * Rebuilt element by element rather than spread into `arrayOf`, so nothing depends on the + * variance of the platform array type this arrives as. + */ + private fun copyOf(projection: Array?, fallback: Array): Array = + if (projection == null) fallback else Array(projection.size) { projection[it] } + + companion object { + + /** Matches `android:authorities` in `app/src/androidTest/AndroidManifest.xml`. */ + const val AUTHORITY: String = "org.libremediaconverter.test.fixtures" + + /** + * What the picker's drawer calls this root. + * + * Deliberately not a word any other root uses. DocumentsUI's drawer also lists + * "Downloads", "Images", "Videos" and the device name, and a UiAutomator selector that + * could match two of them is not a selector. + */ + const val ROOT_TITLE: String = "LMC R38 fixtures" + + /** + * What the file card has to end up showing. + * + * The same string reaches the assertion two ways — as the picker row UiAutomator taps, + * and as `OpenableColumns.DISPLAY_NAME` on the URI the app is handed — which is exactly + * the round trip under test. + */ + const val FIXTURE_DISPLAY_NAME: String = "lmc-r38-fixture.mp4" + + /** + * The type the root advertises, and the one the MIME mutation has to stop matching. + * + * A real type rather than something invented, so the wildcard filter the screen passes + * today is not the only one under which this test could pass. + */ + const val FIXTURE_MIME_TYPE: String = "video/mp4" + + private const val ROOT_ID = "lmc-r38-root" + private const val ROOT_DOCUMENT_ID = "root" + private const val FIXTURE_DOCUMENT_ID = "root/$FIXTURE_DISPLAY_NAME" + + /** Already in this source set, and already a real H.264 MP4 the engines can open. */ + private const val FIXTURE_ASSET = "sample_h264.mp4" + + private val DEFAULT_ROOT_PROJECTION = arrayOf( + Root.COLUMN_ROOT_ID, + Root.COLUMN_DOCUMENT_ID, + Root.COLUMN_TITLE, + Root.COLUMN_SUMMARY, + Root.COLUMN_MIME_TYPES, + Root.COLUMN_FLAGS, + Root.COLUMN_ICON, + ) + + private val DEFAULT_DOCUMENT_PROJECTION = arrayOf( + Document.COLUMN_DOCUMENT_ID, + Document.COLUMN_DISPLAY_NAME, + Document.COLUMN_MIME_TYPE, + Document.COLUMN_FLAGS, + Document.COLUMN_SIZE, + Document.COLUMN_LAST_MODIFIED, + ) + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 8441472..69f21ea 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -42,6 +42,18 @@ annotation = "1.+" junit = "4.+" androidxJunit = "1.+" espressoCore = "3.+" +# UiAutomator. FLOATING, and the argument for it is the one the guard already makes: +# androidx.test.uiautomator is inside `floatedGroupPrefixes` ("androidx."), so `2.+` reads +# as "the newest RELEASED 2.x" exactly the way `work = "2.+"` does -- and this library does +# publish alphas above its stable, so without the guard it would be a pin. +# +# Not pinned like ktlint/detekt/robolectric, because it is not that kind of dependency. Those +# are pinned because a new *rule* or a new *runtime* makes untouched files fail -- the tool +# changes its verdict on code nobody edited. UiAutomator has no verdict: it clicks what a +# selector names, and a selector that stops matching is this repo's test to fix, in a diff +# that says so. `2.` and not bare `+` because 3.x does not exist yet and a major is where the +# selector API would be free to change under exactly that assumption. +uiautomator = "2.+" # PINNED, unlike its neighbours. Under semver a 0.x minor is allowed to break, and # this library is load-bearing exactly where breakage is hardest to see: the wrapper # reaches for smartexception.java.Exceptions only when an FFmpeg call FAILS, so a @@ -139,6 +151,11 @@ junit = { group = "junit", name = "junit", version.ref = "junit" } androidx-junit = { group = "androidx.test.ext", name = "junit", version.ref = "androidxJunit" } androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-core", version.ref = "espressoCore" } +# The only way to touch UI this app does not own. Compose's own matchers stop at this +# process's composition, and the system file picker is a DocumentsUI activity in another +# process -- so a SAF round trip is unreachable without it. +androidx-uiautomator = { group = "androidx.test.uiautomator", name = "uiautomator", version.ref = "uiautomator" } + # Robolectric — an Android runtime for the JVM test source set, so file-lifecycle behaviour # that needs a real Context can be verified without a device. The instrumented suite cannot # run on the development host at all (see CLAUDE.md), so an androidTest-only red test is not