From 650ca8fca31985a1026ecd0215dbb76af8edc541 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Mon, 24 Aug 2026 20:36:31 -0500 Subject: [PATCH] Pick a file the way a user does, then rotate the phone Two things nothing in this repo asserted, and they are one test class because separately the second one asserts nothing new. THE PICKER. ConverterScreen opens SAF with a MIME filter, and a filter is a thing that can hide the user's file. Narrow it and the app still builds, still renders, and still passes every JVM test -- the user taps "Choose file" and gets an empty picker. The round trip now runs for real: DocumentsUI is driven with UiAutomator to a fixture root, and the app is asserted to come back with the file. The file card's name is not the only assertion, because a name proves less than it looks: it comes from a metadata query, which a URI with no read grant answers just as well. The "Container: MP4" detail row only appears once something has opened the file and read its header, so it is what says the picker handed back a URI the app can USE. THE ROTATION. MainActivity declares no configChanges, and ConversionViewModel holds the picked file in a plain MutableStateFlow with NO SavedStateHandle behind it. Nothing persists it. The only thing that carries it across a rotation is the ViewModelStore the Activity retains -- which no test anywhere asserted. Two guards run before that assertion, because both ways it could pass while proving nothing are silent: the display rotation really changed, and MainActivity really was a different instance afterwards. Without the second one this is a recomposition test wearing a rotation's name. MUTATIONS, RUN RATHER THAN ASSERTED, on a local API 34 emulator. Narrowing the filter to arrayOf("application/x-lmc-no-such-type") takes the fixture root out of the picker entirely -- DocumentsUI matches the request against Root.COLUMN_MIME_TYPES and drops roots that cannot answer -- and both tests fail: java.lang.IllegalArgumentException: the system picker never showed BySelector [TEXT='\QLMC R38 fixtures\E'] Making the ViewModel composition-scoped fails ONLY the rotation test: androidx.compose.ui.test.ComposeTimeoutException: Condition (a node tagged converter.fileCard.name exists) still not satisfied after 30000 ms and :app:testDebugUnitTest stays BUILD SUCCESSFUL under it. That divergence is what #64 exists to establish and what its own comment doubted; the PR body has the verdict and why the doubt was reasonable. THE PROVIDER HAD TO BE JAVA. It is the only Java file in the module. A manifest-declared provider is a component of the instrumentation PACKAGE, so the system starts a plain org.libremediaconverter.test process for it with only the test APK on its dex path -- and the test APK is built without the Kotlin stdlib, because the app APK has it and duplicating it is what checkDebugAndroidTestDuplicateClasses prevents. The Kotlin draft died on its first query: java.lang.NoClassDefFoundError: Failed resolution of: Lkotlin/jvm/internal/Intrinsics; at org.libremediaconverter.saf.FixtureDocumentsProvider.queryDocument The compiler emits that reference for the null checks on nearly every function, so no Kotlin dialect avoids it. Same reason nothing in that file imports androidx. No new test tags: CHOOSE_FILE, FILE_CARD_NAME and detailRow already named both ends. Co-Authored-By: Claude Opus 5 (1M context) --- app/build.gradle.kts | 7 +- .../saf/FixtureDocumentsProvider.java | 234 +++++++++++++++++ .../saf/FixtureDocumentsProvider.kt | 197 -------------- .../saf/SafPickerRoundTripTest.kt | 248 ++++++++++++++++++ 4 files changed, 487 insertions(+), 199 deletions(-) create mode 100644 app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.java delete mode 100644 app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.kt create mode 100644 app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 7287b11..a2565d9 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -235,8 +235,11 @@ val jacocoGeneratedExcludes = listOf( ) // AGP 9 compiles Kotlin through its built-in compiler, which writes here rather than to the -// classic `tmp/kotlin-classes/debug`. All hand-written code in this module is Kotlin, so the -// javac output (BuildConfig and R only) is not read at all. +// classic `tmp/kotlin-classes/debug`. All hand-written code in the MAIN source set is Kotlin, so +// the javac output (BuildConfig and R only) is not read at all. There is now one hand-written +// Java file in the module -- androidTest's FixtureDocumentsProvider, which cannot be Kotlin +// because the process it runs in has no Kotlin stdlib; its own header explains why. It is in +// androidTest, so it is not in this task's classDirectories and this stays accurate. val jacocoDebugKotlinClasses = layout.buildDirectory.dir( "intermediates/built_in_kotlinc/debug/compileDebugKotlin/classes", ) diff --git a/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.java b/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.java new file mode 100644 index 0000000..c5d7db9 --- /dev/null +++ b/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.java @@ -0,0 +1,234 @@ +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; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; + +/** + * One file, offered to the system file picker, so that picking one can be tested at all. + * + *

