Compare commits
39
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7b578c1ccf | ||
|
|
9e7f80feaa | ||
|
|
3c5a37fd3c | ||
|
|
af13155c27 | ||
|
|
4ea5afefe1 | ||
|
|
7e4f22322b | ||
|
|
2ee97e30b7 | ||
|
|
d8f1590d2b | ||
|
|
3d51fefeff | ||
|
|
6cd17f25aa | ||
|
|
fea88a281f | ||
|
|
7c69d0699a | ||
|
|
baaaa934e0 | ||
|
|
2bed40d080 | ||
|
|
175472ae88 | ||
|
|
df2e42a2b3 | ||
|
|
64c1a60a97 | ||
|
|
1779f20a03 | ||
|
|
225ecdd7e6 | ||
|
|
b3a705e3da | ||
|
|
577dae998b | ||
|
|
acc71bcaee | ||
|
|
b97d7c36a3 | ||
|
|
93398c4616 | ||
|
|
19a1e66277 | ||
|
|
8fdad6e20b | ||
|
|
742703d360 | ||
|
|
6c34fad17c | ||
|
|
9c4f14202f | ||
|
|
2747bb8627 | ||
|
|
6d6d2189ca | ||
|
|
f8e6bfa2a3 | ||
|
|
5a1b8832d3 | ||
|
|
7ae660ee37 | ||
|
|
775a44753b | ||
|
|
961cfa72a2 | ||
|
|
da6f2807e9 | ||
|
|
792286a2d7 | ||
|
|
739bffa5a0 |
@@ -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.
|
||||
|
||||
@@ -0,0 +1,413 @@
|
||||
name: API 37 debug
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# WHAT THIS IS FOR, AND WHY IT IS SEPARATE
|
||||
#
|
||||
# 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
|
||||
# else: no push, no pull_request, no schedule. Nothing depends on it and it gates
|
||||
# nothing.
|
||||
#
|
||||
# TWO THINGS THIS DELIBERATELY DOES NOT DO:
|
||||
#
|
||||
# - It does not fork .github/scripts/e2e-run.sh. That script owns the FAILED-vs-WEDGED
|
||||
# split, the SIGQUIT thread dump and the streamed logcat, and it is the copy CI
|
||||
# exercises every day. This calls it, exactly as status_check.yml does.
|
||||
# - It does not change status_check.yml. If a configuration here turns out to work,
|
||||
# the change to the real matrix is proposed separately.
|
||||
#
|
||||
# THE WATCHDOG IS THE POINT, not a nicety. reactivecircus/android-emulator-runner calls
|
||||
# killEmulator() from its own catch block, so a run whose emulator never boots is torn
|
||||
# down before a single `script:` line executes -- no probe, no e2e-run.sh, no artifacts,
|
||||
# nothing to read afterwards. That is precisely the failure shape API 37 is suspected of.
|
||||
# The watchdog therefore starts BEFORE the action, from outside it, and samples the device
|
||||
# on its own clock.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
on:
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
api_level:
|
||||
description: 'API level, as the SDK spells it. 37.0, 37.1, 37.2-beta3, 36 ... A bare 37 does not exist and fails during SDK setup.'
|
||||
type: string
|
||||
default: '37.0'
|
||||
target:
|
||||
description: 'System image target. android-37.1 and 37.2-beta* ship ONLY as google_apis_ps16k -- there is no plain google_apis above 37.0.'
|
||||
type: string
|
||||
default: 'google_apis'
|
||||
channel:
|
||||
description: 'SDK channel. beta is required for any 37.2-beta* image.'
|
||||
type: choice
|
||||
options: ['stable', 'beta', 'dev', 'canary']
|
||||
default: 'stable'
|
||||
gpu_mode:
|
||||
description: 'The -gpu argument. swiftshader_indirect is what status_check.yml uses today; swangle_indirect is what works locally on API 37.'
|
||||
type: choice
|
||||
options:
|
||||
- swiftshader_indirect
|
||||
- swangle_indirect
|
||||
- angle_indirect
|
||||
- host
|
||||
- auto
|
||||
- guest
|
||||
- 'off'
|
||||
default: 'swiftshader_indirect'
|
||||
disable_system_ui:
|
||||
description: 'Take SystemUI out before the suite runs, which is what stops SurfaceFlinger RegionSamplingThread reaching the mapper bug. Restarts the framework.'
|
||||
type: boolean
|
||||
default: false
|
||||
run_tests:
|
||||
description: 'Run the instrumented suite. false boots, probes and stops -- the cheap loop when the question is only whether it boots and at what abort rate.'
|
||||
type: boolean
|
||||
default: true
|
||||
emulator_boot_timeout:
|
||||
description: 'Seconds the action waits for sys.boot_completed. Do not lower this for a software renderer: a slow boot would be misreported as a failed one.'
|
||||
type: string
|
||||
default: '600'
|
||||
emulator_extra_options:
|
||||
description: 'Appended verbatim to the emulator command line -- e.g. "-verbose", or "-feature -GLDMA,-GLDMA2". The action interpolates it into a sh -c, so "| tee $RUNNER_TEMP/emulator.log" also works and is uploaded.'
|
||||
type: string
|
||||
default: ''
|
||||
disable_animations:
|
||||
description: 'The action settings-puts three animation scales after boot. Each is an adb call that throws if the framework is mid-restart, which would kill the run before the probe. false removes three of those calls.'
|
||||
type: boolean
|
||||
default: true
|
||||
gradle_extra_args:
|
||||
description: 'Passed to e2e-run.sh as E2E_EXTRA_GRADLE_ARGS, its existing hook -- e.g. "--rerun", or -Pandroid.testInstrumentationRunnerArguments.class=... to run one class instead of the suite.'
|
||||
type: string
|
||||
default: ''
|
||||
measure_baseline:
|
||||
description: 'Measure the abort rate for 45 s BEFORE disabling SystemUI. Answers "how fast is it aborting"; costs 45 s of crash-looping first, which is a worse starting point for the disable.'
|
||||
type: boolean
|
||||
default: true
|
||||
|
||||
run-name: >-
|
||||
api ${{ inputs.api_level }}/${{ inputs.target }} · gpu ${{ inputs.gpu_mode }} ·
|
||||
systemui ${{ inputs.disable_system_ui && 'disabled' || 'running' }} ·
|
||||
tests ${{ inputs.run_tests && 'yes' || 'no' }}
|
||||
|
||||
# No `concurrency` block, unlike status_check.yml. Every dispatch here runs on the same
|
||||
# ref (main), so a group keyed on github.ref with cancel-in-progress would make two
|
||||
# parallel experiments cancel each other -- which is the opposite of what this is for.
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
env:
|
||||
GRADLE_CACHE_PATHS: |
|
||||
~/.gradle/caches
|
||||
~/.gradle/wrapper
|
||||
|
||||
jobs:
|
||||
e2e-api37:
|
||||
name: E2E API ${{ inputs.api_level }} (${{ inputs.gpu_mode }})
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 60
|
||||
steps:
|
||||
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||
|
||||
- uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0
|
||||
with:
|
||||
distribution: temurin
|
||||
java-version: '25' # Matches the daemon JVM pinned in gradle/gradle-daemon-jvm.properties
|
||||
|
||||
- uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
|
||||
with:
|
||||
path: ${{ env.GRADLE_CACHE_PATHS }}
|
||||
key: gradle-${{ runner.os }}-${{ hashFiles('**/*.gradle.kts', 'gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties') }}
|
||||
restore-keys: gradle-${{ runner.os }}-
|
||||
|
||||
# Without this the emulator falls back to software rendering and takes minutes
|
||||
# longer to boot, when it boots at all.
|
||||
- name: Enable KVM
|
||||
run: |
|
||||
echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' \
|
||||
| sudo tee /etc/udev/rules.d/99-kvm4all.rules
|
||||
sudo udevadm control --reload-rules
|
||||
sudo udevadm trigger --name-match=kvm
|
||||
|
||||
# Both helpers live in RUNNER_TEMP rather than in the repository: they are debug
|
||||
# instrumentation for this workflow only, and writing them here keeps the whole
|
||||
# experiment in one file that can be read top to bottom.
|
||||
- name: Write the watchdog and the probe
|
||||
env:
|
||||
LABEL: ${{ inputs.api_level }}
|
||||
run: |
|
||||
cat > "$RUNNER_TEMP/watchdog.sh" <<'WATCHDOG'
|
||||
#!/usr/bin/env bash
|
||||
# Samples the device from outside the emulator action, because the action tears the
|
||||
# emulator down on a boot timeout before any script: line runs. Everything here is
|
||||
# `timeout`-wrapped: a wedged adb must not stall the sampler, and no probe may fail.
|
||||
SERIAL="emulator-5554"
|
||||
|
||||
# adb is resolved by path, not by name. The emulator action puts platform-tools on
|
||||
# PATH with core.addPath, which only affects LATER steps -- this one already exists
|
||||
# by then, so a bare `adb` here is not the runner's adb and may be nothing at all.
|
||||
# The first version of this file assumed otherwise and every sample came back
|
||||
# boot=? dma_aborts=0 while the action's own adb was working fine two steps away.
|
||||
# Re-resolved every iteration because platform-tools may be installed after this
|
||||
# starts, and echoed to stdout so a repeat of that failure is visible immediately.
|
||||
ADB=""
|
||||
resolve_adb() {
|
||||
for c in "$ADB" "${ANDROID_HOME:-}/platform-tools/adb" "${ANDROID_SDK_ROOT:-}/platform-tools/adb" "$(command -v adb 2> /dev/null)"; do
|
||||
if [ -n "$c" ] && [ -x "$c" ]; then
|
||||
[ "$c" = "$ADB" ] || echo "watchdog: adb resolved to $c"
|
||||
ADB="$c"
|
||||
return 0
|
||||
fi
|
||||
done
|
||||
return 1
|
||||
}
|
||||
OUT="$RUNNER_TEMP/watchdog-api$LABEL.txt"
|
||||
CRASH="$RUNNER_TEMP/crash-buffer-api$LABEL.txt"
|
||||
GUESTLOG="$RUNNER_TEMP/watchdog-logcat-api$LABEL.txt"
|
||||
STOP="$RUNNER_TEMP/watchdog.stop"
|
||||
|
||||
# A continuous guest logcat, restarted whenever the device goes away. During a
|
||||
# surfaceflinger crash loop the framework restarts every few seconds and adb goes
|
||||
# with it, so a single `adb logcat` would end at the first restart.
|
||||
(
|
||||
while [ ! -f "$STOP" ]; do
|
||||
if resolve_adb; then
|
||||
timeout 120 "$ADB" -s "$SERIAL" wait-for-device > /dev/null 2>&1 \
|
||||
&& timeout 3000 "$ADB" -s "$SERIAL" logcat -v time >> "$GUESTLOG" 2>&1
|
||||
fi
|
||||
sleep 3
|
||||
done
|
||||
) &
|
||||
|
||||
echo "watchdog started $(date -u +%FT%TZ) -- serial $SERIAL" >> "$OUT"
|
||||
i=0
|
||||
while [ "$i" -lt 300 ]; do
|
||||
i=$((i + 1))
|
||||
[ -f "$STOP" ] && break
|
||||
if ! resolve_adb; then
|
||||
echo "$(date -u +%T) no adb yet" >> "$OUT"
|
||||
sleep 20
|
||||
continue
|
||||
fi
|
||||
boot="$(timeout 20 "$ADB" -s "$SERIAL" shell getprop sys.boot_completed 2> /dev/null | tr -d '\r\n')"
|
||||
sf="$(timeout 20 "$ADB" -s "$SERIAL" shell pidof surfaceflinger 2> /dev/null | tr -d '\r\n')"
|
||||
zy="$(timeout 20 "$ADB" -s "$SERIAL" shell pidof zygote64 2> /dev/null | tr -d '\r\n')"
|
||||
# Kept as a file rather than a variable so the last successful read survives the
|
||||
# action killing the emulator -- which is when it is most worth having.
|
||||
if timeout 30 "$ADB" -s "$SERIAL" logcat -d -b crash > "$CRASH.new" 2> /dev/null; then
|
||||
mv "$CRASH.new" "$CRASH"
|
||||
fi
|
||||
dma="$(grep -c 'hasReadColorBufferDma' "$CRASH" 2> /dev/null || true)"
|
||||
sigabrt="$(grep -c 'signal 6' "$CRASH" 2> /dev/null || true)"
|
||||
printf '%s boot=%-4s surfaceflinger=%-8s zygote64=%-8s dma_aborts=%-5s sigabrt=%s\n' \
|
||||
"$(date -u +%T)" "${boot:-?}" "${sf:-none}" "${zy:-none}" "${dma:-0}" "${sigabrt:-0}" >> "$OUT"
|
||||
# One shot, the first time the device is up: which GLES implementation the guest
|
||||
# actually got. This is the guest-side answer to the same question the emulator's
|
||||
# own gles_mode_selected line answers host-side.
|
||||
if [ "$boot" = "1" ] && [ ! -f "$RUNNER_TEMP/renderer-api$LABEL.txt" ]; then
|
||||
{
|
||||
echo "=== booted at $(date -u +%FT%TZ), watchdog sample $i ==="
|
||||
timeout 30 "$ADB" -s "$SERIAL" shell dumpsys SurfaceFlinger 2>&1 | head -40
|
||||
echo "--- getprop ---"
|
||||
timeout 20 "$ADB" -s "$SERIAL" shell getprop 2>&1 | grep -Ei 'egl|gles|gpu|ranchu|gfxstream' || true
|
||||
} > "$RUNNER_TEMP/renderer-api$LABEL.txt" 2>&1
|
||||
fi
|
||||
sleep 20
|
||||
done
|
||||
echo "watchdog finished $(date -u +%FT%TZ) after $i samples" >> "$OUT"
|
||||
WATCHDOG
|
||||
|
||||
cat > "$RUNNER_TEMP/probe.sh" <<'PROBE'
|
||||
#!/usr/bin/env bash
|
||||
# Runs on the booted device, before the suite. Two jobs: record what the guest got,
|
||||
# and measure the gralloc abort RATE -- which is the number that decides whether a
|
||||
# five-minute test run can survive, and the one comparable with the local figures in
|
||||
# docs/api-37-emulator-crash.md (10-11 per 150 s idle under ANGLE with SystemUI up).
|
||||
#
|
||||
# Never exits non-zero. The action runs script: lines in one try/catch, so a failing
|
||||
# probe would skip e2e-run.sh entirely and the run would measure nothing.
|
||||
exec > >(tee -a "$RUNNER_TEMP/probe-api$LABEL.txt") 2>&1
|
||||
echo "===== probe api$LABEL -- $(date -u +%FT%TZ) ====="
|
||||
adb shell getprop sys.boot_completed
|
||||
adb shell getprop ro.build.fingerprint
|
||||
adb shell getprop ro.build.version.sdk
|
||||
echo "--- SurfaceFlinger (the GLES line names the renderer the guest is on) ---"
|
||||
adb shell dumpsys SurfaceFlinger 2>&1 | head -30
|
||||
echo "--- binder services ---"
|
||||
for s in package activity window; do adb shell service check "$s" 2>&1; done
|
||||
|
||||
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'; }
|
||||
|
||||
if [ "${MEASURE_BASELINE:-true}" = "true" ]; then
|
||||
before="$(count_aborts)"
|
||||
sleep 45
|
||||
after="$(count_aborts)"
|
||||
echo "--- abort rate, SystemUI running: $((after - before)) new in 45 s (total ${after:-0}) ---"
|
||||
else
|
||||
# Skipped on purpose when the question is reliability rather than rate: every
|
||||
# second spent measuring is a second of crash-looping, and the disable is what
|
||||
# has to land. A real CI leg would disable as early as it can, so measure that.
|
||||
echo "--- baseline window skipped (MEASURE_BASELINE=false) ---"
|
||||
fi
|
||||
|
||||
if [ "${DISABLE_SYSTEM_UI:-false}" = "true" ]; then
|
||||
# Three rounds, because ONE round is not reliable and the failure is silent.
|
||||
# Measured: of four runs of the same configuration, three came back with the
|
||||
# suite running and one (32646029143) had SystemUI restarting throughout --
|
||||
# `ActivityManager: Start proc N:com.android.systemui ... GradientColorWallpaper`
|
||||
# eight more times after a `pm disable-user` that had reported
|
||||
# `new state: disabled-user`, and ten more RegionSampling aborts with it. The
|
||||
# framework is being SIGKILLed under this loop, so a package-state change can be
|
||||
# lost with the system_server that accepted it. (An earlier version of this comment
|
||||
# said "every ~20 s". That was the watchdog's SAMPLING interval, not the cadence.
|
||||
# Measured: 20-90 s between aborts, median 60-70 s, 3-5 in a four-minute window --
|
||||
# docs/api-37-emulator-crash.md, "Abort cadence, corrected".)
|
||||
#
|
||||
# Nothing here trusts a command's own report. Each round: disable, take the
|
||||
# framework down and confirm it is DOWN before bringing it back (the previous
|
||||
# version 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), then verify
|
||||
# the package is really disabled and that no abort lands in a quiet window.
|
||||
round=1
|
||||
while [ "$round" -le 3 ]; do
|
||||
echo "--- 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
|
||||
|
||||
# pm disable-user does not retract SystemUI's existing region-sampling
|
||||
# registration -- by the time boot completes it has already registered. Only a
|
||||
# framework restart brings back a SystemUI-less SurfaceFlinger. See
|
||||
# disable_region_sampling in tools/local-emulator/run-e2e.sh.
|
||||
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
|
||||
systemui_disabled && echo "final state: SystemUI disabled" || echo "final state: SystemUI STILL ENABLED -- expect Starting 0 tests"
|
||||
fi
|
||||
|
||||
echo "--- crash buffer (tail 60) ---"
|
||||
adb logcat -d -b crash 2>&1 | tail -60
|
||||
echo "===== probe done ====="
|
||||
exit 0
|
||||
PROBE
|
||||
|
||||
chmod +x "$RUNNER_TEMP/watchdog.sh" "$RUNNER_TEMP/probe.sh"
|
||||
echo "helpers written to $RUNNER_TEMP"
|
||||
|
||||
- name: Start the watchdog
|
||||
env:
|
||||
LABEL: ${{ inputs.api_level }}
|
||||
run: |
|
||||
echo "ANDROID_HOME=${ANDROID_HOME:-<unset>} ANDROID_SDK_ROOT=${ANDROID_SDK_ROOT:-<unset>}"
|
||||
echo "adb on PATH: $(command -v adb || echo '<none -- the watchdog will fall back to ANDROID_HOME>')"
|
||||
nohup bash "$RUNNER_TEMP/watchdog.sh" > "$RUNNER_TEMP/watchdog-stdout.txt" 2>&1 < /dev/null &
|
||||
disown
|
||||
echo "watchdog pid $!"
|
||||
|
||||
- name: Instrumented tests
|
||||
uses: reactivecircus/android-emulator-runner@a421e43855164a8197daf9d8d40fe71c6996bb0d # v2.38.0
|
||||
env:
|
||||
LABEL: ${{ inputs.api_level }}
|
||||
DISABLE_SYSTEM_UI: ${{ inputs.disable_system_ui }}
|
||||
MEASURE_BASELINE: ${{ inputs.measure_baseline }}
|
||||
# e2e-run.sh's own hook, unset in CI's real workflow and therefore inert there.
|
||||
E2E_EXTRA_GRADLE_ARGS: ${{ inputs.gradle_extra_args }}
|
||||
with:
|
||||
api-level: ${{ inputs.api_level }}
|
||||
target: ${{ inputs.target }}
|
||||
channel: ${{ inputs.channel }}
|
||||
arch: x86_64
|
||||
profile: pixel_6
|
||||
emulator-boot-timeout: ${{ inputs.emulator_boot_timeout }}
|
||||
emulator-options: -no-window -gpu ${{ inputs.gpu_mode }} -noaudio -no-boot-anim -camera-back none ${{ inputs.emulator_extra_options }}
|
||||
disable-animations: ${{ inputs.disable_animations }}
|
||||
# disk-size, ram-size: kept exactly as status_check.yml pins them, so this
|
||||
# measures the renderer and not a different device. 8G because the APK plus
|
||||
# FFmpeg does not fit the default userdata partition; 2560M because the
|
||||
# emulator's own RAM floor varies by API level and 2560M is the highest of them.
|
||||
disk-size: 8G
|
||||
ram-size: 2560M
|
||||
# Two lines, because the action splits script: on newlines and runs each as its
|
||||
# own `sh -c`. The first is this workflow's own probe; the second is CI's real
|
||||
# harness, invoked unmodified. run_tests: false replaces it with an echo rather
|
||||
# than a second copy of the job.
|
||||
script: |
|
||||
bash ${{ runner.temp }}/probe.sh
|
||||
${{ inputs.run_tests && format('bash .github/scripts/e2e-run.sh {0}', inputs.api_level) || 'echo "run_tests=false -- suite skipped, boot and probe only"' }}
|
||||
|
||||
- name: Stop the watchdog
|
||||
if: always()
|
||||
run: |
|
||||
touch "$RUNNER_TEMP/watchdog.stop"
|
||||
echo "----- watchdog samples -----"
|
||||
cat "$RUNNER_TEMP/watchdog-api${{ inputs.api_level }}.txt" 2>/dev/null || echo "(no watchdog output)"
|
||||
echo "----- renderer -----"
|
||||
cat "$RUNNER_TEMP/renderer-api${{ inputs.api_level }}.txt" 2>/dev/null || echo "(never booted, or dumpsys unavailable)"
|
||||
echo "----- crash buffer, last read before teardown (tail 80) -----"
|
||||
tail -80 "$RUNNER_TEMP/crash-buffer-api${{ inputs.api_level }}.txt" 2>/dev/null || echo "(none)"
|
||||
|
||||
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
if: always()
|
||||
with:
|
||||
name: api37-debug-run${{ github.run_number }}
|
||||
path: |
|
||||
${{ runner.temp }}/watchdog-api*.txt
|
||||
${{ runner.temp }}/watchdog-logcat-api*.txt
|
||||
${{ runner.temp }}/watchdog-stdout.txt
|
||||
${{ runner.temp }}/crash-buffer-api*.txt
|
||||
${{ runner.temp }}/renderer-api*.txt
|
||||
${{ runner.temp }}/probe-api*.txt
|
||||
${{ runner.temp }}/emulator*.log
|
||||
${{ runner.temp }}/logcat-api*.txt
|
||||
${{ runner.temp }}/diagnostics-api*.txt
|
||||
${{ runner.temp }}/wedge-diagnostics-api*.txt
|
||||
app/build/reports/androidTests/
|
||||
app/build/outputs/androidTest-results/
|
||||
if-no-files-found: warn
|
||||
@@ -156,7 +156,37 @@ jobs:
|
||||
key: gradle-${{ runner.os }}-${{ hashFiles('**/*.gradle.kts', 'gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties') }}
|
||||
restore-keys: gradle-${{ runner.os }}-
|
||||
|
||||
# Shell is the other language in this repo -- four scripts, one of them the CI
|
||||
# entry point itself -- and nothing was checking it. `git ls-files` rather than a
|
||||
# fixed list, so a script added later is covered without editing this workflow.
|
||||
#
|
||||
# Full severity, `info` included. The findings it raises today are answered with
|
||||
# targeted `disable` directives carrying their reason, the same way
|
||||
# config/detekt/detekt.yml carries only the rules this codebase legitimately
|
||||
# breaks. A blanket --severity=warning would have hidden them and the next real
|
||||
# one alike.
|
||||
#
|
||||
# PINNED BY DIGEST, for the reason CLAUDE.md already gives for pinning ktlint,
|
||||
# detekt and JaCoCo: a new rule in a linter makes files nobody touched stop
|
||||
# passing, so CI goes red on a PR whose diff cannot explain it. That is not
|
||||
# hypothetical here. The first cut of this step used the runner's ambient
|
||||
# shellcheck, which is 0.9.0, and 0.9.0 reports a trap handler as seven
|
||||
# unreachable commands (SC2317) where 0.11.0 reports it once on the declaration
|
||||
# (SC2329) -- same script, same directive, different answer, and a red build on
|
||||
# the PR that introduced the step. The version is printed so a finding that
|
||||
# appears out of nowhere can be tied to a bump of this line.
|
||||
- name: shellcheck
|
||||
env:
|
||||
SHELLCHECK: koalaman/shellcheck@sha256:61862eba1fcf09a484ebcc6feea46f1782532571a34ed51fedf90dd25f925a8d
|
||||
run: |
|
||||
docker run --rm "$SHELLCHECK" --version
|
||||
git ls-files -z '*.sh' | xargs -0 -r docker run --rm -v "$PWD:/mnt" "$SHELLCHECK"
|
||||
|
||||
# `!cancelled()` rather than a plain sequence: a shellcheck failure above must not
|
||||
# cost the ktlint/detekt/lint lists. Same reason this step passes --continue -- one
|
||||
# round trip should produce every list, not stop at the first.
|
||||
- name: ktlint, detekt and Android lint
|
||||
if: '!cancelled()'
|
||||
run: ./gradlew :app:ktlintCheck :app:detekt :app:lintDebug --continue --stacktrace
|
||||
|
||||
# The XML matters as much as the HTML: it is the one that can be diffed between
|
||||
@@ -178,8 +208,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 +235,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 +282,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 +349,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 <label>` name, and a check cannot be both required and advisory
|
||||
# under one name.
|
||||
#
|
||||
# It is named for WHAT IT RUNS, deliberately. Both tests drive a full H.264 ->
|
||||
# H.265 hardware transcode through Media3Engine -- which is exactly what
|
||||
# separates them from the two Media3EngineTest cases that pass here, since those
|
||||
# two never decode video. The current theory about why they fail is in the next
|
||||
# paragraph, where it can be corrected without renaming a check that people have
|
||||
# already learned to look for.
|
||||
#
|
||||
# THEORY, NOT SETTLED: the exception surfaces at `dequeueOutputBuffer` on
|
||||
# `c2.goldfish.h264.decoder`, the emulator's own codec, which gets its frames out
|
||||
# of a host-side colour buffer -- the same readback machinery that aborts
|
||||
# surfaceflinger on this image. What is MEASURED is narrower: these two fail on
|
||||
# the API 37 emulator image; pass at API 36 on this runner under the same renderer
|
||||
# AND the same SystemUI-disable path; pass at API 33-36 without that path at all,
|
||||
# since nothing below 37 needs it; and pass on a physical Pixel 10 Pro XL at 37. That the decoder is the culprit rather than something else
|
||||
# the decode path touches is inference. docs/api-37-emulator-crash.md separates
|
||||
# the two, and the images also differ on the encoder side, which is why "broken
|
||||
# h264 decoder" is not written into this job's name.
|
||||
#
|
||||
# WHEN THIS GOES GREEN, delete the annotation rather than this job: the gating
|
||||
# row picks the tests back up automatically, and this job goes empty and can go
|
||||
# with it.
|
||||
# ---------------------------------------------------------------------------
|
||||
e2e-api37-advisory:
|
||||
name: E2E API 37 Media3 hardware transcode (advisory)
|
||||
runs-on: ubuntu-latest
|
||||
needs: ffmpeg
|
||||
timeout-minutes: 60
|
||||
continue-on-error: true
|
||||
env:
|
||||
E2E_LABEL: "37-media3-transcode"
|
||||
steps:
|
||||
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||
|
||||
- uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0
|
||||
with:
|
||||
distribution: temurin
|
||||
java-version: '25'
|
||||
|
||||
- uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
|
||||
with:
|
||||
path: ${{ env.GRADLE_CACHE_PATHS }}
|
||||
key: gradle-${{ runner.os }}-${{ hashFiles('**/*.gradle.kts', 'gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties') }}
|
||||
restore-keys: gradle-${{ runner.os }}-
|
||||
|
||||
- name: Enable KVM
|
||||
run: |
|
||||
echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' \
|
||||
| sudo tee /etc/udev/rules.d/99-kvm4all.rules
|
||||
sudo udevadm control --reload-rules
|
||||
sudo udevadm trigger --name-match=kvm
|
||||
|
||||
- name: Instrumented tests
|
||||
uses: reactivecircus/android-emulator-runner@a421e43855164a8197daf9d8d40fe71c6996bb0d # v2.38.0
|
||||
env:
|
||||
E2E_DISABLE_SYSTEM_UI: "1"
|
||||
# The complement of the gating row's notAnnotation, off the same marker,
|
||||
# so a test can never be excluded from both jobs or run in both.
|
||||
E2E_EXTRA_GRADLE_ARGS: "-Pandroid.testInstrumentationRunnerArguments.annotation=org.libremediaconverter.FailsOnEmulatorApi37"
|
||||
with:
|
||||
# Every device pin below matches the gating row exactly, so a difference
|
||||
# between the two jobs is the test selection and nothing else.
|
||||
api-level: "37.0"
|
||||
target: google_apis
|
||||
arch: x86_64
|
||||
profile: pixel_6
|
||||
emulator-options: -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
|
||||
disable-animations: true
|
||||
disk-size: 8G
|
||||
ram-size: 2560M
|
||||
script: bash .github/scripts/e2e-run.sh ${{ env.E2E_LABEL }}
|
||||
|
||||
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
if: always()
|
||||
with:
|
||||
name: e2e-report-api${{ env.E2E_LABEL }}
|
||||
path: |
|
||||
app/build/reports/androidTests/
|
||||
app/build/outputs/androidTest-results/
|
||||
if-no-files-found: warn
|
||||
|
||||
# Uploaded always, and here it matters more than anywhere else in this file:
|
||||
# this job is EXPECTED to be red, so the logcat is the only thing that says
|
||||
# whether it is red for the known reason or for a new one.
|
||||
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
if: always()
|
||||
with:
|
||||
name: e2e-diagnostics-api${{ env.E2E_LABEL }}
|
||||
path: |
|
||||
${{ runner.temp }}/logcat-api${{ env.E2E_LABEL }}.txt
|
||||
${{ runner.temp }}/diagnostics-api${{ env.E2E_LABEL }}.txt
|
||||
if-no-files-found: warn
|
||||
|
||||
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
if: always()
|
||||
with:
|
||||
name: e2e-wedge-api${{ env.E2E_LABEL }}
|
||||
path: ${{ runner.temp }}/wedge-diagnostics-api${{ env.E2E_LABEL }}.txt
|
||||
if-no-files-found: ignore
|
||||
|
||||
@@ -64,16 +64,28 @@ of the first one that fails.
|
||||
on this machine (below), so without it an androidTest compile error is not discovered until CI.
|
||||
ktlint and detekt also cover the `test`/`androidTest` source sets that `lintDebug` skips.
|
||||
|
||||
## Instrumented tests do not run locally
|
||||
## Instrumented tests: where they actually run
|
||||
|
||||
Two independent reasons, so do not spend time on either:
|
||||
This section said the opposite until 2026-08-24, and both of its claims had been false for two
|
||||
days. Read it as the current answer, and see the git history if you need the old one.
|
||||
|
||||
- **Emulators segfault on this host.** qemu dies on every AVD. Instrumented tests run on CI or on
|
||||
the physical Pixel, never in a local emulator.
|
||||
- **The API 37 image is broken.** `android-37.0` crash-loops surfaceflinger inside its own gralloc
|
||||
mapper, so every test fails there regardless of this app. `docs/api-37-emulator-crash.md` records
|
||||
the evidence and the ruled-out fixes; CI's matrix therefore stops at API 36 even though targetSdk
|
||||
is 37. **API 37 needs a manual check on the Pixel 10 Pro XL before each release.**
|
||||
- **Local emulators work, for API 33-36.** `tools/local-emulator/run-e2e.sh` runs them on this
|
||||
host. The segfault that made this look impossible was not a broken machine: SwiftShader's Reactor
|
||||
JIT writes generated shader code onto the heap and executes it, Fedora's SELinux policy denies
|
||||
`execheap`, and qemu dies. Choosing a different renderer avoids it entirely — `-gpu host`,
|
||||
`angle_indirect` and `swangle_indirect` all boot, while `auto`, `off`, `guest` and
|
||||
`swiftshader_indirect` do not. `docs/local-emulator.md` has the evidence and the per-API renderer
|
||||
table.
|
||||
- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. Two Media3 hardware-transcode
|
||||
tests fail inside the emulator's own `c2.goldfish.h264.decoder` rather than on anything this app
|
||||
does; they carry `@FailsOnEmulatorApi37` and run in a separate `continue-on-error` job,
|
||||
`E2E API 37 Media3 hardware transcode (advisory)`. The gating leg runs the other 55.
|
||||
**That advisory job is red on every PR, by design** — do not read it as your change breaking
|
||||
something, and do not read a green run as evidence those two tests pass.
|
||||
`docs/api-37-emulator-crash.md` has the measurements.
|
||||
|
||||
Still true, and the reason the advisory job is not simply deleted: **API 37 needs a manual check on
|
||||
the Pixel 10 Pro XL before each release.** The advisory pair is the one thing CI cannot answer for.
|
||||
|
||||
On a device or emulator, build only the ABI it can execute:
|
||||
|
||||
@@ -96,10 +108,69 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
- The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
|
||||
decision layer, where one branch is one documented user-visible outcome and the metric counts
|
||||
answers rather than complexity. Every other rule still applies there.
|
||||
- **Coverage is reported, not gated** — currently ~31% of lines. A floor needs a baseline that has
|
||||
settled first.
|
||||
- **Coverage is reported, not gated** — **29.8% of lines (629/2113), 28.7% of branches**, measured
|
||||
on `main` 2026-08-23 with `./gradlew :app:jacocoTestReport`. A floor needs a baseline that has
|
||||
settled first, and this one has not: the figure **fell** from the ~31% recorded earlier even
|
||||
though the JVM suite went from 11 test files to 43. Main source grew 4,114 -> 5,715 lines over
|
||||
the same period, so the denominator outran the numerator. Re-measure before quoting it; do not
|
||||
assume more tests means a higher percentage here.
|
||||
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
|
||||
tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply —
|
||||
a change that is both needs both.
|
||||
|
||||
Three things make that a real bar rather than a slogan here:
|
||||
|
||||
- **Unit-testable is broader than it looks.** The pure-seam pattern — `work/FailureOutcome.kt`
|
||||
documents the reasoning — turns "needs a device" into "a pure function plus a thin edge".
|
||||
Robolectric is in the JVM source set, `compose-ui-test-junit4` with it, so Compose screens are
|
||||
unit testable too. Reach for the seam before concluding something cannot be unit tested.
|
||||
- **E2E is runnable locally**, API 33-36, via `tools/local-emulator/run-e2e.sh` — see
|
||||
"Instrumented tests: where they actually run" above. That was believed impossible until the
|
||||
SELinux/renderer cause was found, and it is what makes the e2e half of this norm enforceable.
|
||||
- **A test has to bite.** Revert the line it covers, confirm it goes red, restore. A review of
|
||||
this codebase ran 46 mutations against a 257-test suite and **9 were vacuous** — five of them
|
||||
passing the whole suite over a completely unguarded code path. Green is not evidence.
|
||||
|
||||
Name what you did not cover and why. Genuine exemptions exist; implied coverage is the problem.
|
||||
|
||||
- `kotlin.code.style=official`. Gradle stays Kotlin DSL.
|
||||
|
||||
- **File one-off issues with `tools/github/file-issue.sh`, not `gh issue create`.** `gh issue
|
||||
create` does not touch the project board, so the issue exists, carries its labels, and is
|
||||
invisible in the Kanban — indistinguishable from never having been filed. Measured 2026-08-24:
|
||||
eight issues filed as a scripted batch all reached the board; one filed as a one-off minutes
|
||||
later did not. A batch carries the board step in its loop; **one-offs are where it slips**, which
|
||||
is what the script is for. It resolves the project and Status ids by name rather than caching
|
||||
them, and it **reads the item back** — a mutation returning 200 is not evidence the board shows
|
||||
what was asked for. Exit 3 means the issue was created but did not reach the board, and prints
|
||||
the number so it cannot be lost quietly.
|
||||
|
||||
`above-cut` and `backlog` are **labels from the 2026-08-22 triage pass** — "worked autonomously
|
||||
overnight" and "held for manual review". They are not board columns. Status carries board state;
|
||||
do not put a cut label on a newly filed ticket.
|
||||
|
||||
- **shellcheck runs in CI**, inside the Static analysis job, over `git ls-files '*.sh'` so a new
|
||||
script is covered without editing the workflow. It runs at full severity, `info` included: the
|
||||
two findings that raises today are answered with targeted `disable` directives carrying their
|
||||
reason, exactly as `config/detekt/detekt.yml` carries only the rules this codebase legitimately
|
||||
breaks. Do not silence it with `--severity=warning` — that hides the next real finding too.
|
||||
**It is pinned by image digest, and joins ktlint/detekt/JaCoCo in the "Dependency versions"
|
||||
rule above** — for exactly the reason stated there, demonstrated the day it was added. The first
|
||||
cut used the runner's ambient shellcheck. That is **0.9.0**, while the container used to check
|
||||
locally was 0.11.0, and the two disagree about how to report a trap handler: 0.11.0 says
|
||||
`SC2329` once on the declaration, 0.9.0 says `SC2317` on each of seven lines in the body. Same
|
||||
script, same directive, one green and one red. Directives that must survive both name both codes.
|
||||
|
||||
Locally, use the same pin rather than whatever is installed:
|
||||
`podman run --rm -v "$PWD:/mnt:z" docker.io/koalaman/shellcheck@sha256:61862eba... <files>`
|
||||
(the digest is in `status_check.yml`; there is no shellcheck system package on this host).
|
||||
|
||||
**It does not cover inline `run:` blocks in the workflows**, and a good deal of this repo's bash
|
||||
lives there. `actionlint` does cover them — it runs shellcheck over each `run:` — and reports one
|
||||
pre-existing `info` finding in `build.yml`. It is not wired in because every action here is
|
||||
pinned by SHA, and actionlint's usual installer is a `curl | bash` off a moving branch; doing it
|
||||
properly means pinning a container digest. Tracked separately rather than bolted on.
|
||||
|
||||
## Dependency versions
|
||||
|
||||
Libraries **float on minor + patch** (`coreKtx = "1.+"`). Three groups deliberately do not:
|
||||
|
||||
@@ -306,6 +306,10 @@ dependencies {
|
||||
// the tests stay green.
|
||||
testImplementation(platform(libs.compose.bom))
|
||||
testImplementation(libs.compose.ui.test.junit4)
|
||||
// For `runTest` alone, in EscapedCoroutineErrors.kt. It arrives transitively with the
|
||||
// rule above anyway; declared because a test file imports it directly, and an import of
|
||||
// something nobody asked for breaks the day the library that pulled it in stops.
|
||||
testImplementation(libs.kotlinx.coroutines.test)
|
||||
|
||||
androidTestImplementation(platform(libs.compose.bom))
|
||||
androidTestImplementation(libs.androidx.junit)
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
package org.libremediaconverter
|
||||
|
||||
/**
|
||||
* Marks an instrumented test that does not pass on the `android-37.x` **emulator** system images.
|
||||
*
|
||||
* This is a marker, not a skip. Nothing reads it except CI, and CI reads it twice — once with
|
||||
* `notAnnotation` to build the gating API 37 leg, and once with `annotation` to build the advisory
|
||||
* one — so a test carrying it runs in exactly one of the two and can never fall through both.
|
||||
* That is the whole reason there is one annotation rather than a pair of test lists: two lists
|
||||
* drift, and the drift is silent in both directions (a test that runs nowhere reads as green).
|
||||
*
|
||||
* It says only what has been measured: **on the emulator, at API 37.** The same tests pass on a
|
||||
* physical Pixel 10 Pro XL at API 37 and at API 33–36 on the same runner under the same renderer,
|
||||
* so this must never be read as "this test is allowed to fail at API 37" — only as "the API 37
|
||||
* emulator image cannot currently answer this one". `docs/api-37-emulator-crash.md` has the
|
||||
* measurements and the one bullet in them that is still inference.
|
||||
*
|
||||
* Removing it is the goal, and the trigger is written down: a new API 37.x system image, or an
|
||||
* ATD image for 37. Delete the annotation from the tests, and the advisory job goes empty and
|
||||
* the gating one grows by two.
|
||||
*/
|
||||
@Retention(AnnotationRetention.RUNTIME)
|
||||
@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION)
|
||||
annotation class FailsOnEmulatorApi37
|
||||
@@ -16,6 +16,7 @@ import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.FailsOnEmulatorApi37
|
||||
import org.libremediaconverter.model.ConversionRequest
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import java.io.File
|
||||
@@ -57,6 +58,7 @@ class Media3EngineTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
@FailsOnEmulatorApi37
|
||||
fun transcodesH264ToH265AndReportsProgress(): Unit = runBlocking {
|
||||
val seen = mutableListOf<Int>()
|
||||
|
||||
@@ -119,6 +121,7 @@ class Media3EngineTest {
|
||||
* HandlerThread indirection holds before any of that lands in Phase 2.
|
||||
*/
|
||||
@Test
|
||||
@FailsOnEmulatorApi37
|
||||
fun runsFromAThreadWithNoLooper() {
|
||||
val pool = Executors.newSingleThreadExecutor()
|
||||
try {
|
||||
|
||||
@@ -33,6 +33,7 @@ import androidx.compose.runtime.saveable.rememberSaveable
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.platform.testTag
|
||||
import androidx.compose.ui.text.style.TextAlign
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.lifecycle.compose.collectAsStateWithLifecycle
|
||||
@@ -51,6 +52,7 @@ import org.libremediaconverter.model.VideoCodec
|
||||
import org.libremediaconverter.ui.PrimaryButtonHeight
|
||||
import org.libremediaconverter.ui.ScreenPaddingHorizontal
|
||||
import org.libremediaconverter.ui.ScreenPaddingVertical
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import java.util.Locale
|
||||
|
||||
@UnstableApi
|
||||
@@ -116,7 +118,8 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
onClick = { pickInput.launch(arrayOf("*/*")) },
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight),
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.Converter.CHOOSE_FILE),
|
||||
) { Text("Choose file") }
|
||||
}
|
||||
|
||||
@@ -147,11 +150,16 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
// The Advanced picker lets an impossible combination be selected on
|
||||
// purpose, so this is what stops it from being run.
|
||||
enabled = validation.isValid,
|
||||
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.Converter.CONVERT),
|
||||
) { Text("Convert") }
|
||||
OutlinedButton(
|
||||
onClick = { pickInput.launch(arrayOf("*/*")) },
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Converter.CHOOSE_DIFFERENT_FILE),
|
||||
) { Text("Choose a different file") }
|
||||
}
|
||||
|
||||
@@ -160,11 +168,13 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
Text("Converting… ${s.percent}%")
|
||||
LinearProgressIndicator(
|
||||
progress = { s.percent / 100f },
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Converter.PROGRESS),
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
|
||||
@@ -183,7 +193,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
|
||||
@@ -202,11 +212,14 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
}
|
||||
Button(
|
||||
onClick = { chooseDestination.launch(s.suggestedName) },
|
||||
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.SAVE_FILE),
|
||||
) { Text("Save file") }
|
||||
OutlinedButton(
|
||||
onClick = viewModel::reset,
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
|
||||
) { Text("Start over") }
|
||||
}
|
||||
|
||||
@@ -214,7 +227,10 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
Text("Saved ${s.displayName}.", style = MaterialTheme.typography.bodyLarge)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.Converter.CONVERT_ANOTHER),
|
||||
) { Text("Convert another") }
|
||||
}
|
||||
|
||||
@@ -226,7 +242,10 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.START_OVER),
|
||||
) { Text("Start over") }
|
||||
}
|
||||
}
|
||||
@@ -237,9 +256,12 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
|
||||
@OptIn(ExperimentalLayoutApi::class)
|
||||
@Composable
|
||||
private fun FormatPicker(selected: OutputFormat?, onSelect: (OutputFormat) -> Unit) {
|
||||
internal fun FormatPicker(selected: OutputFormat?, onSelect: (OutputFormat) -> Unit) {
|
||||
Text("Output format", style = MaterialTheme.typography.titleSmall)
|
||||
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
|
||||
FlowRow(
|
||||
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
||||
modifier = Modifier.testTag(TestTags.Converter.FORMAT_CHIPS),
|
||||
) {
|
||||
OutputFormat.entries.forEach { format ->
|
||||
FilterChip(
|
||||
selected = format == selected,
|
||||
@@ -267,7 +289,7 @@ private fun FormatPicker(selected: OutputFormat?, onSelect: (OutputFormat) -> Un
|
||||
*/
|
||||
@OptIn(ExperimentalLayoutApi::class)
|
||||
@Composable
|
||||
private fun AdvancedPicker(
|
||||
internal fun AdvancedPicker(
|
||||
spec: OutputSpec,
|
||||
validation: Validation,
|
||||
onContainer: (Container) -> Unit,
|
||||
@@ -277,14 +299,23 @@ private fun AdvancedPicker(
|
||||
) {
|
||||
var expanded by rememberSaveable { mutableStateOf(false) }
|
||||
|
||||
TextButton(onClick = { expanded = !expanded }) {
|
||||
TextButton(
|
||||
onClick = { expanded = !expanded },
|
||||
modifier = Modifier.testTag(TestTags.Converter.ADVANCED_TOGGLE),
|
||||
) {
|
||||
Text(if (expanded) "Hide advanced" else "Advanced")
|
||||
}
|
||||
|
||||
AnimatedVisibility(visible = expanded) {
|
||||
Column(verticalArrangement = Arrangement.spacedBy(12.dp)) {
|
||||
Column(
|
||||
verticalArrangement = Arrangement.spacedBy(12.dp),
|
||||
modifier = Modifier.testTag(TestTags.Converter.ADVANCED_PANEL),
|
||||
) {
|
||||
Text("Container", style = MaterialTheme.typography.titleSmall)
|
||||
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
|
||||
FlowRow(
|
||||
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
||||
modifier = Modifier.testTag(TestTags.Converter.ADVANCED_CONTAINER_CHIPS),
|
||||
) {
|
||||
Container.entries.forEach { container ->
|
||||
FilterChip(
|
||||
selected = container == spec.container,
|
||||
@@ -295,7 +326,10 @@ private fun AdvancedPicker(
|
||||
}
|
||||
|
||||
Text("Video", style = MaterialTheme.typography.titleSmall)
|
||||
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
|
||||
FlowRow(
|
||||
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
||||
modifier = Modifier.testTag(TestTags.Converter.ADVANCED_VIDEO_CHIPS),
|
||||
) {
|
||||
VideoCodec.entries.forEach { codec ->
|
||||
FilterChip(
|
||||
selected = codec == spec.videoCodec,
|
||||
@@ -306,7 +340,10 @@ private fun AdvancedPicker(
|
||||
}
|
||||
|
||||
Text("Audio", style = MaterialTheme.typography.titleSmall)
|
||||
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
|
||||
FlowRow(
|
||||
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
||||
modifier = Modifier.testTag(TestTags.Converter.ADVANCED_AUDIO_CHIPS),
|
||||
) {
|
||||
AudioCodec.entries.forEach { codec ->
|
||||
FilterChip(
|
||||
selected = codec == spec.audioCodec,
|
||||
@@ -331,9 +368,11 @@ private fun AdvancedPicker(
|
||||
|
||||
@OptIn(ExperimentalLayoutApi::class)
|
||||
@Composable
|
||||
private fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSpec) -> Unit) {
|
||||
internal fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSpec) -> Unit) {
|
||||
Card(
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Converter.VALIDATION_ERROR),
|
||||
colors = CardDefaults.cardColors(
|
||||
containerColor = MaterialTheme.colorScheme.errorContainer,
|
||||
contentColor = MaterialTheme.colorScheme.onErrorContainer,
|
||||
@@ -347,10 +386,11 @@ private fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSp
|
||||
if (invalid.suggestions.isNotEmpty()) {
|
||||
Text("Try instead:", style = MaterialTheme.typography.labelMedium)
|
||||
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
|
||||
invalid.suggestions.forEach { suggestion ->
|
||||
invalid.suggestions.forEachIndexed { index, suggestion ->
|
||||
AssistChip(
|
||||
onClick = { onSuggestion(suggestion) },
|
||||
label = { Text(describe(suggestion)) },
|
||||
modifier = Modifier.testTag(TestTags.Converter.suggestion(index)),
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -359,7 +399,7 @@ private fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSp
|
||||
}
|
||||
}
|
||||
|
||||
private fun describe(spec: OutputSpec): String {
|
||||
internal fun describe(spec: OutputSpec): String {
|
||||
val video = when (spec.videoCodec) {
|
||||
VideoCodec.NONE -> null
|
||||
else -> spec.videoCodec.label
|
||||
@@ -374,9 +414,12 @@ private fun describe(spec: OutputSpec): String {
|
||||
|
||||
@OptIn(ExperimentalLayoutApi::class)
|
||||
@Composable
|
||||
private fun QualityPicker(selected: QualityTier, onSelect: (QualityTier) -> Unit) {
|
||||
internal fun QualityPicker(selected: QualityTier, onSelect: (QualityTier) -> Unit) {
|
||||
Text("Quality", style = MaterialTheme.typography.titleSmall)
|
||||
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
|
||||
FlowRow(
|
||||
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
||||
modifier = Modifier.testTag(TestTags.Converter.QUALITY_CHIPS),
|
||||
) {
|
||||
QualityTier.entries.forEach { tier ->
|
||||
FilterChip(
|
||||
selected = tier == selected,
|
||||
@@ -390,9 +433,12 @@ private fun QualityPicker(selected: QualityTier, onSelect: (QualityTier) -> Unit
|
||||
|
||||
@OptIn(ExperimentalLayoutApi::class)
|
||||
@Composable
|
||||
private fun EnginePicker(selected: EnginePreference, onSelect: (EnginePreference) -> Unit) {
|
||||
internal fun EnginePicker(selected: EnginePreference, onSelect: (EnginePreference) -> Unit) {
|
||||
Text("Engine", style = MaterialTheme.typography.titleSmall)
|
||||
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
|
||||
FlowRow(
|
||||
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
||||
modifier = Modifier.testTag(TestTags.Converter.ENGINE_CHIPS),
|
||||
) {
|
||||
EnginePreference.entries.forEach { preference ->
|
||||
FilterChip(
|
||||
selected = preference == selected,
|
||||
@@ -403,7 +449,7 @@ private fun EnginePicker(selected: EnginePreference, onSelect: (EnginePreference
|
||||
}
|
||||
}
|
||||
|
||||
private fun EnginePreference.label(): String = when (this) {
|
||||
internal fun EnginePreference.label(): String = when (this) {
|
||||
EnginePreference.AUTO -> "Automatic"
|
||||
EnginePreference.PREFER_HARDWARE -> "Prefer hardware"
|
||||
EnginePreference.FORCE_SOFTWARE -> "Force software"
|
||||
@@ -418,21 +464,34 @@ private fun EnginePreference.label(): String = when (this) {
|
||||
* pretending it has an unknown codec.
|
||||
*/
|
||||
@Composable
|
||||
private fun FileCard(input: InputFile) {
|
||||
Card(modifier = Modifier.fillMaxWidth()) {
|
||||
internal fun FileCard(input: InputFile) {
|
||||
Card(
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Converter.FILE_CARD),
|
||||
) {
|
||||
Column(modifier = Modifier.padding(16.dp)) {
|
||||
Text(input.displayName, style = MaterialTheme.typography.titleMedium)
|
||||
Text(
|
||||
input.displayName,
|
||||
style = MaterialTheme.typography.titleMedium,
|
||||
modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_NAME),
|
||||
)
|
||||
// The null is handled here rather than inside formatBytes, because "no provider would
|
||||
// say" is not a number and a formatter that invented one -- "0 B" -- is the defect
|
||||
// this card would be showing. It degrades in words, like the codec rows below it.
|
||||
Text(
|
||||
input.sizeBytes?.let(::formatBytes) ?: "Size unknown",
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_BYTES),
|
||||
)
|
||||
|
||||
val probe = input.probe
|
||||
if (probe == null) {
|
||||
Text("Reading…", style = MaterialTheme.typography.bodySmall)
|
||||
Text(
|
||||
"Reading…",
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_NOTE),
|
||||
)
|
||||
return@Column
|
||||
}
|
||||
|
||||
@@ -442,6 +501,7 @@ private fun FileCard(input: InputFile) {
|
||||
InputKind.UNPARSEABLE -> Text(
|
||||
"Could not identify this file. It will be converted with FFmpeg.",
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_NOTE),
|
||||
)
|
||||
|
||||
InputKind.IMAGE -> {
|
||||
@@ -481,22 +541,23 @@ private fun FileCard(input: InputFile) {
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun DetailRow(label: String, value: String) {
|
||||
internal fun DetailRow(label: String, value: String) {
|
||||
Text(
|
||||
"$label: $value",
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||
modifier = Modifier.testTag(TestTags.Converter.detailRow(label)),
|
||||
)
|
||||
}
|
||||
|
||||
private fun formatDuration(ms: Long): String {
|
||||
internal fun formatDuration(ms: Long): String {
|
||||
val totalSeconds = ms / 1000
|
||||
val minutes = totalSeconds / 60
|
||||
val seconds = totalSeconds % 60
|
||||
return String.format(Locale.US, "%d:%02d", minutes, seconds)
|
||||
}
|
||||
|
||||
private fun formatBytes(bytes: Long): String = when {
|
||||
internal fun formatBytes(bytes: Long): String = when {
|
||||
bytes >= 1_000_000_000 -> String.format(Locale.US, "%.1f GB", bytes / 1e9)
|
||||
bytes >= 1_000_000 -> String.format(Locale.US, "%.1f MB", bytes / 1e6)
|
||||
bytes >= 1_000 -> String.format(Locale.US, "%.0f kB", bytes / 1e3)
|
||||
|
||||
@@ -21,6 +21,7 @@ import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.platform.testTag
|
||||
import androidx.compose.ui.text.style.TextAlign
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.lifecycle.compose.collectAsStateWithLifecycle
|
||||
@@ -31,6 +32,7 @@ import org.libremediaconverter.model.ConcatStrategy
|
||||
import org.libremediaconverter.ui.PrimaryButtonHeight
|
||||
import org.libremediaconverter.ui.ScreenPaddingHorizontal
|
||||
import org.libremediaconverter.ui.ScreenPaddingVertical
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.libremediaconverter.work.ConcatWorker
|
||||
|
||||
@UnstableApi
|
||||
@@ -82,7 +84,8 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
onClick = { pickInputs.launch(arrayOf("video/*")) },
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight),
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.Join.CHOOSE_FILES),
|
||||
) { Text("Choose files") }
|
||||
}
|
||||
|
||||
@@ -97,11 +100,16 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
s.inputs.forEach { FileRow(it) }
|
||||
Button(
|
||||
onClick = viewModel::join,
|
||||
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.Join.JOIN),
|
||||
) { Text("Join ${s.inputs.size} files") }
|
||||
OutlinedButton(
|
||||
onClick = { pickInputs.launch(arrayOf("video/*")) },
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Join.CHOOSE_DIFFERENT_FILES),
|
||||
) { Text("Choose different files") }
|
||||
}
|
||||
|
||||
@@ -110,10 +118,14 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
// Indeterminate on purpose: FFmpeg reports progress against a
|
||||
// single input's duration, which means nothing across a
|
||||
// concatenation. A fabricated percentage would be worse than none.
|
||||
LinearProgressIndicator(modifier = Modifier.fillMaxWidth())
|
||||
LinearProgressIndicator(
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Join.PROGRESS),
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
|
||||
@@ -127,7 +139,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
|
||||
@@ -146,11 +158,14 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
)
|
||||
Button(
|
||||
onClick = { chooseDestination.launch(s.suggestedName) },
|
||||
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.SAVE_FILE),
|
||||
) { Text("Save file") }
|
||||
OutlinedButton(
|
||||
onClick = viewModel::reset,
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
|
||||
) { Text("Start over") }
|
||||
}
|
||||
|
||||
@@ -158,7 +173,10 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
Text("Saved ${s.displayName}.", style = MaterialTheme.typography.bodyLarge)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.Join.JOIN_MORE),
|
||||
) { Text("Join more") }
|
||||
}
|
||||
|
||||
@@ -170,7 +188,10 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.START_OVER),
|
||||
) { Text("Start over") }
|
||||
}
|
||||
}
|
||||
@@ -180,8 +201,12 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun FileRow(input: InputFile) {
|
||||
Card(modifier = Modifier.fillMaxWidth()) {
|
||||
internal fun FileRow(input: InputFile) {
|
||||
Card(
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Join.fileRow(input.displayName)),
|
||||
) {
|
||||
Column(modifier = Modifier.padding(12.dp)) {
|
||||
Text(input.displayName, style = MaterialTheme.typography.bodyMedium)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,142 @@
|
||||
package org.libremediaconverter.ui
|
||||
|
||||
/**
|
||||
* Where a test finds each affordance on the two screens.
|
||||
*
|
||||
* Every button, picker and card in `ConverterScreen` and `JoinScreen` carries one of these through
|
||||
* `Modifier.testTag`, so a test names a symbol and never a literal. That is the whole reason the
|
||||
* table exists: `"Cancel"`, `"Start over"` and `"Save file"` are each rendered by both screens and
|
||||
* by more than one state branch, so rewording one of them would otherwise redden several
|
||||
* independent test files at once, and none of those diffs would explain why.
|
||||
*
|
||||
* Tags are applied inside `main`, never handed in by the caller. A tag a test passes down as a
|
||||
* `Modifier` proves only that the test set it -- it would stay green with the affordance's own tag
|
||||
* deleted, which is exactly the vacuous test `CLAUDE.md` records nine of.
|
||||
*
|
||||
* ### Public rather than `internal`, deliberately
|
||||
*
|
||||
* `androidTest` **is** a friend source set of `main` here: an `androidTest` file referencing the
|
||||
* `internal` `Destination.CONVERT` compiles clean through `:app:compileDebugAndroidTestKotlin`
|
||||
* under AGP 9.3.1 (measured 2026-08-24 -- nothing in the repo referenced a main `internal` from
|
||||
* `androidTest`, so the question had no in-tree answer until then). `internal` would compile today.
|
||||
*
|
||||
* It is public anyway. That friendship is AGP wiring rather than something this project states, and
|
||||
* this table is a contract read from three source sets: `main` applies the tags, `src/test` and
|
||||
* `src/androidTest` name them. Public buys no external exposure in an application module -- nothing
|
||||
* consumes it from outside -- so the durable answer costs nothing here.
|
||||
*
|
||||
* ### Invariants
|
||||
*
|
||||
* Values are distinct, which `TagTableUniquenessTest` asserts. Two affordances sharing a tag would
|
||||
* break the "resolves to exactly one node" assertion in a file nobody had touched.
|
||||
*/
|
||||
object TestTags {
|
||||
|
||||
/**
|
||||
* Affordances both screens render, under one name each.
|
||||
*
|
||||
* Shared rather than per-screen because only one screen is composed at a time -- the shell
|
||||
* swaps them -- so a tag can only ever resolve within the screen under test.
|
||||
*/
|
||||
const val CANCEL: String = "action.cancel"
|
||||
|
||||
/** Rendered by `Converted`/`Joined` and again by `Failed` on both screens. */
|
||||
const val START_OVER: String = "action.startOver"
|
||||
|
||||
const val SAVE_FILE: String = "action.saveFile"
|
||||
|
||||
/** `ConverterScreen`. */
|
||||
object Converter {
|
||||
const val CHOOSE_FILE: String = "converter.chooseFile"
|
||||
const val CONVERT: String = "converter.convert"
|
||||
const val CHOOSE_DIFFERENT_FILE: String = "converter.chooseDifferentFile"
|
||||
const val CONVERT_ANOTHER: String = "converter.convertAnother"
|
||||
|
||||
/** The determinate bar in `Converting`. It carries no text, so nothing else can find it. */
|
||||
const val PROGRESS: String = "converter.progress"
|
||||
|
||||
const val FILE_CARD: String = "converter.fileCard"
|
||||
const val FILE_CARD_NAME: String = "converter.fileCard.name"
|
||||
|
||||
/**
|
||||
* The byte size, or `"Size unknown"`.
|
||||
*
|
||||
* Named for bytes rather than "size" because the `IMAGE` branch also renders a row labelled
|
||||
* `Size` -- pixel dimensions -- through [detailRow], and the two mean different things.
|
||||
*/
|
||||
const val FILE_CARD_BYTES: String = "converter.fileCard.bytes"
|
||||
|
||||
/**
|
||||
* The one-line explanation that stands in for the detail rows: `"Reading…"` while the probe
|
||||
* is still running, or the unreadable-file line once it has finished and found nothing.
|
||||
* The two are mutually exclusive, so one tag covers both.
|
||||
*/
|
||||
const val FILE_CARD_NOTE: String = "converter.fileCard.note"
|
||||
|
||||
/**
|
||||
* The chip rows, not the pickers around them.
|
||||
*
|
||||
* Each tag sits on the `FlowRow` of chips, so the prose a picker renders beside it -- the
|
||||
* `"Custom — set below."` line under the formats, the tier description under the quality
|
||||
* chips -- is outside the tagged node. Tagging the picker as a whole would mean wrapping
|
||||
* three sibling emissions in a layout that does not exist today.
|
||||
*/
|
||||
const val FORMAT_CHIPS: String = "converter.formatChips"
|
||||
|
||||
const val QUALITY_CHIPS: String = "converter.qualityChips"
|
||||
const val ENGINE_CHIPS: String = "converter.engineChips"
|
||||
|
||||
/** The `Advanced` / `Hide advanced` toggle. Present whether or not the panel is open. */
|
||||
const val ADVANCED_TOGGLE: String = "converter.advanced.toggle"
|
||||
|
||||
/** The panel the toggle gates. Absent from the tree while collapsed. */
|
||||
const val ADVANCED_PANEL: String = "converter.advanced.panel"
|
||||
|
||||
/**
|
||||
* The three chip rows inside the panel, separately.
|
||||
*
|
||||
* Separately because their labels collide: `"Copy"` and `"None"` are both a `VideoCodec`
|
||||
* and an `AudioCodec`, and `"MP3"` and `"FLAC"` are both a `Container` and an `AudioCodec`,
|
||||
* so a text matcher over the open panel is ambiguous for four of the chips.
|
||||
*/
|
||||
const val ADVANCED_CONTAINER_CHIPS: String = "converter.advanced.containerChips"
|
||||
|
||||
const val ADVANCED_VIDEO_CHIPS: String = "converter.advanced.videoChips"
|
||||
const val ADVANCED_AUDIO_CHIPS: String = "converter.advanced.audioChips"
|
||||
|
||||
/** The error card. Rendered outside the panel, so it is reachable while collapsed. */
|
||||
const val VALIDATION_ERROR: String = "converter.validationError"
|
||||
|
||||
/** One detail line of the file card, by the label it renders: `Container`, `Video`, ... */
|
||||
fun detailRow(label: String): String = "converter.fileCard.row:$label"
|
||||
|
||||
/**
|
||||
* One suggested output on the validation card, by position.
|
||||
*
|
||||
* By position rather than by the text of the suggestion, because that text comes from
|
||||
* `describe`, which is itself under test -- a tag derived from it would move whenever the
|
||||
* thing it is meant to locate changed.
|
||||
*/
|
||||
fun suggestion(index: Int): String = "converter.validationError.suggestion:$index"
|
||||
}
|
||||
|
||||
/** `JoinScreen`. */
|
||||
object Join {
|
||||
const val CHOOSE_FILES: String = "join.chooseFiles"
|
||||
const val JOIN: String = "join.join"
|
||||
const val CHOOSE_DIFFERENT_FILES: String = "join.chooseDifferentFiles"
|
||||
const val JOIN_MORE: String = "join.joinMore"
|
||||
|
||||
/** The indeterminate bar in `Joining`. */
|
||||
const val PROGRESS: String = "join.progress"
|
||||
|
||||
/**
|
||||
* One picked input, by the name it displays.
|
||||
*
|
||||
* By name rather than by position, so the tag is derived from data the row already holds
|
||||
* and can stay inside `FileRow`. Passing an index down would mean the call site owned the
|
||||
* tag, and a test that supplies its own tag asserts nothing about the screen.
|
||||
*/
|
||||
fun fileRow(displayName: String): String = "join.fileRow:$displayName"
|
||||
}
|
||||
}
|
||||
@@ -8,7 +8,6 @@ import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.platform.testTag
|
||||
import androidx.compose.ui.test.assertIsSelected
|
||||
import androidx.compose.ui.test.junit4.StateRestorationTester
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
@@ -42,8 +41,11 @@ import org.robolectric.RobolectricTestRunner
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class AppRootRestorationTest {
|
||||
|
||||
// Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors]. Every Compose test
|
||||
// class in this source set starts there, whether or not it is the one that happens to be
|
||||
// running when another test's escaped coroutine error is delivered.
|
||||
@get:Rule
|
||||
val composeRule = createComposeRule()
|
||||
val composeRule = createDrainedComposeRule()
|
||||
|
||||
private val restoration = StateRestorationTester(composeRule)
|
||||
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
package org.libremediaconverter
|
||||
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
import kotlinx.coroutines.test.runTest
|
||||
|
||||
/**
|
||||
* Clears coroutine errors this module's tests deliberately let escape, so they land on the test
|
||||
* that caused them instead of on the next one to start.
|
||||
*
|
||||
* **Every Compose test class in `src/test` has to start here.** `createComposeRule` runs the
|
||||
* composition inside `runTest`, and `runTest` opens by throwing `UncaughtExceptionsBeforeTest` for
|
||||
* anything already sitting in kotlinx-coroutines-test's collector -- a process-wide
|
||||
* `CoroutineExceptionHandler` it installs once and never removes.
|
||||
*
|
||||
* There is one deposit into that collector here, and it is not a mistake:
|
||||
* `ConversionViewModelProbeFailureTest.an OutOfMemoryError is not swallowed` proves an OOM raised
|
||||
* inside the probe is rethrown rather than reported as an unreadable file. `onInputPicked` runs it
|
||||
* in `viewModelScope.launch`, which has no exception handler by design -- the ViewModel's own KDoc
|
||||
* says a real OOM should reach the thread's handler and take the process down. On the JVM the
|
||||
* collector takes it instead, holds it, and hands it to whichever `runTest` starts next.
|
||||
*
|
||||
* It surfaced as two *different* Compose test classes failing on two consecutive runs of the same,
|
||||
* green, code, with a message naming neither the test nor the error's origin. Which class catches
|
||||
* it moves because the throw happens on a real `Dispatchers.IO` thread, after the state assertion
|
||||
* that ends the test that caused it -- so it can be delivered long after that class is done.
|
||||
*
|
||||
* A `@Before` method cannot do this: the compose rule's `runTest` wraps the statement that calls
|
||||
* `@Before`, so it has already thrown. `@BeforeClass` cannot either -- Robolectric runs it outside
|
||||
* the sandbox classloader, where the collector is a different object. Draining while the rule is
|
||||
* being *constructed* is early enough, because JUnit builds a fresh test-class instance, and with
|
||||
* it every `@get:Rule` field, before evaluating any rule.
|
||||
*
|
||||
* The real fix is a seam: give the probe hop an injectable dispatcher the way
|
||||
* `ConversionViewModel`'s constructor already does for `cleanupDispatcher`, and the error would
|
||||
* have somewhere to land. That is a production change, so it belongs in its own commit.
|
||||
*/
|
||||
fun drainEscapedCoroutineErrors() {
|
||||
// Entering a test scope is what flushes the collector; the flush is reported as this
|
||||
// throwing, and there is nothing to assert about an error another test already asserted on.
|
||||
runCatching { runTest {} }
|
||||
}
|
||||
|
||||
/**
|
||||
* [createComposeRule], with [drainEscapedCoroutineErrors] run first. Use this rather than
|
||||
* `createComposeRule` directly in `src/test`.
|
||||
*
|
||||
* It also keeps the one mixed import in one place: the rule comes from the **v2** package
|
||||
* (`androidx.compose.ui.test.junit4.v2`) while `StateRestorationTester`, which takes it, does not.
|
||||
*/
|
||||
fun createDrainedComposeRule() = run {
|
||||
drainEscapedCoroutineErrors()
|
||||
createComposeRule()
|
||||
}
|
||||
@@ -0,0 +1,159 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.os.Bundle
|
||||
import android.os.Parcel
|
||||
import android.os.Parcelable
|
||||
import androidx.compose.runtime.CompositionLocalProvider
|
||||
import androidx.compose.runtime.MutableState
|
||||
import androidx.compose.runtime.saveable.LocalSaveableStateRegistry
|
||||
import androidx.compose.runtime.saveable.SaveableStateRegistry
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.createDrainedComposeRule
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.OutputSpec
|
||||
import org.libremediaconverter.model.Validation
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* What `AdvancedPickerTest`'s restoration test cannot see.
|
||||
*
|
||||
* `StateRestorationTester` saves into an **in-memory map**, never a `Bundle`. That is enough to
|
||||
* discriminate `rememberSaveable` from `remember`, and it is where it stops: the map holds object
|
||||
* references, so a value the platform could never parcel goes in and comes back out looking green.
|
||||
* `AppRootRestorationTest` has the same blind spot and `DestinationSaverTest` is the split it
|
||||
* prompted; this is that split for `expanded`, the only `rememberSaveable` on either screen's
|
||||
* leaves.
|
||||
*
|
||||
* ### The saved representation is not the Boolean
|
||||
*
|
||||
* `var expanded by rememberSaveable { mutableStateOf(false) }` passes no `stateSaver`, so
|
||||
* `autoSaver` saves **the `MutableState` itself**, not the `false` inside it. That works only
|
||||
* because `mutableStateOf` on Android returns a `Parcelable` implementation -- the same call on a
|
||||
* plain JVM returns one that is not. So what stands between an open panel and a rotation that
|
||||
* closes it is a platform-specific detail of a factory function nothing here names directly, and
|
||||
* an in-memory map cannot tell the two apart.
|
||||
*
|
||||
* Pinning it is the move `DestinationSaverTest` makes about names versus ordinals. Passing an
|
||||
* explicit `stateSaver` would save a bare `Boolean` instead and is a perfectly reasonable edit --
|
||||
* it is just not the one in the tree, and it should be made on purpose rather than discovered
|
||||
* after a rotation.
|
||||
*
|
||||
* ### Shared bite, stated rather than implied
|
||||
*
|
||||
* `rememberSaveable` -> `remember` empties the registry, so it reddens this file *and* the
|
||||
* restoration test in `AdvancedPickerTest`. Both failures belong in any report of that mutation.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class AdvancedPanelSavedStateTest {
|
||||
|
||||
// Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors].
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
|
||||
/**
|
||||
* `canBeSaved = { true }` deliberately.
|
||||
*
|
||||
* A predicate mirroring what a `Bundle` accepts would be a hand-written copy of the thing
|
||||
* under test, and a `false` from it *drops* the entry silently -- so the test would fail by
|
||||
* finding nothing saved, which is also how a `remember` regression fails. Two causes, one
|
||||
* symptom, is not a test. The type is checked on the way out instead.
|
||||
*/
|
||||
private val registry = SaveableStateRegistry(restoredValues = null, canBeSaved = { true })
|
||||
|
||||
@Test
|
||||
fun `the panel registers its open state with the registry, and nothing else`() {
|
||||
setPicker()
|
||||
|
||||
// Collapsed is a saved value, not an absent one: `rememberSaveable` registers its provider
|
||||
// on first composition, whatever the state happens to be. Exactly one, because `expanded`
|
||||
// is the only saveable in the subtree -- a second would mean something else began saving.
|
||||
assertEquals(1, savedValues().size)
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
|
||||
|
||||
val saved = theOneSavedValue()
|
||||
assertTrue("saved as ${saved?.javaClass?.name}", saved is MutableState<*>)
|
||||
assertEquals(true, (saved as MutableState<*>).value)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the open panel survives a real Parcel, not just an in-memory map`() {
|
||||
setPicker()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
|
||||
|
||||
val saved = theOneSavedValue()
|
||||
|
||||
// The claim the restoration test cannot make. A `MutableState` that was not `Parcelable`
|
||||
// would satisfy `StateRestorationTester` and then be dropped by the platform.
|
||||
assertTrue("saved as ${saved?.javaClass?.name}", saved is Parcelable)
|
||||
|
||||
val restored = throughARealBundle(saved as Parcelable)
|
||||
|
||||
assertTrue("restored as ${restored.javaClass.name}", restored is MutableState<*>)
|
||||
assertEquals(true, (restored as MutableState<*>).value)
|
||||
}
|
||||
|
||||
private fun setPicker() {
|
||||
composeRule.setContent {
|
||||
CompositionLocalProvider(LocalSaveableStateRegistry provides registry) {
|
||||
AdvancedPicker(
|
||||
spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC),
|
||||
validation = Validation.Valid,
|
||||
onContainer = {},
|
||||
onVideoCodec = {},
|
||||
onAudioCodec = {},
|
||||
onSuggestion = {},
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Every value the picker hands the host to persist, keys dropped -- they are positional. */
|
||||
private fun savedValues(): List<Any?> = composeRule.runOnIdle { registry.performSave().values.flatten() }
|
||||
|
||||
/**
|
||||
* The single saved value, asserted rather than assumed.
|
||||
*
|
||||
* `single()` on an empty list throws `NoSuchElementException: List is empty`, which names
|
||||
* neither the panel nor the registry -- and an empty registry is exactly how the
|
||||
* `rememberSaveable` -> `remember` regression shows up here.
|
||||
*/
|
||||
private fun theOneSavedValue(): Any? {
|
||||
val values = savedValues()
|
||||
assertEquals("the panel should register exactly one saved value", 1, values.size)
|
||||
return values.first()
|
||||
}
|
||||
|
||||
/** A write and a read through a real `Parcel`, which is what the tester's map stands in for. */
|
||||
private fun throughARealBundle(value: Parcelable): Parcelable {
|
||||
val bundle = Bundle().apply { putParcelable(KEY, value) }
|
||||
val parcel = Parcel.obtain()
|
||||
return try {
|
||||
parcel.writeBundle(bundle)
|
||||
parcel.setDataPosition(0)
|
||||
val restored = requireNotNull(parcel.readBundle(javaClass.classLoader)) {
|
||||
"the Bundle did not survive the Parcel"
|
||||
}
|
||||
requireNotNull(restored.getParcelable(KEY, Parcelable::class.java)) {
|
||||
"the saved state did not survive the Parcel"
|
||||
}
|
||||
} finally {
|
||||
parcel.recycle()
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val KEY = "expanded"
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,321 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import androidx.compose.ui.test.assertIsDisplayed
|
||||
import androidx.compose.ui.test.assertTextEquals
|
||||
import androidx.compose.ui.test.hasAnyAncestor
|
||||
import androidx.compose.ui.test.hasTestTag
|
||||
import androidx.compose.ui.test.hasText
|
||||
import androidx.compose.ui.test.junit4.StateRestorationTester
|
||||
import androidx.compose.ui.test.onAllNodesWithTag
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.createDrainedComposeRule
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.ContainerCapabilities
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.model.OutputSpec
|
||||
import org.libremediaconverter.model.Validation
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* The gate over the Advanced chips, and the error card that deliberately sits outside it.
|
||||
*
|
||||
* Two defects, and they pull in opposite directions.
|
||||
*
|
||||
* The first is the chips escaping the gate, or never being reachable through it. `AdvancedPicker`
|
||||
* is the one leaf on this screen that is not stateless -- `expanded` is its own `rememberSaveable`
|
||||
* -- and Container, Video and Audio live inside `AnimatedVisibility(visible = expanded)`. Nothing
|
||||
* else on the screen hides anything, so a refactor that flattened the panel, or wired the toggle to
|
||||
* a state nobody reads, would render an app that looks reasonable in a screenshot and is wrong.
|
||||
*
|
||||
* The second is the opposite mistake, and it is the one this file exists for: **moving the
|
||||
* `ValidationError` call inside the `AnimatedVisibility`**. It is invoked after that block, so an
|
||||
* invalid spec explains itself and offers one-tap fixes *while the section is collapsed*. That is
|
||||
* the only route out of an invalid spec for a user who never opened Advanced -- and since the only
|
||||
* way to reach an invalid spec is through Advanced, hiding the way out behind the same toggle looks
|
||||
* locally sensible and is a trap. Tidying the two `if` blocks into one is a plausible edit, it
|
||||
* compiles, and until this file existed nothing went red. Every assertion about the error card here
|
||||
* therefore runs with the toggle untouched, and asserts the panel is absent in the same test, so a
|
||||
* future `expanded = true` default cannot quietly satisfy it either.
|
||||
*
|
||||
* The invalid specs come from [ContainerCapabilities.validate] rather than from a hand-built
|
||||
* [Validation.Invalid], so the messages and the suggestions are the real pairing. A hand-built one
|
||||
* would keep passing after `validate` stopped producing anything like it.
|
||||
*
|
||||
* Node location is by the three separate chip-row tags, never by text. `"Copy"` and `"None"` are
|
||||
* each both a [VideoCodec] and an [AudioCodec], and `"MP3"` and `"FLAC"` are each both a
|
||||
* [Container] and an [AudioCodec], so a text matcher over the open panel is ambiguous for four
|
||||
* chips -- which is what the separate tags are for.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class AdvancedPickerTest {
|
||||
|
||||
// Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors].
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
|
||||
private val restoration = StateRestorationTester(composeRule)
|
||||
|
||||
private val containers = mutableListOf<Container>()
|
||||
private val videoCodecs = mutableListOf<VideoCodec>()
|
||||
private val audioCodecs = mutableListOf<AudioCodec>()
|
||||
private val applied = mutableListOf<OutputSpec>()
|
||||
|
||||
// --- the expand gate ----------------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `the three chip rows appear only while the panel is expanded`() {
|
||||
setPicker()
|
||||
|
||||
assertPanelHidden()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertExists()
|
||||
ROW_TAGS.forEach { composeRule.onNodeWithTag(it).assertExists() }
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
|
||||
|
||||
// The exit transition outlives the click, so absence has to be waited for rather than
|
||||
// asserted straight away -- unlike the initial collapsed state, which has no animation
|
||||
// in flight.
|
||||
composeRule.waitUntil { nodeCount(TestTags.Converter.ADVANCED_PANEL) == 0 }
|
||||
assertPanelHidden()
|
||||
}
|
||||
|
||||
/** The toggle is the only affordance the collapsed picker offers, so it has to say so. */
|
||||
@Test
|
||||
fun `the toggle names the direction it will move in`() {
|
||||
setPicker()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).assertTextEquals("Advanced")
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE)
|
||||
.assertTextEquals("Hide advanced")
|
||||
}
|
||||
|
||||
/**
|
||||
* The four colliding labels, one per row.
|
||||
*
|
||||
* `"Copy"` is a video codec *and* an audio codec; `"MP3"` is a container *and* an audio codec.
|
||||
* Clicking each through its own row is what proves the rows are wired to different callbacks
|
||||
* -- a picker that handed every chip to `onAudioCodec` would look identical on screen.
|
||||
*/
|
||||
@Test
|
||||
fun `each chip row reports to its own callback, including the labels that collide`() {
|
||||
setPicker()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
|
||||
|
||||
chipIn(TestTags.Converter.ADVANCED_VIDEO_CHIPS, "Copy").performClick()
|
||||
|
||||
assertEquals(listOf(VideoCodec.COPY), videoCodecs)
|
||||
assertEquals(emptyList<AudioCodec>(), audioCodecs)
|
||||
|
||||
chipIn(TestTags.Converter.ADVANCED_AUDIO_CHIPS, "Copy").performClick()
|
||||
|
||||
assertEquals(listOf(AudioCodec.COPY), audioCodecs)
|
||||
|
||||
chipIn(TestTags.Converter.ADVANCED_CONTAINER_CHIPS, "MP3").performClick()
|
||||
|
||||
assertEquals(listOf(Container.MP3), containers)
|
||||
// Still only the one audio click. `MP3` is an AudioCodec label too, and the container row
|
||||
// must not be reporting through that callback.
|
||||
assertEquals(listOf(AudioCodec.COPY), audioCodecs)
|
||||
}
|
||||
|
||||
// --- the error card, which is outside the gate --------------------------
|
||||
|
||||
/**
|
||||
* The headline case. Dropping both tracks is reachable from the collapsed screen -- the
|
||||
* `None`/`None` pair is set inside Advanced, but the user can close it again -- and the
|
||||
* explanation has to still be there.
|
||||
*/
|
||||
@Test
|
||||
fun `an empty output explains itself while the section is collapsed`() {
|
||||
val spec = OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE)
|
||||
val invalid = invalidFor(spec)
|
||||
|
||||
assertEquals("This would produce an empty file — keep at least one track.", invalid.message)
|
||||
|
||||
setPicker(spec, invalid)
|
||||
|
||||
assertPanelHidden()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.VALIDATION_ERROR).assertExists()
|
||||
composeRule.onNodeWithText(invalid.message).assertIsDisplayed()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a codec the container cannot hold explains itself while the section is collapsed`() {
|
||||
val spec = OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.OPUS)
|
||||
val invalid = invalidFor(spec)
|
||||
|
||||
assertEquals("WebM cannot hold H.264 video.", invalid.message)
|
||||
|
||||
setPicker(spec, invalid)
|
||||
|
||||
assertPanelHidden()
|
||||
composeRule.onNodeWithText(invalid.message).assertIsDisplayed()
|
||||
}
|
||||
|
||||
/**
|
||||
* Clicking a suggestion, with the toggle never touched.
|
||||
*
|
||||
* The second suggestion rather than the first, and its count pinned first: with one suggestion
|
||||
* a picker that handed every chip `suggestions[0]` would pass, and `onNodeWithTag` on a
|
||||
* suggestion index that no longer exists reports an unhelpful matcher failure rather than
|
||||
* saying the list shrank.
|
||||
*/
|
||||
@Test
|
||||
fun `a suggestion chip applies its own spec without the section ever being opened`() {
|
||||
val spec = OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.OPUS)
|
||||
val invalid = invalidFor(spec)
|
||||
|
||||
assertEquals(2, invalid.suggestions.size)
|
||||
val second = invalid.suggestions[1]
|
||||
|
||||
setPicker(spec, invalid)
|
||||
|
||||
assertPanelHidden()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.suggestion(1)).assertTextEquals(describe(second))
|
||||
composeRule.onNodeWithTag(TestTags.Converter.suggestion(1)).performClick()
|
||||
|
||||
assertEquals(listOf(second), applied)
|
||||
// What the chips offer is what `validate` said would work, not a repair of the test's own.
|
||||
assertTrue(
|
||||
"suggestion $second should itself validate",
|
||||
ContainerCapabilities.validate(second, PROBE).isValid,
|
||||
)
|
||||
}
|
||||
|
||||
/** A valid spec has nothing to say, collapsed or not. */
|
||||
@Test
|
||||
fun `a valid spec renders no error card`() {
|
||||
setPicker()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.VALIDATION_ERROR).assertDoesNotExist()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.VALIDATION_ERROR).assertDoesNotExist()
|
||||
}
|
||||
|
||||
// --- recreation ---------------------------------------------------------
|
||||
|
||||
/**
|
||||
* `expanded` is the only `rememberSaveable` on either screen's leaves.
|
||||
*
|
||||
* `MainActivity` declares no `configChanges`, so a rotation destroys and rebuilds the whole
|
||||
* composition. A panel the user opened, set three chips in, and left open must not close
|
||||
* itself on the way back. `remember` would.
|
||||
*
|
||||
* What this cannot see is the saved *representation* -- `StateRestorationTester` saves into an
|
||||
* in-memory map rather than a `Bundle`. `AdvancedPanelSavedStateTest` covers that half.
|
||||
*/
|
||||
@Test
|
||||
fun `an open panel is still open after recreation`() {
|
||||
restoration.setContent {
|
||||
AdvancedPicker(
|
||||
spec = VALID_SPEC,
|
||||
validation = Validation.Valid,
|
||||
onContainer = {},
|
||||
onVideoCodec = {},
|
||||
onAudioCodec = {},
|
||||
onSuggestion = {},
|
||||
)
|
||||
}
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertExists()
|
||||
|
||||
restoration.emulateSavedInstanceStateRestore()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertExists()
|
||||
ROW_TAGS.forEach { composeRule.onNodeWithTag(it).assertExists() }
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE)
|
||||
.assertTextEquals("Hide advanced")
|
||||
}
|
||||
|
||||
/** The default has to survive too, or the panel would spring open on every rotation. */
|
||||
@Test
|
||||
fun `a collapsed panel is still collapsed after recreation`() {
|
||||
restoration.setContent {
|
||||
AdvancedPicker(
|
||||
spec = VALID_SPEC,
|
||||
validation = Validation.Valid,
|
||||
onContainer = {},
|
||||
onVideoCodec = {},
|
||||
onAudioCodec = {},
|
||||
onSuggestion = {},
|
||||
)
|
||||
}
|
||||
|
||||
assertPanelHidden()
|
||||
|
||||
restoration.emulateSavedInstanceStateRestore()
|
||||
|
||||
assertPanelHidden()
|
||||
}
|
||||
|
||||
// --- helpers ------------------------------------------------------------
|
||||
|
||||
private fun setPicker(spec: OutputSpec = VALID_SPEC, validation: Validation = Validation.Valid) {
|
||||
composeRule.setContent {
|
||||
AdvancedPicker(
|
||||
spec = spec,
|
||||
validation = validation,
|
||||
onContainer = { containers += it },
|
||||
onVideoCodec = { videoCodecs += it },
|
||||
onAudioCodec = { audioCodecs += it },
|
||||
onSuggestion = { applied += it },
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/** The whole panel, by every tag it owns, so a partial escape counts as a failure. */
|
||||
private fun assertPanelHidden() {
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertDoesNotExist()
|
||||
ROW_TAGS.forEach { composeRule.onNodeWithTag(it).assertDoesNotExist() }
|
||||
}
|
||||
|
||||
private fun nodeCount(tag: String) = composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().size
|
||||
|
||||
private fun chipIn(rowTag: String, label: String) =
|
||||
composeRule.onNode(hasText(label) and hasAnyAncestor(hasTestTag(rowTag)))
|
||||
|
||||
private fun invalidFor(spec: OutputSpec): Validation.Invalid {
|
||||
val validation = ContainerCapabilities.validate(spec, PROBE)
|
||||
return validation as? Validation.Invalid
|
||||
?: throw AssertionError("$spec was expected to be invalid, but validate said $validation")
|
||||
}
|
||||
|
||||
private companion object {
|
||||
val ROW_TAGS = listOf(
|
||||
TestTags.Converter.ADVANCED_CONTAINER_CHIPS,
|
||||
TestTags.Converter.ADVANCED_VIDEO_CHIPS,
|
||||
TestTags.Converter.ADVANCED_AUDIO_CHIPS,
|
||||
)
|
||||
|
||||
val VALID_SPEC = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC)
|
||||
|
||||
/** An ordinary H.264/AAC MP4, so the suggestions have a real source to repair towards. */
|
||||
val PROBE = InputProbe(
|
||||
videoCodec = "h264",
|
||||
audioCodec = "aac",
|
||||
durationMs = 90_000,
|
||||
container = Container.MP4,
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,152 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.EnginePreference
|
||||
import org.libremediaconverter.model.OutputSpec
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
|
||||
/**
|
||||
* The four pure helpers behind the converter screen's prose, pinned at the points where they
|
||||
* change what they say.
|
||||
*
|
||||
* No Compose rule and no Robolectric: these are `String` in, `String` out, and running them under a
|
||||
* device sandbox would buy nothing while hiding the boundaries in a rendered tree.
|
||||
*
|
||||
* The defect each group bites on:
|
||||
*
|
||||
* - **[formatBytes] picks a unit by comparing against three thresholds.** Every one of them is a
|
||||
* `>=`, and a `>` would move a file sitting exactly on a boundary into the unit below -- `1 GB`
|
||||
* shown as `1000.0 MB`. Only a value *on* the threshold can tell the two apart, so each of the
|
||||
* three is asserted at the boundary and one below it. The unit prefixes are decimal, matching
|
||||
* what the file manager and the provider report, not powers of two.
|
||||
* - **[formatDuration] has no hours field.** An hour-long recording reads `60:00`, and that is the
|
||||
* contract rather than an oversight -- the row is a length, not a clock. Pinned so that adding
|
||||
* hours is a deliberate change with a red test in front of it instead of a silent reformat.
|
||||
* - **[describe] builds the suggestion-chip label out of up to three parts**, and the parts are
|
||||
* conditional: [VideoCodec.NONE] and [AudioCodec.NONE] drop out entirely, so an image output
|
||||
* with neither track has to render as the container alone rather than as a container followed
|
||||
* by a dangling separator.
|
||||
* - **[EnginePreference] carries no `label` property**, unlike every other enum the screen
|
||||
* renders; its three display strings live in a `when` in the screen file. Adding a constant is
|
||||
* caught by the compiler because that `when` is exhaustive, but nothing stops two constants
|
||||
* being given the same string, which is what the distinctness assertion is for.
|
||||
*/
|
||||
class ConverterFormattersTest {
|
||||
|
||||
@Test
|
||||
fun `bytes below a kilobyte are counted exactly`() {
|
||||
assertEquals("0 B", formatBytes(0))
|
||||
assertEquals("1 B", formatBytes(1))
|
||||
assertEquals("999 B", formatBytes(999))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `each unit starts exactly on its threshold rather than one byte past it`() {
|
||||
assertEquals("1 kB", formatBytes(1_000))
|
||||
assertEquals("1.0 MB", formatBytes(1_000_000))
|
||||
assertEquals("1.0 GB", formatBytes(1_000_000_000))
|
||||
}
|
||||
|
||||
/**
|
||||
* One byte below each threshold, which is the half a `>=` to `>` change leaves alone. Both
|
||||
* halves are needed: the boundary values alone would still pass if the comparison let
|
||||
* everything through.
|
||||
*/
|
||||
@Test
|
||||
fun `a value just below a threshold stays in the smaller unit`() {
|
||||
assertEquals("999 B", formatBytes(999))
|
||||
assertEquals("1000 kB", formatBytes(999_999))
|
||||
assertEquals("1000.0 MB", formatBytes(999_999_999))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a real file size reads as one decimal place`() {
|
||||
assertEquals("12.3 MB", formatBytes(12_345_678))
|
||||
assertEquals("1.5 GB", formatBytes(1_500_000_000))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a duration is minutes and zero-padded seconds`() {
|
||||
assertEquals("0:00", formatDuration(0))
|
||||
assertEquals("0:01", formatDuration(1_000))
|
||||
assertEquals("0:59", formatDuration(59_000))
|
||||
assertEquals("1:00", formatDuration(60_000))
|
||||
assertEquals("1:30", formatDuration(90_000))
|
||||
}
|
||||
|
||||
/** Sub-second remainders are dropped rather than rounded up into the next second. */
|
||||
@Test
|
||||
fun `a partial second does not become a whole one`() {
|
||||
assertEquals("0:00", formatDuration(999))
|
||||
assertEquals("0:59", formatDuration(59_999))
|
||||
}
|
||||
|
||||
/** No hours field, deliberately: an hour is `60:00` and two hours are `120:00`. */
|
||||
@Test
|
||||
fun `an hour and beyond keeps counting in minutes`() {
|
||||
assertEquals("60:00", formatDuration(3_600_000))
|
||||
assertEquals("61:01", formatDuration(3_661_000))
|
||||
assertEquals("120:00", formatDuration(7_200_000))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a spec with both tracks names the container and joins the two codecs`() {
|
||||
assertEquals(
|
||||
"MP4 · H.264 + AAC",
|
||||
describe(OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC)),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a track set to none is left out instead of being named none`() {
|
||||
assertEquals(
|
||||
"MP3 · MP3",
|
||||
describe(OutputSpec(Container.MP3, VideoCodec.NONE, AudioCodec.MP3)),
|
||||
)
|
||||
assertEquals(
|
||||
"MP4 · H.264",
|
||||
describe(OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.NONE)),
|
||||
)
|
||||
}
|
||||
|
||||
/** An image output has neither track, so there is nothing for the separator to separate. */
|
||||
@Test
|
||||
fun `a spec with no tracks at all is the container alone, with no trailing separator`() {
|
||||
assertEquals("GIF", describe(OutputSpec(Container.GIF, VideoCodec.NONE, AudioCodec.NONE)))
|
||||
assertEquals(
|
||||
"PNG frames",
|
||||
describe(OutputSpec(Container.IMAGE_SEQUENCE, VideoCodec.NONE, AudioCodec.NONE)),
|
||||
)
|
||||
}
|
||||
|
||||
/** `Copy` is a codec here, not the absence of one, so a remux describes both tracks. */
|
||||
@Test
|
||||
fun `a remux names copy on both tracks rather than dropping them`() {
|
||||
assertEquals(
|
||||
"Matroska · Copy + Copy",
|
||||
describe(OutputSpec(Container.MKV, VideoCodec.COPY, AudioCodec.COPY)),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `each engine preference has the wording the chips show`() {
|
||||
assertEquals("Automatic", EnginePreference.AUTO.label())
|
||||
assertEquals("Prefer hardware", EnginePreference.PREFER_HARDWARE.label())
|
||||
assertEquals("Force software", EnginePreference.FORCE_SOFTWARE.label())
|
||||
}
|
||||
|
||||
/**
|
||||
* Two constants sharing a label would render as two identical chips, one of which the user
|
||||
* could not choose deliberately. The exhaustive `when` cannot catch that; this does.
|
||||
*/
|
||||
@Test
|
||||
fun `no two engine preferences render the same chip`() {
|
||||
val labels = EnginePreference.entries.map { it.label() }
|
||||
|
||||
assertEquals(EnginePreference.entries.size, labels.toSet().size)
|
||||
assertEquals(emptyList<String>(), labels.filter { it.isBlank() })
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,196 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.compose.ui.test.assertCountEquals
|
||||
import androidx.compose.ui.test.onAllNodesWithTag
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.createDrainedComposeRule
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.EnginePreference
|
||||
import org.libremediaconverter.model.InputKind
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.model.OutputSpec
|
||||
import org.libremediaconverter.model.QualityTier
|
||||
import org.libremediaconverter.model.Validation
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* Each leaf of the converter screen renders, and each tag it claims resolves to exactly one node.
|
||||
*
|
||||
* The defect this bites on is a tag that is not where the table says it is: dropped by a refactor
|
||||
* that rewrote a `Modifier` chain, applied to the wrong one of two siblings, or duplicated onto a
|
||||
* leaf that is rendered twice. None of that is visible at compile time -- a `testTag` is a string
|
||||
* handed to a modifier -- and none of it shows up in the app either, because nothing but a test
|
||||
* ever reads one.
|
||||
*
|
||||
* It has to be caught here rather than by the children that consume the tags. R38.2, R38.3 and
|
||||
* R38.4 all *begin* by locating a node through one of these, so a tag that had quietly moved would
|
||||
* surface as three unrelated PRs failing on a line their own diffs do not touch. Counting the nodes
|
||||
* rather than asserting existence is deliberate: `onNodeWithTag` on two matches throws about
|
||||
* ambiguity in one place and passes in another, so "exactly one" is the property worth pinning.
|
||||
*
|
||||
* Deliberately *not* the state matrix. Which affordances each `ConversionState` renders is R38.6,
|
||||
* and it needs the state seam R38.5 extracts -- the branch buttons tagged in this change (Convert,
|
||||
* Cancel, Save file, Start over, ...) therefore have no bite yet, which the PR body records.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class ConverterLeafTagsTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
|
||||
private fun assertResolvesToOneNode(tag: String) {
|
||||
composeRule.onAllNodesWithTag(tag).assertCountEquals(1)
|
||||
}
|
||||
|
||||
private fun input(sizeBytes: Long? = 12_345_678L, probe: InputProbe? = VIDEO_PROBE) = InputFile(
|
||||
uri = Uri.parse("content://test/clip.mkv"),
|
||||
displayName = "clip.mkv",
|
||||
sizeBytes = sizeBytes,
|
||||
probe = probe,
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `the format picker tags its chip row`() {
|
||||
composeRule.setContent { FormatPicker(OutputFormat.MP4_H264) {} }
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.FORMAT_CHIPS)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the quality picker tags its chip row`() {
|
||||
composeRule.setContent { QualityPicker(QualityTier.FAST) {} }
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.QUALITY_CHIPS)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the engine picker tags its chip row`() {
|
||||
composeRule.setContent { EnginePicker(EnginePreference.AUTO) {} }
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.ENGINE_CHIPS)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the advanced picker tags its toggle, which is all it renders while collapsed`() {
|
||||
setAdvancedPicker()
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.ADVANCED_TOGGLE)
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertDoesNotExist()
|
||||
}
|
||||
|
||||
/**
|
||||
* The panel and its three rows only exist once the toggle has been clicked, which is R38.4's
|
||||
* subject. Expanding is the only way to reach the tags at all, so the smoke test has to do it.
|
||||
*/
|
||||
@Test
|
||||
fun `expanding the advanced picker tags the panel and each of its three chip rows`() {
|
||||
setAdvancedPicker()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.ADVANCED_PANEL)
|
||||
assertResolvesToOneNode(TestTags.Converter.ADVANCED_CONTAINER_CHIPS)
|
||||
assertResolvesToOneNode(TestTags.Converter.ADVANCED_VIDEO_CHIPS)
|
||||
assertResolvesToOneNode(TestTags.Converter.ADVANCED_AUDIO_CHIPS)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the validation card tags itself and every suggestion on it`() {
|
||||
composeRule.setContent {
|
||||
ValidationError(
|
||||
Validation.Invalid(
|
||||
message = "WebM cannot hold H.264 video.",
|
||||
suggestions = listOf(
|
||||
OutputSpec(Container.MKV, VideoCodec.H264, AudioCodec.AAC),
|
||||
OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.OPUS),
|
||||
),
|
||||
),
|
||||
) {}
|
||||
}
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.VALIDATION_ERROR)
|
||||
assertResolvesToOneNode(TestTags.Converter.suggestion(0))
|
||||
assertResolvesToOneNode(TestTags.Converter.suggestion(1))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the file card tags itself, its name and its size line`() {
|
||||
composeRule.setContent { FileCard(input()) }
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.FILE_CARD)
|
||||
assertResolvesToOneNode(TestTags.Converter.FILE_CARD_NAME)
|
||||
assertResolvesToOneNode(TestTags.Converter.FILE_CARD_BYTES)
|
||||
}
|
||||
|
||||
/**
|
||||
* Both writers of the note line get their own case. They are two separate `Text` calls in two
|
||||
* branches that share one tag, so a test of either alone would leave the other unguarded.
|
||||
*/
|
||||
@Test
|
||||
fun `the file card tags the note it shows while the probe is still running`() {
|
||||
composeRule.setContent { FileCard(input(probe = null)) }
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.FILE_CARD_NOTE)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the file card tags the note it shows when nothing could read the file`() {
|
||||
composeRule.setContent { FileCard(input(probe = InputProbe(kind = InputKind.UNPARSEABLE))) }
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.FILE_CARD_NOTE)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a detail row tags itself with the label it renders`() {
|
||||
composeRule.setContent { DetailRow("Container", "Matroska") }
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.detailRow("Container"))
|
||||
}
|
||||
|
||||
/** The rows the file card builds carry the same per-label tags, one per row it renders. */
|
||||
@Test
|
||||
fun `the file card's detail rows are each tagged by their own label`() {
|
||||
composeRule.setContent { FileCard(input()) }
|
||||
|
||||
assertResolvesToOneNode(TestTags.Converter.detailRow("Container"))
|
||||
assertResolvesToOneNode(TestTags.Converter.detailRow("Video"))
|
||||
assertResolvesToOneNode(TestTags.Converter.detailRow("Audio"))
|
||||
assertResolvesToOneNode(TestTags.Converter.detailRow("Length"))
|
||||
}
|
||||
|
||||
private fun setAdvancedPicker() {
|
||||
composeRule.setContent {
|
||||
AdvancedPicker(
|
||||
spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC),
|
||||
validation = Validation.Valid,
|
||||
onContainer = {},
|
||||
onVideoCodec = {},
|
||||
onAudioCodec = {},
|
||||
onSuggestion = {},
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
val VIDEO_PROBE = InputProbe(
|
||||
videoCodec = "video/avc",
|
||||
audioCodec = "audio/mp4a-latm",
|
||||
durationMs = 90_000,
|
||||
kind = InputKind.VIDEO,
|
||||
container = Container.MKV,
|
||||
width = 1920,
|
||||
height = 1080,
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,169 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import androidx.compose.ui.test.SemanticsNodeInteraction
|
||||
import androidx.compose.ui.test.assertIsNotSelected
|
||||
import androidx.compose.ui.test.assertIsSelected
|
||||
import androidx.compose.ui.test.hasAnyAncestor
|
||||
import androidx.compose.ui.test.hasTestTag
|
||||
import androidx.compose.ui.test.hasText
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.createDrainedComposeRule
|
||||
import org.libremediaconverter.model.EnginePreference
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.model.QualityTier
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* Each picker lights the chip it was handed and reports the constant that was pressed.
|
||||
*
|
||||
* The defect this bites on is a picker that renders perfectly and answers wrongly. All three are
|
||||
* the same dozen lines with a different enum substituted, so the failure mode is a copy-paste that
|
||||
* survives review: an `onClick` that closes over the picker's `selected` parameter instead of the
|
||||
* chip's own entry hands back one constant no matter which chip was tapped, and an inverted
|
||||
* `entry == selected` lights every chip except the right one. Neither throws, neither changes the
|
||||
* set of labels on screen, and a test that only asserted "the callback ran" would pass over both.
|
||||
*
|
||||
* Clicking every chip in turn and comparing the whole recorded list against `entries` is what makes
|
||||
* the constant load-bearing rather than the click count -- a hardcoded `onSelect` fires the same
|
||||
* number of times as a correct one. Selection is asserted over every chip for the same reason: the
|
||||
* one that should be lit proves nothing on its own, because `!=` lights it too whenever the enum
|
||||
* has exactly one entry, and lights all its siblings whenever it has more.
|
||||
*
|
||||
* Labels come from `OutputFormat.label` and `QualityTier.label`; [label], which the screen owns
|
||||
* because `EnginePreference` carries no label of its own, supplies the third set. Retyping any of
|
||||
* them here would turn a rename into a red test that named the wrong cause.
|
||||
*
|
||||
* Not covered, deliberately: the `"Output format"`, `"Quality"` and `"Engine"` headings, which are
|
||||
* untagged `Text` calls with no enum behind them and no behaviour to bite on.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class ConverterPickerSelectionTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
|
||||
/**
|
||||
* The chip carrying [label] inside the row tagged [rowTag].
|
||||
*
|
||||
* By ancestor rather than by direct child: how many semantics nodes Material 3 puts between a
|
||||
* `FlowRow` and its chips is that library's business, and a matcher that assumed "one" would
|
||||
* break on an upgrade that changed nothing this test is about.
|
||||
*/
|
||||
private fun chipIn(rowTag: String, label: String): SemanticsNodeInteraction =
|
||||
composeRule.onNode(hasAnyAncestor(hasTestTag(rowTag)) and hasText(label))
|
||||
|
||||
private fun assertOnlySelected(rowTag: String, labels: List<String>, selected: String?) {
|
||||
labels.forEach { label ->
|
||||
val chip = chipIn(rowTag, label)
|
||||
if (label == selected) chip.assertIsSelected() else chip.assertIsNotSelected()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the format picker lights the selected format and no other`() {
|
||||
composeRule.setContent { FormatPicker(OutputFormat.WEBM_VP9) {} }
|
||||
|
||||
assertOnlySelected(
|
||||
rowTag = TestTags.Converter.FORMAT_CHIPS,
|
||||
labels = OutputFormat.entries.map { it.label },
|
||||
selected = OutputFormat.WEBM_VP9.label,
|
||||
)
|
||||
}
|
||||
|
||||
/** A spec no preset can express lights nothing, which is what the custom line stands in for. */
|
||||
@Test
|
||||
fun `the format picker lights nothing when the spec is custom`() {
|
||||
composeRule.setContent { FormatPicker(null) {} }
|
||||
|
||||
assertOnlySelected(
|
||||
rowTag = TestTags.Converter.FORMAT_CHIPS,
|
||||
labels = OutputFormat.entries.map { it.label },
|
||||
selected = null,
|
||||
)
|
||||
composeRule.onNodeWithText(CUSTOM_SPEC_NOTE).assertExists()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a selected format hides the custom line`() {
|
||||
composeRule.setContent { FormatPicker(OutputFormat.MP3) {} }
|
||||
|
||||
composeRule.onNodeWithText(CUSTOM_SPEC_NOTE).assertDoesNotExist()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `clicking a format chip reports that format`() {
|
||||
val picked = mutableListOf<OutputFormat>()
|
||||
composeRule.setContent { FormatPicker(null) { picked += it } }
|
||||
|
||||
OutputFormat.entries.forEach { chipIn(TestTags.Converter.FORMAT_CHIPS, it.label).performClick() }
|
||||
|
||||
assertEquals(OutputFormat.entries.toList(), picked)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the quality picker lights the selected tier and no other`() {
|
||||
composeRule.setContent { QualityPicker(QualityTier.BEST) {} }
|
||||
|
||||
assertOnlySelected(
|
||||
rowTag = TestTags.Converter.QUALITY_CHIPS,
|
||||
labels = QualityTier.entries.map { it.label },
|
||||
selected = QualityTier.BEST.label,
|
||||
)
|
||||
}
|
||||
|
||||
/** The line under the chips describes what was chosen, not whichever tier was written first. */
|
||||
@Test
|
||||
fun `the quality picker explains the tier that is selected`() {
|
||||
composeRule.setContent { QualityPicker(QualityTier.BEST) {} }
|
||||
|
||||
composeRule.onNodeWithText(QualityTier.BEST.description).assertExists()
|
||||
composeRule.onNodeWithText(QualityTier.FAST.description).assertDoesNotExist()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `clicking a quality chip reports that tier`() {
|
||||
val picked = mutableListOf<QualityTier>()
|
||||
composeRule.setContent { QualityPicker(QualityTier.FAST) { picked += it } }
|
||||
|
||||
QualityTier.entries.forEach { chipIn(TestTags.Converter.QUALITY_CHIPS, it.label).performClick() }
|
||||
|
||||
assertEquals(QualityTier.entries.toList(), picked)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the engine picker lights the selected preference and no other`() {
|
||||
composeRule.setContent { EnginePicker(EnginePreference.FORCE_SOFTWARE) {} }
|
||||
|
||||
assertOnlySelected(
|
||||
rowTag = TestTags.Converter.ENGINE_CHIPS,
|
||||
labels = EnginePreference.entries.map { it.label() },
|
||||
selected = EnginePreference.FORCE_SOFTWARE.label(),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `clicking an engine chip reports that preference`() {
|
||||
val picked = mutableListOf<EnginePreference>()
|
||||
composeRule.setContent { EnginePicker(EnginePreference.AUTO) { picked += it } }
|
||||
|
||||
EnginePreference.entries.forEach { chipIn(TestTags.Converter.ENGINE_CHIPS, it.label()).performClick() }
|
||||
|
||||
assertEquals(EnginePreference.entries.toList(), picked)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/**
|
||||
* Copied byte for byte out of `ConverterScreen.kt` -- it holds a U+2014 em dash, which
|
||||
* retyped as ASCII would match nothing and fail as "no node found" rather than as a reword.
|
||||
*/
|
||||
const val CUSTOM_SPEC_NOTE: String = "Custom — set below."
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,254 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.compose.ui.test.assertCountEquals
|
||||
import androidx.compose.ui.test.assertTextEquals
|
||||
import androidx.compose.ui.test.onChildren
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.createDrainedComposeRule
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.InputKind
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* What the source-info card says when it does not know something.
|
||||
*
|
||||
* The defect is a card that invents an answer instead of admitting it has none. Two of them are
|
||||
* live here and neither had a test before this file:
|
||||
*
|
||||
* - **`InputFile.sizeBytes` is nullable and the card is the reader that has to say so in words.**
|
||||
* `sizeBytes` used to be `0L` for "nobody told me", and [UnknownInputSizeTest] records what that
|
||||
* cost at the space check. The card is the other reader, and its failure mode is the mirror
|
||||
* image: hand the null to `formatBytes` and it renders `"0 B"` -- a measurement, shown to the
|
||||
* user, that no provider ever made. It renders **independently of the probe**, which is why the
|
||||
* same assertion appears twice below, with the probe present and absent. That independence is
|
||||
* the contract; a test covering only the probed case would leave the branch a user actually hits
|
||||
* first -- the card is on screen before the probe finishes -- unguarded.
|
||||
* - **The codec rows degrade in words too.** `CodecNames.describeVideo`/`describeAudio` answer
|
||||
* `"Unknown"` for a codec nothing named, the `VIDEO` branch answers `"No audio track"` for a file
|
||||
* with no audio, and the two `> 0` guards drop the dimension and length rows rather than printing
|
||||
* `0` and `0:00`. Each of those has a case below on **both** sides of the guard, because a test
|
||||
* of the present side alone stays green with the guard deleted.
|
||||
*
|
||||
* ### What cannot be asserted here, so that it is a decision rather than an omission
|
||||
*
|
||||
* The `probe == null` branch exits before `HorizontalDivider`, and **the divider's absence is not
|
||||
* observable from a test**: Material 3 renders it as a `Box` with no semantics modifier, so it
|
||||
* contributes no node to the semantics tree at all. What is asserted instead is everything the
|
||||
* divider precedes -- no detail row for any label the four kind branches can emit -- plus the
|
||||
* card's child count, which pins "these three texts and nothing else" without having to enumerate.
|
||||
*
|
||||
* The early exit itself is enforced by the compiler rather than by this file, which the PR body
|
||||
* records: deleting `return@Column` un-smart-casts `probe`, and the `probe.kind` below it stops
|
||||
* compiling. The mutation that reddens the test here is the compilable form of that regression --
|
||||
* defaulting the null away with `?: InputProbe()` and letting the kind rows render.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class FileCardTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
|
||||
@Test
|
||||
fun `a file no provider could measure says so in words rather than showing a zero`() {
|
||||
setFileCard(input(sizeBytes = null, probe = VIDEO_PROBE))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES)
|
||||
.assertTextEquals("Size unknown")
|
||||
}
|
||||
|
||||
/**
|
||||
* The same line, with no probe at all. Separate from the case above rather than folded into
|
||||
* it because `setContent` may only be called once per rule, and because two independent reds
|
||||
* are the evidence that the size line does not depend on the probe.
|
||||
*/
|
||||
@Test
|
||||
fun `the size line says the same thing while the probe is still running`() {
|
||||
setFileCard(input(sizeBytes = null, probe = null))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES)
|
||||
.assertTextEquals("Size unknown")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a size that was reported is formatted rather than replaced by the unknown line`() {
|
||||
setFileCard(input(sizeBytes = 12_345_678L, probe = VIDEO_PROBE))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME).assertTextEquals("clip.mkv")
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES).assertTextEquals("12.3 MB")
|
||||
}
|
||||
|
||||
/**
|
||||
* The note and the emptiness are one behaviour, so they are one test: a regression that keeps
|
||||
* the note but renders the rows anyway would leave a note-only test green.
|
||||
*/
|
||||
@Test
|
||||
fun `while the probe is still running the card shows the reading note and nothing else`() {
|
||||
setFileCard(input(probe = null))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NOTE)
|
||||
.assertTextEquals("Reading…")
|
||||
assertNoDetailRows()
|
||||
// Name, size, note. Catches a row whose label is not in EVERY_ROW_LABEL as well.
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD).onChildren().assertCountEquals(3)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a file nothing could read gets the explanatory line instead of unknown codecs`() {
|
||||
setFileCard(input(probe = InputProbe(kind = InputKind.UNPARSEABLE)))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NOTE)
|
||||
.assertTextEquals("Could not identify this file. It will be converted with FFmpeg.")
|
||||
assertNoDetailRows()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an image gets its type and its pixel dimensions`() {
|
||||
setFileCard(input(probe = InputProbe(kind = InputKind.IMAGE, width = 1920, height = 1080)))
|
||||
|
||||
assertRow("Type", "Image")
|
||||
assertRow("Size", "1920×1080")
|
||||
}
|
||||
|
||||
/** The `width > 0` guard, from the side that would print `0×0` if it were dropped. */
|
||||
@Test
|
||||
fun `an image whose dimensions nothing reported gets the type row alone`() {
|
||||
setFileCard(input(probe = InputProbe(kind = InputKind.IMAGE)))
|
||||
|
||||
assertRow("Type", "Image")
|
||||
assertNoRow("Size")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an audio-only file says it has no video track rather than leaving the row blank`() {
|
||||
setFileCard(
|
||||
input(
|
||||
probe = InputProbe(
|
||||
audioCodec = "aac",
|
||||
hasVideo = false,
|
||||
durationMs = 90_000,
|
||||
kind = InputKind.AUDIO_ONLY,
|
||||
container = Container.MP3,
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
assertRow("Container", Container.MP3.label)
|
||||
assertRow("Video", "No video track")
|
||||
assertRow("Audio", AudioCodec.AAC.label)
|
||||
assertRow("Length", "1:30")
|
||||
assertNoRow("Type")
|
||||
assertNoRow("Size")
|
||||
}
|
||||
|
||||
/**
|
||||
* Everything the audio branch can fail to know, at once: no container, no codec name, no
|
||||
* duration. Each degrades in its own words, and the length row disappears rather than
|
||||
* claiming `0:00`.
|
||||
*/
|
||||
@Test
|
||||
fun `an audio-only file nothing else could describe degrades one row at a time`() {
|
||||
setFileCard(input(probe = InputProbe(hasVideo = false, kind = InputKind.AUDIO_ONLY)))
|
||||
|
||||
assertRow("Container", "Unknown")
|
||||
assertRow("Video", "No video track")
|
||||
assertRow("Audio", "Unknown")
|
||||
assertNoRow("Length")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a video file composes its codec with its dimensions on one row`() {
|
||||
setFileCard(input(probe = VIDEO_PROBE))
|
||||
|
||||
assertRow("Container", Container.MP4.label)
|
||||
assertRow("Video", "${VideoCodec.H264.label} · 1920×1080")
|
||||
assertRow("Audio", AudioCodec.AAC.label)
|
||||
assertRow("Length", "1:30")
|
||||
}
|
||||
|
||||
/**
|
||||
* `"No audio track"` rather than `describeAudio(null)`'s `"Unknown"`. The video branch knows
|
||||
* the difference between a track it could not name and a track that is not there; the audio
|
||||
* branch above cannot, because a file with no audio is not audio-only.
|
||||
*/
|
||||
@Test
|
||||
fun `a video file with no audio track says so instead of naming an unknown codec`() {
|
||||
setFileCard(input(probe = VIDEO_PROBE.copy(audioCodec = null)))
|
||||
|
||||
assertRow("Audio", "No audio track")
|
||||
}
|
||||
|
||||
/** Both `> 0` guards on the video branch, plus the codec name nothing supplied. */
|
||||
@Test
|
||||
fun `a video file missing its codec, dimensions and duration omits them rather than faking them`() {
|
||||
setFileCard(
|
||||
input(
|
||||
probe = VIDEO_PROBE.copy(
|
||||
videoCodec = null,
|
||||
width = 0,
|
||||
height = 0,
|
||||
durationMs = 0,
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
assertRow("Video", "Unknown")
|
||||
assertNoRow("Length")
|
||||
}
|
||||
|
||||
/**
|
||||
* The row is one node, not a label node beside a value node. A test matching on `"Container"`
|
||||
* alone would pass against either shape.
|
||||
*/
|
||||
@Test
|
||||
fun `a detail row renders its label and its value as a single node`() {
|
||||
composeRule.setContent { DetailRow("Container", "Matroska") }
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.detailRow("Container"))
|
||||
.assertTextEquals("Container: Matroska")
|
||||
}
|
||||
|
||||
private fun setFileCard(input: InputFile) = composeRule.setContent { FileCard(input) }
|
||||
|
||||
private fun input(sizeBytes: Long? = 12_345_678L, probe: InputProbe? = VIDEO_PROBE) = InputFile(
|
||||
uri = Uri.parse("content://test/clip.mkv"),
|
||||
displayName = "clip.mkv",
|
||||
sizeBytes = sizeBytes,
|
||||
probe = probe,
|
||||
)
|
||||
|
||||
private fun assertRow(label: String, value: String) {
|
||||
composeRule.onNodeWithTag(TestTags.Converter.detailRow(label))
|
||||
.assertTextEquals("$label: $value")
|
||||
}
|
||||
|
||||
private fun assertNoRow(label: String) {
|
||||
composeRule.onNodeWithTag(TestTags.Converter.detailRow(label)).assertDoesNotExist()
|
||||
}
|
||||
|
||||
private fun assertNoDetailRows() = EVERY_ROW_LABEL.forEach(::assertNoRow)
|
||||
|
||||
private companion object {
|
||||
/** Every label the four kind branches can emit, so absence can be asserted exhaustively. */
|
||||
val EVERY_ROW_LABEL = listOf("Container", "Video", "Audio", "Length", "Type", "Size")
|
||||
|
||||
val VIDEO_PROBE = InputProbe(
|
||||
videoCodec = "h264",
|
||||
audioCodec = "aac",
|
||||
durationMs = 90_000,
|
||||
kind = InputKind.VIDEO,
|
||||
container = Container.MP4,
|
||||
width = 1920,
|
||||
height = 1080,
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,53 @@
|
||||
package org.libremediaconverter.join
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.compose.ui.test.assertCountEquals
|
||||
import androidx.compose.ui.test.onAllNodesWithTag
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.InputFile
|
||||
import org.libremediaconverter.createDrainedComposeRule
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* The join screen's one leaf renders, and tags itself with the file it is showing.
|
||||
*
|
||||
* `FileRow` is the only place on either screen where the same leaf is rendered more than once at a
|
||||
* time -- one row per picked input -- so it is the only tag that cannot be a constant. It is
|
||||
* derived from `displayName`, inside `FileRow` itself, and that is the part worth a test: a row
|
||||
* that took its tag from the call site would let R38.7 pass a tag in and assert nothing, which is
|
||||
* the vacuous shape `CLAUDE.md` records nine of in one review.
|
||||
*
|
||||
* Two rows are rendered here rather than one, because a tag derived from the wrong thing -- a
|
||||
* constant, an index the row does not have -- would still resolve to one node with a single input
|
||||
* on screen.
|
||||
*
|
||||
* Deliberately not the state matrix: which affordances each `JoinState` renders is R38.7.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class JoinLeafTagsTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
|
||||
private fun input(displayName: String) = InputFile(
|
||||
uri = Uri.parse("content://test/$displayName"),
|
||||
displayName = displayName,
|
||||
sizeBytes = 4_000_000L,
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `each file row is tagged with the name it displays`() {
|
||||
composeRule.setContent {
|
||||
FileRow(input("first.mp4"))
|
||||
FileRow(input("second.mp4"))
|
||||
}
|
||||
|
||||
composeRule.onAllNodesWithTag(TestTags.Join.fileRow("first.mp4")).assertCountEquals(1)
|
||||
composeRule.onAllNodesWithTag(TestTags.Join.fileRow("second.mp4")).assertCountEquals(1)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,40 @@
|
||||
package org.libremediaconverter.ui
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* No two entries of [TestTags] may share a value.
|
||||
*
|
||||
* A duplicated value is the one mistake this table invites -- the constants are added in blocks of
|
||||
* near-identical lines, and a copy-paste that keeps the old string still compiles, still reads
|
||||
* correctly at the call site, and still passes every test in the file that placed it. It surfaces
|
||||
* later, in someone else's PR, as an affordance that "resolves to exactly one node" finding two,
|
||||
* with nothing in that diff to explain it.
|
||||
*
|
||||
* Read by reflection rather than from a hand-written list, because a hand-written list would be a
|
||||
* second copy of the table with the same copy-paste failure in it.
|
||||
*/
|
||||
class TagTableUniquenessTest {
|
||||
|
||||
private fun tagsIn(vararg holders: Class<*>): List<String> = holders.flatMap { holder ->
|
||||
holder.declaredFields
|
||||
.filter { it.type == String::class.java }
|
||||
.map { it.get(null) as String }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `every tag constant has its own value`() {
|
||||
val tags = tagsIn(
|
||||
TestTags::class.java,
|
||||
TestTags.Converter::class.java,
|
||||
TestTags.Join::class.java,
|
||||
)
|
||||
|
||||
// Without this the check would pass on an empty list, which is what a reflection call
|
||||
// that stopped finding the constants would hand it.
|
||||
assertTrue("reflection found only ${tags.size} tag constants, so it is not reading the table", tags.size > 20)
|
||||
assertEquals(emptyList<String>(), tags.groupBy { it }.filterValues { it.size > 1 }.keys.toList())
|
||||
}
|
||||
}
|
||||
+600
-132
@@ -1,127 +1,536 @@
|
||||
# API 37 is not tested in CI: a crash in Google's `android-37.0` emulator image
|
||||
# API 37 on the emulator: a guest gralloc bug that only the host GL renderer triggers
|
||||
|
||||
**Status:** open upstream, worked around by removing API 37 from the E2E matrix.
|
||||
The app itself is verified good on real API 37 hardware — this is an emulator bug only.
|
||||
**Last verified:** 2026-08-21, against emulator `37.1.11.0` and system image revision 6
|
||||
**Status:** the bug is real and still open upstream, but the previous diagnosis in this file was
|
||||
wrong about its most important detail. **The renderer decides whether API 37 boots**, and once it
|
||||
boots, disabling SystemUI collapses the crash rate far enough to run a suite —
|
||||
`tools/local-emulator/run-e2e.sh 37` gets through the whole instrumented suite and comes back with
|
||||
**2 failures, 0 errors and the two by-design skips** (measured 49 / 2 / 0 / 2 at `22c7914`, where
|
||||
the suite was 49 tests — [Reading these totals](#reading-these-totals) before comparing any total
|
||||
with another). The crashes do not stop outright, and the two failures are real; both are quantified
|
||||
below. CI now takes API 37 as two jobs — a gating leg and an advisory one for those two
|
||||
failures — see [So should CI take API 37?](#so-should-ci-take-api-37).
|
||||
**Last verified:** 2026-08-22, emulator `37.1.11.0` (build 15917651), Fedora 44,
|
||||
against system images `android-37.0` rev 6 **and** `android-37.1` rev 8.
|
||||
|
||||
`minSdk` is 33 and `targetSdk` is 37, and the E2E matrix in
|
||||
[`status_check.yml`](../.github/workflows/status_check.yml) runs API 33 through 36.
|
||||
API 37 is deliberately absent. This is why.
|
||||
## The correction
|
||||
|
||||
## Summary
|
||||
This file previously said, under "What was ruled out":
|
||||
|
||||
The `android-37.0` emulator system image crashes `surfaceflinger` in a loop. The app
|
||||
under test never gets a working framework, so every instrumented test fails regardless
|
||||
of what the app does. The bug is in the emulator image, not in this project.
|
||||
> **GPU mode.** Both `swiftshader_indirect` and `host` crash, with the same assertion and
|
||||
> the same frames. The crash is in the gralloc mapper, below the renderer.
|
||||
|
||||
The crash is an assertion inside the emulator's own gralloc implementation:
|
||||
**That is wrong.** The mapper is below the renderer, but *whether the mapper's bad path is
|
||||
reached* is not. Re-measured on 2026-08-22, seven runs, one variable at a time:
|
||||
|
||||
| # | system image | `-gpu` | GLES the emulator chose | booted? | surfaceflinger aborts |
|
||||
|---|---|---|---|---|---|
|
||||
| r01 | `android-37.0` rev 6 | `host` | host (Mesa Iris Xe) | **no**, 422 s | 71, looping |
|
||||
| r02 | `android-37.1` rev 8 | `host` | host (Mesa Iris Xe) | **no**, 362 s | 65, looping |
|
||||
| r03 | `android-37.0` rev 6 | `swangle_indirect` | ANGLE | **yes, 85 s** | 1 |
|
||||
| r04 | `android-37.0` rev 6 | `host` + `-feature -GLDMA,-GLDMA2,-GLDirectMem` | host | **no**, 363 s | 57, looping |
|
||||
| r05 | `android-37.0` rev 6 | `angle_indirect` | ANGLE | **yes, 112 s** | 2 |
|
||||
| r06 | `android-37.1` rev 8 | `swangle_indirect` | ANGLE | **yes, 285 s** | 23 |
|
||||
| r07 | `android-37.0` rev 6 | `host` + `-feature -HostComposition` | host | **no**, wedged adb at 208 s | not readable |
|
||||
|
||||
The discriminator is exact across all seven: **a run boots if and only if the emulator log says
|
||||
something other than `gles_mode_selected:host`.**
|
||||
|
||||
One caveat about how independent those rows are, because the table flatters itself. `-gpu
|
||||
angle_indirect` (r05) and `-gpu swangle_indirect` (r03) both logged `gles_mode_selected:swangle`
|
||||
and both reported the same adapter, differing only in the Vulkan backend beneath
|
||||
(`vulkan_mode_selected:lavapipe` against `swiftshader`). So they are closer to one GLES path
|
||||
reached two ways than to two renderers agreeing — note that at API 33–36
|
||||
[`docs/local-emulator.md`](local-emulator.md) records `angle_indirect` resolving to ANGLE on
|
||||
*llvmpipe*, a genuinely different adapter, which it did not do here. What is 7-for-7 is the
|
||||
host-GLES-versus-not split, not "two independent renderers both work".
|
||||
|
||||
```
|
||||
# r01, r02, r04, r07 -- never boots
|
||||
INFO | emuglConfig_init: vulkan_mode_selected:host gles_mode_selected:host
|
||||
INFO | Graphics Adapter Android Emulator OpenGL ES Translator (Mesa Intel(R) Iris(R) Xe Graphics (TGL GT2))
|
||||
|
||||
# r03, r05, r06 -- boots
|
||||
INFO | emuglConfig_init: vulkan_mode_selected:swiftshader gles_mode_selected:swangle
|
||||
INFO | Graphics Adapter Android Emulator OpenGL ES Translator (ANGLE (Google, Vulkan 1.2.0
|
||||
| (SwiftShader Device (Subzero) (0x0000C0DE)), SwiftShader driver-5.0.0))
|
||||
```
|
||||
|
||||
### Why the wrong claim looked right
|
||||
|
||||
It rested on two samples of two different things, and neither of them was ANGLE.
|
||||
|
||||
- The **local** `swiftshader_indirect` sample was void. On this workstation *every*
|
||||
SwiftShader-GLES launch segfaults the host emulator before the guest matters at all —
|
||||
SELinux denies `execheap` to SwiftShader's Reactor JIT. That is
|
||||
[`docs/local-emulator.md`](local-emulator.md), and it was not yet understood when this file
|
||||
was written. So "`swiftshader_indirect` crashes" was true, for an entirely unrelated reason,
|
||||
and told you nothing about the gralloc assertion.
|
||||
- The **CI** sample was one `swiftshader_indirect` run on a GPU-less `ubuntu-latest`, and the
|
||||
**local** sample was one `-gpu host` run. Two renderers, one measurement each, and the pair
|
||||
written up as "both GPU modes".
|
||||
|
||||
`angle_indirect` and `swangle_indirect` — the two modes that work — had never been tried on
|
||||
API 37. Neither had a second system image.
|
||||
|
||||
The lesson is the same one `docs/local-emulator.md` ends on, which makes it worth repeating:
|
||||
"both backends fail" is a claim about a matrix, and a matrix needs cells, not inference. Two
|
||||
observations of two different configurations do not establish anything about a third.
|
||||
|
||||
## What the bug actually is
|
||||
|
||||
`surfaceflinger` aborts inside the emulator's own gralloc mapper:
|
||||
|
||||
```
|
||||
Executable: /system/bin/surfaceflinger
|
||||
signal 6 (SIGABRT), code -1 (SI_QUEUE), tid: RegionSampling
|
||||
Abort message: 'Assertion failed: !rcEnc->featureInfo()->hasReadColorBufferDma'
|
||||
|
||||
#03 mapper.ranchu.so GoldfishMapper::readFromHost(cb_handle_t const&) const
|
||||
#04 mapper.ranchu.so GoldfishMapper::GoldfishMapper()::'lambda'(...)::__invoke
|
||||
#05 libui.so android::Gralloc5Mapper::lock(...)
|
||||
#06 libui.so android::GraphicBufferMapper::lock(...)
|
||||
#07 libui.so android::GraphicBuffer::lockAsync(...)
|
||||
#08 libui.so android::GraphicBuffer::lock(...)
|
||||
#09 surfaceflinger android::RegionSamplingThread::threadMain()
|
||||
#03 /vendor/lib64/hw/mapper.ranchu.so GoldfishMapper::readFromHost(cb_handle_t const&) const+543
|
||||
#04 /vendor/lib64/hw/mapper.ranchu.so GoldfishMapper::GoldfishMapper()::'lambda'(...)::__invoke+704
|
||||
#05 /system/lib64/libui.so android::Gralloc5Mapper::lock(...)+63
|
||||
#06 /system/lib64/libui.so android::GraphicBufferMapper::lock(...)+198
|
||||
#07 /system/lib64/libui.so android::GraphicBuffer::lockAsync(...)+545
|
||||
#08 /system/lib64/libui.so android::GraphicBuffer::lock(...)+67
|
||||
#09 /system/bin/surfaceflinger android::RegionSamplingThread::threadMain()+2571
|
||||
```
|
||||
|
||||
`RegionSamplingThread` is SystemUI's navigation-bar luma sampling. It calls
|
||||
`GraphicBuffer::lock`, which routes into `GoldfishMapper::readFromHost`, which asserts
|
||||
that the host has *not* negotiated the `ReadColorBufferDma` capability. On this image
|
||||
the host has, so the assertion fails and `surfaceflinger` aborts. It restarts and
|
||||
aborts again.
|
||||
`RegionSamplingThread` is SystemUI's nav-bar luma sampling. It locks a `GraphicBuffer` for CPU
|
||||
read; that routes through the Gralloc5 mapper into `GoldfishMapper::readFromHost`, which is the
|
||||
*non-DMA* readback path and asserts that the host has not negotiated `ReadColorBufferDma`. The
|
||||
host always has, so the assert fires whenever that path is taken.
|
||||
|
||||
## Impact
|
||||
Two facts pin down what "always" means:
|
||||
|
||||
The failure surfaces in two different ways depending on how far the job gets, which is
|
||||
why it took several rounds to identify:
|
||||
- **The capability is negotiated regardless of renderer.** The evidence is the aborts
|
||||
themselves: the assertion that fires is `!hasReadColorBufferDma`, and it fires under ANGLE
|
||||
(r03/r05/r06) as well as under the host translator — just far less often. That is a direct
|
||||
observation of the guest having negotiated DMA readback under both, and it stands alone.
|
||||
(Supporting only, and weaker than it first looks: `ANDROID_EMU_read_color_buffer_dma` appears
|
||||
in exactly one file in the SDK, `emulator/lib64/libgfxstream_backend.so`, which every `-gpu`
|
||||
mode goes through. A string search establishes where the extension is implemented, not that
|
||||
it is negotiated on every path.)
|
||||
- **It is not gated by any feature flag the emulator exposes.** See the ruled-out list below.
|
||||
|
||||
| Guest RAM | Where it dies | What CI reports |
|
||||
|---|---|---|
|
||||
| 1536 MB | during APK install | `Unknown failure: cmd: Can't find service: package` |
|
||||
| 2560 MB | during the test run | `There were failing tests` — all of them |
|
||||
So the renderer does not decide whether the guest *believes* DMA readback exists. It decides how
|
||||
often `RegionSamplingThread` ends up in `readFromHost` — which under the host GL translator is
|
||||
constantly, and under ANGLE is occasionally.
|
||||
|
||||
At 2560 MB the install succeeds and the tests actually execute, then fail wholesale.
|
||||
The first failure in the report is misleading:
|
||||
### Why one abort takes down the whole device
|
||||
|
||||
`surfaceflinger` is a critical service. When it dies, `init` kills the framework with it:
|
||||
|
||||
```
|
||||
kotlin.UninitializedPropertyAccessException: lateinit property output has not
|
||||
been initialized
|
||||
at Media3EngineTest.tearDown(Media3EngineTest.kt:53)
|
||||
|
||||
java.lang.IllegalStateException: WorkManager is not initialized properly.
|
||||
You have explicitly disabled WorkManagerInitializer in your manifest, ...
|
||||
08-22 21:40:28.253 I/init: Sending SIGKILL to service 'zygote' (pid 470) process group...
|
||||
08-22 21:40:28.260 I/init: Service 'zygote' (pid 470) received SIGKILL
|
||||
```
|
||||
|
||||
Neither is a real defect in this project. `tearDown` throws because `setUp` never got
|
||||
far enough to assign `output`, and WorkManager's `InitializationProvider` never runs
|
||||
because content-provider installation fails on a framework whose `surfaceflinger` is
|
||||
crash-looping. The same tests pass at API 33, 34, 35, and 36 in the same CI run, and
|
||||
the first `surfaceflinger` abort is timestamped *before* the test results are reported.
|
||||
|
||||
This is not inference. The full suite was run against a physical API 37 device and
|
||||
passed — see [Verified on real API 37 hardware](#verified-on-real-api-37-hardware)
|
||||
below. `ConversionWorkerTest` and `ConcatWorkerTest`, which drive a real WorkManager
|
||||
round trip and are among the tests that failed this way in CI, both pass there.
|
||||
Everything above zygote goes with it, which is why the symptoms look nothing like a graphics
|
||||
bug. Under `-gpu host` the cycle repeats every five to seven seconds forever and
|
||||
`sys.boot_completed` is never set. Under ANGLE the aborts are sparse enough that the boot
|
||||
usually completes between them — but they do not stop, and each one is a framework restart.
|
||||
That is the difference between "boots" and "is usable", and it is the reason this is not simply
|
||||
fixed by changing the renderer. See [Can the suite run on it?](#can-the-suite-run-on-it) below.
|
||||
|
||||
## Environment
|
||||
|
||||
Reproduced identically in two unrelated environments, so it is not specific to a host
|
||||
GPU, driver, or CI runner.
|
||||
|
||||
| | GitHub Actions | Local workstation |
|
||||
|---|---|---|
|
||||
| Host | `ubuntu-latest`, no GPU | Fedora, Intel Iris Xe (TGL GT2) |
|
||||
| Host | `ubuntu-latest`, no GPU | Fedora 44, Intel Iris Xe (TGL GT2), kernel `7.1.8-200.fc44` |
|
||||
| Emulator | `37.1.11.0` (build 15917651) | `37.1.11.0` (build 15917651) |
|
||||
| GPU mode | `swiftshader_indirect` | `host` |
|
||||
| Result | boots, aborts during tests | aborts before boot completes |
|
||||
| GPU mode measured | `swiftshader_indirect` | `host`, `angle_indirect`, `swangle_indirect` |
|
||||
|
||||
System image: `system-images;android-37.0;google_apis;x86_64`, `Pkg.Revision=6`,
|
||||
`AndroidVersion.ApiLevel=37.0`, `AndroidVersion.ExtensionLevel=22`
|
||||
Images, both reproducing it:
|
||||
|
||||
```
|
||||
Build fingerprint: google/sdk_gphone64_x86_64/emu64xa:17/CE2A.260420.019/15611780:userdebug/dev-keys
|
||||
Kernel Release: 6.12.58-android16-6-gccafb60de224-ab14828483
|
||||
system-images;android-37.0;google_apis;x86_64 Pkg.Revision=6 ApiLevel=37.0 ExtensionLevel=22
|
||||
fingerprint google/sdk_gphone64_x86_64/emu64xa:17/CE2A.260420.019/15611780:userdebug/dev-keys
|
||||
system-images;android-37.1;google_apis_ps16k;x86_64 Pkg.Revision=8 ApiLevel=37.1 ExtensionLevel=23
|
||||
ro.build.version.codename=REL (a release image, not a preview)
|
||||
```
|
||||
|
||||
**Note the `ps16k` in the second one — it is not optional, and it is why the 37.1 result is
|
||||
interpretable.** From API 37.1 onward Google ships *only* 16 KB-page x86_64 images; there is no
|
||||
plain `google_apis` variant to pick. `sdkmanager --list` for 37.1 and 37.2-beta* offers nothing
|
||||
but `google_apis_ps16k` and `google_apis_playstore_ps16k`. That makes page-size alignment a
|
||||
prerequisite rather than a detail: a `.so` that is not 16 KB aligned will not load on such a
|
||||
guest, and the resulting failure looks like an app bug. Checked before the first `ps16k` boot,
|
||||
using the same test `build.yml` applies to release APKs — all 20 libraries in the committed
|
||||
`bin/ffmpeg-kit-next-8.1.1.aar`, both ABIs, report `0x4000`:
|
||||
|
||||
```
|
||||
$ for f in jni/*/*.so; do readelf -lW "$f" | awk '$1=="LOAD"{print $NF}' | sort -u; done
|
||||
0x4000 (x20: libavcodec, libavdevice, libavfilter, libavformat, libavutil,
|
||||
libc++_shared, libffmpegkit, libffmpegkit_abidetect, libswresample, libswscale
|
||||
-- arm64-v8a and x86_64)
|
||||
```
|
||||
|
||||
So when `android-37.1` reproduced the abort, that was the gralloc bug and not a page-size
|
||||
mismatch. `image_pkg_for_api` in `tools/local-emulator/run-e2e.sh` encodes the `ps16k` tag for
|
||||
37.1; if this ever fails after an FFmpeg rebuild, re-run the alignment check first.
|
||||
|
||||
## What was ruled out, and how
|
||||
|
||||
Each of these was tested rather than reasoned about, because the first three attempts
|
||||
at this bug were plausible fixes that turned out to address earlier, unrelated failures.
|
||||
**A newer system image.** This file's own revisit trigger was "a new `android-37.0` system image
|
||||
revision ships (this was revision 6)". That trigger was written too narrowly and would never have
|
||||
fired: `android-37.0` is *still* revision 6, but Google shipped a whole new minor level.
|
||||
`android-37.1` `google_apis_ps16k` revision 8 — a `REL` build, not a beta — was installed and
|
||||
tested (r02, r06) and **behaves identically**: same assertion, same frames, never boots under
|
||||
`-gpu host`, and *worse* under ANGLE (23 aborts to `37.0`'s 1). `android-37.2-beta3` exists too
|
||||
but was not needed; two independent images agreeing settles it, and a beta could not be used by
|
||||
CI anyway.
|
||||
|
||||
**Guest memory.** The emulator raises an undersized guest to a minimum on its own, but
|
||||
only for API levels it recognises, and it does not recognise `"37.0"`. API 33 bumps to
|
||||
2048 MB and 34/35/36 to 2560 MB, while API 37 logged no bump at all and ran at the
|
||||
`pixel_6` default of 1536 MB. Setting `ram-size: 2560M` explicitly fixed that asymmetry
|
||||
and did change the outcome — the job got past install and into the test run — but it is
|
||||
not the underlying bug. At the moment of failure the guest reported `MemTotal 2527392
|
||||
kB` with `MemAvailable 1507104 kB`: 1.5 GB free, and no OOM kills.
|
||||
**An ATD image.** Still does not exist for API 37. `sdkmanager --list` offers `aosp_atd` and
|
||||
`google_atd` for API 30 through 36 and nothing above:
|
||||
|
||||
**GPU mode.** Both `swiftshader_indirect` and `host` crash, with the same assertion and
|
||||
the same frames. The crash is in the gralloc mapper, below the renderer.
|
||||
```
|
||||
system-images;android-36;google_atd;x86_64 | 1 | Google APIs ATD Intel x86_64 Atom System Image
|
||||
(no android-37 ATD of any kind)
|
||||
```
|
||||
|
||||
**Disabling the DMA feature.** `GLDMA` is the host feature that most plausibly backs the
|
||||
guest's `hasReadColorBufferDma`. Launching with `-feature -GLDMA` was accepted by the
|
||||
emulator — the log confirms `Feature 'GLDMA' (51) is overridden to 'disabled'` — and
|
||||
`surfaceflinger` still aborted 13 times and the device never finished booting. Whatever
|
||||
sets that guest capability, it is not this flag.
|
||||
For API 37 the only x86_64 images are `google_apis`, `google_apis_playstore`, their `ps16k`
|
||||
16 KB-page variants, and Wear OS. Check again when revisiting.
|
||||
|
||||
**An ATD image.** `google_atd` / `aosp_atd` images are built for automated testing and
|
||||
ship without the SystemUI package set, which is what drives `RegionSamplingThread` in
|
||||
the first place. That would likely sidestep the bug class entirely, but **no ATD image
|
||||
exists for `android-37.0`** — only `google_apis`, `google_apis_playstore`, the `ps16k`
|
||||
16 KB-page variants, and Wear OS. Check again when revisiting; if an ATD image appears,
|
||||
try it before anything else here.
|
||||
**The DMA feature flags.** `GLDMA` alone was ruled out previously; `GLDMA2` and `GLDirectMem`
|
||||
were not, and the per-image `advancedFeatures.ini` turns all three on. Disabling all three
|
||||
together (r04) is accepted by the emulator and changes nothing:
|
||||
|
||||
```
|
||||
INFO | Feature 'GLDMA' (51) is overridden to 'disabled'
|
||||
INFO | Feature 'GLDMA2' (52) is overridden to 'disabled'
|
||||
INFO | Feature 'GLDirectMem' (53) is overridden to 'disabled'
|
||||
... 57 surfaceflinger aborts, device never boots
|
||||
```
|
||||
|
||||
**Host composition.** `-feature -HostComposition` (r07) was the best remaining guess at what
|
||||
forces the readback. It did not help; it made things worse, wedging adb entirely at 208 s so the
|
||||
crash buffer could not even be read. Recorded as inconclusive rather than ruled out, because no
|
||||
evidence came back from it.
|
||||
|
||||
**Guest feature negotiation differing from API 36.** It does not. The image-level
|
||||
`advancedFeatures.ini` for `android-37.0` is byte-identical to `android-36`'s except for one
|
||||
unrelated line:
|
||||
|
||||
```
|
||||
$ diff android-36/google_apis/x86_64/advancedFeatures.ini android-37.0/google_apis/x86_64/advancedFeatures.ini
|
||||
+QemuCameraSensorOrientation = on
|
||||
```
|
||||
|
||||
`GLDMA`, `GLDMA2`, `GLDirectMem`, `GrallocSync`, `HostComposition` and `YUVCache` are on in
|
||||
both. API 36 boots and passes. So nothing about the host/guest feature handshake changed — the
|
||||
regression is in the guest's Gralloc5 mapper or in what API 37's `RegionSamplingThread` asks of
|
||||
it, not in what the emulator advertises.
|
||||
|
||||
**Guest memory.** Ruled out previously and not revisited; every run above used
|
||||
`hw.ramSize=2560`, the same value the E2E matrix pins, and none of them OOMed.
|
||||
|
||||
**A host-side crash.** Not this bug, and worth stating because the other emulator failure on this
|
||||
workstation *is* host-side. Every run above left `coredumpctl` empty and produced zero
|
||||
`avc: denied` lines, and the qemu process was still alive at the end of the ones that never
|
||||
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.
|
||||
|
||||
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
|
||||
`22c7914`, and a newer checkout reports its own count; see
|
||||
[Reading these totals](#reading-these-totals). And **the Pixel has never run 49**: its green run
|
||||
was 40 / 0 / 0 / 2 at `edd6385`, the same suite nine tests earlier. What compares across the two
|
||||
is two failures against none, and the same two skips — not the totals.
|
||||
|
||||
The same numbers and the same two test names came back twice, which is real corroboration — but
|
||||
by two different routes, and only one of them is the harness. The first was driven by hand
|
||||
(`pm disable-user`, then several minutes of incidental framework restarts, then `e2e-run.sh`
|
||||
directly); the second went through `disable_region_sampling`'s `stop; start`. **The harness path
|
||||
itself has one green measurement.** What would make this routine is a second consecutive
|
||||
`run-e2e.sh 37` whose only failures are the same two.
|
||||
|
||||
### Booting is not the same as being usable
|
||||
|
||||
Changing the renderer gets the device to `sys.boot_completed=1`, and that is all it gets you. The
|
||||
aborts do not stop, and each one is a framework restart. A five-minute test run does not survive
|
||||
that. What it looks like from Gradle:
|
||||
|
||||
```
|
||||
Shell command failed (1): rm -rf "/sdcard/Android/media/org.libremediaconverter/..."
|
||||
rm: ...: Transport endpoint is not connected
|
||||
Starting 0 tests on lmc_e2e_api37(AVD) - 17
|
||||
Shell command failed (20): am get-current-user
|
||||
cmd: Can't find service: activity
|
||||
Device emulator-5572 failed to uninstall test APK org.libremediaconverter.
|
||||
[cmd: Can't find service: package]
|
||||
Test run failed to complete. No test results.
|
||||
onError: commandError=false message=INSTRUMENTATION_ABORTED: System has crashed.
|
||||
```
|
||||
|
||||
Measured idle rate on `android-37.0` under `swangle_indirect`: **10 aborts in 150 s, then 11 more
|
||||
in the next 150 s**. Steady, not a start-up transient.
|
||||
|
||||
### The fix is to remove the region-sampling listener, not to survive it
|
||||
|
||||
`RegionSamplingThread` exists only because SystemUI registers a nav-bar luma-sampling listener.
|
||||
Take SystemUI away and the thread is never started, so the mapper's bad path is never called:
|
||||
|
||||
```
|
||||
$ adb shell pm disable-user --user 0 com.android.systemui
|
||||
Package com.android.systemui new state: disabled-user
|
||||
|
||||
=== aborts at start of measurement: 36
|
||||
=== idle 180s with SystemUI disabled ===
|
||||
=== aborts after: 36 NEW IN WINDOW: 0
|
||||
--- services still up? ---
|
||||
activity Service activity: found
|
||||
package Service package: found
|
||||
window Service window: found
|
||||
```
|
||||
|
||||
**Zero in 180 s, against 10–11 per 150 s.** That is the strongest evidence that region sampling
|
||||
is the dominant trigger, and it is worth recording even by someone who never wants the workaround.
|
||||
It does not establish it as the *only* trigger: the paragraph below has an abort surviving the
|
||||
disable, and nothing measured here says whether that residue is a second caller of the readback
|
||||
path or a disable that did not fully take.
|
||||
|
||||
Do not read that as "the crashes stop", though, because the harness path does not reproduce a
|
||||
clean zero. Its own post-disable check on the run recorded below printed
|
||||
|
||||
```
|
||||
quiet check: 1 new surfaceflinger aborts in 45 s (want 0)
|
||||
surfaceflinger hasReadColorBufferDma aborts: 4 (whole run)
|
||||
```
|
||||
|
||||
So what is reliably achieved is a **rate collapse** — from roughly one abort every fourteen
|
||||
seconds to one every forty-five — which a 47-second Gradle run survives and a five-minute one
|
||||
might not. The 180-second zero above is one measurement on a device that had been up for twelve
|
||||
minutes and had already cycled its framework several times. The harness prints the quiet-check
|
||||
delta on every run precisely so this is visible rather than assumed.
|
||||
|
||||
One ordering detail cost a whole run and is now encoded in `disable_region_sampling`: by the time
|
||||
`sys.boot_completed` flips, SystemUI has **already registered**, and `pm disable-user` does not
|
||||
retract an existing registration — it only stops the package being started again. Disabling it
|
||||
and proceeding straight to the tests fails exactly as before. The harness therefore does
|
||||
`stop; start` afterwards, so the framework that comes back never starts SystemUI at all.
|
||||
|
||||
### The two deviations, stated plainly
|
||||
|
||||
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.**
|
||||
|
||||
### The two remaining failures are the same bug, one layer down
|
||||
|
||||
```
|
||||
org.libremediaconverter.convert.Media3EngineTest > runsFromAThreadWithNoLooper FAILED
|
||||
org.libremediaconverter.convert.Media3EngineTest > transcodesH264ToH265AndReportsProgress FAILED
|
||||
|
||||
androidx.media3.transformer.ExportException: Codec exception:
|
||||
CodecInfo{type=VideoDecoder, ..., mime=video/avc, name=c2.goldfish.h264.decoder}
|
||||
at androidx.media3.transformer.DefaultCodec.maybeDequeueOutputBuffer(DefaultCodec.java:398)
|
||||
Caused by: android.media.MediaCodec$CodecException:
|
||||
at android.media.MediaCodec.native_dequeueOutputBuffer(Native Method)
|
||||
```
|
||||
|
||||
Three measurements say this is the emulator image and not this app, and not the software
|
||||
renderer. A fourth bullet offers a mechanism, and is inference rather than measurement:
|
||||
|
||||
- **Control at API 35 under the identical renderer.** `GPU_MODE=swangle_indirect
|
||||
tools/local-emulator/run-e2e.sh 35` → **49 / 0 / 0 / 2** at `22c7914`, green.
|
||||
`c2.goldfish.h264.decoder` is perfectly happy under ANGLE one API level down, so the renderer is
|
||||
not what breaks it.
|
||||
- **Real API 37 hardware passes**, see below. There is no `c2.goldfish.*` codec on a Pixel.
|
||||
- **API 36 against API 37 on CI, back to back, everything else held.** Same two tests, same
|
||||
`-gpu swiftshader_indirect`, same SystemUI-disable path — `pm disable-user`, `stop`, wait for
|
||||
`system_server` to actually be gone, `start`, then verify against `pm list packages -d`. Both
|
||||
runs were narrowed to the two failing tests:
|
||||
|
||||
```
|
||||
-Pandroid.testInstrumentationRunnerArguments.class=\
|
||||
org.libremediaconverter.convert.Media3EngineTest#transcodesH264ToH265AndReportsProgress,\
|
||||
org.libremediaconverter.convert.Media3EngineTest#runsFromAThreadWithNoLooper
|
||||
```
|
||||
|
||||
and the filter is confirmed three independent ways: `tests="2"` in the XML, `Expected 2 tests`
|
||||
in the abort message, and `run started: 2 tests` in the guest logcat.
|
||||
|
||||
| run | api | result XML |
|
||||
|---|---|---|
|
||||
| [32660148155](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32660148155) | 37.0 | `tests="2" failures="2" errors="0" skipped="0"` |
|
||||
| [32660152961](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32660152961) | 36 | `tests="2" failures="0" errors="0" skipped="0" time="4.603"` |
|
||||
|
||||
API 37 fails with the signature above — `name=c2.goldfish.h264.decoder`,
|
||||
`MediaCodec$CodecException` at `dequeueOutputBuffer(MediaCodec.java:4274)`. API 36 passes both in
|
||||
4.603 s, and `c2.goldfish.h264.decoder` is in *its* logcat too (44 mentions), so the two runs are
|
||||
not being served by different decoder names. **What this falsifies is "the stripped
|
||||
configuration is what breaks these tests"** — a reading none of the other measurements
|
||||
addresses, because they all compare against a device that still had SystemUI. Here SystemUI is
|
||||
absent and the framework has been restarted on both sides, and the healthy image is green anyway.
|
||||
|
||||
Two things it does **not** control, which is why it narrows the claim rather than closing it:
|
||||
|
||||
- **The restarts were not performed under equal conditions.** API 36 did its `stop`/`start` with
|
||||
`dma_aborts=0`; API 37's did the same restart with two aborts already logged. "A framework
|
||||
restart performed while the abort loop is running" therefore remains uncontrolled.
|
||||
- **The images differ on the encoder side.** These tests transcode H.264 → H.265. The API 37
|
||||
logcat carries `c2.goldfish.hevc.decoder` (16 mentions in the control run) where API 36 carries
|
||||
`c2.android.hevc.encoder` (32). The pipeline is not identical end to end, which is a second
|
||||
reason "the image ships a broken h264 decoder" is the wrong *shape* of claim: what is measured
|
||||
is that these two tests fail on the API 37 image, pass at API 36 under the same renderer *and*
|
||||
the same disable path, and pass at 33–36 without needing that path at all — because nothing
|
||||
below 37 has the bug it works around.
|
||||
- The failing call is `dequeueOutputBuffer` on the *goldfish* decoder — the emulator's own codec,
|
||||
which like `RegionSamplingThread` gets its frames out of a host-side colour buffer. Same
|
||||
readback machinery, one layer down. This is inference rather than a measurement, and is flagged
|
||||
as such; what is measured is the first three bullets.
|
||||
|
||||
**Do not try `-feature -HardwareDecoder`.** It is the obvious next idea and it is much worse:
|
||||
forcing the guest onto software decoders took the run from 2 failures to **46**, across
|
||||
`RemuxTest`, `ForcedFailureTest`, `HardwareFallbackTest` and `UnopenableUriTest` as well. The
|
||||
suite depends on those decoders existing.
|
||||
|
||||
### The intact-SystemUI counterfactual cannot be measured on CI
|
||||
|
||||
The control the block above still lacks is the obvious one: run those same two tests at API 37
|
||||
with SystemUI **left running**. Passing would put the failure on the disable rather than on the
|
||||
image; failing on the decoder would make the decoder attribution direct instead of inferred.
|
||||
|
||||
**Seven dispatches of `api37-debug.yml`, zero verdicts.** Not bad luck — a mechanism, which is why
|
||||
this is written down rather than left as a gap for the next person to spend seven runs on:
|
||||
|
||||
| arm | run | result XML | what actually happened |
|
||||
|---|---|---|---|
|
||||
| E1 | [32660528355](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32660528355) | `tests="1" failures="1"`, `<failure>` body empty | `Expected 2 tests, received 0. INSTRUMENTATION_ABORTED: System has crashed.` |
|
||||
| E2 | [32660533845](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32660533845) | `tests="0"` | never installed: `Failed to commit install session ... Failure calling service package: Broken pipe (32)` |
|
||||
| E3 | [32660539259](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32660539259) | `tests="0"` | `Test run failed to complete. No test results.` |
|
||||
| E4 | [32661117237](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32661117237) | `tests="2" failures="2"` | both failed in `@Before`, never reached MediaCodec |
|
||||
| E5 | [32661121972](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32661121972) | `tests="2" failures="2"` | same |
|
||||
| S1 | [32661127224](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32661127224) | `tests="1" failures="1"` | same, single-test arm |
|
||||
| S2 | [32661132024](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32661132024) | `tests="1" failures="1"` | same |
|
||||
|
||||
While the framework is crash-looping, the guest cannot reliably create per-user private
|
||||
directories. An app installed during the loop has no cache directory — and `Media3EngineTest`
|
||||
copies its H.264 fixture into `context.cacheDir` in `@Before`, so it dies there, **before any
|
||||
MediaCodec exists**:
|
||||
|
||||
```
|
||||
W/ContextImpl( 8216): Failed to ensure /data/user/0/org.libremediaconverter/cache
|
||||
I/TestRunner( 8216): run started: 1 tests
|
||||
E/TestRunner( 8216): failed: transcodesH264ToH265AndReportsProgress(...)
|
||||
E/TestRunner( 8216): java.io.FileNotFoundException:
|
||||
/data/user/0/org.libremediaconverter/cache/sample_h264.mp4: open failed: ENOENT
|
||||
at org.libremediaconverter.convert.Media3EngineTest.setUp(Media3EngineTest.kt:47)
|
||||
```
|
||||
|
||||
Not app-specific: `com.google.android.googlesdksetup` and `com.google.android.apps.nexuslauncher`
|
||||
hit the same `Failed to ensure /data/user/0/<pkg>/cache` in the same logcats.
|
||||
|
||||
**The result XML masks this, and reading only the report gets you the wrong bug.** What E4, E5, S1
|
||||
and S2 report is
|
||||
|
||||
```
|
||||
<failure>kotlin.UninitializedPropertyAccessException: lateinit property output has not been initialized
|
||||
at org.libremediaconverter.convert.Media3EngineTest.tearDown(Media3EngineTest.kt:56)
|
||||
```
|
||||
|
||||
— `tearDown` failing because `setUp` threw before it assigned `output`. That looks like a
|
||||
teardown defect in this repository and is not one; the cause is only in the guest logcat.
|
||||
|
||||
So the obstacle is structural: install, data-directory creation and instrumentation start-up do
|
||||
not fit between framework kills, and four of the seven runs show the directory creation itself is
|
||||
broken during the loop. More dispatches of this shape would repeat these outcomes. The
|
||||
counterfactual is still open on the **Pixel 10 Pro XL**, the one API 37 device here that is not an
|
||||
emulator — but a Pixel has no `c2.goldfish.*` codec at all, so it answers "does the app work at
|
||||
API 37", not "is that codec broken".
|
||||
|
||||
#### Abort cadence, corrected
|
||||
|
||||
`.github/workflows/api37-debug.yml` carried "roughly every 20 s" for the kill cycle in its own
|
||||
comments. That number was the watchdog's **sampling** interval, not the cadence, and the two got
|
||||
conflated. Measured across the seven runs above, gaps between successive `hasReadColorBufferDma`
|
||||
aborts run **20 s to 90 s, median 60–70 s — three to five aborts in a four-minute window**.
|
||||
Slower than assumed, and still not slow enough: install, data-directory creation and
|
||||
instrumentation start-up do not fit inside one gap.
|
||||
|
||||
`sys.boot_completed` held at `1` throughout every one of those test windows. The device reports
|
||||
itself booted while zygote is being killed under it, which is why no boot-state check catches
|
||||
this and why `stop`/`start` waits must poll `pidof system_server` and `service check` instead
|
||||
(see `disable_region_sampling` in `tools/local-emulator/run-e2e.sh`).
|
||||
|
||||
### So should CI take API 37?
|
||||
|
||||
**Yes, as two jobs: a gating `E2E API 37` and an advisory leg carrying the two tests that do not
|
||||
pass.** That reverses the answer this section gave, and the reversal is measured rather than
|
||||
argued — two of its three reasons were claims *about CI*, and CI had never been measured. The
|
||||
instrument that measured it is [`.github/workflows/api37-debug.yml`](../.github/workflows/api37-debug.yml),
|
||||
dispatch-only, a copy of the E2E job with the matrix replaced by inputs.
|
||||
|
||||
Every run below is `ubuntu-latest`, KVM on, `pixel_6`, x86_64, disk 8G, RAM 2560M, emulator
|
||||
`37.1.11.0` build 15917651 — the same emulator build the local investigation used. Every **API
|
||||
37** row is `system-images;android-37.0;google_apis;x86_64`; c2 is the API 36 control and runs
|
||||
that level's own image, which is the whole point of it.
|
||||
|
||||
| # | run | api | `-gpu` | SystemUI | suite | verdict |
|
||||
|---|---|---|---|---|---|---|
|
||||
| c1 | [32644947334](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32644947334) | 37.0 | swiftshader_indirect | running | `Starting 0 tests` | FAIL |
|
||||
| c2 | [32644965828](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32644965828) | **36** | swiftshader_indirect | running | 57 tests, BUILD SUCCESSFUL | green control |
|
||||
| c3 | [32644970240](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32644970240) | 37.0 | swangle_indirect | running | `Starting 0 tests` | FAIL |
|
||||
| c5 | [32645543238](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32645543238) | 37.0 | swangle_indirect | disabled | 57 / 2 / 0 / 2 | suite ran |
|
||||
| c6 | [32646029143](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32646029143) | 37.0 | swiftshader_indirect | one-shot disable, **did not hold** | `Starting 0 tests` | FAIL |
|
||||
| c8 | [32646611485](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32646611485) | 37.0 | swiftshader_indirect | disabled, verified | 57 / 2 / 0 / 2 | suite ran |
|
||||
| c9 | [32646615706](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32646615706) | 37.0 | swiftshader_indirect | disabled, verified | 57 / 2 / 0 / 2 | suite ran |
|
||||
| c10 | [32646619472](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32646619472) | 37.0 | swiftshader_indirect | disabled, verified | 57 / 2 / 0 / 2 | suite ran |
|
||||
| c11 | [32647138060](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32647138060) | 37.0 | swiftshader_indirect | disabled, verified | 57 / 2 / 0 / 2 | suite ran |
|
||||
|
||||
57 is that checkout's own `@Test` count at `acc71bc`, so those are whole-suite runs and not
|
||||
truncated ones — see [Reading these totals](#reading-these-totals). Taking the three old reasons
|
||||
in turn:
|
||||
|
||||
1. **"Nothing says a runner would be stable" — measured, and it is.** `-gpu swiftshader_indirect`
|
||||
on a GPU-less runner resolves to `gles_mode_selected:swiftshader`, a third renderer that
|
||||
locally never survives to say anything (Fedora's SELinux denies `execheap` to SwiftShader's
|
||||
Reactor JIT — see [`local-emulator.md`](local-emulator.md)). It boots `android-37.0` in about
|
||||
60 s. The local discriminator — fatal iff `gles_mode_selected:host` — holds, and a runner with
|
||||
no GPU can never select host, so CI was never in the fatal class. Switching CI's `-gpu` changes
|
||||
nothing either way: c1 and c3 both fail with SystemUI up, under swiftshader and swangle
|
||||
respectively, and c5 and c8–c11 show the suite running under either once SystemUI is gone.
|
||||
2. **"Bespoke device surgery" — still true, and now a written caveat rather than a reason to skip
|
||||
the level.** It is one env flag, `E2E_DISABLE_SYSTEM_UI`, read by `.github/scripts/e2e-run.sh`
|
||||
and unset on every other leg. What it costs is stated where it can be read from the failing
|
||||
check: the API 37 row runs a device configuration no other leg and no Pixel run uses. What
|
||||
makes it dependable is verification, not repetition — c6 is the counter-case, a one-shot
|
||||
`pm disable-user` that reported `new state: disabled-user` and then started SystemUI eight more
|
||||
times. The verified form is 4/4; the unverified form was 3/4.
|
||||
3. **"Permanently red or permanently allow-listed" — this was the real objection, and it is the
|
||||
one the split answers.** The two failures are marked `@FailsOnEmulatorApi37` in
|
||||
`app/src/androidTest`. The gating job runs `notAnnotation` on that marker and must be green;
|
||||
the advisory job runs `annotation` on the *same* marker, reports, and never blocks. One marker
|
||||
rather than two lists, so a test cannot silently end up in neither job — which would read as
|
||||
green.
|
||||
|
||||
The cost is about three minutes on the API 37 leg — the `stop`/`start` plus a 45 s quiet window,
|
||||
and another round when the first does not verify. Measured wall clock for the whole job, boot
|
||||
included: 6–7 minutes at API 37 against ~6 at API 36.
|
||||
|
||||
Two things this does **not** buy. The advisory job is expected red, so a *third* failure there is
|
||||
the signal and the run's logcat is the only thing that distinguishes it — which is why that job
|
||||
uploads diagnostics unconditionally. And a green `E2E API 37` still does not replace the release
|
||||
check on the Pixel: the emulator leg runs without SystemUI, and the Pixel does not.
|
||||
|
||||
The *local* story changed at the same time and independently: API 37 is no longer a level nobody
|
||||
can look at. A regression that shows up at 37 and not at 36 can be reproduced on this workstation
|
||||
in about four minutes.
|
||||
|
||||
## Verified on real API 37 hardware
|
||||
|
||||
The bug is confined to the emulator image. On 2026-08-21 the whole instrumented suite
|
||||
was run against a physical device and passed:
|
||||
Unchanged and still true. On 2026-08-21 the whole instrumented suite ran green on a physical
|
||||
device:
|
||||
|
||||
```
|
||||
Device: Pixel 10 Pro XL (mustang), arm64-v8a
|
||||
@@ -132,65 +541,94 @@ API: 37 (Android 17, codename REL -- a release build, not a preview)
|
||||
40 tests, 0 failures, 0 errors, 2 skipped BUILD SUCCESSFUL
|
||||
```
|
||||
|
||||
**That "40" is a measurement of the tree it ran on, not a baseline for today**, and it is not a
|
||||
contradiction of the totals in [`docs/local-emulator.md`](local-emulator.md) either.
|
||||
|
||||
### Reading these totals
|
||||
|
||||
Every total in this file and in [`docs/local-emulator.md`](local-emulator.md) is the size of
|
||||
`app/src/androidTest` on the checkout that produced it, and nothing else. The reported total has
|
||||
equalled that checkout's `@Test` count everywhere it has been checked:
|
||||
|
||||
| checkout | `@Test` methods | total the run reported |
|
||||
|---|---|---|
|
||||
| `edd6385` | 40 | 40 — the Pixel run above |
|
||||
| `22c7914` | 49 | 49 — the four local levels, and API 37 |
|
||||
| `18c53a3` | 57 | not run |
|
||||
|
||||
So the number to expect is not written down here. It is derived from the checkout in front of
|
||||
you, which is the only thing that cannot go stale:
|
||||
|
||||
```bash
|
||||
grep -rho '@Test' app/src/androidTest | wc -l
|
||||
```
|
||||
|
||||
**Before each release, run the suite on the Pixel 10 Pro XL and expect that many tests, 0
|
||||
failures, 0 errors, 2 skipped.** The failure, error and skip counts are the invariant; the total
|
||||
is not. A total that disagrees with your own checkout's count is the signal — an old checkout, a
|
||||
stale build, or tests that never ran — and it is worth stopping on either way.
|
||||
|
||||
The two skips are `RealMediaBenchmark.hardwareVersusSoftwareOnRealVideo` and
|
||||
`av1InputRoutesAccordingToDeviceDecodeSupport`, which `assumeTrue` their sample files
|
||||
are present and skip when they are not. That is by design and unrelated to API level.
|
||||
`av1InputRoutesAccordingToDeviceDecodeSupport`, which `assumeTrue` their sample files are present
|
||||
and skip when they are not. That is by design and unrelated to API level.
|
||||
|
||||
One harmless warning appears during the run and can be ignored:
|
||||
`No UID for androidx.test.services in user 0`, from an `appops` call the test services
|
||||
package makes before it is fully registered.
|
||||
|
||||
So the app is correct on Android 17. What is missing is only *automated* coverage in
|
||||
CI. Until the image is fixed, run the suite on a physical API 37 device before release;
|
||||
that is the substitute for the missing matrix row.
|
||||
`No UID for androidx.test.services in user 0`, from an `appops` call the test services package
|
||||
makes before it is fully registered.
|
||||
|
||||
## Reproducing it
|
||||
|
||||
Locally, with `-gpu host` so the emulator itself does not segfault on Intel graphics:
|
||||
Both halves, so the renderer claim can be checked rather than taken on trust:
|
||||
|
||||
```bash
|
||||
export ANDROID_HOME="$HOME/Android/Sdk"
|
||||
export PATH="$ANDROID_HOME/platform-tools:$ANDROID_HOME/emulator:$ANDROID_HOME/cmdline-tools/latest/bin:$PATH"
|
||||
|
||||
sdkmanager --install "system-images;android-37.0;google_apis;x86_64"
|
||||
echo no | avdmanager create avd -n api37_repro \
|
||||
-k "system-images;android-37.0;google_apis;x86_64" -d pixel_6 --force
|
||||
|
||||
$ANDROID_HOME/emulator/emulator -avd api37_repro \
|
||||
-no-window -gpu host -noaudio -no-boot-anim -camera-back none -no-snapshot &
|
||||
# never boots -- surfaceflinger aborts every ~6 s, forever
|
||||
emulator -avd api37_repro -no-window -gpu host \
|
||||
-noaudio -no-boot-anim -camera-back none -no-snapshot &
|
||||
|
||||
# Boot never completes. Count the aborts:
|
||||
adb logcat -d -b crash | grep -c hasReadColorBufferDma
|
||||
# boots in ~85 s, having aborted once or twice on the way
|
||||
emulator -avd api37_repro -no-window -gpu swangle_indirect \
|
||||
-noaudio -no-boot-anim -camera-back none -no-snapshot &
|
||||
```
|
||||
|
||||
`sys.boot_completed` never reaches `1`, `pgrep -f system_server` stays empty, and
|
||||
`keystore2`'s watchdog logs `await_boot_completed ... Overdue` indefinitely.
|
||||
Count the aborts either way:
|
||||
|
||||
To see the CI-side form instead, restore the API 37 row in the E2E matrix of
|
||||
`status_check.yml` (`api-level: "37.0"` — a bare `37` fails earlier still, during SDK
|
||||
setup, because there is no `platforms;android-37`).
|
||||
```bash
|
||||
adb -s emulator-5554 logcat -d -b crash | grep -c hasReadColorBufferDma
|
||||
```
|
||||
|
||||
Under `-gpu host`, `sys.boot_completed` never reaches `1`, `pgrep -f system_server` stays empty,
|
||||
and `keystore2`'s watchdog logs `await_boot_completed ... Overdue` indefinitely.
|
||||
|
||||
`tools/local-emulator/run-e2e.sh 37` does all of this, with the working renderer picked
|
||||
automatically — see `gpu_for_api` in that file.
|
||||
|
||||
## Filing this upstream
|
||||
|
||||
Not yet filed. To file it:
|
||||
Not yet filed. The report is stronger than it was, because the renderer dependency narrows it:
|
||||
|
||||
1. Go to <https://issuetracker.google.com/> and sign in with a Google account.
|
||||
2. Choose **Report an issue**, then pick the component for the Android emulator — search
|
||||
the component picker for "Emulator"; it sits under the Android Studio component tree.
|
||||
If the picker is unclear, Android Studio's **Help → Submit Feedback** opens the same
|
||||
tracker with the component preselected, and the emulator's own **Extended controls →
|
||||
Help → File a bug** does likewise.
|
||||
3. Title it for the mechanism, not the symptom, so it is searchable — for example:
|
||||
`surfaceflinger aborts in GoldfishMapper::readFromHost (hasReadColorBufferDma) on
|
||||
android-37.0 google_apis x86_64`.
|
||||
4. Paste the assertion and backtrace from the top of this document, the environment
|
||||
table, and the reproduction steps above. State explicitly that it reproduces on two
|
||||
unrelated hosts under both GPU modes — that is the detail that stops it being closed
|
||||
as a local graphics problem.
|
||||
5. List what was ruled out. Bugs that arrive with `-feature -GLDMA` already eliminated
|
||||
tend not to bounce back asking for it.
|
||||
6. Attach:
|
||||
- the guest tombstone, via `adb pull /data/tombstones` (or the `pbtombstone` output
|
||||
the crash log names)
|
||||
1. Go to <https://issuetracker.google.com/>, **Report an issue**, and pick the Android emulator
|
||||
component (search the component picker for "Emulator"; Android Studio's **Help → Submit
|
||||
Feedback** opens the same tracker with it preselected).
|
||||
2. Title it for the mechanism: `surfaceflinger aborts in GoldfishMapper::readFromHost
|
||||
(hasReadColorBufferDma) on android-37.0 and android-37.1 x86_64 -- fatal under -gpu host,
|
||||
intermittent under ANGLE`.
|
||||
3. Paste the assertion and backtrace, the environment block, and the seven-row matrix. The
|
||||
matrix is the valuable part: it shows the abort is not renderer-specific but its *frequency*
|
||||
is, which points at the readback path rather than at any one GL implementation.
|
||||
4. State that it reproduces on two independent system images (`37.0` rev 6 and `37.1` rev 8) and
|
||||
on two unrelated hosts, and that `-feature -GLDMA,-GLDMA2,-GLDirectMem` does not suppress it.
|
||||
5. Attach:
|
||||
- the guest tombstone, via `adb pull /data/tombstones` (or the `pbtombstone` output the crash
|
||||
log names)
|
||||
- `adb logcat -d -b crash > crash.txt`
|
||||
- the emulator's own stdout log, captured by redirecting the launch command
|
||||
- the emulator's own stdout log (`-verbose -debug all`, redirected)
|
||||
- the AVD's `config.ini`
|
||||
- a link to a failing CI job, which shows it on hardware you do not control:
|
||||
<https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32545625459/job/96963461184>
|
||||
@@ -199,16 +637,46 @@ Record the issue number here once filed.
|
||||
|
||||
## When to revisit
|
||||
|
||||
Re-add the API 37 row when any of these happens:
|
||||
The old trigger list named "a new `android-37.0` revision", which is why nothing ever fired even
|
||||
though a new API level shipped. Watch for these instead:
|
||||
|
||||
- a new `android-37.0` system image revision ships (this was revision 6)
|
||||
- an ATD image appears for API 37
|
||||
- the upstream issue is marked fixed
|
||||
- **any new API 37.x system image**, not just a new revision of `37.0` — `37.1` rev 8 and
|
||||
`37.2-beta*` already exist, and more will. Test with `-gpu host`: if it boots, the guest mapper
|
||||
is fixed.
|
||||
- **an ATD image for API 37.** Still none as of 2026-08-22. ATD images ship without SystemUI,
|
||||
which is what drives `RegionSamplingThread`, so one would very likely sidestep the bug
|
||||
entirely. Try it before anything else here.
|
||||
- **the upstream issue being marked fixed.**
|
||||
- **`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.
|
||||
|
||||
Until then the gap is narrower than the missing row suggests. `targetSdk` is 37, so the
|
||||
app is compiled and unit-tested against it; the API-dependent behaviour this matrix
|
||||
exists to exercise — the foreground service type, absent below 34, `dataSync` at 34,
|
||||
`mediaProcessing` from 35 — is covered at 35 and 36; and the full instrumented suite has
|
||||
been run green on real API 37 hardware. What is missing is *automated* API 37 coverage,
|
||||
so a regression there would not be caught by a pull request. Run the suite on a physical
|
||||
API 37 device before each release for as long as this row is absent.
|
||||
## Correction owed to `CLAUDE.md`
|
||||
|
||||
`CLAUDE.md` currently says:
|
||||
|
||||
> - **The API 37 image is broken.** `android-37.0` crash-loops surfaceflinger inside its own
|
||||
> gralloc mapper, so every test fails there regardless of this app.
|
||||
> `docs/api-37-emulator-crash.md` records the evidence and the ruled-out fixes; CI's matrix
|
||||
> therefore stops at API 36 even though targetSdk is 37.
|
||||
|
||||
The first sentence is right, and now under-specified in one direction and over-specified in the
|
||||
other: it is not only `android-37.0` (it is `37.1` too), and it does not crash-loop under every
|
||||
renderer. **The last clause is now simply false: CI's matrix does not stop at API 36 any more.**
|
||||
Proposed replacement, offered for review rather than applied here — `CLAUDE.md` is left alone
|
||||
deliberately, because several branches touch it:
|
||||
|
||||
> - **The API 37 images crash-loop surfaceflinger under the host GL renderer.** Both
|
||||
> `android-37.0` and `android-37.1` abort inside their own gralloc mapper
|
||||
> (`RegionSamplingThread` → `GoldfishMapper::readFromHost`), and when surfaceflinger dies init
|
||||
> SIGKILLs zygote, so the framework restarts under the test run. Under `-gpu host` it never
|
||||
> boots at all; under `-gpu swangle_indirect` it boots and the aborts merely become
|
||||
> intermittent. `docs/api-37-emulator-crash.md` has the seven-run matrix and the ruled-out
|
||||
> list, and `tools/local-emulator/run-e2e.sh` picks the working renderer per API level.
|
||||
> CI takes API 37 as two jobs: a gating `E2E API 37` that disables SystemUI first, and an
|
||||
> advisory leg carrying the two `@FailsOnEmulatorApi37` tests. The gating leg therefore runs a
|
||||
> device configuration nothing else does. **API 37 still needs a manual check on the Pixel 10
|
||||
> Pro XL before each release** — it is the only API 37 run with SystemUI intact.
|
||||
|
||||
+52
-17
@@ -1,8 +1,10 @@
|
||||
# Emulators do run on this host: the segfault is SwiftShader's JIT against SELinux
|
||||
|
||||
**Status:** solved. Local instrumented runs work with `-gpu host`, and the suite is green
|
||||
on API 33–36 — 49 tests, 0 failures, 0 errors, 2 skipped on every level. See
|
||||
[The sweep, run](#the-sweep-run).
|
||||
**Status:** solved. Local instrumented runs work with `-gpu host`, and the whole suite is green
|
||||
on API 33–36 — 0 failures, 0 errors and the two by-design skips on every level, measured as
|
||||
49 / 0 / 0 / 2 at `22c7914`, where the suite was 49 tests. See [The sweep, run](#the-sweep-run),
|
||||
and [Reading these totals](api-37-emulator-crash.md#reading-these-totals) before comparing any
|
||||
total with another checkout's.
|
||||
**Last verified:** 2026-08-22, emulator `37.1.11.0` (build 15917651), Fedora 44,
|
||||
kernel `7.1.8-200.fc44`, `selinux-policy-44.6-1.fc44`
|
||||
|
||||
@@ -190,11 +192,28 @@ emulator -avd <name> -no-window -gpu host \
|
||||
`.github/scripts/e2e-run.sh` for the run itself. Use it rather than the raw command:
|
||||
|
||||
```bash
|
||||
tools/local-emulator/run-e2e.sh # API 33 34 35 36
|
||||
tools/local-emulator/run-e2e.sh # API 33 34 35 36 37
|
||||
tools/local-emulator/run-e2e.sh 35 # one level
|
||||
tools/local-emulator/run-e2e.sh 37 37.1 # both API 37 images
|
||||
GPU_MODE=swangle_indirect tools/local-emulator/run-e2e.sh 35
|
||||
```
|
||||
|
||||
Levels are the labels above, not SDK ints: API 37's SDK directories are dotted
|
||||
(`android-37.0`, `android-37.1`) and there is no `android-37`, so `37` is accepted as a
|
||||
spelling of `37.0`. Setting `GPU_MODE` forces one renderer on every level, which is what
|
||||
you want when measuring a mode; leaving it unset lets `gpu_for_api` pick, which is what
|
||||
you want when running the suite — 33–36 need `host` and 37 must not have it.
|
||||
|
||||
**A bare `run-e2e.sh` exits 1, and that is the design.** API 37 is in the default list
|
||||
deliberately — 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: `Media3EngineTest` cannot drive the emulator's
|
||||
`c2.goldfish.h264.decoder` on those images, which
|
||||
[`api-37-emulator-crash.md`](api-37-emulator-crash.md) pins on the image and not on this app
|
||||
(API 35 under the same renderer is green). The summary row names the two expected failures so
|
||||
that a third is visibly new, and the script repeats the point on the way out. Anything that
|
||||
treats a non-zero exit as breakage — a wrapper, a hook, a habit — should name the levels it
|
||||
wants: `run-e2e.sh 33 34 35 36` is the sweep that can be green.
|
||||
|
||||
`swangle_indirect` is the fallback worth knowing about. It is entirely software, so it
|
||||
does not depend on reaching the session's GPU — useful over plain SSH, where `-gpu host`
|
||||
has not been tested and may not find a device. It is also the closest local analogue to
|
||||
@@ -236,8 +255,9 @@ after an AGP upgrade.
|
||||
## The sweep, run
|
||||
|
||||
`tools/local-emulator/run-e2e.sh`, one invocation per level so each got a freshly created
|
||||
AVD, `-gpu host` throughout, 2026-08-22 19:42–19:56. Every level matches the physical
|
||||
Pixel 10 Pro XL (API 37) baseline of 49 / 0 / 0 / 2 exactly:
|
||||
AVD, `-gpu host` throughout, 2026-08-22 19:42–19:56, on `22c7914`. All four levels agree exactly,
|
||||
and 49 is that checkout's whole suite — every `@Test` in `app/src/androidTest`, two of which skip
|
||||
by design everywhere:
|
||||
|
||||
| API | Android | AVD | Boot | `connectedDebugAndroidTest` | Tests | Failures | Errors | Skipped |
|
||||
|---|---|---|---|---|---|---|---|---|
|
||||
@@ -246,6 +266,12 @@ Pixel 10 Pro XL (API 37) baseline of 49 / 0 / 0 / 2 exactly:
|
||||
| 35 | 15 | `lmc_e2e_api35` | 40 s | 3 m 46 s | 49 | 0 | 0 | 2 |
|
||||
| 36 | 16 | `lmc_e2e_api36` | 90 s | 2 m 18 s | 49 | 0 | 0 | 2 |
|
||||
|
||||
The physical Pixel has never reported 49, and an earlier version of this paragraph said the
|
||||
sweep matched it exactly. Its green API 37 run was 40 / 0 / 0 / 2, at `edd6385` — the same suite
|
||||
nine tests earlier. What matches is 0 failures, 0 errors and the same two skips; totals only ever
|
||||
match between runs of one checkout, which
|
||||
[`api-37-emulator-crash.md`](api-37-emulator-crash.md#reading-these-totals) sets out.
|
||||
|
||||
Thirteen and a half minutes for the four levels, AVD creation and cold boots included;
|
||||
fifteen with the pre-warm build in front of them. Nothing needed a retry, and no level
|
||||
produced a `diagnostics-api*.txt` — `e2e-run.sh` writes that only on the failure path, so
|
||||
@@ -352,9 +378,11 @@ and the same binaries against the same kernel boot fine under `-gpu host`.
|
||||
before `sys.boot_completed` is ever set. Nothing in the guest — system image variant,
|
||||
RAM, disk size, ATD versus `google_apis` — can influence a host-side `mprotect` denial,
|
||||
so none of those axes was varied. (The API 37 failure in
|
||||
[`api-37-emulator-crash.md`](api-37-emulator-crash.md) is genuinely guest-side and
|
||||
genuinely unrelated: there the host emulator survives and the guest's `surfaceflinger`
|
||||
aborts.)
|
||||
[`api-37-emulator-crash.md`](api-37-emulator-crash.md) is genuinely guest-side — there the host
|
||||
emulator survives and the guest's `surfaceflinger` aborts — but it is **not** unrelated, as this
|
||||
paragraph originally claimed. Both are decided by the renderer, in opposite directions: below 37
|
||||
you must avoid SwiftShader GLES and `-gpu host` is the answer; at 37 you must avoid the *host* GL
|
||||
translator and `-gpu host` is the thing that never boots.)
|
||||
|
||||
**Turning the SELinux boolean on** — deliberately *not* done, though it would almost
|
||||
certainly work:
|
||||
@@ -401,8 +429,13 @@ are easy to forget to look at.
|
||||
"SwiftShader 4.0.0.1" as reported by the GLES translator.
|
||||
- **If `-gpu host` regresses** after a Mesa or kernel update, fall back to
|
||||
`GPU_MODE=swangle_indirect`, which needs no GPU at all.
|
||||
- **This changes nothing about API 37.** That image is broken for a different reason and
|
||||
still must be checked on the physical Pixel 10 Pro XL before each release.
|
||||
- **API 37 needs the opposite renderer, and this file used to say it needed nothing.** The
|
||||
original bullet here read "This changes nothing about API 37"; that turned out to be wrong.
|
||||
The API 37 images abort `surfaceflinger` under the *host* GL translator and boot under ANGLE —
|
||||
the exact mirror of the rule above — and `run-e2e.sh` therefore picks the renderer per API
|
||||
level. See [`api-37-emulator-crash.md`](api-37-emulator-crash.md), which was rewritten on
|
||||
2026-08-22 with the seven-run matrix. API 37 still must be checked on the physical Pixel 10 Pro
|
||||
XL before each release.
|
||||
|
||||
## Correction owed to `CLAUDE.md`
|
||||
|
||||
@@ -432,12 +465,14 @@ Proposed replacement for the section, offered for review rather than applied her
|
||||
> `auto` (the default), `off` and `guest` do when headless. `-gpu host` works, and the
|
||||
> harness both picks it and refuses the others. `docs/local-emulator.md` has the
|
||||
> backtrace and the mode matrix.
|
||||
> - **The API 37 image is broken.** `android-37.0` crash-loops surfaceflinger inside its
|
||||
> own gralloc mapper, so every test fails there regardless of this app —
|
||||
> `docs/api-37-emulator-crash.md` records the evidence and the ruled-out fixes. This is
|
||||
> unrelated to the renderer above: it is a guest-side bug that CI hits too, which is why
|
||||
> the matrix stops at API 36 even though targetSdk is 37. **API 37 needs a manual check
|
||||
> on the Pixel 10 Pro XL before each release.**
|
||||
> - **API 37 needs the opposite renderer, and SystemUI turned off.** Both `android-37.0` and
|
||||
> `android-37.1` abort surfaceflinger inside their own gralloc mapper, and init SIGKILLs
|
||||
> zygote each time. Under `-gpu host` they never boot; under `-gpu swangle_indirect` they
|
||||
> boot, and disabling SystemUI removes the trigger. `run-e2e.sh` does all of that per level,
|
||||
> and the local API 37 result is two failures and the two usual skips, not a clean run. CI
|
||||
> takes API 37 as a gating leg plus an advisory one carrying those two tests.
|
||||
> `docs/api-37-emulator-crash.md` has the matrix and the reasoning. **API 37 needs a manual
|
||||
> check on the Pixel 10 Pro XL before each release.**
|
||||
|
||||
The wording is worth getting right rather than merely correcting, because the original was
|
||||
not a careless sentence — it was a reasonable inference from three crashes, written down
|
||||
|
||||
@@ -80,6 +80,17 @@ jacoco = "0.8.15"
|
||||
# 4.16.1 is the newest RELEASED version; the 4.17 line is beta-only at the time of writing.
|
||||
robolectric = "4.16.1"
|
||||
|
||||
# kotlinx-coroutines-test. Already on the unit-test classpath transitively, through
|
||||
# compose-ui-test-junit4 -- declared here because a source file now imports it, and a direct
|
||||
# import of a transitive is a dependency nobody chose.
|
||||
#
|
||||
# PINNED, for the same reason as robolectric above: org.jetbrains.kotlinx is not one of the
|
||||
# groups in the prerelease guard's `floatedGroupPrefixes`, so a "1.+" here would be free to
|
||||
# resolve to a milestone build. This value is what the Compose BOM already resolves it to, so
|
||||
# stating it changes nothing in the graph today; if the BOM moves ahead, Gradle takes the
|
||||
# higher version and this stays a floor rather than a conflict.
|
||||
coroutinesTest = "1.9.0"
|
||||
|
||||
[libraries]
|
||||
androidx-core-ktx = { group = "androidx.core", name = "core-ktx", version.ref = "coreKtx" }
|
||||
androidx-activity-compose = { group = "androidx.activity", name = "activity-compose", version.ref = "activityCompose" }
|
||||
@@ -134,6 +145,10 @@ androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-co
|
||||
# a TDD loop anyone here can execute.
|
||||
robolectric = { group = "org.robolectric", name = "robolectric", version.ref = "robolectric" }
|
||||
|
||||
# Only for its `runTest`, and only to drain the collector kotlinx-coroutines-test installs
|
||||
# process-wide. See EscapedCoroutineErrors.kt in the JVM test source set.
|
||||
kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "coroutinesTest" }
|
||||
|
||||
[plugins]
|
||||
# com.android.application and org.jetbrains.kotlin.plugin.compose are deliberately absent.
|
||||
# They come from the root buildscript classpath (see build.gradle.kts) so that a newer KGP
|
||||
|
||||
Executable
+253
@@ -0,0 +1,253 @@
|
||||
#!/usr/bin/env bash
|
||||
#
|
||||
# Files a GitHub issue AND puts it on the project board, as one operation.
|
||||
#
|
||||
# Usage: tools/github/file-issue.sh --title TITLE (--body TEXT | --body-file PATH) [options]
|
||||
#
|
||||
# --status NAME board column, matched case-insensitively against the board's own
|
||||
# options; a miss lists what is available. Default: Backlog
|
||||
# --label NAME repeatable. Passed through to `gh issue create` unchanged.
|
||||
# --project N project number. Default: $ISSUE_PROJECT_NUMBER, else 6
|
||||
# --repo OWNER/NAME default: whatever `gh repo view` resolves in the working directory
|
||||
# --dry-run resolve and validate everything, create nothing
|
||||
#
|
||||
# EXIT CODE: 0 only when the issue exists, is on the board, AND reads back carrying the
|
||||
# Status that was asked for. 2 for a usage or validation error, before anything is created.
|
||||
# **3 means the issue was created but did not reach the board** -- the number is printed on
|
||||
# a line of its own, because that combination is the entire failure this script exists to
|
||||
# prevent and it must never be quiet.
|
||||
#
|
||||
# WHY THIS EXISTS
|
||||
#
|
||||
# `gh issue create` does not touch the project board. The issue is created, carries its
|
||||
# labels, and is invisible in the Kanban -- which looks exactly like a ticket nobody filed.
|
||||
# Measured 2026-08-24: eight issues filed as a scripted batch all reached the board; one
|
||||
# filed as a one-off a few minutes later did not, and was caught only because someone went
|
||||
# looking. A batch carries the board step inside its loop. One-offs are where it slips, so
|
||||
# one-offs are what this is for.
|
||||
#
|
||||
# Adding an item and setting a field value are GraphQL-only. REST can list project items
|
||||
# and field definitions, but the `fields` array it returns on an item carries Title and
|
||||
# nothing else -- a REST-only check reports every item's Status as unset, which is why the
|
||||
# read-back at the end is a GraphQL query rather than the cheaper REST one.
|
||||
#
|
||||
# WHAT IT DELIBERATELY DOES NOT DO
|
||||
#
|
||||
# It does not cache the project, field or option ids. Resolving them by name costs one
|
||||
# GraphQL query per run, and it means a renamed or reordered column cannot make this write
|
||||
# a stale id. The ids are the fragile part; the names are what people actually use.
|
||||
#
|
||||
# It does not apply triage labels for you. `above-cut` and `backlog` are labels from one
|
||||
# specific 2026-08-22 triage pass -- they mean "worked autonomously overnight" and "held for
|
||||
# manual review", not "this is in the Backlog column". Status carries board state. Pass
|
||||
# --label only for things that are true about the issue itself.
|
||||
#
|
||||
# It does not create the project, the Status field, or a missing option. Anything absent is
|
||||
# an error to report, not to invent.
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
readonly EXIT_USAGE=2
|
||||
readonly EXIT_ORPHANED=3
|
||||
|
||||
die() {
|
||||
printf 'file-issue: %s\n' "$1" >&2
|
||||
exit "${2:-$EXIT_USAGE}"
|
||||
}
|
||||
|
||||
title=""
|
||||
body=""
|
||||
body_file=""
|
||||
status="Backlog"
|
||||
project="${ISSUE_PROJECT_NUMBER:-6}"
|
||||
repo=""
|
||||
dry_run=0
|
||||
labels=()
|
||||
|
||||
while [ $# -gt 0 ]; do
|
||||
case "$1" in
|
||||
--title) [ $# -ge 2 ] || die "--title needs a value"; title="$2"; shift 2 ;;
|
||||
--body) [ $# -ge 2 ] || die "--body needs a value"; body="$2"; shift 2 ;;
|
||||
--body-file) [ $# -ge 2 ] || die "--body-file needs a path"; body_file="$2"; shift 2 ;;
|
||||
--status) [ $# -ge 2 ] || die "--status needs a value"; status="$2"; shift 2 ;;
|
||||
--label) [ $# -ge 2 ] || die "--label needs a value"; labels+=("$2"); shift 2 ;;
|
||||
--project) [ $# -ge 2 ] || die "--project needs a number"; project="$2"; shift 2 ;;
|
||||
--repo) [ $# -ge 2 ] || die "--repo needs OWNER/NAME"; repo="$2"; shift 2 ;;
|
||||
--dry-run) dry_run=1; shift ;;
|
||||
-h|--help) awk 'NR > 1 && /^#/ { sub(/^# ?/, ""); print; next } NR > 1 { exit }' "$0"
|
||||
exit 0 ;;
|
||||
*) die "unknown argument: $1" ;;
|
||||
esac
|
||||
done
|
||||
|
||||
[ -n "$title" ] || die "--title is required"
|
||||
if [ -n "$body" ] && [ -n "$body_file" ]; then
|
||||
die "pass --body or --body-file, not both"
|
||||
fi
|
||||
[ -n "$body" ] || [ -n "$body_file" ] || die "one of --body or --body-file is required"
|
||||
if [ -n "$body_file" ] && [ ! -r "$body_file" ]; then
|
||||
die "--body-file is not readable: $body_file"
|
||||
fi
|
||||
case "$project" in
|
||||
''|*[!0-9]*) die "--project must be a number, got: $project" ;;
|
||||
esac
|
||||
|
||||
command -v gh >/dev/null 2>&1 || die "gh is not on PATH"
|
||||
|
||||
if [ -z "$repo" ]; then
|
||||
repo=$(gh repo view --json nameWithOwner --jq '.nameWithOwner') \
|
||||
|| die "could not resolve the repository; pass --repo OWNER/NAME"
|
||||
fi
|
||||
owner="${repo%%/*}"
|
||||
[ -n "$owner" ] || die "could not read an owner out of: $repo"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Resolve the board by NAME. Every id below is read fresh; none is hardcoded.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# The $names in the query are GraphQL variables, declared by the query and bound by the
|
||||
# -f flags. Expanding them in the shell would send this shell's idea of $owner to the
|
||||
# API instead of declaring a parameter -- which is why every query here is single-quoted.
|
||||
# shellcheck disable=SC2016
|
||||
board=$(gh api graphql \
|
||||
-f query='
|
||||
query($owner: String!, $number: Int!) {
|
||||
user(login: $owner) {
|
||||
projectV2(number: $number) {
|
||||
id
|
||||
title
|
||||
field(name: "Status") {
|
||||
... on ProjectV2SingleSelectField { id options { id name } }
|
||||
}
|
||||
}
|
||||
}
|
||||
}' \
|
||||
-f owner="$owner" -F number="$project" 2>&1) \
|
||||
|| die "could not read project $project for $owner. A 403 naming scopes means gh is
|
||||
missing 'project'; a 403 naming a rate limit is the GraphQL budget, not permissions. The
|
||||
API said: $board"
|
||||
|
||||
project_id=$(printf '%s' "$board" | jq -r '.data.user.projectV2.id // empty')
|
||||
field_id=$(printf '%s' "$board" | jq -r '.data.user.projectV2.field.id // empty')
|
||||
project_title=$(printf '%s' "$board" | jq -r '.data.user.projectV2.title // empty')
|
||||
|
||||
[ -n "$project_id" ] || die "no project number $project under user $owner"
|
||||
[ -n "$field_id" ] || die "project $project has no single-select field named 'Status'"
|
||||
|
||||
# Case-insensitive match, so "backlog" and "Backlog" both work. The canonical name is
|
||||
# what gets reported back, so a sloppy argument still produces an exact log line.
|
||||
option=$(printf '%s' "$board" | jq -r --arg want "$status" '
|
||||
.data.user.projectV2.field.options[]
|
||||
| select((.name | ascii_downcase) == ($want | ascii_downcase))
|
||||
| "\(.id)\t\(.name)"' | head -n 1)
|
||||
|
||||
if [ -z "$option" ]; then
|
||||
printf 'file-issue: no Status option named %s. Available:\n' "$status" >&2
|
||||
printf '%s' "$board" | jq -r '.data.user.projectV2.field.options[] | " " + .name' >&2
|
||||
exit "$EXIT_USAGE"
|
||||
fi
|
||||
option_id="${option%%$'\t'*}"
|
||||
status_canonical="${option#*$'\t'}"
|
||||
|
||||
printf 'repo %s\n' "$repo"
|
||||
printf 'board %s (project %s)\n' "$project_title" "$project"
|
||||
printf 'status %s\n' "$status_canonical"
|
||||
printf 'labels %s\n' "${labels[*]:-(none)}"
|
||||
printf 'title %s\n' "$title"
|
||||
|
||||
if [ "$dry_run" -eq 1 ]; then
|
||||
printf '\ndry run: everything above resolved; nothing was created.\n'
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Create. Past this line a failure can leave an issue off the board, so every
|
||||
# error path prints the number.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
create_args=(--repo "$repo" --title "$title")
|
||||
if [ -n "$body_file" ]; then
|
||||
create_args+=(--body-file "$body_file")
|
||||
else
|
||||
create_args+=(--body "$body")
|
||||
fi
|
||||
for label in ${labels[@]+"${labels[@]}"}; do
|
||||
create_args+=(--label "$label")
|
||||
done
|
||||
|
||||
issue_url=$(gh issue create "${create_args[@]}") || die "gh issue create failed; nothing was filed"
|
||||
issue_number="${issue_url##*/}"
|
||||
case "$issue_number" in
|
||||
''|*[!0-9]*) die "could not read an issue number out of: $issue_url" ;;
|
||||
esac
|
||||
|
||||
orphaned() {
|
||||
printf 'file-issue: %s\n' "$1" >&2
|
||||
printf 'file-issue: THE ISSUE EXISTS BUT IS NOT ON THE BOARD. Fix it by hand:\n' >&2
|
||||
printf '%s\n' "$issue_url" >&2
|
||||
exit "$EXIT_ORPHANED"
|
||||
}
|
||||
|
||||
content_id=$(gh api "/repos/$repo/issues/$issue_number" --jq '.node_id') \
|
||||
|| orphaned "could not read the node id for #$issue_number"
|
||||
|
||||
# shellcheck disable=SC2016 # GraphQL variables, as above
|
||||
item_id=$(gh api graphql \
|
||||
-f query='
|
||||
mutation($project: ID!, $content: ID!) {
|
||||
addProjectV2ItemById(input: {projectId: $project, contentId: $content}) {
|
||||
item { id }
|
||||
}
|
||||
}' \
|
||||
-f project="$project_id" -f content="$content_id" \
|
||||
--jq '.data.addProjectV2ItemById.item.id') \
|
||||
|| orphaned "could not add #$issue_number to the board"
|
||||
[ -n "$item_id" ] || orphaned "the board add returned no item id for #$issue_number"
|
||||
|
||||
# shellcheck disable=SC2016 # GraphQL variables, as above
|
||||
gh api graphql \
|
||||
-f query='
|
||||
mutation($project: ID!, $item: ID!, $field: ID!, $option: String!) {
|
||||
updateProjectV2ItemFieldValue(input: {
|
||||
projectId: $project, itemId: $item, fieldId: $field,
|
||||
value: {singleSelectOptionId: $option}
|
||||
}) { projectV2Item { id } }
|
||||
}' \
|
||||
-f project="$project_id" -f item="$item_id" -f field="$field_id" -f option="$option_id" \
|
||||
>/dev/null \
|
||||
|| orphaned "#$issue_number is on the board but its Status could not be set"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Read back. A mutation returning 200 is not evidence the board shows what was
|
||||
# asked for -- this is the only check that is.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# shellcheck disable=SC2016 # GraphQL variables, as above
|
||||
readback=$(gh api graphql \
|
||||
-f query='
|
||||
query($item: ID!) {
|
||||
node(id: $item) {
|
||||
... on ProjectV2Item {
|
||||
content { ... on Issue { number } }
|
||||
fieldValueByName(name: "Status") {
|
||||
... on ProjectV2ItemFieldSingleSelectValue { name }
|
||||
}
|
||||
}
|
||||
}
|
||||
}' \
|
||||
-f item="$item_id") \
|
||||
|| orphaned "#$issue_number was written but could not be read back"
|
||||
|
||||
seen_number=$(printf '%s' "$readback" | jq -r '.data.node.content.number // empty')
|
||||
seen_status=$(printf '%s' "$readback" | jq -r '.data.node.fieldValueByName.name // empty')
|
||||
|
||||
if [ "$seen_number" != "$issue_number" ]; then
|
||||
orphaned "read-back names issue #${seen_number:-<none>}, expected #$issue_number"
|
||||
fi
|
||||
if [ "$seen_status" != "$status_canonical" ]; then
|
||||
orphaned "read-back Status is ${seen_status:-<unset>}, expected $status_canonical"
|
||||
fi
|
||||
|
||||
printf '\n#%s on %s as %s -- verified by read-back\n' \
|
||||
"$issue_number" "$project_title" "$seen_status"
|
||||
printf '%s\n' "$issue_url"
|
||||
+348
-48
@@ -3,13 +3,24 @@
|
||||
# Runs the instrumented suite on a local emulator, on this workstation, for one or more
|
||||
# API levels.
|
||||
#
|
||||
# Usage: tools/local-emulator/run-e2e.sh [API ...] # default: 33 34 35 36
|
||||
# Usage: tools/local-emulator/run-e2e.sh [API ...] # default: 33 34 35 36 37
|
||||
#
|
||||
# GPU_MODE=host renderer to use; see the refusal list below
|
||||
# API levels are the labels below, not SDK ints: 33-36, plus `37` (= `37.0`) and `37.1`.
|
||||
#
|
||||
# GPU_MODE= force one renderer on every level; unset means per-API (gpu_for_api)
|
||||
# EMULATOR_PORT=5560 console port, so the serial is deterministic
|
||||
# BOOT_TIMEOUT=300 seconds to wait for sys.boot_completed
|
||||
# KEEP_AVD=1 do not delete an AVD this script created
|
||||
#
|
||||
# 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.
|
||||
#
|
||||
# WHY THIS EXISTS, AND WHAT IT DELIBERATELY DOES NOT DO
|
||||
#
|
||||
# It is a *launcher*, not a second test harness. The diagnostics -- the FAILED-vs-WEDGED
|
||||
@@ -34,6 +45,14 @@
|
||||
# those modes was measured crashing. docs/local-emulator.md has the backtrace, the faulting
|
||||
# page's RW-without-E segment flags, and the full mode matrix.
|
||||
#
|
||||
# AND THE ONE THING API 37 NEEDS THAT 33-36 DO NOT: the opposite renderer. On the API 37
|
||||
# images the guest's Gralloc5 mapper aborts surfaceflinger from RegionSamplingThread
|
||||
# (`Assertion failed: !rcEnc->featureInfo()->hasReadColorBufferDma`). Under `-gpu host` that
|
||||
# repeats every few seconds and the device never boots; under ANGLE it fires a handful of
|
||||
# times and the boot survives. So `host` is required below 37 and forbidden at 37, which is
|
||||
# why the renderer is chosen per level in gpu_for_api rather than set once.
|
||||
# docs/api-37-emulator-crash.md has that matrix.
|
||||
#
|
||||
# THE OTHER LOCAL-ONLY HAZARD: a physical Pixel is usually plugged into this machine, so
|
||||
# `adb` is ambiguous in a way it never is on a runner, and an unpinned run would install
|
||||
# and execute this suite on the phone. Every path below pins the emulator serial.
|
||||
@@ -65,12 +84,14 @@ export ANDROID_HOME="${ANDROID_HOME:-$HOME/Android/Sdk}"
|
||||
export ANDROID_SDK_ROOT="$ANDROID_HOME"
|
||||
export PATH="$ANDROID_HOME/platform-tools:$ANDROID_HOME/emulator:$ANDROID_HOME/cmdline-tools/latest/bin:$PATH"
|
||||
|
||||
GPU_MODE="${GPU_MODE:-host}"
|
||||
# Empty means "let each level pick" -- see gpu_for_api. Setting GPU_MODE forces one renderer
|
||||
# on every level, which is what you want when measuring a mode, not when running the suite.
|
||||
GPU_MODE="${GPU_MODE:-}"
|
||||
EMULATOR_PORT="${EMULATOR_PORT:-5560}"
|
||||
BOOT_TIMEOUT="${BOOT_TIMEOUT:-300}"
|
||||
SERIAL="emulator-${EMULATOR_PORT}"
|
||||
APIS=("$@")
|
||||
[ "${#APIS[@]}" -eq 0 ] && APIS=(33 34 35 36)
|
||||
[ "${#APIS[@]}" -eq 0 ] && APIS=(33 34 35 36 37)
|
||||
|
||||
# Matches CI. `disk-size: 8G` because the FFmpeg libraries do not fit the default userdata
|
||||
# partition; `ram-size: 2560M` because the emulator's own floor varies by API level and
|
||||
@@ -83,21 +104,28 @@ RESULTS_DIR="app/build/outputs/androidTest-results"
|
||||
LOG_DIR="${TMPDIR:-/tmp}/lmc-local-e2e"
|
||||
mkdir -p "$LOG_DIR"
|
||||
|
||||
# The two things that outlive a level, declared here rather than where they are first
|
||||
# assigned, because the cleanup trap below can fire before either has been reached.
|
||||
EMU_PID=""
|
||||
CREATED_AVDS=()
|
||||
|
||||
# ---------------------------------------------------------------- renderer preflight ---
|
||||
case "$GPU_MODE" in
|
||||
swiftshader_indirect | auto | off | guest)
|
||||
echo "REFUSING to launch with -gpu $GPU_MODE."
|
||||
echo "On this host that resolves to SwiftShader's GLES, whose JIT is denied execheap by"
|
||||
echo "SELinux; the emulator segfaults (exit 139) before boot. See docs/local-emulator.md."
|
||||
echo "Working modes: host (default), angle_indirect, swangle_indirect."
|
||||
exit 2
|
||||
;;
|
||||
host | angle_indirect | swangle_indirect) ;;
|
||||
*)
|
||||
echo "Unrecognised GPU_MODE '$GPU_MODE'. Known-good: host, angle_indirect, swangle_indirect."
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
if [ -n "$GPU_MODE" ]; then
|
||||
case "$GPU_MODE" in
|
||||
swiftshader_indirect | auto | off | guest)
|
||||
echo "REFUSING to launch with -gpu $GPU_MODE."
|
||||
echo "On this host that resolves to SwiftShader's GLES, whose JIT is denied execheap by"
|
||||
echo "SELinux; the emulator segfaults (exit 139) before boot. See docs/local-emulator.md."
|
||||
echo "Working modes: host, angle_indirect, swangle_indirect."
|
||||
exit 2
|
||||
;;
|
||||
host | angle_indirect | swangle_indirect) ;;
|
||||
*)
|
||||
echo "Unrecognised GPU_MODE '$GPU_MODE'. Known-good: host, angle_indirect, swangle_indirect."
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
fi
|
||||
|
||||
# A courtesy, not a gate: the boolean being on means SwiftShader would work too, and the
|
||||
# refusal list above could be relaxed. It is off on a stock Fedora.
|
||||
@@ -121,14 +149,90 @@ host_forensics() {
|
||||
journalctl --since "$since" --no-pager 2> /dev/null | grep -E 'avc: .*denied' | tail -10 || echo " (none)"
|
||||
}
|
||||
|
||||
# The API 37 counterpart of host_forensics. `-gpu host` there aborts surfaceflinger in a loop
|
||||
# and the device never boots; the working renderers abort it a few times and survive. Either way
|
||||
# the count is the number to look at, and the crash buffer is where it lives -- so print it on
|
||||
# every 37 level, not only on the failure path, because a level that passed with 40 aborts is
|
||||
# telling you something a level that passed with 1 is not.
|
||||
guest_forensics() {
|
||||
local api="$1" n
|
||||
case "$api" in 37 | 37.*) ;; *) return 0 ;; esac
|
||||
n="$(emu_adb logcat -d -b crash 2> /dev/null | grep -c 'hasReadColorBufferDma')"
|
||||
echo " surfaceflinger hasReadColorBufferDma aborts: ${n:-?} (docs/api-37-emulator-crash.md)"
|
||||
}
|
||||
|
||||
# API label -> system image. API 33-36 are plain integers with a `google_apis` image. API 37
|
||||
# is not: its SDK directories are dotted minor versions (`android-37.0`, `android-37.1`), there
|
||||
# is no `android-37`, and from 37.1 onwards Google ships only 16 KB-page (`ps16k`) images for
|
||||
# x86_64. `37` is accepted as a spelling of `37.0` because that is what people type.
|
||||
image_pkg_for_api() {
|
||||
case "$1" in
|
||||
37 | 37.0) echo "system-images;android-37.0;google_apis;x86_64" ;;
|
||||
37.1) echo "system-images;android-37.1;google_apis_ps16k;x86_64" ;;
|
||||
*) echo "system-images;android-$1;google_apis;x86_64" ;;
|
||||
esac
|
||||
}
|
||||
|
||||
# The renderer requirement is per-API and the two levels want OPPOSITE things, which is why this
|
||||
# is a function and not a constant.
|
||||
#
|
||||
# 33-36: must NOT be SwiftShader GLES (host-side SELinux/execheap segfault) -- `host` is right.
|
||||
# 37.x: must NOT be the host GL translator. With `-gpu host` the guest's Gralloc5 mapper
|
||||
# aborts surfaceflinger in a loop and the device never boots; under ANGLE the same
|
||||
# assertion fires a handful of times and the boot survives it. Measured, not guessed --
|
||||
# docs/api-37-emulator-crash.md has the matrix.
|
||||
#
|
||||
# `swangle_indirect` rather than `angle_indirect` for 37: both boot, and swangle names its
|
||||
# renderer outright instead of resolving through `auto`'s path.
|
||||
gpu_for_api() {
|
||||
if [ -n "$GPU_MODE" ]; then
|
||||
echo "$GPU_MODE"
|
||||
return
|
||||
fi
|
||||
case "$1" in
|
||||
37 | 37.*) echo "swangle_indirect" ;;
|
||||
*) echo "host" ;;
|
||||
esac
|
||||
}
|
||||
|
||||
# `lmc_e2e_api37.0` would be a legal AVD name but an awkward one to type and to grep for.
|
||||
# `37` and `37.0` therefore give two AVD names (`lmc_e2e_api37`, `lmc_e2e_api37_0`) for the one
|
||||
# image. Harmless -- two AVDs off the same system image cost only disk -- and deliberately not
|
||||
# normalised, so that `run-e2e.sh 37 37.0` does not have both levels fight over one AVD.
|
||||
avd_for_api() { echo "lmc_e2e_api${1//./_}"; }
|
||||
|
||||
# Where avdmanager actually put the AVD. `$HOME/.android/avd` is only the default:
|
||||
# ANDROID_AVD_HOME, ANDROID_USER_HOME and ANDROID_SDK_HOME each move it, and hardcoding the
|
||||
# default meant a machine that sets any of them silently ran every level at stock RAM and
|
||||
# userdata size. Rather than encode a precedence that cannot be verified from here, look in
|
||||
# every location avdmanager honours and let the existence check pick.
|
||||
avd_config_path() {
|
||||
local avd="$1" base cfg
|
||||
for base in "${ANDROID_AVD_HOME:-}" \
|
||||
"${ANDROID_USER_HOME:+$ANDROID_USER_HOME/avd}" \
|
||||
"${ANDROID_SDK_HOME:+$ANDROID_SDK_HOME/.android/avd}" \
|
||||
"$HOME/.android/avd"; do
|
||||
[ -n "$base" ] || continue
|
||||
cfg="$base/${avd}.avd/config.ini"
|
||||
if [ -f "$cfg" ]; then
|
||||
echo "$cfg"
|
||||
return 0
|
||||
fi
|
||||
done
|
||||
return 1
|
||||
}
|
||||
|
||||
ensure_avd() {
|
||||
local api="$1" avd="$2"
|
||||
local pkg="system-images;android-${api};google_apis;x86_64"
|
||||
local pkg
|
||||
pkg="$(image_pkg_for_api "$api")"
|
||||
local img_dir="$ANDROID_HOME/system-images/${pkg#system-images;}"
|
||||
img_dir="${img_dir//;//}"
|
||||
|
||||
if avdmanager list avd -c 2> /dev/null | grep -qx "$avd"; then
|
||||
echo " reusing existing AVD $avd"
|
||||
else
|
||||
if [ ! -d "$ANDROID_HOME/system-images/android-${api}/google_apis/x86_64" ]; then
|
||||
if [ ! -d "$img_dir" ]; then
|
||||
echo " installing $pkg"
|
||||
yes | sdkmanager --install "$pkg" > /dev/null 2>&1 || {
|
||||
echo " FAILED to install $pkg"
|
||||
@@ -144,18 +248,37 @@ ensure_avd() {
|
||||
fi
|
||||
|
||||
# Written into config.ini rather than passed on the command line, which is how
|
||||
# reactivecircus/android-emulator-runner applies the same two settings in CI.
|
||||
local cfg="$HOME/.android/avd/${avd}.avd/config.ini"
|
||||
sed -i -e '/^disk\.dataPartition\.size=/d' -e '/^hw\.ramSize=/d' "$cfg"
|
||||
printf 'disk.dataPartition.size=%s\nhw.ramSize=%s\n' "$DISK_SIZE_BYTES" "$RAM_SIZE_MB" >> "$cfg"
|
||||
# reactivecircus/android-emulator-runner applies the same two settings in CI. On the reuse
|
||||
# path too, so an AVD left over from an older run gets today's pins.
|
||||
#
|
||||
# A level that cannot be pinned FAILS rather than running at the defaults. Unpinned, it
|
||||
# dies much later with "not enough space", which reads as a device problem -- CI's own
|
||||
# history is where that lesson comes from -- and nothing points back to a `sed` that
|
||||
# edited a path this script guessed wrong.
|
||||
local cfg
|
||||
if ! cfg="$(avd_config_path "$avd")"; then
|
||||
echo " FAILED: no config.ini for $avd in any directory avdmanager uses"
|
||||
echo " (ANDROID_AVD_HOME=${ANDROID_AVD_HOME:-unset}, ANDROID_USER_HOME=${ANDROID_USER_HOME:-unset},"
|
||||
echo " ANDROID_SDK_HOME=${ANDROID_SDK_HOME:-unset}, HOME=$HOME)"
|
||||
return 1
|
||||
fi
|
||||
if ! sed -i -e '/^disk\.dataPartition\.size=/d' -e '/^hw\.ramSize=/d' "$cfg"; then
|
||||
echo " FAILED to rewrite $cfg"
|
||||
return 1
|
||||
fi
|
||||
if ! printf 'disk.dataPartition.size=%s\nhw.ramSize=%s\n' \
|
||||
"$DISK_SIZE_BYTES" "$RAM_SIZE_MB" >> "$cfg"; then
|
||||
echo " FAILED to write the RAM/disk pins into $cfg"
|
||||
return 1
|
||||
fi
|
||||
}
|
||||
|
||||
boot_emulator() {
|
||||
local avd="$1" api="$2"
|
||||
local avd="$1" api="$2" gpu="$3"
|
||||
local boot_log="$LOG_DIR/emulator-api${api}.log"
|
||||
|
||||
emulator -avd "$avd" -port "$EMULATOR_PORT" \
|
||||
-no-window -gpu "$GPU_MODE" -noaudio -no-boot-anim -camera-back none -no-snapshot \
|
||||
-no-window -gpu "$gpu" -noaudio -no-boot-anim -camera-back none -no-snapshot \
|
||||
> "$boot_log" 2>&1 &
|
||||
EMU_PID=$!
|
||||
|
||||
@@ -181,6 +304,92 @@ boot_emulator() {
|
||||
return 1
|
||||
}
|
||||
|
||||
# API 37 only, and the reason API 37 can be run at all.
|
||||
#
|
||||
# The abort that breaks these images is reached from SurfaceFlinger's RegionSamplingThread,
|
||||
# which exists only because SystemUI registers a nav-bar luma-sampling listener. Each abort
|
||||
# kills surfaceflinger, and init responds by SIGKILLing zygote -- so the whole framework
|
||||
# restarts underneath the test run, which arrives as `Can't find service: package` and
|
||||
# `INSTRUMENTATION_ABORTED: System has crashed`. Under the host GL renderer that repeats
|
||||
# forever; under ANGLE it is roughly one every fifteen seconds, which a five-minute suite does
|
||||
# not survive either.
|
||||
#
|
||||
# Removing the listener removes the whole chain. Measured on android-37.0 under
|
||||
# swangle_indirect: 10-11 aborts per 150 s idle with SystemUI running, and 0 in 180 s with it
|
||||
# disabled, framework services up throughout.
|
||||
#
|
||||
# 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.
|
||||
#
|
||||
# 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
|
||||
# failed exactly that way, with `cmd: Can't find service: package`.
|
||||
#
|
||||
# The framework restart at the end is not optional, and finding that out cost a run. By the
|
||||
# time `sys.boot_completed` flips, SystemUI has already registered its region-sampling listener,
|
||||
# and `pm disable-user` does not retract a registration that already happened -- it only stops
|
||||
# the package being started again. So the first attempt disabled SystemUI, reported success, and
|
||||
# then died exactly as before with `Starting 0 tests` and four more aborts. `stop; start` cycles
|
||||
# zygote deliberately, and the framework that comes back up does not start SystemUI at all.
|
||||
disable_region_sampling() {
|
||||
local api="$1" out i before after ready
|
||||
case "$api" in 37 | 37.*) ;; *) return 0 ;; esac
|
||||
|
||||
out=""
|
||||
for i in $(seq 1 20); do
|
||||
out="$(emu_adb shell pm disable-user --user 0 com.android.systemui 2>&1 | tr -d '\r')"
|
||||
case "$out" in
|
||||
*"new state: disabled"*)
|
||||
echo " SystemUI disabled on attempt $i"
|
||||
break
|
||||
;;
|
||||
esac
|
||||
out=""
|
||||
sleep 5
|
||||
done
|
||||
if [ -z "$out" ]; then
|
||||
echo " WARNING: could not disable SystemUI after 20 attempts."
|
||||
echo " Expect INSTRUMENTATION_ABORTED -- docs/api-37-emulator-crash.md"
|
||||
return 0
|
||||
fi
|
||||
|
||||
echo " restarting the framework so the region-sampling listener goes with it"
|
||||
emu_adb shell stop > /dev/null 2>&1
|
||||
emu_adb shell start > /dev/null 2>&1
|
||||
# There is no property worth waiting on here, and an earlier version of this only looked
|
||||
# like it was waiting on one: `stop` does not clear sys.boot_completed, so it still reads
|
||||
# `1` throughout the restart and any loop over it returns at once. The loop below is the
|
||||
# wait -- and it polls the better thing anyway, since `Can't find service: package` is the
|
||||
# failure it exists to prevent.
|
||||
ready=0
|
||||
for i in $(seq 1 30); do
|
||||
if emu_adb shell service check package 2> /dev/null | grep -q ': found' \
|
||||
&& emu_adb shell service check activity 2> /dev/null | grep -q ': found'; then
|
||||
ready=1
|
||||
break
|
||||
fi
|
||||
sleep 5
|
||||
done
|
||||
if [ "$ready" -ne 1 ]; then
|
||||
echo " WARNING: package and activity services still absent 150 s after the restart."
|
||||
echo " Expect INSTRUMENTATION_ABORTED -- docs/api-37-emulator-crash.md"
|
||||
fi
|
||||
|
||||
# Prove it worked rather than assume it. Zero new aborts over this window is what makes the
|
||||
# difference between a run that completes and one that reports `Starting 0 tests`.
|
||||
before="$(emu_adb logcat -d -b crash 2> /dev/null | grep -c 'hasReadColorBufferDma')"
|
||||
emu_adb shell 'sleep 45' > /dev/null 2>&1
|
||||
after="$(emu_adb logcat -d -b crash 2> /dev/null | grep -c 'hasReadColorBufferDma')"
|
||||
echo " quiet check: $((after - before)) new surfaceflinger aborts in 45 s (want 0)"
|
||||
if [ "$((after - before))" -ne 0 ]; then
|
||||
echo " WARNING: region sampling is still live; the run may not survive."
|
||||
fi
|
||||
return 0
|
||||
}
|
||||
|
||||
# CI gets this from the action's `disable-animations: true`.
|
||||
disable_animations() {
|
||||
local s
|
||||
@@ -189,17 +398,81 @@ disable_animations() {
|
||||
done
|
||||
}
|
||||
|
||||
# `${EMU_PID:-0}` used to guard these three calls, and it guarded the wrong thing: EMU_PID
|
||||
# is *empty*, not unset, if the background launch never produced a job, and `kill` reads pid
|
||||
# 0 as "the sender's whole process group" -- this script and, on a terminal, everything else
|
||||
# in the foreground group with it. The `kill -0` wait loop had the same shape and would have
|
||||
# spent its full grace period testing the group. Nothing to stop is now a return, never a
|
||||
# guess. (boot_emulator's own `kill -0 "$EMU_PID"` is unguarded and cannot reach that form:
|
||||
# it runs only after the assignment.)
|
||||
#
|
||||
# max_wait is a parameter so the interrupt path need not sit through the full grace period.
|
||||
stop_emulator() {
|
||||
local max_wait="${1:-30}" waited=0
|
||||
[ -n "${EMU_PID:-}" ] || return 0
|
||||
emu_adb emu kill > /dev/null 2>&1
|
||||
local waited=0
|
||||
while kill -0 "${EMU_PID:-0}" 2> /dev/null && [ "$waited" -lt 30 ]; do
|
||||
while kill -0 "$EMU_PID" 2> /dev/null && [ "$waited" -lt "$max_wait" ]; do
|
||||
sleep 2
|
||||
waited=$((waited + 2))
|
||||
done
|
||||
kill -9 "${EMU_PID:-0}" 2> /dev/null
|
||||
wait "${EMU_PID:-0}" 2> /dev/null
|
||||
kill -9 "$EMU_PID" 2> /dev/null
|
||||
wait "$EMU_PID" 2> /dev/null
|
||||
EMU_PID=""
|
||||
}
|
||||
|
||||
delete_created_avds() {
|
||||
local avd
|
||||
[ "${KEEP_AVD:-0}" = "1" ] && return 0
|
||||
for avd in ${CREATED_AVDS[@]+"${CREATED_AVDS[@]}"}; do
|
||||
# A SIGKILLed emulator does not get to remove its own lock files, and avdmanager can
|
||||
# refuse over them. Staying silent there would leak the very thing this exists to clean.
|
||||
avdmanager delete avd -n "$avd" > /dev/null 2>&1 \
|
||||
|| echo " WARNING: could not delete AVD $avd -- 'avdmanager delete avd -n $avd' by hand"
|
||||
done
|
||||
CREATED_AVDS=()
|
||||
}
|
||||
|
||||
# What an interrupted sweep used to leave behind: a headless emulator holding console port
|
||||
# $EMULATOR_PORT, and an lmc_e2e_apiNN AVD. The next run's `emulator -port` then collides
|
||||
# with the orphan, and `emu_adb` can resolve to it -- on a workstation that also has the
|
||||
# Pixel plugged in, exactly the ambiguity the ANDROID_SERIAL pinning exists to prevent. A
|
||||
# sweep is up to five boots long, so the window for one Ctrl-C is not small.
|
||||
#
|
||||
# Idempotent, and called explicitly on the normal path so its output cannot land after the
|
||||
# summary; the EXIT trap then finds nothing left to do. The emulator logs are deliberately
|
||||
# NOT removed -- they live in $LOG_DIR and are the only evidence a failed boot leaves.
|
||||
CLEANED=0
|
||||
cleanup() {
|
||||
[ "$CLEANED" = "1" ] && return 0
|
||||
CLEANED=1
|
||||
stop_emulator "${1:-30}"
|
||||
delete_created_avds
|
||||
}
|
||||
|
||||
# 6 s, not 30: Ctrl-C has already reached the emulator through the foreground process group,
|
||||
# so this is only waiting for it to finish writing, and `kill -9` follows regardless. The
|
||||
# EXIT trap is disarmed before exiting so the status below is the one that survives.
|
||||
#
|
||||
# bash runs a trap only between commands, so this starts when whatever was in the foreground
|
||||
# returns -- which for Ctrl-C is immediately, because the same interrupt reached that command
|
||||
# too. `kill -INT` aimed at this script alone waits for the foreground command to finish.
|
||||
# Invoked indirectly -- installed as the INT and TERM trap a few lines below. Both codes,
|
||||
# because shellcheck 0.9.0 reports this as unreachable commands (SC2317) and 0.11.0 as an
|
||||
# uninvoked function (SC2329); CI pins 0.11.0 but a local install may be either.
|
||||
# shellcheck disable=SC2317,SC2329
|
||||
on_signal() {
|
||||
echo
|
||||
echo "interrupted (SIG$1) -- stopping the emulator and removing the AVDs this run created"
|
||||
echo " emulator logs kept in $LOG_DIR"
|
||||
cleanup 6
|
||||
trap - EXIT
|
||||
exit "$2"
|
||||
}
|
||||
|
||||
trap 'on_signal INT 130' INT
|
||||
trap 'on_signal TERM 143' TERM
|
||||
trap cleanup EXIT
|
||||
|
||||
# The XML is authoritative. The console counter double-counts skips, so a run that reports
|
||||
# "42 tests" on stdout can be 40 in the report.
|
||||
#
|
||||
@@ -236,37 +509,44 @@ PY
|
||||
}
|
||||
|
||||
# ------------------------------------------------------------------------------ main ---
|
||||
CREATED_AVDS=()
|
||||
SUMMARY=()
|
||||
overall=0
|
||||
|
||||
for api in "${APIS[@]}"; do
|
||||
if [ "$api" = "37" ] || [ "$api" = "37.0" ]; then
|
||||
echo "SKIPPING API $api: the android-37.0 image crash-loops surfaceflinger."
|
||||
echo " See docs/api-37-emulator-crash.md. Test API 37 on the physical Pixel."
|
||||
continue
|
||||
fi
|
||||
# Whether the red exit is the expected one depends on which level produced it, and only the
|
||||
# loop knows that -- so it is recorded where `overall` is set rather than guessed from the
|
||||
# summary afterwards. A note at the end claiming a genuine API 34 failure was "by design"
|
||||
# would be the same defect it is there to prevent, one layer up.
|
||||
NON37_RED=0
|
||||
mark_red() {
|
||||
overall=1
|
||||
case "$1" in 37 | 37.*) ;; *) NON37_RED=1 ;; esac
|
||||
}
|
||||
|
||||
avd="lmc_e2e_api${api}"
|
||||
for api in "${APIS[@]}"; do
|
||||
avd="$(avd_for_api "$api")"
|
||||
gpu="$(gpu_for_api "$api")"
|
||||
started="$(date '+%Y-%m-%d %H:%M:%S')"
|
||||
echo "=============================================================="
|
||||
echo "API $api (avd=$avd gpu=$GPU_MODE serial=$SERIAL)"
|
||||
echo "API $api (avd=$avd gpu=$gpu serial=$SERIAL)"
|
||||
echo " image: $(image_pkg_for_api "$api")"
|
||||
echo "=============================================================="
|
||||
|
||||
if ! ensure_avd "$api" "$avd"; then
|
||||
SUMMARY+=("API $api: AVD SETUP FAILED")
|
||||
overall=1
|
||||
mark_red "$api"
|
||||
continue
|
||||
fi
|
||||
|
||||
if ! boot_emulator "$avd" "$api"; then
|
||||
if ! boot_emulator "$avd" "$api" "$gpu"; then
|
||||
host_forensics "$started"
|
||||
guest_forensics "$api"
|
||||
SUMMARY+=("API $api: BOOT FAILED")
|
||||
overall=1
|
||||
mark_red "$api"
|
||||
stop_emulator
|
||||
continue
|
||||
fi
|
||||
|
||||
disable_region_sampling "$api"
|
||||
disable_animations
|
||||
rm -rf "$RESULTS_DIR"
|
||||
|
||||
@@ -281,23 +561,43 @@ for api in "${APIS[@]}"; do
|
||||
unset ANDROID_SERIAL E2E_EXTRA_GRADLE_ARGS
|
||||
|
||||
line="$(summarise_results "$api")"
|
||||
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.
|
||||
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"
|
||||
;;
|
||||
esac
|
||||
if [ "$rc" -ne 0 ]; then
|
||||
line="$line [gradle exit $rc]"
|
||||
overall=1
|
||||
mark_red "$api"
|
||||
host_forensics "$started"
|
||||
fi
|
||||
SUMMARY+=("$line")
|
||||
stop_emulator
|
||||
done
|
||||
|
||||
if [ "${KEEP_AVD:-0}" != "1" ]; then
|
||||
for avd in ${CREATED_AVDS[@]+"${CREATED_AVDS[@]}"}; do
|
||||
avdmanager delete avd -n "$avd" > /dev/null 2>&1
|
||||
done
|
||||
fi
|
||||
cleanup
|
||||
|
||||
echo
|
||||
echo "===================== LOCAL E2E SUMMARY ======================"
|
||||
printf '%s\n' ${SUMMARY[@]+"${SUMMARY[@]}"}
|
||||
echo "=============================================================="
|
||||
|
||||
# An unexplained red exit trains people to stop reading exit codes, and this one is expected
|
||||
# whenever API 37 is in the sweep -- which the default list makes the common case. Said here
|
||||
# rather than only in the docs, because this is where it is actually read. Only when 37.x is
|
||||
# the ONLY thing that went red: a note calling a real failure elsewhere "by design" would be
|
||||
# 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."
|
||||
fi
|
||||
|
||||
exit "$overall"
|
||||
|
||||
Reference in New Issue
Block a user