Compare commits
20
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
3b0c262030 | ||
|
|
18aff51c98 | ||
|
|
b23ff0f082 | ||
|
|
fa10d94192 | ||
|
|
69d5392227 | ||
|
|
e7c3e5688f | ||
|
|
b3d4318273 | ||
|
|
07f7ed4259 | ||
|
|
dc6ee3dc9b | ||
|
|
7fd95ddede | ||
|
|
350b179c9e | ||
|
|
0cc4c4f3a3 | ||
|
|
557b3edab4 | ||
|
|
97558c259f | ||
|
|
745c4f62ce | ||
|
|
e2f8ef2918 | ||
|
|
393b931fff | ||
|
|
d759ef32f1 | ||
|
|
4d090d9a81 | ||
|
|
39327beea7 |
@@ -184,6 +184,46 @@ out="$(run_report "$root")"
|
|||||||
assert_contains "same-line annotation removed: counts 2, so it was worth 1" "$out" \
|
assert_contains "same-line annotation removed: counts 2, so it was worth 1" "$out" \
|
||||||
" baseline DEVIATION: the tree carries 2 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3 — update FAILS_ON_EMULATOR_API37_BASELINE"
|
" baseline DEVIATION: the tree carries 2 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3 — update FAILS_ON_EMULATOR_API37_BASELINE"
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# 4. A run the abort truncated, with fewer failures than the baseline: NOT a deviation.
|
||||||
|
#
|
||||||
|
# `expected` comes from `Starting N tests`, printed before anything can abort, so it still
|
||||||
|
# answers "is the marked set the size the baseline says". `failed` is a tally of what actually
|
||||||
|
# ran, and on a truncated run the tests after the abort never start. Measured on 2026-09-05, two
|
||||||
|
# api37-debug dispatches of the same four marked tests: 4/4/4 and then 4/3/3. Announcing the
|
||||||
|
# second as "one now passes" is the wrong reading, and #120 is the standing lesson about a notice
|
||||||
|
# that is wrong often enough to be skimmed past.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
root="$(make_root "$FIXTURE_DIR" 3)"
|
||||||
|
cat > "$root/gradle.log" <<'TRUNCATED'
|
||||||
|
> Task :app:connectedDebugAndroidTest
|
||||||
|
Starting 3 tests on test(AVD) - 16
|
||||||
|
There was 2 failure(s).
|
||||||
|
Test run failed to complete. Expected 3 tests, received 2. onError: commandError=false message=INSTRUMENTATION_ABORTED: System has crashed.
|
||||||
|
TRUNCATED
|
||||||
|
out="$(run_report "$root")"
|
||||||
|
assert_contains "truncated run: the truncation is reported" "$out" ' completed cleanly: no'
|
||||||
|
assert_absent "truncated run: the short failure count is not a deviation" "$out" 'tests failed, the baseline is'
|
||||||
|
# And the match line has to say what actually happened rather than repeat the baseline: PR #245's
|
||||||
|
# advisory leg printed `failed: 4` three lines above `matches (5 expected, 5 failed)`.
|
||||||
|
assert_contains "truncated run: the match line does not claim the baseline's failure count" "$out" \
|
||||||
|
' baseline: matches (3 expected; 2 of 3 failed, on a run the abort truncated — not compared)'
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# 5. The same short failure count on a run that finished IS a deviation.
|
||||||
|
#
|
||||||
|
# The pair is the point: case 4 must not have bought its quiet by disabling the check outright.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
root="$(make_root "$FIXTURE_DIR" 3)"
|
||||||
|
cat > "$root/gradle.log" <<'CLEAN'
|
||||||
|
> Task :app:connectedDebugAndroidTest
|
||||||
|
Starting 3 tests on test(AVD) - 16
|
||||||
|
There was 2 failure(s).
|
||||||
|
CLEAN
|
||||||
|
out="$(run_report "$root")"
|
||||||
|
assert_contains "clean run, short by one: the deviation fires" "$out" \
|
||||||
|
'2 tests failed, the baseline is 3'
|
||||||
|
|
||||||
echo
|
echo
|
||||||
if [ "$failures" -eq 0 ]; then
|
if [ "$failures" -eq 0 ]; then
|
||||||
echo "e2e-report-shape-test.sh: all checks passed"
|
echo "e2e-report-shape-test.sh: all checks passed"
|
||||||
|
|||||||
@@ -252,8 +252,22 @@ if [ -n "$baseline" ]; then
|
|||||||
if [ "$expected" != "unknown" ] && [ "$expected" != "$baseline" ]; then
|
if [ "$expected" != "unknown" ] && [ "$expected" != "$baseline" ]; then
|
||||||
deviations+=("the runner started $expected tests, the baseline is $baseline")
|
deviations+=("the runner started $expected tests, the baseline is $baseline")
|
||||||
fi
|
fi
|
||||||
|
# `expected` is compared on every run and `failed` only on a run that finished, and the
|
||||||
|
# difference is the truncation this file already records rather than compares. `expected`
|
||||||
|
# comes from `Starting N tests`, which is printed before anything can abort, so it answers
|
||||||
|
# "is the marked set the size the baseline says" whatever happens afterwards. `failed` is a
|
||||||
|
# tally of what actually ran: on a truncated run the tests after the abort never start, so
|
||||||
|
# comparing it to the baseline announces a deviation about the framework dying rather than
|
||||||
|
# about the test list. Measured on 2026-09-05, two api37-debug dispatches of the same four
|
||||||
|
# marked tests: 4/4/4 and then 4/3/3, the second having lost the last test to the abort.
|
||||||
|
# Announcing that as "one now passes" is exactly the wrong reading, and #120 is the standing
|
||||||
|
# lesson about a notice that is wrong often enough to be skimmed past.
|
||||||
if [ "$failed" != "unknown" ] && [ "$failed" != "$baseline" ]; then
|
if [ "$failed" != "unknown" ] && [ "$failed" != "$baseline" ]; then
|
||||||
deviations+=("$failed tests failed, the baseline is $baseline — every test carrying the marker is expected to fail on this image, so fewer means one now passes and more means a new one joined")
|
if [ "$completed" = "**no**" ]; then
|
||||||
|
echo "::debug::$failed of $baseline marked tests failed, on a run the abort truncated — not compared"
|
||||||
|
else
|
||||||
|
deviations+=("$failed tests failed, the baseline is $baseline — every test carrying the marker is expected to fail on this image, so fewer means one now passes and more means a new one joined")
|
||||||
|
fi
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
if [ -n "$marked" ] && [ "$marked" != "$baseline" ]; then
|
if [ -n "$marked" ] && [ "$marked" != "$baseline" ]; then
|
||||||
@@ -283,7 +297,16 @@ if [ -n "$failed_names" ]; then
|
|||||||
fi
|
fi
|
||||||
if [ "$advisory" = "yes" ]; then
|
if [ "$advisory" = "yes" ]; then
|
||||||
if [ "${#deviations[@]}" -eq 0 ]; then
|
if [ "${#deviations[@]}" -eq 0 ]; then
|
||||||
echo " baseline: matches ($baseline expected, $baseline failed)"
|
# Two spellings, because one of them would be a lie half the time. `$baseline expected,
|
||||||
|
# $baseline failed` is only true of a run that finished; on a truncated one `failed` is a
|
||||||
|
# tally of the tests that got to run before the framework died, and printing the baseline in
|
||||||
|
# its place claims a number nobody measured. Seen on PR #245's advisory leg, which reported
|
||||||
|
# `failed: 4` three lines above `matches (5 expected, 5 failed)`.
|
||||||
|
if [ "$failed" != "unknown" ] && [ "$failed" != "$baseline" ]; then
|
||||||
|
echo " baseline: matches ($baseline expected; $failed of $baseline failed, on a run the abort truncated — not compared)"
|
||||||
|
else
|
||||||
|
echo " baseline: matches ($baseline expected, $baseline failed)"
|
||||||
|
fi
|
||||||
else
|
else
|
||||||
printf ' baseline DEVIATION: %s\n' "${deviations[@]}"
|
printf ' baseline DEVIATION: %s\n' "${deviations[@]}"
|
||||||
fi
|
fi
|
||||||
|
|||||||
+66
-50
@@ -52,40 +52,72 @@ WEDGE_TIMEOUT=1200
|
|||||||
# the same shape as E2E_EXTRA_GRADLE_ARGS below. The other four E2E legs run byte-identical
|
# the same shape as E2E_EXTRA_GRADLE_ARGS below. The other four E2E legs run byte-identical
|
||||||
# commands with it unset.
|
# commands with it unset.
|
||||||
#
|
#
|
||||||
# WHY IT RUNS HERE, BEFORE THE LOGCAT STREAM: `adb shell stop` ends the `adb logcat` started
|
# WHY IT RUNS HERE, BEFORE THE LOGCAT STREAM: it is a 45-second wait, and the stream below is
|
||||||
# below, and nothing restarts it, so a disable performed after that point would cost this leg
|
# meant to cover the suite rather than the wait. Everything this function counts comes from
|
||||||
# its whole diagnostic story for the part of the run that matters. Everything this function
|
# `adb logcat -d -b crash`, a fresh read each time and independent of the stream. (The original
|
||||||
# counts comes from `adb logcat -d -b crash`, which is a fresh read each time and independent
|
# reason was stronger and no longer applies: `adb shell stop` would have ended the streamed
|
||||||
# of the stream.
|
# `adb logcat` and nothing restarts it. There is no `stop` here any more -- see below.)
|
||||||
#
|
#
|
||||||
# WHAT IT IS FOR: the android-37.x images abort surfaceflinger from RegionSamplingThread inside
|
# WHAT IT IS FOR -- AND THE NAME IS NOW WRONG, WHICH IS WHY THIS PARAGRAPH IS LONG.
|
||||||
# their own gralloc mapper (docs/api-37-emulator-crash.md). surfaceflinger is a critical service,
|
# The android-37.x images abort surfaceflinger from RegionSamplingThread inside their own gralloc
|
||||||
# so init SIGKILLs zygote with it and the framework restarts under the run -- Gradle then reports
|
# 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
|
# `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
|
# because SystemUI registers a nav-bar luma-sampling listener, so this was written to remove the
|
||||||
# the whole chain. Measured cadence of those kills: 20-90 s apart, median 60-70 s, three to five
|
# package and with it the whole chain. Measured cadence of those kills on `-gpu host`: 20-90 s
|
||||||
# in a four-minute window -- fast enough that install and instrumentation start-up do not fit
|
# apart, median 60-70 s, three to five in a four-minute window.
|
||||||
# inside one gap.
|
#
|
||||||
|
# **THE DISABLE HALF OF THAT HAS NEVER WORKED, AND THE QUIET WINDOW IS WHAT THE LEG ACTUALLY
|
||||||
|
# GETS.** Measured 2026-09-05, two ways that agree:
|
||||||
|
#
|
||||||
|
# - On CI, in the gating leg of run 34006456986: `pm disable-user` is accepted at 02:28:37.9 and
|
||||||
|
# `com.android.systemui` really is in `pm list packages -d` at 02:29:33 -- and SystemUI is
|
||||||
|
# started anyway at 02:28:39.5 and again at 02:28:52.3, the second of which (pid 4275) is
|
||||||
|
# alive for the whole instrumentation run, logging `WindowManagerShell ...
|
||||||
|
# app=com.android.systemui` minutes after this function prints its final line.
|
||||||
|
# - Locally on android-37.0, with the package verified disabled before AND after a deliberate
|
||||||
|
# `stop; start`: `com.android.systemui` comes up 3 s after `system_server` regardless.
|
||||||
|
#
|
||||||
|
# So `pm disable-user --user 0 com.android.systemui` does not stop SystemUI starting on this
|
||||||
|
# image, whatever else happens. The name `E2E_DISABLE_SYSTEM_UI` and the name of this function are
|
||||||
|
# kept because the matrix row, both workflows and two documents refer to them, and a rename would
|
||||||
|
# touch all of that to no benefit -- read this comment, not the name.
|
||||||
|
#
|
||||||
|
# WHAT IS LEFT IS LOAD-BEARING, so do not delete the function as dead weight. It is the 45-second
|
||||||
|
# window with zero new `hasReadColorBufferDma` aborts. The boot-time aborts land close together --
|
||||||
|
# 02:28:18 and 02:28:43 in that same run -- and the wait is what puts instrumentation (02:32:42)
|
||||||
|
# after them rather than inside one. That is what stops a leg reporting `Starting 0 tests`, and it
|
||||||
|
# is why the three-round retry stays.
|
||||||
|
#
|
||||||
|
# THE `pm disable-user` CALL STAYS TOO, for a narrower reason than it was written for: every green
|
||||||
|
# leg and every measurement quoted anywhere about this row was taken with it applied and SystemUI
|
||||||
|
# running. Removing it would change the configuration the numbers came from, which is not a change
|
||||||
|
# to make while fixing a flake.
|
||||||
|
#
|
||||||
|
# AND THE FRAMEWORK RESTART IS GONE, having been measured to be worse than nothing. It was written
|
||||||
|
# as `adb shell stop; adb shell start`, which are root-only; adbd is not root, so every leg printed
|
||||||
|
# `Must be root` twice and restarted nothing. Adding `adb root` made it real, and api37-debug run
|
||||||
|
# 34010167885 is what that looks like: `pm disable-user` reports success, the stop lands ~2 s later
|
||||||
|
# and kills system_server before PackageManager has flushed its delayed write of package
|
||||||
|
# restrictions, so the state is gone on the way back up -- `NOT DISABLED after the restart`, three
|
||||||
|
# rounds, `final state: SystemUI STILL ENABLED`, and the leg then reported `expected: 0,
|
||||||
|
# received: 0`. A 15 s pause before the stop does make the state survive (bisected locally), and it
|
||||||
|
# still does not help, because of the two measurements above. So the restart is removed rather than
|
||||||
|
# repaired: it cost the leg every test it had, and there is nothing for it to buy.
|
||||||
#
|
#
|
||||||
# NOTHING HERE TRUSTS A COMMAND'S OWN REPORT, and that is not paranoia: of four runs of an
|
# 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
|
# 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
|
# started SystemUI eight more times. So this reports what `pm list packages -d` says AND what
|
||||||
# a system_server that is SIGKILLed before the state is written, and `pm disable-user` does not
|
# `pidof` says, side by side, rather than one line implying both.
|
||||||
# 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'; }
|
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'; }
|
systemui_disabled() { adb shell pm list packages -d 2> /dev/null | grep -q 'com.android.systemui'; }
|
||||||
|
systemui_pid() { adb shell pidof com.android.systemui 2> /dev/null | tr -d '\r\n'; }
|
||||||
|
|
||||||
disable_region_sampling() {
|
disable_region_sampling() {
|
||||||
local round=1 i out before after
|
local round=1 i out pid before after
|
||||||
while [ "$round" -le 3 ]; do
|
while [ "$round" -le 3 ]; do
|
||||||
echo "--- SystemUI disable, round $round ---"
|
echo "--- round $round ---"
|
||||||
for i in $(seq 1 10); do
|
for i in $(seq 1 10); do
|
||||||
out="$(adb shell pm disable-user --user 0 com.android.systemui 2>&1 | tr -d '\r')"
|
out="$(adb shell pm disable-user --user 0 com.android.systemui 2>&1 | tr -d '\r')"
|
||||||
echo " pm attempt $i: $out"
|
echo " pm attempt $i: $out"
|
||||||
@@ -93,36 +125,20 @@ disable_region_sampling() {
|
|||||||
sleep 5
|
sleep 5
|
||||||
done
|
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
|
if systemui_disabled; then
|
||||||
echo " verified: com.android.systemui is in pm list packages -d"
|
echo " pm list packages -d: com.android.systemui is in it"
|
||||||
else
|
else
|
||||||
echo " NOT DISABLED after the restart -- the package state did not survive"
|
echo " pm list packages -d: com.android.systemui is NOT in it"
|
||||||
round=$((round + 1))
|
|
||||||
continue
|
|
||||||
fi
|
fi
|
||||||
|
# Printed next to the line above precisely because the two disagree on this image, and a
|
||||||
|
# reader who sees only the first will believe something that is not true.
|
||||||
|
pid="$(systemui_pid)"
|
||||||
|
echo " com.android.systemui pid: ${pid:-none} (expected: a pid -- see the header)"
|
||||||
|
|
||||||
before="$(count_aborts)"
|
before="$(count_aborts)"
|
||||||
sleep 45
|
sleep 45
|
||||||
after="$(count_aborts)"
|
after="$(count_aborts)"
|
||||||
echo " abort rate, SystemUI disabled: $((after - before)) new in 45 s (total ${after:-0})"
|
echo " aborts: $((after - before)) new in 45 s (total ${after:-0})"
|
||||||
[ "$((after - before))" -eq 0 ] && break
|
[ "$((after - before))" -eq 0 ] && break
|
||||||
echo " still aborting after round $round"
|
echo " still aborting after round $round"
|
||||||
round=$((round + 1))
|
round=$((round + 1))
|
||||||
@@ -131,16 +147,16 @@ disable_region_sampling() {
|
|||||||
# A warning rather than an exit. If the disable did not take, the run is about to report
|
# 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
|
# `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.
|
# buffer and the diagnostics attached, which is more useful than dying here with none of it.
|
||||||
if systemui_disabled; then
|
if [ "$((after - before))" -eq 0 ]; then
|
||||||
echo " final state: SystemUI disabled"
|
echo " final state: 45 s with no new aborts -- the suite starts here"
|
||||||
else
|
else
|
||||||
echo "::warning::E2E api${LABEL}: SystemUI is still enabled -- expect INSTRUMENTATION_ABORTED"
|
echo "::warning::E2E api${LABEL}: still aborting after three rounds -- expect INSTRUMENTATION_ABORTED"
|
||||||
fi
|
fi
|
||||||
return 0
|
return 0
|
||||||
}
|
}
|
||||||
|
|
||||||
if [ "${E2E_DISABLE_SYSTEM_UI:-}" = "1" ]; then
|
if [ "${E2E_DISABLE_SYSTEM_UI:-}" = "1" ]; then
|
||||||
echo "::group::E2E api${LABEL} -- removing the region-sampling listener"
|
echo "::group::E2E api${LABEL} -- waiting out the boot-time gralloc aborts"
|
||||||
disable_region_sampling
|
disable_region_sampling
|
||||||
echo "::endgroup::"
|
echo "::endgroup::"
|
||||||
fi
|
fi
|
||||||
|
|||||||
@@ -28,6 +28,15 @@ name: API 37 debug
|
|||||||
# - It does not fork .github/scripts/e2e-run.sh. That script owns the FAILED-vs-WEDGED
|
# - 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
|
# 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.
|
# exercises every day. This calls it, exactly as status_check.yml does.
|
||||||
|
#
|
||||||
|
# The SystemUI disable below is the exception, and it is a real one: this workflow
|
||||||
|
# drives it from its own probe step so `disable_system_ui` can be turned off for a
|
||||||
|
# dispatch, where the real leg gets it through `E2E_DISABLE_SYSTEM_UI`. Two copies of
|
||||||
|
# that logic therefore exist and must be changed together. **This instrument is also
|
||||||
|
# what established that the disable half of it does nothing** -- run 34010167885, in
|
||||||
|
# which making its framework restart real cost the leg every test it had. Read
|
||||||
|
# .github/scripts/e2e-run.sh's header for the measurements; the restart is gone from
|
||||||
|
# both copies and what remains is the 45-second quiet window.
|
||||||
# - It does not change status_check.yml. If a configuration here turns out to work,
|
# - 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 change to the real matrix is proposed separately.
|
||||||
#
|
#
|
||||||
@@ -291,45 +300,30 @@ jobs:
|
|||||||
sleep 5
|
sleep 5
|
||||||
done
|
done
|
||||||
|
|
||||||
# pm disable-user does not retract SystemUI's existing region-sampling
|
# NO FRAMEWORK RESTART. There was one here, and making it work (it needed
|
||||||
# registration -- by the time boot completes it has already registered. Only a
|
# `adb root`) is what proved the whole disable is ineffective on this image:
|
||||||
# framework restart brings back a SystemUI-less SurfaceFlinger. See
|
# SystemUI starts anyway, measured on CI and locally, and the restart itself
|
||||||
# disable_region_sampling in tools/local-emulator/run-e2e.sh.
|
# loses the package state to PackageManager's delayed write and leaves the leg
|
||||||
echo " restarting the framework"
|
# reporting `Starting 0 tests`. e2e-run.sh's header carries the measurements.
|
||||||
adb shell stop
|
# What is left, and what is load-bearing, is the quiet window below.
|
||||||
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
|
if systemui_disabled; then
|
||||||
echo " verified: com.android.systemui is in pm list packages -d"
|
echo " pm list packages -d: com.android.systemui is in it"
|
||||||
else
|
else
|
||||||
echo " NOT DISABLED after the restart -- the package state did not survive"
|
echo " pm list packages -d: com.android.systemui is NOT in it"
|
||||||
round=$((round + 1))
|
|
||||||
continue
|
|
||||||
fi
|
fi
|
||||||
|
# Beside it, because the two disagree on this image and the first line alone
|
||||||
|
# reads as a claim about the process that is not true.
|
||||||
|
echo " com.android.systemui pid: $(adb shell pidof com.android.systemui 2> /dev/null | tr -d '\r\n')"
|
||||||
|
|
||||||
before="$(count_aborts)"
|
before="$(count_aborts)"
|
||||||
sleep 45
|
sleep 45
|
||||||
after="$(count_aborts)"
|
after="$(count_aborts)"
|
||||||
echo "--- abort rate, SystemUI disabled: $((after - before)) new in 45 s (total ${after:-0}) ---"
|
echo "--- aborts: $((after - before)) new in 45 s (total ${after:-0}) ---"
|
||||||
[ "$((after - before))" -eq 0 ] && break
|
[ "$((after - before))" -eq 0 ] && break
|
||||||
echo " still aborting after round $round"
|
echo " still aborting after round $round"
|
||||||
round=$((round + 1))
|
round=$((round + 1))
|
||||||
done
|
done
|
||||||
systemui_disabled && echo "final state: SystemUI disabled" || echo "final state: SystemUI STILL ENABLED -- expect Starting 0 tests"
|
systemui_disabled && echo "final state: com.android.systemui is disabled in pm (it still runs)" || echo "final state: com.android.systemui is not even disabled in pm"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
echo "--- crash buffer (tail 60) ---"
|
echo "--- crash buffer (tail 60) ---"
|
||||||
|
|||||||
@@ -254,31 +254,31 @@ jobs:
|
|||||||
api-level: "36"
|
api-level: "36"
|
||||||
# API 37, and it is NOT the same device as the four rows above it.
|
# 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
|
# THE CAVEAT THAT USED TO BE HERE IS WITHDRAWN, 2026-09-05, and the
|
||||||
# SystemUI disabled and the framework restarted under it. No other leg
|
# withdrawal is good news. It said this leg "runs with SystemUI disabled
|
||||||
# and no Pixel run uses that configuration. It is defensible only because
|
# and the framework restarted under it", that no other leg or Pixel run
|
||||||
# nothing THIS LEG RUNS touches system UI -- Media3, FFmpeg and
|
# uses that configuration, and that anything depending on system UI must
|
||||||
# WorkManager tests -- and because the alternative is no CI coverage of
|
# not trust this row. **None of that was ever true.** Measured: the
|
||||||
# the level this app targets. **Anything that ever does depend on system
|
# framework restart is two root-only adb commands that answered `Must be
|
||||||
# UI must not trust this row.** E2E_DISABLE_SYSTEM_UI is what does it;
|
# root` on every leg ever run, and `pm disable-user` does not stop SystemUI
|
||||||
# .github/scripts/e2e-run.sh explains the mechanism and why every step of
|
# starting on this image anyway -- in run 34006456986 the package is
|
||||||
# it is verified rather than assumed.
|
# verified disabled at 02:29:33 and SystemUI (pid 4275) is up from 02:28:52
|
||||||
|
# for the whole run. So this row's device configuration is the same as the
|
||||||
|
# other four's, and a green here means what a green on 33-36 means.
|
||||||
#
|
#
|
||||||
# "this leg" and not "this suite", since 2026-08-24, and the difference is
|
# E2E_DISABLE_SYSTEM_UI still exists and still runs, because what it
|
||||||
# now load-bearing: SafPickerRoundTripTest DOES touch system UI. It drives
|
# actually buys is a 45-second window with no new gralloc aborts before the
|
||||||
# DocumentsUI and rotates the display, and both reach the gralloc mapper
|
# suite starts -- the boot-time ones land close together and instrumentation
|
||||||
# this image aborts in -- disabling SystemUI removes the IDLE trigger, not
|
# has to begin after them, not between them. The name is stale and kept:
|
||||||
# those. Measured per method on android-37.0: the ROTATION test takes the
|
# read .github/scripts/e2e-run.sh's header, which carries the measurements.
|
||||||
# framework down (INSTRUMENTATION_ABORTED) and carries
|
|
||||||
# @FailsOnEmulatorApi37, so notAnnotation below keeps it off this row; the
|
|
||||||
# PICKER test passes and runs here like anything else. A rotation rebuilds
|
|
||||||
# every surface at once, and starting another app's activity does not.
|
|
||||||
#
|
#
|
||||||
# So this row does now run one test that depends on system UI, and the
|
# notAnnotation below keeps five tests off this row, and one of
|
||||||
# caveat above still applies to it: a green here is not evidence the picker
|
# them is new. SafPickerRoundTripTest's PICKER test was measured on
|
||||||
# works on a device with SystemUI running -- the Pixel release check is.
|
# 2026-08-24 as passing here and was left on the leg; four gating logcats
|
||||||
# docs/api-37-emulator-crash.md has the per-method measurements, and the
|
# read on 2026-09-05 show it aborting system_server from the task-snapshot
|
||||||
# correction that produced them.
|
# path on every single run, pass or fail, which is what had been failing
|
||||||
|
# unrelated PRs (#108). Both of that class's tests now carry the marker.
|
||||||
|
# docs/api-37-emulator-crash.md has the timings and the correction.
|
||||||
#
|
#
|
||||||
# api-level must be a POINT release. A bare 37 is not an SDK package and
|
# api-level must be a POINT release. A bare 37 is not an SDK package and
|
||||||
# fails during setup, which cost a run to discover. `37.0` is the choice
|
# fails during setup, which cost a run to discover. `37.0` is the choice
|
||||||
|
|||||||
@@ -76,17 +76,45 @@ days. Read it as the current answer, and see the git history if you need the old
|
|||||||
`angle_indirect` and `swangle_indirect` all boot, while `auto`, `off`, `guest` and
|
`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
|
`swiftshader_indirect` do not. `docs/local-emulator.md` has the evidence and the per-API renderer
|
||||||
table.
|
table.
|
||||||
- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Three** of the 60 instrumented
|
- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Five** of the 69 instrumented
|
||||||
tests cannot pass on that image, for two unrelated reasons: two Media3 hardware transcodes fail
|
tests cannot be *run* on that image, for three unrelated reasons: three Media3 tests fail inside
|
||||||
inside the emulator's own `c2.goldfish.h264.decoder`, and one SAF test takes the framework down
|
the emulator's own `c2.goldfish.h264.decoder`, one SAF test takes the framework down when it
|
||||||
when it rotates the display. All three carry `@FailsOnEmulatorApi37` and run in a separate
|
rotates the display, and its sibling — the SAF picker round trip — aborts `system_server` from
|
||||||
`continue-on-error` job; the gating leg runs the other 57.
|
the task-snapshot path whether it passes or not. All five carry `@FailsOnEmulatorApi37` and run
|
||||||
|
in a separate `continue-on-error` job; the gating leg runs the other 64.
|
||||||
|
|
||||||
|
**These two numbers move with the suite and are derived, not remembered.** `grep -cE
|
||||||
|
'^\s*@Test' ` over `app/src/androidTest` is the first; the second is that minus the marker
|
||||||
|
count `.github/scripts/e2e-report-shape.sh` greps. Cross-check against any run's shape rather
|
||||||
|
than trusting the sentence: a leg below 37 reports the first as `expected`, and the API 37
|
||||||
|
gating leg reports the second.
|
||||||
|
|
||||||
|
**That third reason is why "cannot pass" became "cannot be run" on 2026-09-05.** Four gating
|
||||||
|
runs were read logcat-first — 34006456986, 34001744574, 34001377499 and the green 34002313300 —
|
||||||
|
and each carries exactly two `hasReadColorBufferDma` aborts before the suite (surfaceflinger,
|
||||||
|
during boot and the SystemUI disable) and exactly **one** during it: `system_server`, thread
|
||||||
|
`TaskSnapshotPer`, always inside the picker test's window, and nothing else in the gating set
|
||||||
|
reached the mapper at all. Whether the leg went red was luck — one run passed the test and lost
|
||||||
|
the leg anyway with `failed: 0`, another passed it 0.6 s after the abort and went green. That is
|
||||||
|
#108, it cost roughly a third of the gating legs over the wave-4 landings (#190), and a marker
|
||||||
|
is what it needed. `docs/api-37-emulator-crash.md` has the timings.
|
||||||
|
|
||||||
|
**A second thing came out of those logcats, and it withdraws a caveat rather than adding one.**
|
||||||
|
The API 37 row was documented as the one leg running "with SystemUI disabled and the framework
|
||||||
|
restarted under it", which nothing else does. Neither half was ever happening: `adb shell stop`
|
||||||
|
and `start` are root-only and answered `Must be root` on every leg ever run, and `pm
|
||||||
|
disable-user` does not stop SystemUI starting on this image anyway — measured on CI and locally,
|
||||||
|
with and without a real restart. **So this row's device configuration is the same as the other
|
||||||
|
four's, and a green here means what a green at 33–36 means.** `E2E_DISABLE_SYSTEM_UI` is kept
|
||||||
|
under its now-stale name because what it really buys is a 45-second window with no new gralloc
|
||||||
|
aborts before the suite starts, which is load-bearing; `.github/scripts/e2e-run.sh`'s header is
|
||||||
|
where that is written down.
|
||||||
|
|
||||||
That job is still called `E2E API 37 Media3 hardware transcode (advisory)`, which no longer
|
That job is still called `E2E API 37 Media3 hardware transcode (advisory)`, which no longer
|
||||||
describes everything in it. The name is kept deliberately — it is not a required context and
|
describes everything in it. The name is kept deliberately — it is not a required context and
|
||||||
people have learned to look for it — so **read the marker, not the name**, for what it holds.
|
people have learned to look for it — so **read the marker, not the name**, for what it holds.
|
||||||
**It is red on every PR, by design**: do not read it as your change breaking something, and do
|
**It 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 three tests pass.
|
not read a green run as evidence those five tests pass.
|
||||||
`docs/api-37-emulator-crash.md` has the measurements.
|
`docs/api-37-emulator-crash.md` has the measurements.
|
||||||
|
|
||||||
**That instruction is also why nobody looks, so the job now reports its own shape** — expected,
|
**That instruction is also why nobody looks, so the job now reports its own shape** — expected,
|
||||||
@@ -106,7 +134,7 @@ days. Read it as the current answer, and see the git history if you need the old
|
|||||||
is gradle never returning, so the log it left says nothing about it.
|
is gradle never returning, so the log it left says nothing about it.
|
||||||
|
|
||||||
Still true, and the reason the advisory job is not simply deleted: **API 37 needs a manual check on
|
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.** Those three tests are the one thing CI cannot answer
|
the Pixel 10 Pro XL before each release.** Those five tests are the one thing CI cannot answer
|
||||||
for.
|
for.
|
||||||
|
|
||||||
On a device or emulator, build only the ABI it can execute:
|
On a device or emulator, build only the ABI it can execute:
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
package org.libremediaconverter
|
package org.libremediaconverter
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Marks an instrumented test that does not pass on the `android-37.x` **emulator** system images.
|
* Marks an instrumented test that cannot be run 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
|
* 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
|
* `notAnnotation` to build the gating API 37 leg, and once with `annotation` to build the advisory
|
||||||
@@ -9,6 +9,15 @@ package org.libremediaconverter
|
|||||||
* That is the whole reason there is one annotation rather than a pair of test lists: two lists
|
* 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).
|
* drift, and the drift is silent in both directions (a test that runs nowhere reads as green).
|
||||||
*
|
*
|
||||||
|
* **"Cannot be run" covers two things, and it said only the first until 2026-09-05.** Four of the
|
||||||
|
* five carriers simply fail: three Media3 tests die in the image's own `c2.goldfish.h264.decoder`,
|
||||||
|
* and the SAF rotation test takes the framework down with it. The fifth —
|
||||||
|
* `SafPickerRoundTripTest.pickingAFileThroughTheSystemPickerFillsInTheFileCard` — **passes about
|
||||||
|
* half the time and aborts `system_server` every time**, which is worse for a gating leg than an
|
||||||
|
* honest failure: it fails the leg from the teardown, with no failing test to point at (#108).
|
||||||
|
* The wording was widened rather than the test excused; that test's own KDoc has the four-run
|
||||||
|
* measurement.
|
||||||
|
*
|
||||||
* It says only what has been measured: **on the emulator, at API 37.** The same tests pass on a
|
* 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,
|
* 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
|
* so this must never be read as "this test is allowed to fail at API 37" — only as "the API 37
|
||||||
@@ -37,11 +46,28 @@ annotation class FailsOnEmulatorApi37
|
|||||||
* keep printing with nothing to compare to, so it announces that it could not read the baseline
|
* keep printing with nothing to compare to, so it announces that it could not read the baseline
|
||||||
* rather than falling quiet. If you see that notice, this line is what it means.
|
* rather than falling quiet. If you see that notice, this line is what it means.
|
||||||
*
|
*
|
||||||
* **One number, both checks, and that is what the marker means.** A test carrying it cannot pass
|
* **One number, both checks, and that is what the marker means.** A test carrying it cannot be run
|
||||||
* on this image, so the count is simultaneously how many the advisory leg runs and how many fail.
|
* on this image, so the count is simultaneously how many the advisory leg runs and how many fail.
|
||||||
* A *smaller* failure count is the interesting direction: it means one of them now passes, which
|
* A *smaller* failure count is the interesting direction: it means one of them now passes, which
|
||||||
* is the trigger the KDoc above names for deleting the annotation.
|
* is the trigger the KDoc above names for deleting the annotation.
|
||||||
*
|
*
|
||||||
|
* **The picker test is the one to read that sentence carefully for.**
|
||||||
|
* `pickingAFileThroughTheSystemPickerFillsInTheFileCard` was marked on 2026-09-05 for aborting
|
||||||
|
* `system_server` rather than for failing (#108), and on the gating leg it passed two runs of
|
||||||
|
* four. It fails on the advisory leg because the rotation test runs before it and takes the
|
||||||
|
* framework down first — measured, `api37-debug.yml` run 34008889182, which reports
|
||||||
|
* `expected: 4, received: 4, failed: 4` with the four in the order Media3, Media3, rotation,
|
||||||
|
* picker. (Those dispatches predate the third Media3 marker landing on `main`, so their totals
|
||||||
|
* are four rather than five; the ordering they establish is what matters here.)
|
||||||
|
*
|
||||||
|
* **But a second dispatch of the identical configuration reported 4/3/3**, having lost the last
|
||||||
|
* test to the abort rather than to anything about the test list, and that is why
|
||||||
|
* `e2e-report-shape.sh` compares `failed` only on a run that finished. `expected` is compared
|
||||||
|
* always — it comes from `Starting N tests`, which is printed before anything can abort, so it is
|
||||||
|
* the field that answers "is the marked set the size this number says". Read a *clean* run
|
||||||
|
* reporting fewer failures than this as one of them now passing; read a truncated one as the
|
||||||
|
* framework having died, which is this job's normal.
|
||||||
|
*
|
||||||
* So: adding or removing a [FailsOnEmulatorApi37] means changing this number, in this file, in
|
* So: adding or removing a [FailsOnEmulatorApi37] means changing this number, in this file, in
|
||||||
* the same diff. The report says so on the run itself if you forget — it prints the tree's own
|
* the same diff. The report says so on the run itself if you forget — it prints the tree's own
|
||||||
* `grep` count beside this one.
|
* `grep` count beside this one.
|
||||||
@@ -52,4 +78,4 @@ annotation class FailsOnEmulatorApi37
|
|||||||
* `INSTRUMENTATION_ABORTED`, so the count is a number taken from a partial run. The report
|
* `INSTRUMENTATION_ABORTED`, so the count is a number taken from a partial run. The report
|
||||||
* records the truncation next to the counts for that reason.
|
* records the truncation next to the counts for that reason.
|
||||||
*/
|
*/
|
||||||
const val FAILS_ON_EMULATOR_API37_BASELINE = 4
|
const val FAILS_ON_EMULATOR_API37_BASELINE = 6
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ import org.junit.runner.RunWith
|
|||||||
import org.libremediaconverter.convert.MediaProbe
|
import org.libremediaconverter.convert.MediaProbe
|
||||||
import org.libremediaconverter.convert.StagingNames
|
import org.libremediaconverter.convert.StagingNames
|
||||||
import org.libremediaconverter.ffmpeg.ConcatEngine
|
import org.libremediaconverter.ffmpeg.ConcatEngine
|
||||||
|
import org.libremediaconverter.ffmpeg.FFmpegEngine
|
||||||
import org.libremediaconverter.model.ConcatStrategy
|
import org.libremediaconverter.model.ConcatStrategy
|
||||||
import org.libremediaconverter.work.ConcatWorker
|
import org.libremediaconverter.work.ConcatWorker
|
||||||
import java.io.File
|
import java.io.File
|
||||||
@@ -236,6 +237,55 @@ class ConcatEngineTest {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A failed join tells the user the return code and what FFmpeg said.
|
||||||
|
*
|
||||||
|
* **This is the device half of #203/#217**, whose PR closed by noting the join legs had not
|
||||||
|
* been run. Running them would not have answered it: nothing on either source set drove a real
|
||||||
|
* join *failure*, so the unified message was asserted only against values a JVM test hands to
|
||||||
|
* `sessionOutcome` directly.
|
||||||
|
*
|
||||||
|
* What is device-only here is that the three reads behind that message work against a real
|
||||||
|
* native session at all — `getReturnCode`, `getFailStackTrace` and `getAllLogsAsString`. If
|
||||||
|
* the log tail came back null or empty on a device, the user would get `Joining failed (1): `
|
||||||
|
* with nothing after the colon and every JVM test would still pass.
|
||||||
|
*
|
||||||
|
* **What this deliberately does not pin is the preference between the two detail sources.** On
|
||||||
|
* an ordinary non-zero return code FFmpegKit reports no fail stack trace, so the stack-trace-
|
||||||
|
* first rule and the log-tail-first rule produce the same text and no assertion here can tell
|
||||||
|
* them apart. That ordering is [SessionOutcomeTest][org.libremediaconverter.ffmpeg.SessionOutcomeTest]'s
|
||||||
|
* job, where both sources can be non-blank at once. Asserting it here would be a test whose
|
||||||
|
* KDoc claims more than it checks — the `probeForConcat` mistake wave 3 caught.
|
||||||
|
*
|
||||||
|
* The failure is forced with an input that does not exist, which the concat demuxer rejects
|
||||||
|
* the same way on every FFmpeg build, rather than with malformed media whose handling varies.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun aFailedJoinReportsTheReturnCodeAndWhatFFmpegSaid(): Unit = runBlocking {
|
||||||
|
val missing = File(context.cacheDir, "no_such_clip.mp4").also { it.delete() }
|
||||||
|
val out = output("joined_failure.mp4")
|
||||||
|
|
||||||
|
val failure = runCatching {
|
||||||
|
engine.join(listOf(Uri.fromFile(clipA), Uri.fromFile(missing)), out)
|
||||||
|
}.exceptionOrNull()
|
||||||
|
|
||||||
|
assertTrue(
|
||||||
|
"a join over a missing input must fail, got $failure",
|
||||||
|
failure is FFmpegEngine.FFmpegException,
|
||||||
|
)
|
||||||
|
val message = failure?.message.orEmpty()
|
||||||
|
assertTrue(
|
||||||
|
"the message must name the operation and carry the return code, was: '$message'",
|
||||||
|
message.startsWith("Joining failed ("),
|
||||||
|
)
|
||||||
|
// The half a JVM test cannot reach: a real session actually produced detail to show.
|
||||||
|
val detail = message.substringAfter("): ", "")
|
||||||
|
assertTrue(
|
||||||
|
"the message stopped at the return code and told the user nothing, was: '$message'",
|
||||||
|
detail.isNotBlank(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun theListFileIsCleanedUpAfterJoining(): Unit = runBlocking {
|
fun theListFileIsCleanedUpAfterJoining(): Unit = runBlocking {
|
||||||
val out = output("joined_cleanup.mp4")
|
val out = output("joined_cleanup.mp4")
|
||||||
|
|||||||
+108
-4
@@ -14,6 +14,8 @@ import java.io.FileOutputStream;
|
|||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.io.InputStream;
|
import java.io.InputStream;
|
||||||
import java.io.OutputStream;
|
import java.io.OutputStream;
|
||||||
|
import java.util.ArrayList;
|
||||||
|
import java.util.List;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* One file, offered to the system file picker, so that picking one can be tested at all.
|
* One file, offered to the system file picker, so that picking one can be tested at all.
|
||||||
@@ -104,6 +106,18 @@ public final class FixtureDocumentsProvider extends DocumentsProvider {
|
|||||||
private static final String ROOT_DOCUMENT_ID = "root";
|
private static final String ROOT_DOCUMENT_ID = "root";
|
||||||
private static final String FIXTURE_DOCUMENT_ID = "root/" + FIXTURE_DISPLAY_NAME;
|
private static final String FIXTURE_DOCUMENT_ID = "root/" + FIXTURE_DISPLAY_NAME;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Prefix for documents this provider CREATES, as opposed to the one it serves for reading.
|
||||||
|
*
|
||||||
|
* <p>Two namespaces rather than one so a destination can never be confused with the fixture.
|
||||||
|
* The fixture is read-only and must stay that way for the picker tests; a destination is
|
||||||
|
* writable and deletable, which is what {@code PublishToRealSafDestinationTest} needs.
|
||||||
|
*/
|
||||||
|
public static final String DESTINATION_PREFIX = "dest/";
|
||||||
|
|
||||||
|
/** Document ids {@link #deleteDocument} was called with, newest last. Cleared by {@link #reset}. */
|
||||||
|
private static final List<String> DELETED = new ArrayList<>();
|
||||||
|
|
||||||
/** Already in this source set, and already a real H.264 MP4 the engines can open. */
|
/** Already in this source set, and already a real H.264 MP4 the engines can open. */
|
||||||
private static final String FIXTURE_ASSET = "sample_h264.mp4";
|
private static final String FIXTURE_ASSET = "sample_h264.mp4";
|
||||||
|
|
||||||
@@ -147,7 +161,7 @@ public final class FixtureDocumentsProvider extends DocumentsProvider {
|
|||||||
.add(Root.COLUMN_TITLE, ROOT_TITLE)
|
.add(Root.COLUMN_TITLE, ROOT_TITLE)
|
||||||
.add(Root.COLUMN_SUMMARY, "Instrumentation fixture")
|
.add(Root.COLUMN_SUMMARY, "Instrumentation fixture")
|
||||||
.add(Root.COLUMN_MIME_TYPES, FIXTURE_MIME_TYPE)
|
.add(Root.COLUMN_MIME_TYPES, FIXTURE_MIME_TYPE)
|
||||||
.add(Root.COLUMN_FLAGS, Root.FLAG_LOCAL_ONLY)
|
.add(Root.COLUMN_FLAGS, Root.FLAG_LOCAL_ONLY | Root.FLAG_SUPPORTS_CREATE)
|
||||||
.add(Root.COLUMN_ICON, android.R.drawable.ic_menu_gallery);
|
.add(Root.COLUMN_ICON, android.R.drawable.ic_menu_gallery);
|
||||||
return cursor;
|
return cursor;
|
||||||
}
|
}
|
||||||
@@ -159,6 +173,8 @@ public final class FixtureDocumentsProvider extends DocumentsProvider {
|
|||||||
addDirectoryRow(cursor);
|
addDirectoryRow(cursor);
|
||||||
} else if (FIXTURE_DOCUMENT_ID.equals(documentId)) {
|
} else if (FIXTURE_DOCUMENT_ID.equals(documentId)) {
|
||||||
addFixtureRow(cursor);
|
addFixtureRow(cursor);
|
||||||
|
} else if (documentId != null && documentId.startsWith(DESTINATION_PREFIX)) {
|
||||||
|
addDestinationRow(cursor, documentId);
|
||||||
} else {
|
} else {
|
||||||
throw new FileNotFoundException("no such document: " + documentId);
|
throw new FileNotFoundException("no such document: " + documentId);
|
||||||
}
|
}
|
||||||
@@ -178,10 +194,76 @@ public final class FixtureDocumentsProvider extends DocumentsProvider {
|
|||||||
@Override
|
@Override
|
||||||
public ParcelFileDescriptor openDocument(String documentId, String mode, CancellationSignal signal)
|
public ParcelFileDescriptor openDocument(String documentId, String mode, CancellationSignal signal)
|
||||||
throws FileNotFoundException {
|
throws FileNotFoundException {
|
||||||
if (!FIXTURE_DOCUMENT_ID.equals(documentId)) {
|
if (FIXTURE_DOCUMENT_ID.equals(documentId)) {
|
||||||
|
return ParcelFileDescriptor.open(fixtureFile(), ParcelFileDescriptor.MODE_READ_ONLY);
|
||||||
|
}
|
||||||
|
if (documentId == null || !documentId.startsWith(DESTINATION_PREFIX)) {
|
||||||
throw new FileNotFoundException("no such document: " + documentId);
|
throw new FileNotFoundException("no such document: " + documentId);
|
||||||
}
|
}
|
||||||
return ParcelFileDescriptor.open(fixtureFile(), ParcelFileDescriptor.MODE_READ_ONLY);
|
int flags = "r".equals(mode)
|
||||||
|
? ParcelFileDescriptor.MODE_READ_ONLY
|
||||||
|
: ParcelFileDescriptor.MODE_READ_WRITE | ParcelFileDescriptor.MODE_TRUNCATE;
|
||||||
|
return ParcelFileDescriptor.open(destinationFile(documentId), flags);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Creates a real, empty file and reports the document id for it.
|
||||||
|
*
|
||||||
|
* <p><b>Empty is the whole point, and this provider does not get to decide it.</b> The premise
|
||||||
|
* under test in {@code PublishToRealSafDestinationTest} is what <i>DocumentsUI</i> hands back
|
||||||
|
* from {@code ACTION_CREATE_DOCUMENT}, and {@code OutputPublisher.destinationIsKnownEmpty}
|
||||||
|
* authorises its cleanup delete only on a positive zero. This creates the file and writes
|
||||||
|
* nothing to it, which is what the SAF contract documents; the test asserts what actually came
|
||||||
|
* back rather than trusting either side.
|
||||||
|
*/
|
||||||
|
@Override
|
||||||
|
public String createDocument(String parentDocumentId, String mimeType, String displayName)
|
||||||
|
throws FileNotFoundException {
|
||||||
|
if (!ROOT_DOCUMENT_ID.equals(parentDocumentId)) {
|
||||||
|
throw new FileNotFoundException("cannot create in: " + parentDocumentId);
|
||||||
|
}
|
||||||
|
String documentId = DESTINATION_PREFIX + displayName;
|
||||||
|
File file = destinationFile(documentId);
|
||||||
|
try {
|
||||||
|
if (!file.createNewFile() && !file.exists()) {
|
||||||
|
throw new FileNotFoundException("could not create: " + documentId);
|
||||||
|
}
|
||||||
|
} catch (IOException e) {
|
||||||
|
throw new FileNotFoundException("could not create " + documentId + ": " + e);
|
||||||
|
}
|
||||||
|
return documentId;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void deleteDocument(String documentId) throws FileNotFoundException {
|
||||||
|
if (documentId == null || !documentId.startsWith(DESTINATION_PREFIX)) {
|
||||||
|
throw new FileNotFoundException("refusing to delete: " + documentId);
|
||||||
|
}
|
||||||
|
synchronized (DELETED) {
|
||||||
|
DELETED.add(documentId);
|
||||||
|
}
|
||||||
|
destinationFile(documentId).delete();
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Document ids {@link #deleteDocument} was called with, newest last. */
|
||||||
|
public static List<String> deletedDocumentIds() {
|
||||||
|
synchronized (DELETED) {
|
||||||
|
return new ArrayList<>(DELETED);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Forgets recorded deletes and removes created destinations. The process outlives one class. */
|
||||||
|
public static void reset(File filesDir) {
|
||||||
|
synchronized (DELETED) {
|
||||||
|
DELETED.clear();
|
||||||
|
}
|
||||||
|
File dir = new File(filesDir, "destinations");
|
||||||
|
File[] children = dir.listFiles();
|
||||||
|
if (children != null) {
|
||||||
|
for (File child : children) {
|
||||||
|
child.delete();
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private void addDirectoryRow(MatrixCursor cursor) {
|
private void addDirectoryRow(MatrixCursor cursor) {
|
||||||
@@ -189,10 +271,32 @@ public final class FixtureDocumentsProvider extends DocumentsProvider {
|
|||||||
.add(Document.COLUMN_DOCUMENT_ID, ROOT_DOCUMENT_ID)
|
.add(Document.COLUMN_DOCUMENT_ID, ROOT_DOCUMENT_ID)
|
||||||
.add(Document.COLUMN_DISPLAY_NAME, ROOT_TITLE)
|
.add(Document.COLUMN_DISPLAY_NAME, ROOT_TITLE)
|
||||||
.add(Document.COLUMN_MIME_TYPE, Document.MIME_TYPE_DIR)
|
.add(Document.COLUMN_MIME_TYPE, Document.MIME_TYPE_DIR)
|
||||||
.add(Document.COLUMN_FLAGS, 0)
|
.add(Document.COLUMN_FLAGS, Document.FLAG_DIR_SUPPORTS_CREATE)
|
||||||
.add(Document.COLUMN_SIZE, null);
|
.add(Document.COLUMN_SIZE, null);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private void addDestinationRow(MatrixCursor cursor, String documentId) throws FileNotFoundException {
|
||||||
|
File file = destinationFile(documentId);
|
||||||
|
if (!file.exists()) {
|
||||||
|
throw new FileNotFoundException("no such document: " + documentId);
|
||||||
|
}
|
||||||
|
cursor.newRow()
|
||||||
|
.add(Document.COLUMN_DOCUMENT_ID, documentId)
|
||||||
|
.add(Document.COLUMN_DISPLAY_NAME, documentId.substring(DESTINATION_PREFIX.length()))
|
||||||
|
.add(Document.COLUMN_MIME_TYPE, FIXTURE_MIME_TYPE)
|
||||||
|
.add(Document.COLUMN_FLAGS, Document.FLAG_SUPPORTS_DELETE | Document.FLAG_SUPPORTS_WRITE)
|
||||||
|
.add(Document.COLUMN_SIZE, file.length())
|
||||||
|
.add(Document.COLUMN_LAST_MODIFIED, file.lastModified());
|
||||||
|
}
|
||||||
|
|
||||||
|
private File destinationFile(String documentId) throws FileNotFoundException {
|
||||||
|
File dir = new File(getContext().getFilesDir(), "destinations");
|
||||||
|
if (!dir.isDirectory() && !dir.mkdirs()) {
|
||||||
|
throw new FileNotFoundException("could not make the destinations directory");
|
||||||
|
}
|
||||||
|
return new File(dir, documentId.substring(DESTINATION_PREFIX.length()));
|
||||||
|
}
|
||||||
|
|
||||||
private void addFixtureRow(MatrixCursor cursor) throws FileNotFoundException {
|
private void addFixtureRow(MatrixCursor cursor) throws FileNotFoundException {
|
||||||
File file = fixtureFile();
|
File file = fixtureFile();
|
||||||
cursor.newRow()
|
cursor.newRow()
|
||||||
|
|||||||
@@ -1,12 +1,18 @@
|
|||||||
package org.libremediaconverter.saf
|
package org.libremediaconverter.saf
|
||||||
|
|
||||||
import android.app.UiAutomation
|
import android.app.UiAutomation
|
||||||
|
import android.content.Context
|
||||||
|
import android.net.Uri
|
||||||
|
import android.provider.DocumentsContract
|
||||||
|
import android.provider.OpenableColumns
|
||||||
import androidx.compose.ui.test.ComposeTimeoutException
|
import androidx.compose.ui.test.ComposeTimeoutException
|
||||||
|
import androidx.compose.ui.test.assertIsEnabled
|
||||||
import androidx.compose.ui.test.assertTextEquals
|
import androidx.compose.ui.test.assertTextEquals
|
||||||
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
||||||
import androidx.compose.ui.test.onAllNodesWithTag
|
import androidx.compose.ui.test.onAllNodesWithTag
|
||||||
import androidx.compose.ui.test.onNodeWithTag
|
import androidx.compose.ui.test.onNodeWithTag
|
||||||
import androidx.compose.ui.test.performClick
|
import androidx.compose.ui.test.performClick
|
||||||
|
import androidx.compose.ui.test.performScrollTo
|
||||||
import androidx.media3.common.util.UnstableApi
|
import androidx.media3.common.util.UnstableApi
|
||||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||||
import androidx.test.platform.app.InstrumentationRegistry
|
import androidx.test.platform.app.InstrumentationRegistry
|
||||||
@@ -20,14 +26,22 @@ import androidx.test.uiautomator.StaleObjectException
|
|||||||
import androidx.test.uiautomator.UiDevice
|
import androidx.test.uiautomator.UiDevice
|
||||||
import androidx.test.uiautomator.Until
|
import androidx.test.uiautomator.Until
|
||||||
import org.junit.After
|
import org.junit.After
|
||||||
|
import org.junit.Assert.assertArrayEquals
|
||||||
|
import org.junit.Assert.assertEquals
|
||||||
import org.junit.Assert.assertNotEquals
|
import org.junit.Assert.assertNotEquals
|
||||||
|
import org.junit.Assert.assertNotNull
|
||||||
|
import org.junit.Assert.assertTrue
|
||||||
import org.junit.Rule
|
import org.junit.Rule
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
import org.junit.runner.RunWith
|
import org.junit.runner.RunWith
|
||||||
import org.libremediaconverter.FailsOnEmulatorApi37
|
import org.libremediaconverter.FailsOnEmulatorApi37
|
||||||
import org.libremediaconverter.MainActivity
|
import org.libremediaconverter.MainActivity
|
||||||
|
import org.libremediaconverter.convert.ConversionDependencies
|
||||||
|
import org.libremediaconverter.convert.OutputPublisher
|
||||||
import org.libremediaconverter.ui.TestTags
|
import org.libremediaconverter.ui.TestTags
|
||||||
|
import java.io.File
|
||||||
import java.util.concurrent.atomic.AtomicInteger
|
import java.util.concurrent.atomic.AtomicInteger
|
||||||
|
import java.util.regex.Pattern
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Choosing a file, through the real system picker, and still having it after a rotation.
|
* Choosing a file, through the real system picker, and still having it after a rotation.
|
||||||
@@ -241,12 +255,72 @@ import java.util.concurrent.atomic.AtomicInteger
|
|||||||
* file".** That is what API 33 through 36 are for, and they answer it.
|
* file".** That is what API 33 through 36 are for, and they answer it.
|
||||||
*/
|
*/
|
||||||
@UnstableApi
|
@UnstableApi
|
||||||
|
/**
|
||||||
|
* Reads what SAF handed back, then publishes for real.
|
||||||
|
*
|
||||||
|
* The premise `OutputPublisher.destinationIsKnownEmpty` depends on has only ever been asserted
|
||||||
|
* against a fake built to match it — `OutputPublisherPublishTest` writes `ByteArray(0)` into
|
||||||
|
* `FakeSafProvider` before each case, under a comment stating this is how `CreateDocument` behaves.
|
||||||
|
* This records what stock DocumentsUI actually produced, at the moment `publish` sees it and before
|
||||||
|
* a byte is written, and then lets the real copy proceed. See #226.
|
||||||
|
*/
|
||||||
|
private class RecordingPublisher(private val app: Context) : OutputPublisher(app) {
|
||||||
|
|
||||||
|
override fun publish(staged: File, destination: Uri) {
|
||||||
|
seenDestination = destination
|
||||||
|
seenIsDocumentUri = DocumentsContract.isDocumentUri(app, destination)
|
||||||
|
seenSizeBefore = app.contentResolver
|
||||||
|
.query(destination, arrayOf(OpenableColumns.SIZE), null, null, null)
|
||||||
|
?.use { row ->
|
||||||
|
val column = row.getColumnIndex(OpenableColumns.SIZE)
|
||||||
|
if (column >= 0 && row.moveToFirst() && !row.isNull(column)) row.getLong(column) else null
|
||||||
|
}
|
||||||
|
// Read before the copy: the ViewModel deletes the staged file once publish returns.
|
||||||
|
savedBytes = staged.readBytes()
|
||||||
|
super.publish(staged, destination)
|
||||||
|
}
|
||||||
|
|
||||||
|
companion object {
|
||||||
|
var savedBytes: ByteArray = ByteArray(0)
|
||||||
|
var seenDestination: Uri? = null
|
||||||
|
var seenIsDocumentUri: Boolean? = null
|
||||||
|
var seenSizeBefore: Long? = null
|
||||||
|
|
||||||
|
fun reset() {
|
||||||
|
savedBytes = ByteArray(0)
|
||||||
|
seenDestination = null
|
||||||
|
seenIsDocumentUri = null
|
||||||
|
seenSizeBefore = null
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@RunWith(AndroidJUnit4::class)
|
@RunWith(AndroidJUnit4::class)
|
||||||
class SafPickerRoundTripTest {
|
class SafPickerRoundTripTest {
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Installs [RecordingPublisher] before the Activity exists.
|
||||||
|
*
|
||||||
|
* `ConversionViewModel` resolves its publisher through `ConversionDependencies` **at
|
||||||
|
* construction**, and the Compose rule launches `MainActivity` as part of the rule chain —
|
||||||
|
* which wraps `@Before`, so `@Before` is already too late. JUnit constructs the test instance
|
||||||
|
* before it evaluates the rules, so an initialiser is early enough, and it needs no
|
||||||
|
* `@BeforeClass` (this class's companion is private, and JUnit wants a public static there).
|
||||||
|
*
|
||||||
|
* Harmless for the other two tests: neither saves, so `publish` is never called and the
|
||||||
|
* subclass behaves exactly like `OutputPublisher`. `restoreOrientation` puts the seam back.
|
||||||
|
*/
|
||||||
|
init {
|
||||||
|
RecordingPublisher.reset()
|
||||||
|
ConversionDependencies.publisher = { RecordingPublisher(it) }
|
||||||
|
}
|
||||||
|
|
||||||
@get:Rule
|
@get:Rule
|
||||||
val composeRule = createAndroidComposeRule<MainActivity>()
|
val composeRule = createAndroidComposeRule<MainActivity>()
|
||||||
|
|
||||||
|
private val context: Context =
|
||||||
|
InstrumentationRegistry.getInstrumentation().targetContext
|
||||||
|
|
||||||
private val device: UiDevice =
|
private val device: UiDevice =
|
||||||
UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
|
UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
|
||||||
|
|
||||||
@@ -291,6 +365,8 @@ class SafPickerRoundTripTest {
|
|||||||
*/
|
*/
|
||||||
@After
|
@After
|
||||||
fun restoreOrientation() {
|
fun restoreOrientation() {
|
||||||
|
// The suite runs without Android Test Orchestrator, so a swapped seam outlives the class.
|
||||||
|
ConversionDependencies.reset()
|
||||||
ActivityLifecycleMonitorRegistry.getInstance().removeLifecycleCallback(recreationWatcher)
|
ActivityLifecycleMonitorRegistry.getInstance().removeLifecycleCallback(recreationWatcher)
|
||||||
if (!rotated) return
|
if (!rotated) return
|
||||||
device.setOrientationNatural()
|
device.setOrientationNatural()
|
||||||
@@ -298,7 +374,35 @@ class SafPickerRoundTripTest {
|
|||||||
device.waitForIdle()
|
device.waitForIdle()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* **Marked for API 37 because of what it does to the image, not because it fails there.**
|
||||||
|
*
|
||||||
|
* This is the one place the marker's KDoc phrase "cannot pass on this image" does not fit, and
|
||||||
|
* the distinction is worth keeping rather than smoothing over. Across the four gating API 37
|
||||||
|
* runs whose logcats were read on 2026-09-05 — 34006456986, 34001744574, 34001377499 and the
|
||||||
|
* green 34002313300 — the leg carries exactly two `hasReadColorBufferDma` aborts before the
|
||||||
|
* suite starts (both `surfaceflinger`, during boot and the SystemUI disable) and then exactly
|
||||||
|
* **one** during it. Every time, that one is `system_server` on the `TaskSnapshotPer` thread,
|
||||||
|
* and every time it lands inside this test's window. No other test in the gating set reaches
|
||||||
|
* the mapper at all.
|
||||||
|
*
|
||||||
|
* So this test kills the framework on that image whether it passes or not, and whether the leg
|
||||||
|
* goes red is luck: 34001377499 passed it and lost the leg anyway (`failed: 0`, teardown
|
||||||
|
* broken), 34002313300 passed it 0.6 s after the abort and went green. That is #108, and it is
|
||||||
|
* why the leg was failing on unrelated PRs.
|
||||||
|
*
|
||||||
|
* `docs/api-37-emulator-crash.md` measured this test on 2026-08-24, recorded "passes, 4 aborts
|
||||||
|
* in the window", and concluded that a rotation reaches the mapper where starting DocumentsUI
|
||||||
|
* does not. The aborts were seen; what was not drawn out is that they are this test's own and
|
||||||
|
* are not intermittent.
|
||||||
|
*
|
||||||
|
* The marker is what routes it off the gating leg and into the advisory job beside its
|
||||||
|
* rotation sibling. **It is not a statement about the picker**: the same test passes on API
|
||||||
|
* 33–36 on the same runner and on the Pixel 10 Pro XL, which is where API 37's answer comes
|
||||||
|
* from.
|
||||||
|
*/
|
||||||
@Test
|
@Test
|
||||||
|
@FailsOnEmulatorApi37
|
||||||
fun pickingAFileThroughTheSystemPickerFillsInTheFileCard() {
|
fun pickingAFileThroughTheSystemPickerFillsInTheFileCard() {
|
||||||
pickTheFixture()
|
pickTheFixture()
|
||||||
|
|
||||||
@@ -378,6 +482,173 @@ class SafPickerRoundTripTest {
|
|||||||
* are all warm and the only thing being waited on is one screen. That is what keeps the cost
|
* are all warm and the only thing being waited on is one screen. That is what keeps the cost
|
||||||
* of a genuinely absent root bounded — see the class KDoc.
|
* of a genuinely absent root bounded — see the class KDoc.
|
||||||
*/
|
*/
|
||||||
|
/**
|
||||||
|
* The save side of SAF, end to end, against a document stock DocumentsUI created (#226).
|
||||||
|
*
|
||||||
|
* ## What this settles
|
||||||
|
*
|
||||||
|
* `publish` deletes a destination it could not write to — `docs/defect-audit.md` **D4**'s fix,
|
||||||
|
* so a failed save does not leave a truncated file at the name the user chose — but only when
|
||||||
|
* that destination was **positively zero bytes** first. `destinationIsKnownEmpty` is careful
|
||||||
|
* that "I could not tell" never authorises a delete, which is right, and which makes the
|
||||||
|
* precondition load-bearing.
|
||||||
|
*
|
||||||
|
* Until now that precondition was asserted only against a fake built to match it:
|
||||||
|
* `OutputPublisherPublishTest` writes `ByteArray(0)` into `FakeSafProvider` before each case,
|
||||||
|
* under a comment stating this is how `CreateDocument` behaves. **If it is false in production,
|
||||||
|
* D4's fix is inert and every existing test still passes.** [RecordingPublisher] reads what SAF
|
||||||
|
* actually handed over, at the moment `publish` sees it and before a byte is written.
|
||||||
|
*
|
||||||
|
* ## Why it has to go through the app, and through the picker
|
||||||
|
*
|
||||||
|
* Through the **picker** because a `DocumentsProvider` cannot be reached any other way —
|
||||||
|
* measured three ways and recorded as **E7** in `docs/e2e-read-findings.md`: an unprotected one
|
||||||
|
* is refused at install, instrumentation carries the app's uid so the test APK's own identity
|
||||||
|
* is no help, and shell identity is denied too, each denial naming `ACTION_OPEN_DOCUMENT`.
|
||||||
|
*
|
||||||
|
* Through the **app** because the same constraint sinks the obvious alternative. A host
|
||||||
|
* Activity in this source set that owns a `CreateDocument` launcher cannot be started:
|
||||||
|
* `ActivityScenario` refuses with *"Intent in process org.libremediaconverter resolved to
|
||||||
|
* different process org.libremediaconverter.test"*. Instrumentation runs in the target app's
|
||||||
|
* process, so the only Activity available to drive is the app's own — which is also the more
|
||||||
|
* faithful thing to drive.
|
||||||
|
*
|
||||||
|
* ## The conversion is setup, not subject
|
||||||
|
*
|
||||||
|
* Save is only offered on `Converted`, so the test converts first. MP3 is chosen because the
|
||||||
|
* router sends it to FFmpeg unconditionally at every API level, so the setup cannot depend on
|
||||||
|
* the device's codecs — #223 is what that costs.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
@FailsOnEmulatorApi37
|
||||||
|
fun aSaveWritesToTheDocumentTheSystemPickerCreated() {
|
||||||
|
pickTheFixture()
|
||||||
|
convertToTheDefaultFormat()
|
||||||
|
|
||||||
|
saveThroughTheSystemPicker()
|
||||||
|
|
||||||
|
val destination = RecordingPublisher.seenDestination
|
||||||
|
assertNotNull("publish was never reached, so nothing was saved", destination)
|
||||||
|
assertTrue(
|
||||||
|
"SAF handed back something that is not a document URI, so publish's cleanup can " +
|
||||||
|
"never run and D4's fix is inert: $destination",
|
||||||
|
RecordingPublisher.seenIsDocumentUri == true,
|
||||||
|
)
|
||||||
|
assertEquals(
|
||||||
|
"SAF handed back a document that is not positively empty, so " +
|
||||||
|
"destinationIsKnownEmpty answers false and a failed save keeps its partial file",
|
||||||
|
0L,
|
||||||
|
RecordingPublisher.seenSizeBefore,
|
||||||
|
)
|
||||||
|
|
||||||
|
// And the bytes really arrived, which only the failure side was covered for on a device.
|
||||||
|
val staged = File(context.cacheDir, "conversions")
|
||||||
|
assertArrayEquals(
|
||||||
|
"the destination did not receive what was staged",
|
||||||
|
RecordingPublisher.savedBytes,
|
||||||
|
context.contentResolver.openInputStream(destination!!)!!.use { it.readBytes() },
|
||||||
|
)
|
||||||
|
assertTrue("staging should be empty after a successful save", staged.listFiles().isNullOrEmpty())
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Runs the conversion, leaving the screen on `Converted`.
|
||||||
|
*
|
||||||
|
* **The format is left at its default, and that is a constraint rather than laziness.**
|
||||||
|
* `ConverterScreen` registers `CreateDocument` with the *output's* MIME type, and
|
||||||
|
* [FixtureDocumentsProvider] advertises `Root.COLUMN_MIME_TYPES` of `video/mp4` — deliberately,
|
||||||
|
* so the picker's MIME filter has a mutation with a shape. DocumentsUI honours that on the save
|
||||||
|
* side too: choosing MP3 makes the destination type `audio/mpeg`, and the fixture root is then
|
||||||
|
* filtered out of the save dialog entirely. Measured, as *"the create-document dialog never
|
||||||
|
* showed LMC R38 fixtures"*. The default `MP4_H265` produces `video/mp4` and the root is
|
||||||
|
* offered.
|
||||||
|
*
|
||||||
|
* **The notification dialog is dismissed rather than pre-granted, and that is the honest
|
||||||
|
* version.** Convert never calls `convert()` directly — it launches `RequestPermission` for
|
||||||
|
* `POST_NOTIFICATIONS` and converts from the callback **whichever way the answer goes**. So the
|
||||||
|
* dialog only has to be got out of the way; denying it is a real user's path and the conversion
|
||||||
|
* still runs. Granting it programmatically was tried first and did not take —
|
||||||
|
* `GrantPermissionsActivity` appeared anyway, the click that followed went to it rather than to
|
||||||
|
* the app, and the screen sat in `Ready` with nothing enqueued.
|
||||||
|
*
|
||||||
|
* **Both taps scroll first.** On `Ready` the screen carries a file card, five pickers and then
|
||||||
|
* the button, so Convert is below the fold on a phone. `performClick` on an off-screen node
|
||||||
|
* dispatches at a position that hits nothing and throws nothing, and `assertIsEnabled` passes
|
||||||
|
* either way — the first version of this sat waiting for a `Converted` that could never come.
|
||||||
|
*/
|
||||||
|
private fun convertToTheDefaultFormat() {
|
||||||
|
composeRule.onNodeWithTag(TestTags.Converter.CONVERT)
|
||||||
|
.performScrollTo()
|
||||||
|
.assertIsEnabled()
|
||||||
|
.performClick()
|
||||||
|
|
||||||
|
dismissThePermissionDialog()
|
||||||
|
awaitNode(TestTags.SAVE_FILE, CONVERSION_TIMEOUT_MS)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Gets the `POST_NOTIFICATIONS` dialog out of the way, if this device shows one.
|
||||||
|
*
|
||||||
|
* Backing out of it is a denial, and a denial is fine here: the conversion starts either way,
|
||||||
|
* and what that costs the user is a progress notification confined to the Task Manager. Waiting
|
||||||
|
* only briefly, because on a device where the permission is already held no dialog appears at
|
||||||
|
* all and the conversion is already under way.
|
||||||
|
*/
|
||||||
|
private fun dismissThePermissionDialog() {
|
||||||
|
if (device.wait(Until.hasObject(By.pkg(PERMISSION_UI_PACKAGE)), PERMISSION_DIALOG_MS) != true) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
device.pressBack()
|
||||||
|
device.wait(Until.gone(By.pkg(PERMISSION_UI_PACKAGE)), PERMISSION_DIALOG_MS)
|
||||||
|
// And wait for the app to be in front again before anything asks Compose about it.
|
||||||
|
// Querying while another window still owns the screen raises "No compose hierarchies found
|
||||||
|
// in the app", which is what this test did on an API 35 leg: the back press had landed but
|
||||||
|
// the dialog had not finished going away.
|
||||||
|
//
|
||||||
|
// Asked of UiAutomator rather than through awaitAppFocus, which is the opposite of what the
|
||||||
|
// class KDoc argues for elsewhere and is right here: awaitAppFocus goes through
|
||||||
|
// composeRule.waitUntil, so it would raise the very error it is being used to avoid.
|
||||||
|
device.wait(Until.hasObject(By.pkg(context.packageName)), FOCUS_TIMEOUT_MS)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Taps Save and drives the create-document dialog into the fixture root.
|
||||||
|
*
|
||||||
|
* Retried whole, for the reason [pickTheFixture] documents: a dialog that came up unreadable
|
||||||
|
* cannot be recovered from inside, and a fresh one is the only answer.
|
||||||
|
*/
|
||||||
|
private fun saveThroughTheSystemPicker() {
|
||||||
|
var missing: BySelector? = null
|
||||||
|
repeat(PICK_ATTEMPTS) { attempt ->
|
||||||
|
requireAReadableScreen()
|
||||||
|
composeRule.onNodeWithTag(TestTags.SAVE_FILE).performClick()
|
||||||
|
missing = walkTheSaveDialog(
|
||||||
|
if (attempt == 0) PICKER_TIMEOUT_MS else REOPENED_TIMEOUT_MS,
|
||||||
|
)
|
||||||
|
if (missing == null) {
|
||||||
|
awaitNode(TestTags.Converter.CONVERT_ANOTHER, SAVE_TIMEOUT_MS)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
dismissThePicker()
|
||||||
|
}
|
||||||
|
throw AssertionError(
|
||||||
|
"the create-document dialog never showed $missing, in $PICK_ATTEMPTS separate " +
|
||||||
|
"dialogs (the last one left ${device.currentPackageName} in front)",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Into the fixture root, then Save. Returns the selector never found, or null. */
|
||||||
|
private fun walkTheSaveDialog(timeoutMs: Long): BySelector? {
|
||||||
|
val picker = By.pkg(DOCUMENTS_UI_PACKAGE)
|
||||||
|
val root = By.text(FixtureDocumentsProvider.ROOT_TITLE)
|
||||||
|
return when {
|
||||||
|
device.wait(Until.hasObject(picker), timeoutMs) != true -> picker
|
||||||
|
!tapPickerNode(root, timeoutMs, ifAbsent = ::openTheRootsDrawer) -> root
|
||||||
|
!tapPickerNode(SAVE_BUTTON, timeoutMs) -> SAVE_BUTTON
|
||||||
|
else -> null
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private fun pickTheFixture() {
|
private fun pickTheFixture() {
|
||||||
var missing: BySelector? = null
|
var missing: BySelector? = null
|
||||||
repeat(PICK_ATTEMPTS) { attempt ->
|
repeat(PICK_ATTEMPTS) { attempt ->
|
||||||
@@ -604,6 +875,9 @@ class SafPickerRoundTripTest {
|
|||||||
* It is also why this counts backs rather than pressing a fixed number of them. One back is
|
* It is also why this counts backs rather than pressing a fixed number of them. One back is
|
||||||
* enough from Recent and two are needed from inside the root, but a third from Recent would
|
* enough from Recent and two are needed from inside the root, but a third from Recent would
|
||||||
* finish `MainActivity` and take the rest of the test with it.
|
* finish `MainActivity` and take the rest of the test with it.
|
||||||
|
*
|
||||||
|
* **[forceStopThePicker] is the escalation after the presses, and it exists because a back
|
||||||
|
* press is not always deliverable.** See its own KDoc for the measurement.
|
||||||
*/
|
*/
|
||||||
private fun dismissThePicker() {
|
private fun dismissThePicker() {
|
||||||
repeat(BACK_PRESSES) {
|
repeat(BACK_PRESSES) {
|
||||||
@@ -618,15 +892,50 @@ class SafPickerRoundTripTest {
|
|||||||
// The check after the last press, and not a spare one: `repeat` presses on its final
|
// The check after the last press, and not a spare one: `repeat` presses on its final
|
||||||
// iteration too, so without this a dismissal that worked on the last press would still be
|
// iteration too, so without this a dismissal that worked on the last press would still be
|
||||||
// reported as a failure to close.
|
// reported as a failure to close.
|
||||||
|
if (awaitAppFocus()) return
|
||||||
|
forceStopThePicker()
|
||||||
if (!awaitAppFocus()) {
|
if (!awaitAppFocus()) {
|
||||||
throw AssertionError(
|
throw AssertionError(
|
||||||
"the system picker would not close: after $BACK_PRESSES back presses the app " +
|
"the system picker would not close: after $BACK_PRESSES back presses and a " +
|
||||||
"still does not have the window focus, and ${device.currentPackageName} is " +
|
"force-stop of $DOCUMENTS_UI_PACKAGE the app still does not have the window " +
|
||||||
"in front. What could be seen: " + describeWindows(),
|
"focus, and ${device.currentPackageName} is in front. What could be seen: " +
|
||||||
|
describeWindows(),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Kills the picker's process, for when no back press can reach it.
|
||||||
|
*
|
||||||
|
* **The failure this exists for cannot be answered with input, and that is the whole point.**
|
||||||
|
* Measured on the gating API 37 legs of runs 34006456986 and 34001744574, which fail this way
|
||||||
|
* and whose logcats say the same thing in the same order. `UiObject2.click()` on the fixture's
|
||||||
|
* root is injected at the node's centre and the framework discards it —
|
||||||
|
* `InputDispatcher: No new touched window at (539.0, 525.0) in display 0` — because
|
||||||
|
* `PickActivity` has published accessibility nodes but has no touchable window there yet.
|
||||||
|
* `click()` cannot see that and returns normally, so the walk goes on to wait out
|
||||||
|
* [PICKER_TIMEOUT_MS] for a fixture that was never navigated to. By the time this function's
|
||||||
|
* caller starts pressing back, WindowManager is still saying
|
||||||
|
* `no window has focus but ...PickActivity may eventually add a window when it finishes
|
||||||
|
* starting up` — and goes on saying it for another 63 s. Every one of the four presses is
|
||||||
|
* dropped, and DocumentsUI ANRs on `Input dispatching timed out`.
|
||||||
|
*
|
||||||
|
* So the picker is in front, unreachable by key or by touch, and [pickTheFixture]'s whole
|
||||||
|
* point — that a second `PickActivity` rebuilds every window and list in it — is unreachable
|
||||||
|
* with it. `am force-stop` goes around input entirely: `UiAutomation` runs shell commands as
|
||||||
|
* uid 2000, which holds `FORCE_STOP_PACKAGES`, so the picker's process is killed, its
|
||||||
|
* activity leaves the task it was launched into, and `MainActivity` — the activity below it in
|
||||||
|
* that same task — is resumed with the focus.
|
||||||
|
*
|
||||||
|
* **Only on the failure path**, after every back press has been spent, so a picker that closes
|
||||||
|
* the ordinary way never reaches this and is not altered by it. If the framework itself is
|
||||||
|
* gone, this cannot help either, and the caller still reports what it could see.
|
||||||
|
*/
|
||||||
|
private fun forceStopThePicker() {
|
||||||
|
device.executeShellCommand("am force-stop $DOCUMENTS_UI_PACKAGE")
|
||||||
|
device.waitForIdle()
|
||||||
|
}
|
||||||
|
|
||||||
/** True once [MainActivity] has the window focus, false if it does not take it in time. */
|
/** True once [MainActivity] has the window focus, false if it does not take it in time. */
|
||||||
private fun awaitAppFocus(): Boolean = try {
|
private fun awaitAppFocus(): Boolean = try {
|
||||||
composeRule.waitUntil("the app has the window focus back", FOCUS_TIMEOUT_MS) {
|
composeRule.waitUntil("the app has the window focus back", FOCUS_TIMEOUT_MS) {
|
||||||
@@ -729,8 +1038,8 @@ class SafPickerRoundTripTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun awaitNode(tag: String) {
|
private fun awaitNode(tag: String, timeoutMs: Long = APP_TIMEOUT_MS) {
|
||||||
composeRule.waitUntil("a node tagged $tag exists", APP_TIMEOUT_MS) {
|
composeRule.waitUntil("a node tagged $tag exists", timeoutMs) {
|
||||||
composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().isNotEmpty()
|
composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().isNotEmpty()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -745,6 +1054,24 @@ class SafPickerRoundTripTest {
|
|||||||
const val PICKER_TIMEOUT_MS = 30_000L
|
const val PICKER_TIMEOUT_MS = 30_000L
|
||||||
const val APP_TIMEOUT_MS = 30_000L
|
const val APP_TIMEOUT_MS = 30_000L
|
||||||
|
|
||||||
|
/** The runtime-permission dialog's package, so it can be recognised and dismissed. */
|
||||||
|
const val PERMISSION_UI_PACKAGE = "com.google.android.permissioncontroller"
|
||||||
|
|
||||||
|
/** Short: either the dialog is up almost immediately, or the permission was already held. */
|
||||||
|
const val PERMISSION_DIALOG_MS = 5_000L
|
||||||
|
|
||||||
|
/** A 3 s clip to MP3 on an emulator is about a second; this only bounds a hang. */
|
||||||
|
const val CONVERSION_TIMEOUT_MS = 120_000L
|
||||||
|
|
||||||
|
/** The copy is a few kilobytes, but it crosses a provider. */
|
||||||
|
const val SAVE_TIMEOUT_MS = 30_000L
|
||||||
|
|
||||||
|
/**
|
||||||
|
* DocumentsUI's save button. Case-insensitive because the label is "SAVE" on some images
|
||||||
|
* and "Save" on others, and the difference is not what this test is about.
|
||||||
|
*/
|
||||||
|
val SAVE_BUTTON: BySelector = By.text(Pattern.compile("save", Pattern.CASE_INSENSITIVE))
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The same wait once a picker has already come and gone, and shorter for a reason.
|
* The same wait once a picker has already come and gone, and shorter for a reason.
|
||||||
*
|
*
|
||||||
|
|||||||
+189
-18
@@ -315,25 +315,90 @@ clean zero. Its own post-disable check on the run recorded below printed
|
|||||||
|
|
||||||
So what is reliably achieved is a **rate collapse** — from roughly one abort every fourteen
|
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
|
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
|
might not.
|
||||||
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
|
**And that restart has never happened — which is how the disable turned out not to work either.**
|
||||||
`sys.boot_completed` flips, SystemUI has **already registered**, and `pm disable-user` does not
|
Corrected 2026-09-05; this replaces the two paragraphs above rather than qualifying them.
|
||||||
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
|
`adb shell stop` and `start` are root-only, adbd is not root on a booted emulator, and all three
|
||||||
`stop; start` afterwards, so the framework that comes back never starts SystemUI at all.
|
copies of this logic called them without `adb root`. On CI both printed `Must be root`, between
|
||||||
|
lines that read as if the restart had happened; `run-e2e.sh` sent them to `/dev/null`, so its
|
||||||
|
`Must be root` was never even visible. Neither number in those logs was an observation either —
|
||||||
|
the `pidof` loop breaks when the process is gone and otherwise falls out at its last iteration,
|
||||||
|
and the old code printed the iteration count either way, so `system_server down after ~40 s` is
|
||||||
|
what a stop that did nothing looks like.
|
||||||
|
|
||||||
|
Adding `adb root` made the restart real, and **that is what proved the disable ineffective**.
|
||||||
|
`api37-debug` run 34010167885, `disable_system_ui=true`:
|
||||||
|
|
||||||
|
```
|
||||||
|
--- disable round 1 ---
|
||||||
|
pm attempt 1: Package com.android.systemui new state: disabled-user
|
||||||
|
restarting the framework
|
||||||
|
adbd is running as root
|
||||||
|
system_server down after 2 s
|
||||||
|
services back after 10 s
|
||||||
|
NOT DISABLED after the restart -- the package state did not survive
|
||||||
|
```
|
||||||
|
|
||||||
|
Three rounds of that, then `final state: SystemUI STILL ENABLED`, and the leg reported
|
||||||
|
`expected: 0, received: 0` — `Starting 0 tests`, the exact failure this function exists to
|
||||||
|
prevent.
|
||||||
|
|
||||||
|
Bisected locally on `android-37.0`, which explains the lost state and nothing else:
|
||||||
|
|
||||||
|
| arm | sequence | disabled after the restart? |
|
||||||
|
|---|---|---|
|
||||||
|
| A | `pm disable-user`, then `stop` at once | **no** |
|
||||||
|
| B | `pm disable-user`, wait 15 s, then `stop` | **yes** |
|
||||||
|
|
||||||
|
That is PackageManager's delayed write of package restrictions: the stop kills `system_server`
|
||||||
|
before the settings are flushed, and arm A is what CI did. **Arm B does not help either**, which
|
||||||
|
is the measurement that matters. With the package verified `disabled-user` before *and* after a
|
||||||
|
further clean restart:
|
||||||
|
|
||||||
|
```
|
||||||
|
package still disabled? YES
|
||||||
|
processes:
|
||||||
|
9275 00:17 system_server
|
||||||
|
9695 00:14 com.android.systemui <- started 3 s after system_server
|
||||||
|
```
|
||||||
|
|
||||||
|
CI's own logcat says the same without any restart at all. In the gating leg of run 34006456986,
|
||||||
|
`pm disable-user` is accepted at 02:28:37.9 and the package really is in `pm list packages -d` at
|
||||||
|
02:29:33 — and SystemUI is started at 02:28:39.5 and again at 02:28:52.3, the second of which
|
||||||
|
(pid 4275) is alive for the whole instrumentation run.
|
||||||
|
|
||||||
|
**So `pm disable-user --user 0 com.android.systemui` does not stop SystemUI starting on this
|
||||||
|
image**, with or without a framework restart, on CI or locally. The premise this section was
|
||||||
|
built on — "the framework that comes back never starts SystemUI at all" — is false.
|
||||||
|
|
||||||
|
Two things follow, pointing in opposite directions.
|
||||||
|
|
||||||
|
- **The restart is removed rather than repaired**, in all three copies. It cost a leg every test
|
||||||
|
it had and there is nothing for it to buy. What is kept is the 45-second window with zero new
|
||||||
|
aborts, which was always the part doing the work: in that same run the boot aborts land at
|
||||||
|
02:28:18 and 02:28:43, and the wait is what puts instrumentation at 02:32:42 — after them
|
||||||
|
rather than inside one. The `pm disable-user` call is kept too, for a narrower reason than it
|
||||||
|
was written for: every green leg and every number quoted about this row was measured with it
|
||||||
|
applied, and changing the configuration while fixing a flake is not a trade worth making.
|
||||||
|
- **The rate collapse recorded above is not evidence of what it says.** Both arms of that
|
||||||
|
comparison had SystemUI running. What it measured is a device twelve minutes into its uptime
|
||||||
|
against one that had just booted — a real difference, and a different claim. The quiet gate is
|
||||||
|
still worth having on exactly that reading.
|
||||||
|
|
||||||
### The two deviations, stated plainly
|
### The two deviations, stated plainly
|
||||||
|
|
||||||
1. **The renderer is ANGLE, not the host GPU.** Shared with nothing else in the matrix — API
|
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`.
|
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
|
2. **SystemUI is asked to be disabled, and runs anyway.** This was written as the deviation that
|
||||||
other leg or as the Pixel. It was defensible here because nothing in this suite touched
|
mattered — "anything that ever does depend on system UI must not trust this leg" — and the
|
||||||
system UI — Media3, FFmpeg and WorkManager tests — and because the alternative is no local
|
measurements above say the deviation does not exist: the package is marked `disabled-user` and
|
||||||
API 37 coverage at all. **Anything that ever does depend on system UI must not trust this
|
`com.android.systemui` is up for the whole leg regardless. **The correction is good news
|
||||||
leg.** Something now does; see the section below.
|
rather than bad.** This row is *more* comparable to API 33–36 and to the Pixel than it has
|
||||||
|
been claiming, not less, and the test that depends on system UI (see the section below) was
|
||||||
|
never running in the exotic configuration this bullet describes. What `pm disable-user` leaves
|
||||||
|
behind is a package-manager flag nothing acts on.
|
||||||
|
|
||||||
### Something does depend on system UI now, and half of it is excluded
|
### Something does depend on system UI now, and half of it is excluded
|
||||||
|
|
||||||
@@ -341,12 +406,13 @@ Added 2026-08-24, and the first entry on this page that is not a codec.
|
|||||||
|
|
||||||
`SafPickerRoundTripTest` drives the real system file picker and rotates the display. Both reach
|
`SafPickerRoundTripTest` drives the real system file picker and rotates the display. Both reach
|
||||||
the gralloc mapper — DocumentsUI is another app's windows, and a rotation rebuilds every surface
|
the gralloc mapper — DocumentsUI is another app's windows, and a rotation rebuilds every surface
|
||||||
on screen — and **disabling SystemUI does not help**, because it removes the *idle* trigger
|
on screen — and **disabling SystemUI does not help**. Two reasons now, and only the first was
|
||||||
(RegionSamplingThread's nav-bar luma sampling) and not this one.
|
known when this was written: it removes the *idle* trigger (RegionSamplingThread's nav-bar luma
|
||||||
|
sampling) and not this one, and — see the section above — it does not remove SystemUI either.
|
||||||
|
|
||||||
Measured one method per fresh emulator, `android-37.0`, `swangle_indirect`, SystemUI disabled and
|
Measured one method per fresh emulator, `android-37.0`, `swangle_indirect`, with the disable
|
||||||
verified quiet — separately, because inferring the second from the first is the mistake this
|
applied and verified quiet — separately, because inferring the second from the first is the
|
||||||
page's opening correction is about:
|
mistake this page's opening correction is about:
|
||||||
|
|
||||||
| test | result on android-37.0 | `hasReadColorBufferDma` aborts in the window |
|
| test | result on android-37.0 | `hasReadColorBufferDma` aborts in the window |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
@@ -357,6 +423,111 @@ So a rotation, which rebuilds every surface at once, is what the mapper does not
|
|||||||
starting DocumentsUI is not. Only the rotation test carries `@FailsOnEmulatorApi37`; the picker
|
starting DocumentsUI is not. Only the rotation test carries `@FailsOnEmulatorApi37`; the picker
|
||||||
test runs on the gating leg like anything else.
|
test runs on the gating leg like anything else.
|
||||||
|
|
||||||
|
#### That last sentence was wrong for twelve days, and the aborts in the table said so
|
||||||
|
|
||||||
|
**Corrected 2026-09-05.** Read the second row again: the picker test passes *and takes four
|
||||||
|
`hasReadColorBufferDma` aborts with it*. This section counted them, put them in the table, and then
|
||||||
|
drew the conclusion from the pass/fail column alone. The right question is not "does the test
|
||||||
|
pass" but "does the image survive it", and the answer had been printed in the right-hand column
|
||||||
|
from the day it was written.
|
||||||
|
|
||||||
|
Four gating API 37 runs read logcat-first — 34006456986, 34001744574, 34001377499, and the **green**
|
||||||
|
34002313300 — say it without ambiguity. Each carries exactly two aborts before the suite starts
|
||||||
|
(both `surfaceflinger`, during boot and the SystemUI disable) and then exactly **one** during it:
|
||||||
|
|
||||||
|
| run | picker test window | the run's only in-suite abort | leg |
|
||||||
|
|---|---|---|---|
|
||||||
|
| 34006456986 | 02:33:04.2 → 02:34:46.9, **failed** | 02:34:46.845 | red, `failed: 1` |
|
||||||
|
| 34001744574 | 00:55:41.4 → 00:57:23.9, **failed** | 00:57:23.794 | red, `failed: 1` |
|
||||||
|
| 34001377499 | 00:35:53.3 → 00:36:00.6, passed | 00:35:59.662 | red, `failed: 0` |
|
||||||
|
| 34002313300 | 00:58:12.7 → 00:58:19.8, passed | 00:58:19.218 | green |
|
||||||
|
|
||||||
|
Every one is `system_server`, thread `TaskSnapshotPer`, and every one lands inside that test's
|
||||||
|
window. Nothing else in the gating set reached the mapper at all. So the picker test is
|
||||||
|
**deterministic** in what it does to the image and a coin flip in what the leg reports: 34001377499
|
||||||
|
passed it and lost the leg from teardown with no failing test to name, and 34002313300 passed it
|
||||||
|
0.6 s after the abort and went green.
|
||||||
|
|
||||||
|
That is #108, which had been filed against this behaviour in August and left open because the
|
||||||
|
trigger was unknown. The trigger is this test. It now carries `@FailsOnEmulatorApi37` too, and the
|
||||||
|
marker's KDoc had to widen from "does not pass on this image" to "cannot be run on this image" to
|
||||||
|
say so honestly.
|
||||||
|
|
||||||
|
The stack, for the record, is a different caller from either of the two above:
|
||||||
|
|
||||||
|
```
|
||||||
|
Cmdline: system_server name: TaskSnapshotPer
|
||||||
|
Abort message: 'Assertion failed: !rcEnc->featureInfo()->hasReadColorBufferDma'
|
||||||
|
|
||||||
|
#04 mapper.ranchu.so GoldfishMapper::readFromHost(cb_handle_t const&) const+543
|
||||||
|
#06 libui.so android::Gralloc5Mapper::lock(...)+63
|
||||||
|
#10 libandroid_runtime.so android::lockImageFromBuffer(...)+374
|
||||||
|
#15 framework.jar android.media.ImageReader$SurfaceImage.getPlanes+50
|
||||||
|
#17 services.jar com.android.server.wm.TaskSnapshotConvertUtil.copyToSwBitmapDirect+56
|
||||||
|
#28 services.jar com.android.server.wm.SnapshotPersistQueue$StoreWriteQueueItem.writeBuffer+66
|
||||||
|
#32 services.jar com.android.server.wm.SnapshotPersistQueue$1.run+186
|
||||||
|
```
|
||||||
|
|
||||||
|
WindowManager writing a task snapshot to disk, which needs the buffer as a software bitmap, which
|
||||||
|
is the non-DMA readback path. `PickActivity` is started **into the app's own task** (`Task #11
|
||||||
|
A=10234:org.libremediaconverter` in the logcat), so the snapshot being persisted is that task's,
|
||||||
|
and the churn at the end of the pick is what schedules it.
|
||||||
|
|
||||||
|
#### There is no shell knob for task snapshots, and that was checked rather than assumed
|
||||||
|
|
||||||
|
#108 asks whether `TaskSnapshotPersister` is suppressible the way the region-sampling listener was.
|
||||||
|
Probed on a local `android-37.0 google_apis x86_64` AVD, 2026-09-05:
|
||||||
|
|
||||||
|
```
|
||||||
|
getprop | grep -i snapshot # nothing but apexd-snapshotde
|
||||||
|
settings list global | grep -iE 'snapshot|recents' # empty
|
||||||
|
device_config list window_manager | grep -i snapshot # empty
|
||||||
|
cmd window help # no snapshot or screenshot command
|
||||||
|
dumpsys window | grep -i snapshot # mSnapshotEnabled=true, for Task and Activity
|
||||||
|
```
|
||||||
|
|
||||||
|
`mSnapshotEnabled` is real state and there is nothing that sets it from outside. The only
|
||||||
|
`device_config` hits anywhere in the tree are aconfig flags — e.g.
|
||||||
|
`windowing_frontend/com.android.window.flags.respect_requested_task_snapshot_resolution` — which
|
||||||
|
tune the snapshot rather than disable it. So the marker is the available answer, not the lazy one.
|
||||||
|
|
||||||
|
#### When the picker test does fail, the abort is the coda and not the cause
|
||||||
|
|
||||||
|
Worth separating, because the failure message points the wrong way. In both runs where the test
|
||||||
|
itself went red, it had been broken for 98 seconds before the abort landed. The discriminator is
|
||||||
|
one line, present in both reds and absent from the green:
|
||||||
|
|
||||||
|
```
|
||||||
|
I/InputDispatcher: No new touched window at (539.0, 525.0) in display 0
|
||||||
|
```
|
||||||
|
|
||||||
|
(539, 525) is the centre of the fixture's root row — the same coordinates the green run clicks.
|
||||||
|
The touch reaches no window and is discarded; `UiObject2.click()` cannot see that and returns
|
||||||
|
normally. DocumentsUI then logs nothing at all, where the green run logs `DocumentStack` and
|
||||||
|
`Creating new directory loader` 40 ms after its click. The walk waits out its timeout twice for a
|
||||||
|
fixture it never navigated to, and by the time the back presses start, WindowManager is still
|
||||||
|
saying `no window has focus but ...PickActivity may eventually add a window when it finishes
|
||||||
|
starting up` — for another 63 s. All four presses are dropped, DocumentsUI ANRs on
|
||||||
|
`Input dispatching timed out`, and only *then* does the abort fire and make the failure message
|
||||||
|
read `no windows at all`.
|
||||||
|
|
||||||
|
`SafPickerRoundTripTest.forceStopThePicker` is the answer to that half: `am force-stop` goes around
|
||||||
|
input entirely, so the picker's process can be removed from a task no key press can reach and
|
||||||
|
`pickTheFixture`'s whole-picker retry — which exists for exactly this — becomes reachable again.
|
||||||
|
That is a fix to the test on every level, not to API 37.
|
||||||
|
|
||||||
|
**It was made to bite before it was believed.** On a local API 36 emulator, with the walk cut short
|
||||||
|
so the picker is left open and in front and with `device.pressBack()` removed, so that nothing but
|
||||||
|
the force-stop can close it:
|
||||||
|
|
||||||
|
| | result |
|
||||||
|
|---|---|
|
||||||
|
| with `forceStopThePicker()` | **passes** — `ActivityManager: Force stopping com.google.android.documentsui ... from pid 5334`, `Killing 5269:com.google.android.documentsui (adj 0)`, a second `PickActivity` opens, the retry completes the pick |
|
||||||
|
| with the one call removed | **fails** — `the system picker would not close: after 4 back presses ... com.google.android.documentsui is in front`, which is the API 37 failure verbatim |
|
||||||
|
|
||||||
|
The unmutated class passes on that emulator either way, which is the point of running the mutation
|
||||||
|
at all: the recovery path is unreachable on a healthy device, so a green suite says nothing about it.
|
||||||
|
|
||||||
#### The correction that produced that table
|
#### The correction that produced that table
|
||||||
|
|
||||||
**The first version of this section said both tests failed, and put the marker on the class.** The
|
**The first version of this section said both tests failed, and put the marker on the class.** The
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
# E2E-read findings
|
# E2E-read findings
|
||||||
|
|
||||||
**Status:** seven findings; E4 fixed, the rest standing, none urgent — **plus one confirmed vacuous test, which is a
|
**Status:** seven findings; E4 fixed, E7 extended and its ticket closed, the rest standing — **plus one confirmed vacuous test, which is a
|
||||||
ticket rather than an entry here** (see [Not covered here](#not-covered-here)). `E1`–`E6` came from
|
ticket rather than an entry here** (see [Not covered here](#not-covered-here)). `E1`–`E6` came from
|
||||||
the 2026-09-05 read of the instrumented suite. Every entry here is a *test-suite* observation —
|
the 2026-09-05 read of the instrumented suite. Every entry here is a *test-suite* observation —
|
||||||
something a new test would not fix, because the test already exists and the problem is what it
|
something a new test would not fix, because the test already exists and the problem is what it
|
||||||
@@ -274,6 +274,27 @@ ticket is about.
|
|||||||
**So any test of `publish` against a real `DocumentsProvider` must drive DocumentsUI**, and pays
|
**So any test of `publish` against a real `DocumentsProvider` must drive DocumentsUI**, and pays
|
||||||
#190's flake tax. The work is one item at that cost, not two, and #226 was updated to say so.
|
#190's flake tax. The work is one item at that cost, not two, and #226 was updated to say so.
|
||||||
|
|
||||||
|
**Updated 2026-09-06, doing it: there is a second constraint underneath, and it has the same
|
||||||
|
cause.** The obvious way to avoid driving the app was a host Activity in `androidTest` owning its
|
||||||
|
own `CreateDocument` launcher. It cannot be started at all:
|
||||||
|
|
||||||
|
```
|
||||||
|
java.lang.RuntimeException: Intent in process org.libremediaconverter resolved to different
|
||||||
|
process org.libremediaconverter.test
|
||||||
|
at android.app.Instrumentation.startActivitySync
|
||||||
|
```
|
||||||
|
|
||||||
|
Instrumentation runs in the target app's process, so a component declared in the instrumentation
|
||||||
|
APK is in the wrong one — the same fact that sinks approach 2 above, arriving from the other side.
|
||||||
|
**The app's own Save button is the only launcher available to drive**, which is also the more
|
||||||
|
faithful thing to drive. `SafPickerRoundTripTest.aSaveWritesToTheDocumentTheSystemPickerCreated` is
|
||||||
|
what came of it.
|
||||||
|
|
||||||
|
**And the premise turned out to be true**, which is the answer #226 was filed for: on API 34,
|
||||||
|
stock DocumentsUI hands back a document URI reporting a size of exactly zero. `deletePartialOutput`
|
||||||
|
can fire, and D4's fix is live rather than inert. A "no defect found" — and not one that could have
|
||||||
|
been reached by reading.
|
||||||
|
|
||||||
### What this does *not* block, which is the useful half
|
### What this does *not* block, which is the useful half
|
||||||
|
|
||||||
`FFmpegKitConfig.getSafParameterForRead` — the bridge on every real conversion and join — needs no
|
`FFmpegKitConfig.getSafParameterForRead` — the bridge on every real conversion and join — needs no
|
||||||
@@ -361,7 +382,7 @@ decision, not a detail — see **E6** for why no third option exists — and **#
|
|||||||
| **#223** | `HardwareFallbackTest` never attempts the hardware path on any emulator leg | closed — it skips instead of passing vacuously |
|
| **#223** | `HardwareFallbackTest` never attempts the hardware path on any emulator leg | closed — it skips instead of passing vacuously |
|
||||||
| **#224** | Cancelling a *running* native session, in any of the three engines | closed — all three engines |
|
| **#224** | Cancelling a *running* native session, in any of the three engines | closed — all three engines |
|
||||||
| **#225** | No `content://` input has reached a *successful* conversion — the ffkitsaf bridge | closed, and it found **#238** |
|
| **#225** | No `content://` input has reached a *successful* conversion — the ffkitsaf bridge | closed, and it found **#238** |
|
||||||
| **#226** | `OutputPublisher.publish` against a real `DocumentsProvider` | **open** — re-scoped by E7; one picker-driven item, not two |
|
| **#226** | `OutputPublisher.publish` against a real `DocumentsProvider` | closed — the premise holds; see E7 |
|
||||||
| **#227** | The notification's Cancel action has never been fired | closed |
|
| **#227** | The notification's Cancel action has never been fired | closed |
|
||||||
| **#228** | `encodesFlacLosslessAudio` and `encodesOpus` pass on any non-empty file | closed |
|
| **#228** | `encodesFlacLosslessAudio` and `encodesOpus` pass on any non-empty file | closed |
|
||||||
| **#229** | FFmpeg's progress percentage is computed everywhere and asserted nowhere | closed |
|
| **#229** | FFmpeg's progress percentage is computed everywhere and asserted nowhere | closed |
|
||||||
|
|||||||
@@ -355,12 +355,10 @@ boot_emulator() {
|
|||||||
# may be in one of its restarts and `pm` is simply not published yet. The first attempt at this
|
# 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`.
|
# 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
|
# This used to end with a framework restart, described here as "not optional". It was neither
|
||||||
# time `sys.boot_completed` flips, SystemUI has already registered its region-sampling listener,
|
# optional nor happening -- see the block inside the function. What the first attempt's
|
||||||
# and `pm disable-user` does not retract a registration that already happened -- it only stops
|
# `Starting 0 tests` and four more aborts actually showed is that a `pm disable-user` on its own
|
||||||
# the package being started again. So the first attempt disabled SystemUI, reported success, and
|
# buys nothing, which is still true; what was wrong is the conclusion that a restart would.
|
||||||
# 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() {
|
disable_region_sampling() {
|
||||||
local api="$1" out i before after ready
|
local api="$1" out i before after ready
|
||||||
case "$api" in 37 | 37.*) ;; *) return 0 ;; esac
|
case "$api" in 37 | 37.*) ;; *) return 0 ;; esac
|
||||||
@@ -383,14 +381,18 @@ disable_region_sampling() {
|
|||||||
return 0
|
return 0
|
||||||
fi
|
fi
|
||||||
|
|
||||||
echo " restarting the framework so the region-sampling listener goes with it"
|
# NO FRAMEWORK RESTART, and the two lines that used to be here are why this comment is long.
|
||||||
emu_adb shell stop > /dev/null 2>&1
|
# They were `emu_adb shell stop` and `emu_adb shell start`, both redirected to /dev/null, and
|
||||||
emu_adb shell start > /dev/null 2>&1
|
# both root-only -- so what they printed there was `Must be root` and what they did was nothing,
|
||||||
# There is no property worth waiting on here, and an earlier version of this only looked
|
# here and in the two CI copies alike. Making them real (2026-09-05) is what established that
|
||||||
# like it was waiting on one: `stop` does not clear sys.boot_completed, so it still reads
|
# the disable never worked in the first place: with the package verified `disabled-user` before
|
||||||
# `1` throughout the restart and any loop over it returns at once. The loop below is the
|
# AND after a clean restart on android-37.0, `com.android.systemui` comes up 3 s after
|
||||||
# wait -- and it polls the better thing anyway, since `Can't find service: package` is the
|
# `system_server` regardless, and the same is visible in CI's own logcat. The restart also loses
|
||||||
# failure it exists to prevent.
|
# the package state to PackageManager's delayed write if it lands too soon after the `pm` call,
|
||||||
|
# which cost api37-debug run 34010167885 every test in the leg.
|
||||||
|
#
|
||||||
|
# So the useful part of this function is the quiet window below, not the disable. See
|
||||||
|
# .github/scripts/e2e-run.sh's header, and docs/api-37-emulator-crash.md.
|
||||||
ready=0
|
ready=0
|
||||||
for i in $(seq 1 30); do
|
for i in $(seq 1 30); do
|
||||||
if emu_adb shell service check package 2> /dev/null | grep -q ': found' \
|
if emu_adb shell service check package 2> /dev/null | grep -q ': found' \
|
||||||
|
|||||||
Reference in New Issue
Block a user