Pick a file through the real system picker, then rotate the phone #80

Merged
JMR-dev merged 4 commits from test/r38-8-saf-e2e into main 2026-08-25 02:23:37 +00:00
4 Commits
Author SHA1 Message Date
JMR-devandClaude Opus 5 3925f1aa9f Re-find the picker node when it goes stale, and re-measure API 37
CI found a flake this workstation could not, and fixing it overturned half of what
the previous commit recorded about API 37.

THE FLAKE. UiObject2 caches the AccessibilityNodeInfo it was found with, and
DocumentsUI is still settling when a node first appears -- its list rebinds, the
roots strip lays out, a window animates. If the node is replaced in that gap,
click() throws against the handle rather than missing the target:

  androidx.test.uiautomator.StaleObjectException
    at androidx.test.uiautomator.UiObject2.getAccessibilityNodeInfo(UiObject2.java:1042)
    at androidx.test.uiautomator.UiObject2.click(UiObject2.java:526)
    at SafPickerRoundTripTest.pickTheFixture(SafPickerRoundTripTest.kt:223)

It is not intermittent on a COLD emulator -- CI hit it on API 33, 34 and 35, every
one of them, on the first run. It never appeared here because the local emulator had
been warm for an hour. tapPickerNode now re-finds the node and taps again, three
attempts. That retries acquiring a handle to a node that has to be there anyway:
every attempt still goes through awaitPickerNode, which fails outright if it is
absent, so the MIME mutation's bite is untouched. Verified with `pm clear
com.google.android.documentsui` between runs, five for five green on API 34.

AND THE CORRECTION IT FORCED. The previous commit marked the whole class
@FailsOnEmulatorApi37 on the strength of two measured failures. One of them was
this bug. Re-measured with the fix, one method per fresh android-37.0 emulator:

  thePickedInputSurvivesARealRotation             INSTRUMENTATION_ABORTED:
                                                  System has crashed.
  pickingAFileThroughTheSystemPickerFillsInTheFileCard              PASSED

So a rotation, which rebuilds every surface at once, is what the gralloc mapper does
not survive; starting another app's activity is not. The marker moves to the one
method that earned it, and the picker test runs on the gating API 37 leg like
anything else. The workflow comment, run-e2e.sh and the doc all say that now.

The lesson is worth more than the measurement, and the doc keeps it: an annotation
is a claim about an IMAGE, and a broken test makes every image look broken. Both a
framework abort and a stale node read as "the run fell over". Re-measure after
fixing a test before deciding what the platform did.

Also measured rather than assumed, since it is what keeps the gating leg green: the
runner's annotation filter honours a class-level marker, expanding it to every
method. On API 34, `annotation=` selected exactly 4 tests (2 Media3EngineTest + 2
here) and `notAnnotation=` selected 55 with neither of these in it. CI's own gating
API 37 leg then reported 55 / 0 on the previous push. That is why moving the marker
to a single method is a narrowing rather than a repair.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 21:14:07 -05:00
JMR-devandClaude Opus 5 a3c835b7c9 Keep the picker test off the API 37 gating leg, having measured why
The API 37 emulator images abort surfaceflinger inside the guest's Gralloc5 mapper,
init SIGKILLs zygote with it, and the framework restarts under the run. run-e2e.sh
and the CI leg disable SystemUI to remove the trigger -- but that removes the IDLE
one, RegionSamplingThread's nav-bar luma sampling. Driving DocumentsUI and rotating
the display are not idle. They are the first things in this suite that generate
surface traffic of their own.

Both tests were measured on android-37.0 under swangle_indirect with SystemUI
disabled and verified quiet, and measured SEPARATELY -- inferring the second from
the first is the mistake docs/api-37-emulator-crash.md opens by correcting. They
fail in the two shapes a framework restart produces:

  thePickedInputSurvivesARealRotation
    INSTRUMENTATION_ABORTED: System has crashed.
    Expected 59 tests, received 50
    (5 hasReadColorBufferDma aborts; the framework dies DURING the test, so six
     later tests never run and the XML carries a failure with no text at all)

  pickingAFileThroughTheSystemPickerFillsInTheFileCard
    androidx.test.uiautomator.StaleObjectException
      at androidx.test.uiautomator.UiObject2.click(UiObject2.java:526)
    (3 aborts; the picker's root node was rebuilt between finding it and tapping it)

Both pass on API 33 and API 36 locally -- whole suite, 59/0/0/2 on each -- which is
the same evidence pattern that made the Media3EngineTest pair the image rather than
the app.

So the class carries @FailsOnEmulatorApi37 and runs on the advisory leg.

THREE PLACES SAID "nothing in this suite touches system UI", and that is what makes
the SystemUI-disable deviation defensible. It is no longer true of the suite, and all
three are corrected rather than left to rot -- the workflow comment, run-e2e.sh's
header, and the doc. The rule they state is being APPLIED, not broken: the thing that
depends on system UI is excluded from the leg that cannot be trusted for it.

Two consequences stated rather than left to be discovered:

- run-e2e.sh applies no annotation filter, unlike CI, so a local `run-e2e.sh 37`
  reports these two on top of the Media3 pair AND DOES NOT FINISH. Its totals come
  back short and which later tests ran is arbitrary. The summary row now says so;
  it previously promised "exactly two failures", which would have read as a
  regression in someone else's diff.
- The advisory job is still named "E2E API 37 Media3 hardware transcode", and half
  of what it now runs is neither. Renaming a check touches branch protection, so it
  is deliberately not done here; the doc records the staleness and the revisit
  trigger now says the marker covers two unrelated bugs that can go green apart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 20:58:34 -05:00
JMR-devandClaude Opus 5 650ca8fca3 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) <noreply@anthropic.com>
2026-08-24 20:36:31 -05:00
JMR-devandClaude Opus 5 b18f45def7 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) <noreply@anthropic.com>
2026-08-24 20:15:33 -05:00