DocumentsUI does not browse a filesystem: it lists what {@link 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, because a {@code ContentProvider} is instantiated + * by the system and cannot be registered from test code. {@code + * app/src/androidTest/AndroidManifest.xml} is that declaration and says why each of its + * attributes is load-bearing. + * + *

The only Java file in this module, and it has to be

+ * + *

Everything else here is Kotlin. This cannot be: the Kotlin standard library is not on + * this class's classpath at runtime. + * + *

Instrumentation code normally never notices. The test APK's dex is loaded into the app's + * process, where the app APK supplies {@code kotlin.jvm.internal.Intrinsics} — so the test APK is + * built without it, deliberately, since packaging a second copy is what {@code + * checkDebugAndroidTestDuplicateClasses} exists to prevent. A provider is different. It is a + * component of the instrumentation package, so when DocumentsUI queries it the system + * starts a plain {@code org.libremediaconverter.test} process with only the test APK on its dex + * path, and no app APK anywhere. The Kotlin version of this file crashed there on its first + * query, before returning a single row: + * + * 

+ * FATAL EXCEPTION: binder:6369_2
+ * Process: org.libremediaconverter.test
+ * java.lang.NoClassDefFoundError: Failed resolution of: Lkotlin/jvm/internal/Intrinsics;
+ *     at org.libremediaconverter.saf.FixtureDocumentsProvider.queryDocument
+ *
+ * + *

The compiler emits that reference for the null checks on almost every function, so there is + * no Kotlin dialect that avoids it. For the same reason nothing here imports {@code androidx.*}: + * those classes are absent from this process for exactly the same reason. Framework and JDK only. + * + *

Why a provider rather than a file in Downloads

+ * + *

That would have worked, and it would have tested less. Two properties are what {@code + * SafPickerRoundTripTest} actually needs: + * + *

+ * + *

The descriptor is opened on a real file rather than served through a pipe, deliberately. + * {@code InputQuery.sizeOf} falls back to {@code ParcelFileDescriptor.statSize} when a provider + * omits {@code OpenableColumns.SIZE}, and a pipe's {@code statSize} is {@code -1} — an unknown + * size, which is a different case with a screen of its own. This fixture is meant to be an + * ordinary, fully described file, so that the one thing under test is the round trip. + */ +public final class FixtureDocumentsProvider extends DocumentsProvider { + + /** + * What the picker calls this root. + * + *

Deliberately not a word any other root uses. The picker's own landing screen already + * offers "Images", "Audio", "Videos" and "Documents", and a UiAutomator selector that could + * match two things is not a selector. + */ + public static final String ROOT_TITLE = "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 {@code OpenableColumns.DISPLAY_NAME} on the URI the app is handed — which is exactly the + * round trip under test. + */ + public static final String FIXTURE_DISPLAY_NAME = "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 filter under which this test could pass. + */ + public static final String FIXTURE_MIME_TYPE = "video/mp4"; + + private static final String ROOT_ID = "lmc-r38-root"; + private static final String ROOT_DOCUMENT_ID = "root"; + private static final String FIXTURE_DOCUMENT_ID = "root/" + FIXTURE_DISPLAY_NAME; + + /** Already in this source set, and already a real H.264 MP4 the engines can open. */ + private static final String FIXTURE_ASSET = "sample_h264.mp4"; + + private static final String[] DEFAULT_ROOT_PROJECTION = { + 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 static final String[] DEFAULT_DOCUMENT_PROJECTION = { + Document.COLUMN_DOCUMENT_ID, + Document.COLUMN_DISPLAY_NAME, + Document.COLUMN_MIME_TYPE, + Document.COLUMN_FLAGS, + Document.COLUMN_SIZE, + Document.COLUMN_LAST_MODIFIED, + }; + + @Override + public boolean onCreate() { + return true; + } + + /** + * The single root. + * + *

{@link 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 + public Cursor queryRoots(String[] projection) { + MatrixCursor cursor = new MatrixCursor(projection != null ? projection : DEFAULT_ROOT_PROJECTION); + cursor.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); + return cursor; + } + + @Override + public Cursor queryDocument(String documentId, String[] projection) throws FileNotFoundException { + MatrixCursor cursor = new MatrixCursor(projection != null ? projection : DEFAULT_DOCUMENT_PROJECTION); + if (ROOT_DOCUMENT_ID.equals(documentId)) { + addDirectoryRow(cursor); + } else if (FIXTURE_DOCUMENT_ID.equals(documentId)) { + addFixtureRow(cursor); + } else { + throw new FileNotFoundException("no such document: " + documentId); + } + return cursor; + } + + @Override + public Cursor queryChildDocuments(String parentDocumentId, String[] projection, String sortOrder) + throws FileNotFoundException { + MatrixCursor cursor = new MatrixCursor(projection != null ? projection : DEFAULT_DOCUMENT_PROJECTION); + if (ROOT_DOCUMENT_ID.equals(parentDocumentId)) { + addFixtureRow(cursor); + } + return cursor; + } + + @Override + public ParcelFileDescriptor openDocument(String documentId, String mode, CancellationSignal signal) + throws FileNotFoundException { + if (!FIXTURE_DOCUMENT_ID.equals(documentId)) { + throw new FileNotFoundException("no such document: " + documentId); + } + return ParcelFileDescriptor.open(fixtureFile(), ParcelFileDescriptor.MODE_READ_ONLY); + } + + private void addDirectoryRow(MatrixCursor cursor) { + cursor.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 void addFixtureRow(MatrixCursor cursor) throws FileNotFoundException { + File file = fixtureFile(); + cursor.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, file.length()) + .add(Document.COLUMN_LAST_MODIFIED, file.lastModified()); + } + + /** + * The fixture on disk, unpacked from this APK's own assets the first time anything asks. + * + *

On demand rather than seeded once in {@link #onCreate()}, because this process is started + * by whoever queries the provider and can be killed between two queries of the same test. + * + *

A failure here is reported as {@link FileNotFoundException} rather than swallowed. A + * provider that answers with a zero-byte file would put the test on the "Size unknown" screen + * with nothing saying why. + */ + private File fixtureFile() throws FileNotFoundException { + File file = new File(getContext().getFilesDir(), FIXTURE_DISPLAY_NAME); + if (file.length() > 0L) { + return file; + } + try (InputStream source = getContext().getAssets().open(FIXTURE_ASSET); + OutputStream sink = new FileOutputStream(file)) { + byte[] buffer = new byte[8192]; + int read; + while ((read = source.read(buffer)) != -1) { + sink.write(buffer, 0, read); + } + } catch (IOException e) { + throw new FileNotFoundException("could not unpack " + FIXTURE_ASSET + ": " + e); + } + return file; + } +} diff --git a/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.kt b/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.kt deleted file mode 100644 index 3b20033..0000000 --- a/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.kt +++ /dev/null @@ -1,197 +0,0 @@ -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/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt b/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt new file mode 100644 index 0000000..c245bee --- /dev/null +++ b/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt @@ -0,0 +1,248 @@ +package org.libremediaconverter.saf + +import androidx.compose.ui.test.assertTextEquals +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.performClick +import androidx.media3.common.util.UnstableApi +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import androidx.test.uiautomator.By +import androidx.test.uiautomator.BySelector +import androidx.test.uiautomator.UiDevice +import androidx.test.uiautomator.Until +import org.junit.After +import org.junit.Assert.assertNotEquals +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.MainActivity +import org.libremediaconverter.ui.TestTags + +/** + * Choosing a file, through the real system picker, and still having it after a rotation. + * + * Two defects, and neither is reachable from anywhere else in this repo. + * + * **The picker is opened with a filter, and a filter can hide the user's file.** `ConverterScreen` + * launches `ActivityResultContracts.OpenDocument` with a MIME array; DocumentsUI hides every root + * and every document that array does not match. Narrow it and the app still compiles, still + * renders, still passes every JVM test — and the user taps "Choose file" and is shown an empty + * picker. Nothing in either source set drove SAF **as a picker** before this: the only SAF coverage + * is the publish side, in `OutputPublisherPublishTest`, against hand-written `ContentProvider` + * fakes. The launcher wiring, the filter, and the read grant that comes back had never been + * executed by a test. + * + * **The picked file has to survive a rotation.** `MainActivity` declares no `configChanges`, so + * every rotation destroys and recreates it, and `ConversionViewModel` holds the picked file in a + * plain `MutableStateFlow` with no `SavedStateHandle` behind it. The only thing that carries it + * across is the retained `ViewModelStore` the Activity gets from resolving the ViewModel through + * `LocalViewModelStoreOwner`. Scope it to the composition instead and the file is gone. + * + * ### Why these two are one test class + * + * A rotation test alone has no bite of its own. `AppRootRestorationTest` already catches + * `rememberSaveable` -> `remember` on the JVM, and a second test whose only mutation is one an + * existing test catches is the vacuous test this whole decomposition exists to prevent. So the + * rotation here runs **from a real picked input**, which is a state no JVM test can produce: + * `AppRootRestorationTest` injects a stub `content` lambda specifically to avoid standing up + * either ViewModel, and `StateRestorationTester` saves into an in-memory map rather than a + * `Bundle`. + * + * ### The mutations, and what they printed + * + * Both were run, not asserted. Narrowing the wildcard array `ConverterScreen.kt` passes to + * `pickInput.launch` — to `arrayOf("application/x-lmc-no-such-type")` — empties the picker of the + * fixture root entirely, and [pickingAFileThroughTheSystemPickerFillsInTheFileCard] fails on the + * assertion that names it. + * Making the ViewModel composition-scoped leaves the picker test alone and fails + * [thePickedInputSurvivesARealRotation], with `:app:testDebugUnitTest` still BUILD SUCCESSFUL — + * which is the divergence this ticket was filed to establish, and which was doubted on it. It is + * `viewModel()` -> `viewModel(viewModelStoreOwner = remember { })`, + * **plus** `factory = ViewModelProvider.AndroidViewModelFactory()` and a `MutableCreationExtras` + * carrying `APPLICATION_KEY`. The factory half is not decoration: an owner that is not a + * `HasDefaultViewModelProviderFactory` contributes no creation extras, and the default factory + * cannot construct an `AndroidViewModel` without them — so the owner swap alone crashes on + * construction instead of demonstrating the scope. The PR body quotes both failures verbatim. + * + * ### It has to be an unlocked emulator + * + * The Pixel 10 Pro XL is secure-locked and cannot be unlocked from a shell, so the picker cannot be + * driven there at all. That is why this gap survived as long as it did. + * `tools/local-emulator/run-e2e.sh` runs API 33-36 on the development host. + */ +@UnstableApi +@RunWith(AndroidJUnit4::class) +class SafPickerRoundTripTest { + + @get:Rule + val composeRule = createAndroidComposeRule() + + private val device: UiDevice = + UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) + + /** Set by the one test that rotates, read by [restoreOrientation]. See its KDoc. */ + private var rotated = false + + /** + * Leave the device the way it was found — and only if this test moved it. + * + * Two things are deliberate here, and both are about the *other* tests on the device rather + * than about these two. + * + * The flag, because this runs after every test in the class, not only the one that rotated. An + * unconditional restore issues a WindowManager rotation request after the picker test as well, + * which has nothing to undo; JUnit does not promise method order, so that is an interaction + * between two tests that no single-class run would ever show. Tracked as a flag rather than + * read back off `isNaturalOrientation`, because a device whose *natural* orientation is + * landscape would answer that question the wrong way round. + * + * And `unfreezeRotation`, because `setOrientationNatural` does not merely rotate: it freezes + * the rotation there. A run that stopped after it would hand the next test a device that + * cannot rotate at all. + */ + @After + fun restoreOrientation() { + if (!rotated) return + device.setOrientationNatural() + device.unfreezeRotation() + device.waitForIdle() + } + + @Test + fun pickingAFileThroughTheSystemPickerFillsInTheFileCard() { + pickTheFixture() + + composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME) + .assertTextEquals(FixtureDocumentsProvider.FIXTURE_DISPLAY_NAME) + + // Not the same assertion twice. The name above comes from a metadata query, which a URI + // with no read grant answers just as well; this line only appears once something has + // opened the file and read its header. It is what says the picker handed back a URI the + // app can actually USE -- delete grantUriPermissions from the fixture's manifest entry and + // the name still arrives while this goes red. + // + // The whole "Container: MP4" and not "MP4": DetailRow renders the label and the value as + // one semantics node. + awaitNode(TestTags.Converter.detailRow(CONTAINER_LABEL)) + composeRule.onNodeWithTag(TestTags.Converter.detailRow(CONTAINER_LABEL)) + .assertTextEquals("$CONTAINER_LABEL: MP4") + } + + @Test + fun thePickedInputSurvivesARealRotation() { + pickTheFixture() + // The identity hash rather than the Activity itself, so nothing here keeps a destroyed + // Activity reachable across the recreation it is being used to detect. + val before = System.identityHashCode(composeRule.activity) + + device.setOrientationLandscape() + rotated = true + composeRule.waitForIdle() + + // Two guards before the assertion that matters, because both of the ways this test could + // pass while proving nothing are silent ones. + // + // A device that ignored the rotation request would leave the app exactly as it was, and + // "the file is still there" would then be a statement about a screen nothing happened to. + assertNotEquals( + "the device did not actually rotate, so nothing below is about a rotation", + NATURAL_ROTATION, + device.displayRotation, + ) + // And a rotation that did NOT recreate the Activity -- a configChanges attribute added to + // the manifest, an aspect-ratio or orientation lock -- would make this a recomposition + // test. The retained ViewModelStore is only interesting because the Activity around it + // really was destroyed and rebuilt. + assertNotEquals( + "the rotation did not recreate MainActivity, so the retained ViewModelStore was never used", + before, + System.identityHashCode(composeRule.activity), + ) + + awaitNode(TestTags.Converter.FILE_CARD_NAME) + composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME) + .assertTextEquals(FixtureDocumentsProvider.FIXTURE_DISPLAY_NAME) + } + + // --- driving the picker --------------------------------------------------------------- + + /** + * Taps "Choose file", walks the system picker to the fixture, and returns once the app has it. + * + * Everything between the first tap and the last belongs to `com.google.android.documentsui`, + * which is why UiAutomator is here at all: Compose's matchers stop at this process's + * composition and Espresso's at its view hierarchy, and the picker is neither. + */ + private fun pickTheFixture() { + composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).performClick() + + // THIS is the line the MIME filter mutation fails on. DocumentsUI matches the requested + // types against Root.COLUMN_MIME_TYPES and drops the roots that cannot answer, so a filter + // the fixture root does not satisfy takes the root out of the picker altogether -- along + // with "Images", "Audio", "Videos" and "Documents", measured on API 34. + val root = awaitPickerNode(By.text(FixtureDocumentsProvider.ROOT_TITLE)) { + // Which screen the picker opens on is its own business: it lands on Recent, where the + // roots are a strip at the bottom, but a device with a populated Recent may need the + // drawer. Looking in the second place widens where the root is searched for; it does + // not weaken what has to be found, which is still this root. + device.findObject(By.desc(SHOW_ROOTS_DESCRIPTION))?.click() + } + root.click() + + awaitPickerNode(By.text(FixtureDocumentsProvider.FIXTURE_DISPLAY_NAME)).click() + + awaitNode(TestTags.Converter.FILE_CARD_NAME) + } + + /** + * The picker node [selector] names, or a failure that says which one was missing. + * + * [ifAbsent] runs once, after the first wait comes up empty, and then the wait is repeated. A + * null return from `findObject` is deliberately not an error there: it is the "already on the + * right screen" case. + */ + private fun awaitPickerNode(selector: BySelector, ifAbsent: () -> Unit = {}) = + device.wait(Until.findObject(selector), PICKER_TIMEOUT_MS) + ?: run { + ifAbsent() + requireNotNull(device.wait(Until.findObject(selector), PICKER_TIMEOUT_MS)) { + "the system picker never showed $selector" + } + } + + /** + * Blocks until [tag] is in the composition, so an assertion cannot race the picker's result. + * + * The described overload of `waitUntil`, not the bare one. A timeout is how both of this + * class's mutations report themselves, and the bare overload's message is + * `Condition still not satisfied after 30000 ms` — which names neither the node nor the test. + * With the description it says which affordance never arrived, which is the whole finding. + */ + private fun awaitNode(tag: String) { + composeRule.waitUntil("a node tagged $tag exists", APP_TIMEOUT_MS) { + composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().isNotEmpty() + } + } + + private companion object { + + /** + * Generous on purpose. This waits on another app being started, and on FFprobe spawning a + * native process over a `content://` URI; a timeout that merely usually passes is a flaky + * gating leg on five API levels, which costs far more than the seconds it saves. + */ + const val PICKER_TIMEOUT_MS = 30_000L + const val APP_TIMEOUT_MS = 30_000L + + /** `Surface.ROTATION_0`, named rather than `0` so the comparison reads. */ + const val NATURAL_ROTATION = 0 + + /** DocumentsUI's drawer button. It carries no text, only this description. */ + const val SHOW_ROOTS_DESCRIPTION = "Show roots" + + /** The detail row `MediaProbe` fills in for anything it could open and identify. */ + const val CONTAINER_LABEL = "Container" + } +}