JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

Re-find the picker node when it goes stale, and re-measure API 37

Browse Source 
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>
This commit is contained in:
JMR-dev
2026-08-24 21:14:07 -05:00
co-authored by Claude Opus 5
parent a3c835b7c9
commit 3925f1aa9f
4 changed files with 154 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/status_check.yml
+15 -8
View File
@@ -247,14 +247,21 @@ jobs:
# .github/scripts/e2e-run.sh explains the mechanism and why every step of
# it is verified rather than assumed.
#
# "this leg" and not "this suite", since 2026-08-24: the suite now has a
# test that DOES touch system UI. SafPickerRoundTripTest drives DocumentsUI
# and rotates the display, both of which reach the gralloc mapper this image
# aborts in -- disabling SystemUI removes the idle trigger, not that one. It
# was measured failing here, per test, and carries @FailsOnEmulatorApi37, so
# notAnnotation below keeps it off this row. The rule the caveat states is
# doing its job rather than being violated; docs/api-37-emulator-crash.md
# has both failures.
# "this leg" and not "this suite", since 2026-08-24, and the difference is
# now load-bearing: SafPickerRoundTripTest DOES touch system UI. It drives
# DocumentsUI and rotates the display, and both reach the gralloc mapper
# this image aborts in -- disabling SystemUI removes the IDLE trigger, not
# those. Measured per method on android-37.0: the ROTATION test takes the
# framework down (INSTRUMENTATION_ABORTED) and carries
# @FailsOnEmulatorApi37, so notAnnotation below keeps it off this row; the
# PICKER test passes and runs here like anything else. A rotation rebuilds
# every surface at once, and starting another app's activity does not.
#
# So this row does now run one test that depends on system UI, and the
# caveat above still applies to it: a green here is not evidence the picker
# works on a device with SystemUI running -- the Pixel release check is.
# docs/api-37-emulator-crash.md has the per-method measurements, and the
# correction that produced them.
#
# api-level must be "37.0". A bare 37 is not an SDK package and fails
# during setup, which cost a run to discover.
app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt
+74 -25
View File
@@ -10,6 +10,7 @@ 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.StaleObjectException
import androidx.test.uiautomator.UiDevice
import androidx.test.uiautomator.Until
import org.junit.After
@@ -74,36 +75,37 @@ import org.libremediaconverter.ui.TestTags
* `tools/local-emulator/run-e2e.sh` runs API 33-36 on the development host, and both tests pass
* there: **59 / 0 / 0 / 2 at API 33 and again at API 36**, whole suite, 2026-08-24.
*
* ### Why [FailsOnEmulatorApi37] is on this class
* ### Why only the rotation test carries [FailsOnEmulatorApi37]
*
* Measured, per that annotation's own rule, and measured **per test** rather than inferred from
* one of them — see `docs/api-37-emulator-crash.md`, which this is the first entry in that is not
* a codec.
*
* This is the first thing in the suite that touches system UI, and the android-37.x images are
* where that stops being free: surfaceflinger aborts inside the guest's Gralloc5 mapper, init
* This class is the first thing in the suite that touches system UI, and the android-37.x images
* are where that stops being free: surfaceflinger aborts inside the guest's Gralloc5 mapper, init
* SIGKILLs zygote with it, and the framework restarts underneath the run. Disabling SystemUI --
* the deviation the API 37 leg already makes -- removes the *idle* trigger, not this one. Driving
* DocumentsUI and rotating the display generate exactly the surface traffic that reaches the
* mapper. Both tests fail on `android-37.0` under `swangle_indirect` with SystemUI disabled, and
* they fail in the two shapes a framework restart produces:
* the deviation the API 37 leg already makes -- removes the *idle* trigger, not this one.
*
* The marker is on one method and not on the class, because that is what was measured, one method
* per fresh emulator, on `android-37.0` under `swangle_indirect`:
*
* ```
* thePickedInputSurvivesARealRotation
* INSTRUMENTATION_ABORTED: System has crashed. (5 hasReadColorBufferDma aborts; the run
* Expected 59 tests, received 50 never finished, taking 6 later tests out)
*
* pickingAFileThroughTheSystemPickerFillsInTheFileCard
* androidx.test.uiautomator.StaleObjectException (3 aborts; the picker's root node was
* at UiObject2.click(UiObject2.java:526) rebuilt between finding it and tapping it)
* thePickedInputSurvivesARealRotation INSTRUMENTATION_ABORTED: System has crashed.
* Expected 1 tests, received 0
* pickingAFileThroughTheSystemPickerFillsInTheFileCard PASSED
* ```
*
* The annotation says only that, and CI reads it twice, so this class runs on the advisory API 37
* leg and not on the gating one. **Do not read it as "a rotation is allowed to lose the file".**
* That is what API 33 through 36 are for, and they answer it.
* A rotation rebuilds every surface on screen at once, which the mapper does not survive; merely
* starting DocumentsUI does not.
*
* **The first version of this said the class, and it was wrong.** The picker test had failed at
* API 37 too -- with a `StaleObjectException` that turned out to be this file's own bug rather
* than the image's, and which CI then reproduced deterministically at API 33, 34 and 35. Fixing
* it ([tapPickerNode]) and re-measuring is what separated the two. An annotation is a claim about
* an image, and a broken test makes every image look broken; **re-measure after fixing a test
* before deciding what the platform did.**
*
* The annotation says only that, and CI reads it twice, so the rotation test runs on the advisory
* API 37 leg and not the gating one. **Do not read it as "a rotation is allowed to lose the
* file".** That is what API 33 through 36 are for, and they answer it.
*/
@UnstableApi
@FailsOnEmulatorApi37
@RunWith(AndroidJUnit4::class)
class SafPickerRoundTripTest {
@@ -162,6 +164,7 @@ class SafPickerRoundTripTest {
}
@Test
@FailsOnEmulatorApi37
fun thePickedInputSurvivesARealRotation() {
pickTheFixture()
// The identity hash rather than the Activity itself, so nothing here keeps a destroyed
@@ -213,20 +216,56 @@ class SafPickerRoundTripTest {
// 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)) {
tapPickerNode(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()
tapPickerNode(By.text(FixtureDocumentsProvider.FIXTURE_DISPLAY_NAME))
awaitNode(TestTags.Converter.FILE_CARD_NAME)
}
/**
* Finds the picker node [selector] names and taps it, re-finding it if it goes stale.
*
* **The re-finding is not padding, and this is not a retry of the assertion.** A `UiObject2`
* holds an `AccessibilityNodeInfo` captured when it was found, and DocumentsUI is still
* settling when the node first appears — its list rebinds, the roots strip lays out, a window
* animates. If the node is replaced in that gap, `click()` throws `StaleObjectException`
* against the handle rather than missing the target. Measured on a cold API 34 emulator:
*
* ```
* androidx.test.uiautomator.StaleObjectException
* at androidx.test.uiautomator.UiObject2.getAccessibilityNodeInfo(UiObject2.java:1042)
* at androidx.test.uiautomator.UiObject2.click(UiObject2.java:526)
* ```
*
* So what is retried is *acquiring a handle to a node that has to be there anyway* — every
* attempt still goes through [awaitPickerNode], which fails outright if the node is absent.
* The MIME mutation's bite is untouched: a root that is not in the picker is not found on any
* attempt, and the failure is still "the system picker never showed" rather than a stale one.
*/
private fun tapPickerNode(selector: BySelector, ifAbsent: () -> Unit = {}) {
var stale: StaleObjectException? = null
repeat(TAP_ATTEMPTS) { attempt ->
// ifAbsent only on the first attempt: it navigates, and re-navigating from a screen it
// already reached would walk away from the node.
val node = awaitPickerNode(selector, if (attempt == 0) ifAbsent else ({}))
device.waitForIdle()
try {
node.click()
return
} catch (e: StaleObjectException) {
stale = e
}
}
throw AssertionError("$selector kept going stale between finding it and tapping it", stale)
}
/**
* The picker node [selector] names, or a failure that says which one was missing.
*
@@ -270,6 +309,16 @@ class SafPickerRoundTripTest {
/** `Surface.ROTATION_0`, named rather than `0` so the comparison reads. */
const val NATURAL_ROTATION = 0
/**
* How many times a picker node may be re-found before its staleness is the finding.
*
* Three, not "until the timeout". Each attempt already waits up to [PICKER_TIMEOUT_MS] for
* the node to exist, so this bounds only the settling window after it does; a node that is
* still being replaced after three of those is telling you something about the device, and
* a loop that hid it would be the flake rather than the fix.
*/
const val TAP_ATTEMPTS = 3
/** DocumentsUI's drawer button. It carries no text, only this description. */
const val SHOW_ROOTS_DESCRIPTION = "Show roots"
docs/api-37-emulator-crash.md
+38 -22
View File
@@ -331,39 +331,55 @@ and proceeding straight to the tests fails exactly as before. The harness theref
API 37 coverage at all. **Anything that ever does depend on system UI must not trust this
leg.** Something now does; see the section below.
### Something does depend on system UI now, and it is excluded rather than trusted
### Something does depend on system UI now, and half of it is excluded
Added 2026-08-24, and it is the first entry on this page that is not a codec.
Added 2026-08-24, and the first entry on this page that is not a codec.
`SafPickerRoundTripTest` drives the real system file picker and rotates the display. Both reach
the gralloc mapper — DocumentsUI is another app's windows, and a rotation rebuilds every surface
on screen — and **disabling SystemUI does not help**, because it removes the *idle* trigger
(RegionSamplingThread's nav-bar luma sampling) and not this one. The two tests fail on
`android-37.0` under `swangle_indirect` with SystemUI disabled and verified quiet, and they were
measured **separately**, because inferring the second from the first would have been the same
mistake this page's opening correction is about:
(RegionSamplingThread's nav-bar luma sampling) and not this one.
| test | how it fails | `hasReadColorBufferDma` aborts in the window |
Measured one method per fresh emulator, `android-37.0`, `swangle_indirect`, SystemUI disabled and
verified quiet — separately, because inferring the second from the first is the mistake this
page's opening correction is about:
| test | result on android-37.0 | `hasReadColorBufferDma` aborts in the window |
|---|---|---|
| `thePickedInputSurvivesARealRotation` | `INSTRUMENTATION_ABORTED: System has crashed.` — `Expected 59 tests, received 50`. The framework dies **during** it, so six later tests never run and the JUnit XML carries a failure with no text at all. | 5 |
| `pickingAFileThroughTheSystemPickerFillsInTheFileCard` | `androidx.test.uiautomator.StaleObjectException` at `UiObject2.click`, tapping the fixture root the previous line had just found. A restart rebuilt the window between the two. | 3 |
| `thePickedInputSurvivesARealRotation` | **fails**: `INSTRUMENTATION_ABORTED: System has crashed.`, `Expected 1 tests, received 0`. The framework dies **during** it, so the JUnit XML carries a failure with no text at all. | 3 |
| `pickingAFileThroughTheSystemPickerFillsInTheFileCard` | **passes** | 4 |
Both pass on API 33 and API 36 locally, whole suite, `59 / 0 / 0 / 2` on each — so this is the
image, on the same evidence pattern as the codec failures above.
So a rotation, which rebuilds every surface at once, is what the mapper does not survive. Merely
starting DocumentsUI is not. Only the rotation test carries `@FailsOnEmulatorApi37`; the picker
test runs on the gating leg like anything else.
The class therefore carries `@FailsOnEmulatorApi37` and runs on the advisory leg. **The rule the
deviation states is being applied, not broken:** the thing that depends on system UI does not
trust this leg.
#### The correction that produced that table
Two consequences worth stating rather than discovering:
**The first version of this section said both tests failed, and put the marker on the class.** The
picker test had indeed failed at API 37 — with `androidx.test.uiautomator.StaleObjectException`,
which looked like a framework restart invalidating an accessibility node, because that is exactly
what it looks like.
- **`run-e2e.sh 37` applies no annotation filter**, so a local API 37 run reports these on top of
the two `Media3EngineTest` ones, and — new — **does not finish**. Its totals come back short,
and which later tests ran is arbitrary. The summary row says so.
- **The advisory job is still named `E2E API 37 Media3 hardware transcode (advisory)`**, and it
now carries two tests that are neither Media3 nor a transcode. Renaming a check is a branch-
protection change and was deliberately not made in the same PR; the name is stale, the
behaviour is correct.
It was the test's own bug. `UiObject2` caches the `AccessibilityNodeInfo` it was found with, and
DocumentsUI is still settling when a node first appears; the handle went stale before `click()`.
CI then reproduced it **deterministically** at API 33, 34 and 35 — every cold runner emulator, not
intermittently — which is what made it obviously not an API 37 property. It had passed locally
only because the emulator was warm.
The lesson is worth more than the measurement: **an annotation is a claim about an image, and a
broken test makes every image look broken.** Re-measure after fixing a test before deciding what
the platform did. Both the abort and the stale node produce "the run fell over", and only one of
them was the image.
#### Two consequences worth stating rather than discovering
- **`run-e2e.sh 37` applies no annotation filter**, unlike CI, so a local API 37 run includes the
rotation test and therefore **does not finish**: its totals come back short and which later
tests ran is arbitrary. The summary row says so.
- **The advisory job is still named `E2E API 37 Media3 hardware transcode (advisory)`** and now
carries a test that is neither Media3 nor a transcode. Renaming a check is a branch-protection
change and was deliberately not made in the same PR; the name is stale, the behaviour is
correct.
### The two remaining failures are the same bug, one layer down
tools/local-emulator/run-e2e.sh
+27 -22
View File
@@ -15,11 +15,13 @@
# EXIT CODE: 0 only if every level was green; 1 if any level failed, wedged or could not be
# set up; 2 if it refused to start at all. **A bare `run-e2e.sh` therefore exits 1 by design.**
# API 37 is in the default list on purpose -- leaving it out is what left the level unlooked-at
# for as long as it was -- and it is permanently two failures short of green, on the emulator's
# own c2.goldfish.h264.decoder rather than on anything this app does. The summary names the two,
# so a third is visibly new, and the last line printed says the same thing. Anything that reads a
# non-zero exit as breakage should name the levels it wants: `run-e2e.sh 33 34 35 36` is the
# sweep that can be green. docs/api-37-emulator-crash.md has the measurements.
# for as long as it was -- and it is permanently short of green, on the emulator image rather
# than on anything this app does. Since 2026-08-24 it does not even FINISH: one of its expected
# failures kills the framework, so the totals come back short with an arbitrary tail. The summary
# names every failure it expects, so an unnamed one is visibly new, and the last line printed
# says the same thing. Anything that reads a non-zero exit as breakage should name the levels it
# wants: `run-e2e.sh 33 34 35 36` is the sweep that can be green.
# docs/api-37-emulator-crash.md has the measurements.
#
# WHY THIS EXISTS, AND WHAT IT DELIBERATELY DOES NOT DO
#
@@ -326,12 +328,14 @@ boot_emulator() {
#
# "Touched", past tense, since 2026-08-24. SafPickerRoundTripTest drives DocumentsUI and rotates
# the display, and both reach the gralloc mapper these images abort in -- disabling SystemUI
# removes the IDLE trigger, not that one. Unlike CI, THIS SCRIPT APPLIES NO ANNOTATION FILTER, so
# a local `run-e2e.sh 37` runs it and reports a third and fourth failure on top of the two
# Media3EngineTest ones the summary names. Measured 2026-08-24: the rotation test takes the
# framework down outright (INSTRUMENTATION_ABORTED, and six later tests never run), the picker
# test dies on a StaleObjectException. CI's gating leg does not see either -- they carry
# @FailsOnEmulatorApi37 and it filters on notAnnotation.
# removes the IDLE trigger, not those. Measured per method on android-37.0: the ROTATION test
# takes the framework down (INSTRUMENTATION_ABORTED) and carries @FailsOnEmulatorApi37; the
# picker test passes.
#
# THIS SCRIPT APPLIES NO ANNOTATION FILTER, unlike CI, so a local `run-e2e.sh 37` runs the
# rotation test anyway -- and because that test kills the framework rather than merely failing,
# THE LEVEL DOES NOT FINISH. Its totals come back short and which later tests ran is arbitrary.
# CI's gating leg never sees it.
#
# The retry loop is not defensive padding: at the moment boot_completed flips, the framework
# may be in one of its restarts and `pm` is simply not published yet. The first attempt at this
@@ -577,20 +581,21 @@ for api in "${APIS[@]}"; do
# code. So the row NAMES the expected ones, and anything else is then obviously new.
#
# The list grew on 2026-08-24 and the shape of the row changed with it. The two
# Media3EngineTest failures are a codec; SafPickerRoundTripTest is the gralloc bug reached
# through system UI, and its rotation test takes the framework down rather than merely
# failing -- so that level does not finish, and the totals come back SHORT (50 of 59 at the
# time of writing) with the later tests never run. A run whose totals do not add up is
# therefore expected here too, which it never was before, and is the reason this says
# "at least".
# Media3EngineTest failures are a codec; the third is the gralloc bug reached through system
# UI, and it takes the framework DOWN rather than merely failing -- so the level does not
# finish, and the totals come back SHORT (50 of 59 when this was written) with the later
# tests never run. A run whose totals do not add up is expected here now, which it never
# was before.
case "$api" in
37 | 37.*)
line="$line
expected here, at least: 2 Media3EngineTest failures on c2.goldfish.h264.decoder, plus
both SafPickerRoundTripTest tests -- and the run ABORTS partway, so the total is short
and which later tests ran is arbitrary. Anything else is new.
CI's gating leg sees only the first two: the SAF class carries @FailsOnEmulatorApi37 and
this script, unlike CI, applies no annotation filter. docs/api-37-emulator-crash.md"
expected here: 2 Media3EngineTest failures on c2.goldfish.h264.decoder, plus
SafPickerRoundTripTest.thePickedInputSurvivesARealRotation -- which kills the framework
rather than merely failing, so the run ABORTS partway and the total comes back SHORT with
an arbitrary tail. That is expected here too, and never was before. Anything else is new.
CI's gating leg sees only the first two: the rotation test carries @FailsOnEmulatorApi37
and this script, unlike CI, applies no annotation filter.
docs/api-37-emulator-crash.md"
;;
esac
if [ "$rc" -ne 0 ]; then