diff --git a/.github/scripts/e2e-run.sh b/.github/scripts/e2e-run.sh index c5759cb..b736815 100755 --- a/.github/scripts/e2e-run.sh +++ b/.github/scripts/e2e-run.sh @@ -40,6 +40,104 @@ WEDGE_LOG="$TMP/wedge-diagnostics-api${LABEL}.txt" # enough under the job's 60-min cap that a genuine wedge still leaves time to capture it. WEDGE_TIMEOUT=1200 +# --------------------------------------------------------------------------- +# API 37 only, and nothing else sets it, so this is inert everywhere it is not wanted -- +# the same shape as E2E_EXTRA_GRADLE_ARGS below. The other four E2E legs run byte-identical +# commands with it unset. +# +# WHY IT RUNS HERE, BEFORE THE LOGCAT STREAM: `adb shell stop` ends the `adb logcat` started +# below, and nothing restarts it, so a disable performed after that point would cost this leg +# its whole diagnostic story for the part of the run that matters. Everything this function +# counts comes from `adb logcat -d -b crash`, which is a fresh read each time and independent +# of the stream. +# +# WHAT IT IS FOR: the android-37.x images abort surfaceflinger from RegionSamplingThread inside +# their own gralloc mapper (docs/api-37-emulator-crash.md). surfaceflinger is a critical service, +# so init SIGKILLs zygote with it and the framework restarts under the run -- Gradle then reports +# `cmd: Can't find service: package` and `Starting 0 tests`. RegionSamplingThread exists only +# because SystemUI registers a nav-bar luma-sampling listener, so removing the package removes +# the whole chain. Measured cadence of those kills: 20-90 s apart, median 60-70 s, three to five +# in a four-minute window -- fast enough that install and instrumentation start-up do not fit +# inside one gap. +# +# NOTHING HERE TRUSTS A COMMAND'S OWN REPORT, and that is not paranoia: of four runs of an +# earlier one-shot version, one (32646029143) reported `new state: disabled-user` and then +# started SystemUI eight more times, with ten more aborts. `pm disable-user` can be accepted by +# a system_server that is SIGKILLed before the state is written, and `pm disable-user` does not +# retract SystemUI's existing region-sampling registration either -- by the time boot completes +# it has already registered, so only a framework restart brings back a SystemUI-less +# surfaceflinger. Hence: disable, take the framework DOWN and confirm system_server is really +# gone (an earlier probe asked `service check` 0.3 s after `stop` and got `found` from the +# system_server that was still exiting, so its wait was not a wait), bring it back, verify the +# package against `pm list packages -d`, and require a 45 s window with zero new aborts. +# Three rounds, because one is not reliable and the failure is silent. +# --------------------------------------------------------------------------- +count_aborts() { adb logcat -d -b crash 2> /dev/null | grep -c 'hasReadColorBufferDma'; } +systemui_disabled() { adb shell pm list packages -d 2> /dev/null | grep -q 'com.android.systemui'; } + +disable_region_sampling() { + local round=1 i out before after + while [ "$round" -le 3 ]; do + echo "--- SystemUI disable, round $round ---" + for i in $(seq 1 10); do + out="$(adb shell pm disable-user --user 0 com.android.systemui 2>&1 | tr -d '\r')" + echo " pm attempt $i: $out" + case "$out" in *"new state: disabled"*) break ;; esac + sleep 5 + done + + echo " restarting the framework" + adb shell stop + for i in $(seq 1 20); do + [ -z "$(adb shell pidof system_server 2> /dev/null | tr -d '\r\n')" ] && break + sleep 2 + done + echo " system_server down after ~$((i * 2)) s" + adb shell start + for i in $(seq 1 30); do + if adb shell service check package 2> /dev/null | grep -q ': found' \ + && adb shell service check activity 2> /dev/null | grep -q ': found' \ + && [ -n "$(adb shell pidof system_server 2> /dev/null | tr -d '\r\n')" ]; then + echo " services back after ~$((i * 5)) s" + break + fi + sleep 5 + done + + if systemui_disabled; then + echo " verified: com.android.systemui is in pm list packages -d" + else + echo " NOT DISABLED after the restart -- the package state did not survive" + round=$((round + 1)) + continue + fi + + before="$(count_aborts)" + sleep 45 + after="$(count_aborts)" + echo " abort rate, SystemUI disabled: $((after - before)) new in 45 s (total ${after:-0})" + [ "$((after - before))" -eq 0 ] && break + echo " still aborting after round $round" + round=$((round + 1)) + done + + # A warning rather than an exit. If the disable did not take, the run is about to report + # `Starting 0 tests` and fail on its own -- and it will do so with the logcat, the crash + # buffer and the diagnostics attached, which is more useful than dying here with none of it. + if systemui_disabled; then + echo " final state: SystemUI disabled" + else + echo "::warning::E2E api${LABEL}: SystemUI is still enabled -- expect INSTRUMENTATION_ABORTED" + fi + return 0 +} + +if [ "${E2E_DISABLE_SYSTEM_UI:-}" = "1" ]; then + echo "::group::E2E api${LABEL} -- removing the region-sampling listener" + disable_region_sampling + echo "::endgroup::" +fi + # Stream logcat from now until the step ends, into a file that survives to the artifact upload. # Without this, a failure that happens on-device leaves nothing behind: `adb logcat -d` at the # end only has whatever is still in the ring buffer, and a chatty test run evicts the cause. diff --git a/.github/workflows/api37-debug.yml b/.github/workflows/api37-debug.yml index 3a223a5..31e3a87 100644 --- a/.github/workflows/api37-debug.yml +++ b/.github/workflows/api37-debug.yml @@ -3,14 +3,20 @@ name: API 37 debug # --------------------------------------------------------------------------- # WHAT THIS IS FOR, AND WHY IT IS SEPARATE # -# status_check.yml's E2E matrix stops at API 36 because the android-37.x emulator -# images abort surfaceflinger inside their own gralloc mapper. That was established -# locally, under -gpu host and under ANGLE (docs/api-37-emulator-crash.md). It was -# NOT established on a GitHub runner: CI runs -gpu swiftshader_indirect, and the one -# local measurement of that mode was void for a purely local reason (Fedora's SELinux -# denies execheap to SwiftShader's Reactor JIT -- docs/local-emulator.md). So what CI -# actually does at API 37 is an open question, and this workflow is the instrument for -# answering it. +# The android-37.x emulator images abort surfaceflinger inside their own gralloc +# mapper. That was established locally, under -gpu host and under ANGLE +# (docs/api-37-emulator-crash.md), but NOT on a GitHub runner: CI runs +# -gpu swiftshader_indirect, and the one local measurement of that mode was void for a +# purely local reason (Fedora's SELinux denies execheap to SwiftShader's Reactor JIT -- +# docs/local-emulator.md). What CI actually does at API 37 was an open question, and +# this workflow is the instrument that answered it. +# +# IT IS STILL THE INSTRUMENT. status_check.yml now carries API 37 -- a gating leg that +# disables SystemUI first, and an advisory one for the two @FailsOnEmulatorApi37 tests +# -- so this file's job is no longer to decide that, but to test a change to it for one +# dispatch instead of one commit. The next questions it exists for are written down +# under "When to revisit" in docs/api-37-emulator-crash.md: a new API 37.x image, or an +# ATD image for 37, either of which could retire the whole workaround. # # It is a copy of that E2E job with the matrix replaced by workflow_dispatch inputs, # so one hypothesis costs one dispatch rather than one commit. It triggers on nothing diff --git a/.github/workflows/status_check.yml b/.github/workflows/status_check.yml index 171163b..f650df4 100644 --- a/.github/workflows/status_check.yml +++ b/.github/workflows/status_check.yml @@ -178,8 +178,9 @@ jobs: # across it -- none below 34, dataSync at 34, mediaProcessing from 35. Testing a # single level would leave two thirds of that branch unexercised. # - # It stops at 36 rather than targetSdk 37 because the android-37.0 emulator image - # is broken, not because 37 does not matter. See docs/api-37-emulator-crash.md. + # It reaches targetSdk 37, but the API 37 row is not like the other four and the + # comment on it says how. Two tests are excluded there and run in their own + # advisory job below. See docs/api-37-emulator-crash.md. # # FFmpeg is not built here. The AAR is committed under bin/, so a red run means the # code is broken rather than that a cross-compile hiccuped. @@ -204,13 +205,28 @@ jobs: api-level: "35" - label: "36" api-level: "36" - # No API 37 row. targetSdk is 37, but the android-37.0 emulator image - # crash-loops surfaceflinger inside its own gralloc mapper, so every test - # fails there no matter what this app does. Ruling that in took four CI - # rounds, so the evidence and the ruled-out fixes are written down rather - # than left to be rediscovered: docs/api-37-emulator-crash.md. That file - # also records what to try first when re-adding it -- note that the row - # needs api-level "37.0", since a bare 37 fails during SDK setup. + # API 37, and it is NOT the same device as the four rows above it. + # + # 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 + # 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. + # + # api-level must be "37.0". A bare 37 is not an SDK package and fails + # during setup, which cost a run to discover. + # + # notAnnotation removes the two tests that do not pass on this image; they + # run in the advisory job below, off the same marker so they cannot end up + # in both or neither. docs/api-37-emulator-crash.md has the measurements. + - label: "37" + api-level: "37.0" + disable-system-ui: "1" + gradle-args: "-Pandroid.testInstrumentationRunnerArguments.notAnnotation=org.libremediaconverter.FailsOnEmulatorApi37" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -236,6 +252,12 @@ jobs: - name: Instrumented tests uses: reactivecircus/android-emulator-runner@a421e43855164a8197daf9d8d40fe71c6996bb0d # v2.38.0 + # Both of these are empty on every row but 37, and both are read with a + # `:-` default in e2e-run.sh, so the four legs below 37 run the identical + # gradle command they always have. + env: + E2E_DISABLE_SYSTEM_UI: ${{ matrix.disable-system-ui }} + E2E_EXTRA_GRADLE_ARGS: ${{ matrix.gradle-args }} with: api-level: ${{ matrix.api-level }} target: google_apis @@ -297,3 +319,111 @@ jobs: name: e2e-wedge-api${{ matrix.label }} path: ${{ runner.temp }}/wedge-diagnostics-api${{ matrix.label }}.txt if-no-files-found: ignore + + # --------------------------------------------------------------------------- + # The two API 37 tests the gating row above excludes, run on their own so they + # stay visible instead of disappearing behind a notAnnotation. + # + # continue-on-error: it reports, it never blocks. That is the whole reason it is + # a separate job rather than a sixth matrix row: a row would share the gating + # job's `E2E API