diff --git a/.github/workflows/status_check.yml b/.github/workflows/status_check.yml index 1c95fff..19ccdab 100644 --- a/.github/workflows/status_check.yml +++ b/.github/workflows/status_check.yml @@ -240,13 +240,22 @@ jobs: # CAVEAT, read this before trusting a green here: this leg runs with # SystemUI disabled and the framework restarted under it. No other leg # and no Pixel run uses that configuration. It is defensible only because - # nothing in this suite touches system UI -- these are Media3, FFmpeg and + # nothing THIS LEG RUNS touches system UI -- Media3, FFmpeg and # WorkManager tests -- and because the alternative is no CI coverage of # the level this app targets. **Anything that ever does depend on system # UI must not trust this row.** E2E_DISABLE_SYSTEM_UI is what does it; # .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. + # # api-level must be "37.0". A bare 37 is not an SDK package and fails # during setup, which cost a run to discover. # diff --git a/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt b/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt index c245bee..51d122b 100644 --- a/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt +++ b/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt @@ -17,6 +17,7 @@ import org.junit.Assert.assertNotEquals import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.libremediaconverter.FailsOnEmulatorApi37 import org.libremediaconverter.MainActivity import org.libremediaconverter.ui.TestTags @@ -70,9 +71,39 @@ import org.libremediaconverter.ui.TestTags * * 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. + * `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 + * + * 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 + * 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: + * + * ``` + * 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) + * ``` + * + * 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. */ @UnstableApi +@FailsOnEmulatorApi37 @RunWith(AndroidJUnit4::class) class SafPickerRoundTripTest { diff --git a/docs/api-37-emulator-crash.md b/docs/api-37-emulator-crash.md index ff93b26..3e53fdd 100644 --- a/docs/api-37-emulator-crash.md +++ b/docs/api-37-emulator-crash.md @@ -230,11 +230,17 @@ booted (`emulator_alive=yes`). The host emulator is fine; the guest is not. ## Can the suite run on it? -**Almost.** `tools/local-emulator/run-e2e.sh 37` now runs the whole suite locally, and all of it -passes except two tests. Measured at `22c7914`: **49 tests, 2 failures, 0 errors, 2 skipped** — 45 -passed, the two `Media3EngineTest` failures dissected below, and the two `assumeTrue` skips every -level has. It costs two deviations from how every other level is run, and both are worth -understanding before trusting the leg. +**Almost, and less so than it was.** `tools/local-emulator/run-e2e.sh 37` runs the whole suite +locally. Measured at `22c7914`: **49 tests, 2 failures, 0 errors, 2 skipped** — 45 passed, the two +`Media3EngineTest` failures dissected below, and the two `assumeTrue` skips every level has. It +costs two deviations from how every other level is run, and both are worth understanding before +trusting the leg. + +**That was the high-water mark.** On 2026-08-24 a test that touches system UI joined the suite, +and the level stopped *finishing* rather than merely failing two — +[see below](#something-does-depend-on-system-ui-now-and-it-is-excluded-rather-than-trusted). +Two `Media3EngineTest` failures is what **CI's gating leg** expects, because it filters on +`notAnnotation`; a local `run-e2e.sh 37` does not filter and sees more. Two things about that total before it is compared with anything. It is the size of the suite on the checkout that ran, not a property of API 37 — `app/src/androidTest` held 49 `@Test` methods at @@ -320,10 +326,44 @@ and proceeding straight to the tests fails exactly as before. The harness theref 1. **The renderer is ANGLE, not the host GPU.** Shared with nothing else in the matrix — API 33–36 run `-gpu host` locally, and CI runs `swiftshader_indirect`. 2. **SystemUI is disabled.** The API 37 leg does not run the same device configuration as any - other leg or as the Pixel. It is defensible here only because nothing in this suite touches - system UI — these are Media3, FFmpeg and WorkManager tests — and because the alternative is no - local API 37 coverage at all. **Anything that ever does depend on system UI must not trust - this leg.** + other leg or as the Pixel. It was defensible here because nothing in this suite touched + system UI — Media3, FFmpeg and WorkManager tests — and because the alternative is no local + 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 + +Added 2026-08-24, and it is 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: + +| test | how it fails | `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 | + +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. + +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. + +Two consequences worth stating rather than discovering: + +- **`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. ### The two remaining failures are the same bug, one layer down @@ -650,9 +690,15 @@ though a new API level shipped. Watch for these instead: - **`E2E API 37 Media3 hardware transcode (advisory)` going green.** Nothing announces this: the job is `continue-on-error`, so it fixing itself looks exactly like a check nobody reads quietly ceasing to be red. It is listed here because that makes it the *least* likely of these - triggers to be noticed, not the most. When it happens, delete `@FailsOnEmulatorApi37` from the - two tests rather than the job — the gating leg picks them back up on its own, and the advisory - job then runs nothing and can go. + triggers to be noticed, not the most. When it happens, delete `@FailsOnEmulatorApi37` from + everything carrying it rather than deleting the job — the gating leg picks them back up on its + own, and the advisory job then runs nothing and can go. + + **It is not two tests any more.** As of 2026-08-24 the marker is on `Media3EngineTest`'s two + methods *and* on `SafPickerRoundTripTest` as a class, and the two groups fail for unrelated + reasons — a codec and the gralloc mapper. They can go green independently, so check both before + concluding the marker is done; and the job's name still says "Media3 hardware transcode", which + half of what it runs is not. ## Correction owed to `CLAUDE.md` diff --git a/tools/local-emulator/run-e2e.sh b/tools/local-emulator/run-e2e.sh index ed34159..417a8dd 100755 --- a/tools/local-emulator/run-e2e.sh +++ b/tools/local-emulator/run-e2e.sh @@ -320,9 +320,18 @@ boot_emulator() { # # THIS IS A DEVIATION, and it is deliberately loud rather than silent. The API 37 leg does not # run the same device configuration as API 33-36 or as the Pixel. It is defensible only -# because nothing in this suite touches SystemUI -- these are Media3, FFmpeg and WorkManager -# tests -- and because the alternative is no API 37 coverage at all. Anything that ever does -# depend on system UI must not trust this leg. docs/api-37-emulator-crash.md explains why. +# because nothing in this suite touched SystemUI -- Media3, FFmpeg and WorkManager tests -- +# and because the alternative is no API 37 coverage at all. Anything that ever does depend on +# system UI must not trust this leg. docs/api-37-emulator-crash.md explains why. +# +# "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. # # 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 @@ -564,13 +573,24 @@ for api in "${APIS[@]}"; do guest_forensics "$api" # API 37 is in the default list on purpose, and it is expected to be red. Leaving it out would # put the level back where this whole exercise found it -- untested and unlooked-at -- but a - # summary that just says "2 failures" with no explanation trains people to ignore the exit - # code. So the row says which two, and a THIRD failure is then obviously new. + # summary that just says "N failures" with no explanation trains people to ignore the exit + # 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". case "$api" in 37 | 37.*) line="$line - expected here: 2 failures, both Media3EngineTest, on c2.goldfish.h264.decoder. - A third is new -- docs/api-37-emulator-crash.md" + 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" ;; esac if [ "$rc" -ne 0 ]; then @@ -596,8 +616,9 @@ echo "==============================================================" # worse than no note at all. if [ "$overall" -ne 0 ] && [ "$NON37_RED" -eq 0 ]; then echo "note: the only level that went red is API 37, which exits non-zero by design -- it is" - echo " permanently 2 failures short of green. Confirm its row above shows exactly those" - echo " two and nothing else; docs/api-37-emulator-crash.md says why they are the image." + echo " permanently short of green, and since 2026-08-24 it does not even finish. Confirm" + echo " its row above names every failure it shows; docs/api-37-emulator-crash.md says why" + echo " each of them is the image rather than this app." fi exit "$overall"