From fa10d9419265dd49b94569e3087b73bf0bf2535d Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Sun, 6 Sep 2026 10:04:06 -0500 Subject: [PATCH 1/2] Save to a document stock DocumentsUI created (#226) publish deletes a destination it could not write to -- D4's fix, so a failed save does not leave a truncated file at the name the user chose -- but only when that destination was positively zero bytes first. destinationIsKnownEmpty is careful that "I could not tell" never authorises a delete, which makes the precondition load-bearing. Until now that precondition was asserted only against a fake built to match it: OutputPublisherPublishTest writes ByteArray(0) into FakeSafProvider before each case, under a comment stating this is how CreateDocument behaves. If it were false in production, D4's fix would be inert and every existing test would still pass. It is not false. Measured on an API 34 emulator against the real dialog: the document SAF hands back is a document URI and reports a size of exactly zero before anything writes to it. RecordingPublisher reads both at the moment publish sees them, through the ConversionDependencies seam, then lets the real copy proceed so the bytes are checked too. This has to go through the picker, and through the app, and both are platform constraints rather than choices. E7 in docs/e2e-read-findings.md records the first: a DocumentsProvider is reachable only through a picker-issued grant. The second was measured here -- a host Activity in this source set owning its own CreateDocument launcher cannot be started at all, because instrumentation runs in the target app's process and ActivityScenario refuses with "Intent in process org.libremediaconverter resolved to different process org.libremediaconverter.test". So #226 has no cheap half, which is what its comment now says. Three things the flow needed, each measured rather than guessed: Both taps scroll first. On Ready the screen carries a file card, five pickers and then the button, so Convert is below the fold; performClick on an off-screen node dispatches where nothing is and throws nothing, while assertIsEnabled passes either way. The first version sat waiting for a Converted that could never come. The format stays at its default. FixtureDocumentsProvider advertises video/mp4 so the picker's MIME filter has a mutation with a shape, and DocumentsUI honours that on the save side too: choosing MP3 makes the destination audio/mpeg and the fixture root is filtered out of the save dialog entirely. The notification dialog is dismissed rather than pre-granted. Convert converts from the permission callback whichever way the answer goes, so denying is a real user's path and enough. Granting programmatically did not take -- GrantPermissionsActivity appeared anyway and swallowed the tap. The provider gains create, write and delete support, which it needs to be a save target at all. It carries @FailsOnEmulatorApi37 because anything that puts DocumentsUI on screen aborts system_server on that image, as #245 established for the other two; baseline 5 -> 6. Verified on a local API 34 emulator: three full-suite runs at 70/0/0/3, and a publish that writes no bytes fails it with "array lengths differed, expected.length=58677 actual.length=0". Co-Authored-By: Claude Opus 5 (1M context) --- .../FailsOnEmulatorApi37.kt | 2 +- .../saf/FixtureDocumentsProvider.java | 112 +++++++- .../saf/SafPickerRoundTripTest.kt | 255 +++++++++++++++++- 3 files changed, 362 insertions(+), 7 deletions(-) diff --git a/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt b/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt index fd1c93f..5b01383 100644 --- a/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt +++ b/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt @@ -78,4 +78,4 @@ annotation class FailsOnEmulatorApi37 * `INSTRUMENTATION_ABORTED`, so the count is a number taken from a partial run. The report * records the truncation next to the counts for that reason. */ -const val FAILS_ON_EMULATOR_API37_BASELINE = 5 +const val FAILS_ON_EMULATOR_API37_BASELINE = 6 diff --git a/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.java b/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.java index c5d7db9..3a082cd 100644 --- a/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.java +++ b/app/src/androidTest/java/org/libremediaconverter/saf/FixtureDocumentsProvider.java @@ -14,6 +14,8 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.ArrayList; +import java.util.List; /** * One file, offered to the system file picker, so that picking one can be tested at all. @@ -104,6 +106,18 @@ public final class FixtureDocumentsProvider extends DocumentsProvider { private static final String ROOT_DOCUMENT_ID = "root"; private static final String FIXTURE_DOCUMENT_ID = "root/" + FIXTURE_DISPLAY_NAME; + /** + * Prefix for documents this provider CREATES, as opposed to the one it serves for reading. + * + *

Two namespaces rather than one so a destination can never be confused with the fixture. + * The fixture is read-only and must stay that way for the picker tests; a destination is + * writable and deletable, which is what {@code PublishToRealSafDestinationTest} needs. + */ + public static final String DESTINATION_PREFIX = "dest/"; + + /** Document ids {@link #deleteDocument} was called with, newest last. Cleared by {@link #reset}. */ + private static final List DELETED = new ArrayList<>(); + /** 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"; @@ -147,7 +161,7 @@ public final class FixtureDocumentsProvider extends DocumentsProvider { .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_FLAGS, Root.FLAG_LOCAL_ONLY | Root.FLAG_SUPPORTS_CREATE) .add(Root.COLUMN_ICON, android.R.drawable.ic_menu_gallery); return cursor; } @@ -159,6 +173,8 @@ public final class FixtureDocumentsProvider extends DocumentsProvider { addDirectoryRow(cursor); } else if (FIXTURE_DOCUMENT_ID.equals(documentId)) { addFixtureRow(cursor); + } else if (documentId != null && documentId.startsWith(DESTINATION_PREFIX)) { + addDestinationRow(cursor, documentId); } else { throw new FileNotFoundException("no such document: " + documentId); } @@ -178,10 +194,76 @@ public final class FixtureDocumentsProvider extends DocumentsProvider { @Override public ParcelFileDescriptor openDocument(String documentId, String mode, CancellationSignal signal) throws FileNotFoundException { - if (!FIXTURE_DOCUMENT_ID.equals(documentId)) { + if (FIXTURE_DOCUMENT_ID.equals(documentId)) { + return ParcelFileDescriptor.open(fixtureFile(), ParcelFileDescriptor.MODE_READ_ONLY); + } + if (documentId == null || !documentId.startsWith(DESTINATION_PREFIX)) { throw new FileNotFoundException("no such document: " + documentId); } - return ParcelFileDescriptor.open(fixtureFile(), ParcelFileDescriptor.MODE_READ_ONLY); + int flags = "r".equals(mode) + ? ParcelFileDescriptor.MODE_READ_ONLY + : ParcelFileDescriptor.MODE_READ_WRITE | ParcelFileDescriptor.MODE_TRUNCATE; + return ParcelFileDescriptor.open(destinationFile(documentId), flags); + } + + /** + * Creates a real, empty file and reports the document id for it. + * + *

Empty is the whole point, and this provider does not get to decide it. The premise + * under test in {@code PublishToRealSafDestinationTest} is what DocumentsUI hands back + * from {@code ACTION_CREATE_DOCUMENT}, and {@code OutputPublisher.destinationIsKnownEmpty} + * authorises its cleanup delete only on a positive zero. This creates the file and writes + * nothing to it, which is what the SAF contract documents; the test asserts what actually came + * back rather than trusting either side. + */ + @Override + public String createDocument(String parentDocumentId, String mimeType, String displayName) + throws FileNotFoundException { + if (!ROOT_DOCUMENT_ID.equals(parentDocumentId)) { + throw new FileNotFoundException("cannot create in: " + parentDocumentId); + } + String documentId = DESTINATION_PREFIX + displayName; + File file = destinationFile(documentId); + try { + if (!file.createNewFile() && !file.exists()) { + throw new FileNotFoundException("could not create: " + documentId); + } + } catch (IOException e) { + throw new FileNotFoundException("could not create " + documentId + ": " + e); + } + return documentId; + } + + @Override + public void deleteDocument(String documentId) throws FileNotFoundException { + if (documentId == null || !documentId.startsWith(DESTINATION_PREFIX)) { + throw new FileNotFoundException("refusing to delete: " + documentId); + } + synchronized (DELETED) { + DELETED.add(documentId); + } + destinationFile(documentId).delete(); + } + + /** Document ids {@link #deleteDocument} was called with, newest last. */ + public static List deletedDocumentIds() { + synchronized (DELETED) { + return new ArrayList<>(DELETED); + } + } + + /** Forgets recorded deletes and removes created destinations. The process outlives one class. */ + public static void reset(File filesDir) { + synchronized (DELETED) { + DELETED.clear(); + } + File dir = new File(filesDir, "destinations"); + File[] children = dir.listFiles(); + if (children != null) { + for (File child : children) { + child.delete(); + } + } } private void addDirectoryRow(MatrixCursor cursor) { @@ -189,10 +271,32 @@ public final class FixtureDocumentsProvider extends DocumentsProvider { .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_FLAGS, Document.FLAG_DIR_SUPPORTS_CREATE) .add(Document.COLUMN_SIZE, null); } + private void addDestinationRow(MatrixCursor cursor, String documentId) throws FileNotFoundException { + File file = destinationFile(documentId); + if (!file.exists()) { + throw new FileNotFoundException("no such document: " + documentId); + } + cursor.newRow() + .add(Document.COLUMN_DOCUMENT_ID, documentId) + .add(Document.COLUMN_DISPLAY_NAME, documentId.substring(DESTINATION_PREFIX.length())) + .add(Document.COLUMN_MIME_TYPE, FIXTURE_MIME_TYPE) + .add(Document.COLUMN_FLAGS, Document.FLAG_SUPPORTS_DELETE | Document.FLAG_SUPPORTS_WRITE) + .add(Document.COLUMN_SIZE, file.length()) + .add(Document.COLUMN_LAST_MODIFIED, file.lastModified()); + } + + private File destinationFile(String documentId) throws FileNotFoundException { + File dir = new File(getContext().getFilesDir(), "destinations"); + if (!dir.isDirectory() && !dir.mkdirs()) { + throw new FileNotFoundException("could not make the destinations directory"); + } + return new File(dir, documentId.substring(DESTINATION_PREFIX.length())); + } + private void addFixtureRow(MatrixCursor cursor) throws FileNotFoundException { File file = fixtureFile(); cursor.newRow() diff --git a/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt b/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt index c82e2ce..2d97bbd 100644 --- a/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt +++ b/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt @@ -1,12 +1,18 @@ package org.libremediaconverter.saf import android.app.UiAutomation +import android.content.Context +import android.net.Uri +import android.provider.DocumentsContract +import android.provider.OpenableColumns import androidx.compose.ui.test.ComposeTimeoutException +import androidx.compose.ui.test.assertIsEnabled 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.compose.ui.test.performScrollTo import androidx.media3.common.util.UnstableApi import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry @@ -20,14 +26,22 @@ import androidx.test.uiautomator.StaleObjectException import androidx.test.uiautomator.UiDevice import androidx.test.uiautomator.Until import org.junit.After +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertEquals import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.libremediaconverter.FailsOnEmulatorApi37 import org.libremediaconverter.MainActivity +import org.libremediaconverter.convert.ConversionDependencies +import org.libremediaconverter.convert.OutputPublisher import org.libremediaconverter.ui.TestTags +import java.io.File import java.util.concurrent.atomic.AtomicInteger +import java.util.regex.Pattern /** * Choosing a file, through the real system picker, and still having it after a rotation. @@ -241,12 +255,72 @@ import java.util.concurrent.atomic.AtomicInteger * file".** That is what API 33 through 36 are for, and they answer it. */ @UnstableApi +/** + * Reads what SAF handed back, then publishes for real. + * + * The premise `OutputPublisher.destinationIsKnownEmpty` depends on has only ever been asserted + * against a fake built to match it — `OutputPublisherPublishTest` writes `ByteArray(0)` into + * `FakeSafProvider` before each case, under a comment stating this is how `CreateDocument` behaves. + * This records what stock DocumentsUI actually produced, at the moment `publish` sees it and before + * a byte is written, and then lets the real copy proceed. See #226. + */ +private class RecordingPublisher(private val app: Context) : OutputPublisher(app) { + + override fun publish(staged: File, destination: Uri) { + seenDestination = destination + seenIsDocumentUri = DocumentsContract.isDocumentUri(app, destination) + seenSizeBefore = app.contentResolver + .query(destination, arrayOf(OpenableColumns.SIZE), null, null, null) + ?.use { row -> + val column = row.getColumnIndex(OpenableColumns.SIZE) + if (column >= 0 && row.moveToFirst() && !row.isNull(column)) row.getLong(column) else null + } + // Read before the copy: the ViewModel deletes the staged file once publish returns. + savedBytes = staged.readBytes() + super.publish(staged, destination) + } + + companion object { + var savedBytes: ByteArray = ByteArray(0) + var seenDestination: Uri? = null + var seenIsDocumentUri: Boolean? = null + var seenSizeBefore: Long? = null + + fun reset() { + savedBytes = ByteArray(0) + seenDestination = null + seenIsDocumentUri = null + seenSizeBefore = null + } + } +} + @RunWith(AndroidJUnit4::class) class SafPickerRoundTripTest { + /** + * Installs [RecordingPublisher] before the Activity exists. + * + * `ConversionViewModel` resolves its publisher through `ConversionDependencies` **at + * construction**, and the Compose rule launches `MainActivity` as part of the rule chain — + * which wraps `@Before`, so `@Before` is already too late. JUnit constructs the test instance + * before it evaluates the rules, so an initialiser is early enough, and it needs no + * `@BeforeClass` (this class's companion is private, and JUnit wants a public static there). + * + * Harmless for the other two tests: neither saves, so `publish` is never called and the + * subclass behaves exactly like `OutputPublisher`. `restoreOrientation` puts the seam back. + */ + init { + RecordingPublisher.reset() + ConversionDependencies.publisher = { RecordingPublisher(it) } + } + @get:Rule val composeRule = createAndroidComposeRule() + private val context: Context = + InstrumentationRegistry.getInstrumentation().targetContext + private val device: UiDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) @@ -291,6 +365,8 @@ class SafPickerRoundTripTest { */ @After fun restoreOrientation() { + // The suite runs without Android Test Orchestrator, so a swapped seam outlives the class. + ConversionDependencies.reset() ActivityLifecycleMonitorRegistry.getInstance().removeLifecycleCallback(recreationWatcher) if (!rotated) return device.setOrientationNatural() @@ -406,6 +482,163 @@ class SafPickerRoundTripTest { * are all warm and the only thing being waited on is one screen. That is what keeps the cost * of a genuinely absent root bounded — see the class KDoc. */ + /** + * The save side of SAF, end to end, against a document stock DocumentsUI created (#226). + * + * ## What this settles + * + * `publish` deletes a destination it could not write to — `docs/defect-audit.md` **D4**'s fix, + * so a failed save does not leave a truncated file at the name the user chose — but only when + * that destination was **positively zero bytes** first. `destinationIsKnownEmpty` is careful + * that "I could not tell" never authorises a delete, which is right, and which makes the + * precondition load-bearing. + * + * Until now that precondition was asserted only against a fake built to match it: + * `OutputPublisherPublishTest` writes `ByteArray(0)` into `FakeSafProvider` before each case, + * under a comment stating this is how `CreateDocument` behaves. **If it is false in production, + * D4's fix is inert and every existing test still passes.** [RecordingPublisher] reads what SAF + * actually handed over, at the moment `publish` sees it and before a byte is written. + * + * ## Why it has to go through the app, and through the picker + * + * Through the **picker** because a `DocumentsProvider` cannot be reached any other way — + * measured three ways and recorded as **E7** in `docs/e2e-read-findings.md`: an unprotected one + * is refused at install, instrumentation carries the app's uid so the test APK's own identity + * is no help, and shell identity is denied too, each denial naming `ACTION_OPEN_DOCUMENT`. + * + * Through the **app** because the same constraint sinks the obvious alternative. A host + * Activity in this source set that owns a `CreateDocument` launcher cannot be started: + * `ActivityScenario` refuses with *"Intent in process org.libremediaconverter resolved to + * different process org.libremediaconverter.test"*. Instrumentation runs in the target app's + * process, so the only Activity available to drive is the app's own — which is also the more + * faithful thing to drive. + * + * ## The conversion is setup, not subject + * + * Save is only offered on `Converted`, so the test converts first. MP3 is chosen because the + * router sends it to FFmpeg unconditionally at every API level, so the setup cannot depend on + * the device's codecs — #223 is what that costs. + */ + @Test + @FailsOnEmulatorApi37 + fun aSaveWritesToTheDocumentTheSystemPickerCreated() { + pickTheFixture() + convertToTheDefaultFormat() + + saveThroughTheSystemPicker() + + val destination = RecordingPublisher.seenDestination + assertNotNull("publish was never reached, so nothing was saved", destination) + assertTrue( + "SAF handed back something that is not a document URI, so publish's cleanup can " + + "never run and D4's fix is inert: $destination", + RecordingPublisher.seenIsDocumentUri == true, + ) + assertEquals( + "SAF handed back a document that is not positively empty, so " + + "destinationIsKnownEmpty answers false and a failed save keeps its partial file", + 0L, + RecordingPublisher.seenSizeBefore, + ) + + // And the bytes really arrived, which only the failure side was covered for on a device. + val staged = File(context.cacheDir, "conversions") + assertArrayEquals( + "the destination did not receive what was staged", + RecordingPublisher.savedBytes, + context.contentResolver.openInputStream(destination!!)!!.use { it.readBytes() }, + ) + assertTrue("staging should be empty after a successful save", staged.listFiles().isNullOrEmpty()) + } + + /** + * Runs the conversion, leaving the screen on `Converted`. + * + * **The format is left at its default, and that is a constraint rather than laziness.** + * `ConverterScreen` registers `CreateDocument` with the *output's* MIME type, and + * [FixtureDocumentsProvider] advertises `Root.COLUMN_MIME_TYPES` of `video/mp4` — deliberately, + * so the picker's MIME filter has a mutation with a shape. DocumentsUI honours that on the save + * side too: choosing MP3 makes the destination type `audio/mpeg`, and the fixture root is then + * filtered out of the save dialog entirely. Measured, as *"the create-document dialog never + * showed LMC R38 fixtures"*. The default `MP4_H265` produces `video/mp4` and the root is + * offered. + * + * **The notification dialog is dismissed rather than pre-granted, and that is the honest + * version.** Convert never calls `convert()` directly — it launches `RequestPermission` for + * `POST_NOTIFICATIONS` and converts from the callback **whichever way the answer goes**. So the + * dialog only has to be got out of the way; denying it is a real user's path and the conversion + * still runs. Granting it programmatically was tried first and did not take — + * `GrantPermissionsActivity` appeared anyway, the click that followed went to it rather than to + * the app, and the screen sat in `Ready` with nothing enqueued. + * + * **Both taps scroll first.** On `Ready` the screen carries a file card, five pickers and then + * the button, so Convert is below the fold on a phone. `performClick` on an off-screen node + * dispatches at a position that hits nothing and throws nothing, and `assertIsEnabled` passes + * either way — the first version of this sat waiting for a `Converted` that could never come. + */ + private fun convertToTheDefaultFormat() { + composeRule.onNodeWithTag(TestTags.Converter.CONVERT) + .performScrollTo() + .assertIsEnabled() + .performClick() + + dismissThePermissionDialog() + awaitNode(TestTags.SAVE_FILE, CONVERSION_TIMEOUT_MS) + } + + /** + * Gets the `POST_NOTIFICATIONS` dialog out of the way, if this device shows one. + * + * Backing out of it is a denial, and a denial is fine here: the conversion starts either way, + * and what that costs the user is a progress notification confined to the Task Manager. Waiting + * only briefly, because on a device where the permission is already held no dialog appears at + * all and the conversion is already under way. + */ + private fun dismissThePermissionDialog() { + if (device.wait(Until.hasObject(By.pkg(PERMISSION_UI_PACKAGE)), PERMISSION_DIALOG_MS) == true) { + device.pressBack() + device.wait(Until.gone(By.pkg(PERMISSION_UI_PACKAGE)), PERMISSION_DIALOG_MS) + } + } + + /** + * Taps Save and drives the create-document dialog into the fixture root. + * + * Retried whole, for the reason [pickTheFixture] documents: a dialog that came up unreadable + * cannot be recovered from inside, and a fresh one is the only answer. + */ + private fun saveThroughTheSystemPicker() { + var missing: BySelector? = null + repeat(PICK_ATTEMPTS) { attempt -> + requireAReadableScreen() + composeRule.onNodeWithTag(TestTags.SAVE_FILE).performClick() + missing = walkTheSaveDialog( + if (attempt == 0) PICKER_TIMEOUT_MS else REOPENED_TIMEOUT_MS, + ) + if (missing == null) { + awaitNode(TestTags.Converter.CONVERT_ANOTHER, SAVE_TIMEOUT_MS) + return + } + dismissThePicker() + } + throw AssertionError( + "the create-document dialog never showed $missing, in $PICK_ATTEMPTS separate " + + "dialogs (the last one left ${device.currentPackageName} in front)", + ) + } + + /** Into the fixture root, then Save. Returns the selector never found, or null. */ + private fun walkTheSaveDialog(timeoutMs: Long): BySelector? { + val picker = By.pkg(DOCUMENTS_UI_PACKAGE) + val root = By.text(FixtureDocumentsProvider.ROOT_TITLE) + return when { + device.wait(Until.hasObject(picker), timeoutMs) != true -> picker + !tapPickerNode(root, timeoutMs, ifAbsent = ::openTheRootsDrawer) -> root + !tapPickerNode(SAVE_BUTTON, timeoutMs) -> SAVE_BUTTON + else -> null + } + } + private fun pickTheFixture() { var missing: BySelector? = null repeat(PICK_ATTEMPTS) { attempt -> @@ -795,8 +1028,8 @@ class SafPickerRoundTripTest { } } - private fun awaitNode(tag: String) { - composeRule.waitUntil("a node tagged $tag exists", APP_TIMEOUT_MS) { + private fun awaitNode(tag: String, timeoutMs: Long = APP_TIMEOUT_MS) { + composeRule.waitUntil("a node tagged $tag exists", timeoutMs) { composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().isNotEmpty() } } @@ -811,6 +1044,24 @@ class SafPickerRoundTripTest { const val PICKER_TIMEOUT_MS = 30_000L const val APP_TIMEOUT_MS = 30_000L + /** The runtime-permission dialog's package, so it can be recognised and dismissed. */ + const val PERMISSION_UI_PACKAGE = "com.google.android.permissioncontroller" + + /** Short: either the dialog is up almost immediately, or the permission was already held. */ + const val PERMISSION_DIALOG_MS = 5_000L + + /** A 3 s clip to MP3 on an emulator is about a second; this only bounds a hang. */ + const val CONVERSION_TIMEOUT_MS = 120_000L + + /** The copy is a few kilobytes, but it crosses a provider. */ + const val SAVE_TIMEOUT_MS = 30_000L + + /** + * DocumentsUI's save button. Case-insensitive because the label is "SAVE" on some images + * and "Save" on others, and the difference is not what this test is about. + */ + val SAVE_BUTTON: BySelector = By.text(Pattern.compile("save", Pattern.CASE_INSENSITIVE)) + /** * The same wait once a picker has already come and gone, and shorter for a reason. * -- 2.47.3 From b23ff0f082f01e750303ec4ec67093873b194f0f Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Sun, 6 Sep 2026 10:28:02 -0500 Subject: [PATCH 2/2] Wait for the app to come back before asking Compose about it The save test failed an API 35 leg with "No compose hierarchies found in the app". Dismissing the POST_NOTIFICATIONS dialog presses back and waits for the permission UI to be gone, but going away and the app being in front again are not the same moment, and the next Compose query landed in the gap. Asked of UiAutomator rather than through awaitAppFocus, which is the opposite of what this class argues for elsewhere and is right here: awaitAppFocus goes through composeRule.waitUntil, so it would raise the very error it is being used to avoid. Two more local API 34 runs at 70/0/0/3. Co-Authored-By: Claude Opus 5 (1M context) --- .../saf/SafPickerRoundTripTest.kt | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt b/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt index 2d97bbd..aacd7b5 100644 --- a/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt +++ b/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt @@ -595,10 +595,20 @@ class SafPickerRoundTripTest { * all and the conversion is already under way. */ private fun dismissThePermissionDialog() { - if (device.wait(Until.hasObject(By.pkg(PERMISSION_UI_PACKAGE)), PERMISSION_DIALOG_MS) == true) { - device.pressBack() - device.wait(Until.gone(By.pkg(PERMISSION_UI_PACKAGE)), PERMISSION_DIALOG_MS) + if (device.wait(Until.hasObject(By.pkg(PERMISSION_UI_PACKAGE)), PERMISSION_DIALOG_MS) != true) { + return } + device.pressBack() + device.wait(Until.gone(By.pkg(PERMISSION_UI_PACKAGE)), PERMISSION_DIALOG_MS) + // And wait for the app to be in front again before anything asks Compose about it. + // Querying while another window still owns the screen raises "No compose hierarchies found + // in the app", which is what this test did on an API 35 leg: the back press had landed but + // the dialog had not finished going away. + // + // Asked of UiAutomator rather than through awaitAppFocus, which is the opposite of what the + // class KDoc argues for elsewhere and is right here: awaitAppFocus goes through + // composeRule.waitUntil, so it would raise the very error it is being used to avoid. + device.wait(Until.hasObject(By.pkg(context.packageName)), FOCUS_TIMEOUT_MS) } /** -- 2.47.3