Re-derive the API 37 emulator failure, and harden the local sweep #47

Merged
JMR-dev merged 7 commits from tools/api-37-emulator into main 2026-08-23 05:21:26 +00:00
7 Commits
Author SHA1 Message Date
JMR-dev f8e6bfa2a3 Merge remote-tracking branch 'origin/main' into tools/api-37-emulator 2026-08-22 23:52:58 -05:00
JMR-dev 7ae660ee37 Merge remote-tracking branch 'origin/main' into tools/api-37-emulator 2026-08-22 23:21:47 -05:00
JMR-devandClaude Opus 5 775a44753b Say that the default sweep is red on purpose, and narrow two claims
R16 / #25 -- the branch put API 37 into the default APIS list, where it is permanently two
failures short of green, so a bare `run-e2e.sh` exits 1 by design and nothing said so.
Somebody running it from habit, a wrapper or a hook gets a red exit forever and either
stops reading exit codes or debugs a normal state.

Documented rather than suppressed. The script's own comment already argued that an
expected-red level belongs in the exit code -- reversing that is the branch owner's call,
not a correction -- and the review's alternative needs an exact-set comparison of the
failing test names before it can subtract 37's contribution, which is a new mechanism that
cannot be validated without a device. So:

- the header now states the exit code (0 all green / 1 any level red / 2 refused to
  start), says a bare run is 1 by design and why, and gives `run-e2e.sh 33 34 35 36` as
  the sweep that can be green;
- a red sweep prints one note after the summary saying the same thing, because the exit
  code is read in the terminal and not in the docs -- but ONLY when 37.x is the only level
  that went red. `overall` is set by any red level, so a note keyed on "37 was in the
  list" would have called a genuine API 34 failure "by design", which is the defect this
  is meant to prevent, one layer up. mark_red records which level it was, where the loop
  already knows;
- docs/local-emulator.md says it where the default is documented.

R27 / #36 -- 792286a appended the caveat that the harness path reproduces a rate collapse
rather than a clean zero, but left "that is the confirmation that region sampling is the
sole trigger" standing three lines above it, which the caveat contradicts. Now "the
strongest evidence that region sampling is the dominant trigger", with the residue named:
no measurement here separates a second caller of the readback path from a disable that did
not fully take, and the file says so rather than picking one.

R28 / #37 -- "the capability is negotiated regardless of renderer" leaned on the string
search, which shows only that `ANDROID_EMU_read_color_buffer_dma` is implemented in one
shared component, not that it is negotiated on every path. The aborts are the actual
evidence -- the assertion that fires is `!hasReadColorBufferDma` and it fires under ANGLE
too -- and they suffice alone; the string search is demoted to a supporting note. Worth
getting right because the doc says the upstream report should lead with this model.

Two follow-ons that belong with R17 / #26 and land here rather than in their own commit:
bash runs a trap only between commands, so the handler starts when the foreground command
returns -- immediate under Ctrl-C, which reaches that command too, but not under a `kill
-INT` aimed at the script alone; that is now written next to the handler. And
delete_created_avds no longer discards avdmanager's status: an emulator that was SIGKILLed
did not get to remove its own lock files, avdmanager can refuse over them, and silence
there would leak exactly what the trap exists to clean up.

`bash -n` clean; the stub smoke harness (real script, fake SDK binaries, boot-failure path,
no Gradle and no emulator) now also checks that a 37-only red prints the note after the
summary, that a red API 34 alongside it suppresses the note, that a 34-only sweep says
nothing, and that a refused AVD deletion is reported. shellcheck is not installed here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 23:17:19 -05:00
JMR-devandClaude Opus 5 961cfa72a2 Derive the suite size instead of writing it down in two documents
R4 / #13 and R20 / #29 are one defect: an absolute test total in an unregenerated
document, written the same day it went stale. This branch was cut at 22c7914, where
app/src/androidTest held 49 @Test methods; main is 57 (ReattachOnLaunchTest added eight
in ec969c4). So the release instruction "expect 49 / 0 / 0 / 2, and if you get 40 you are
on an old checkout" becomes false the moment this branch merges -- on the one check that
has no CI backstop -- and docs/local-emulator.md's headline promises a 49-test local
baseline main no longer produces.

Re-derived rather than renumbered, because a third total would go stale the same way:

- The total is the size of app/src/androidTest on the checkout that ran, and the reported
  total has equalled that checkout's @Test count everywhere it has been checked: 40 at
  edd6385 (the Pixel run), 49 at 22c7914 (the four local levels and API 37), 57 at
  18c53a3 (counted, not run). The new "Reading these totals" section states that, gives
  the one-line grep, and makes the *mismatch* the signal: a total that disagrees with
  your own checkout's count means an old checkout, a stale build or tests that never ran.
  The pre-release Pixel instruction now reads "that many tests, 0 failures, 0 errors, 2
  skipped" -- the invariant, not the total.
- Measurements are kept verbatim and anchored to 22c7914 (the sweep table, the API 35
  control, the tests="49" XML quote, the 51-on-screen console block). Only the claims
  built on top of them were rewritten.

Two claims went with the number, both of which a rebase would have preserved:

- "47 of 49" is not a defensible ratio when two of the 49 are skips. 49 = 45 passed + 2
  failed + 2 skipped, and that is what it now says.
- "against the Pixel's 49 of 49" and "matches the physical Pixel 10 Pro XL baseline of
  49 / 0 / 0 / 2 exactly" describe a run that never happened: the Pixel measured
  40 / 0 / 0 / 2 at edd6385, nine tests earlier, as the same file says a hundred lines
  further down. Both documents projected the local total onto the phone and called it a
  match. What compares between them is 0 failures and the same two skips.

Also re-derived in the CLAUDE.md wording docs/local-emulator.md proposes, since that text
is meant to be pasted out of the branch and would have carried "49 tests / 2 failures /
2 skipped" with it. CLAUDE.md itself is still untouched.

Counts re-checked with git grep at each of the three commits; nothing here needed a
device, and none was used.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 23:09:30 -05:00
JMR-devandClaude Opus 5 da6f2807e9 Stop an interrupted sweep leaking the emulator, the AVD and the port
Four corrections to the harness, none of which changes what a successful sweep does.

R17 / #26 -- no trap. Ctrl-C during a sweep (now up to five boots long) left headless
qemu on console port 5560 and an lmc_e2e_apiNN AVD behind. The next run's `emulator
-port` then collides with the orphan and `emu_adb` can resolve to it -- on a workstation
with the Pixel plugged in, exactly the ambiguity the ANDROID_SERIAL pinning exists to
prevent. `cleanup` (stop_emulator + delete_created_avds, KEEP_AVD honoured) is now on
EXIT, INT and TERM. It is idempotent and the normal path calls it explicitly before the
summary, so cleanup output cannot land after the summary and the EXIT trap finds nothing
to redo. The interrupt path passes a 6-second grace rather than 30: Ctrl-C has already
reached the emulator through the foreground process group, so that wait is only for it to
finish writing, and `kill -9` follows regardless. `exit "$overall"` stays the last line,
so the exit code an EXIT trap could have swallowed is still the one that escapes. The
emulator logs in $LOG_DIR are deliberately kept -- they are the only evidence a failed
boot leaves.

R31 / #40 -- `kill -9 "${EMU_PID:-0}"`. EMU_PID is empty, not unset, if the background
launch never produced a job, so `:-0` converted "nothing to kill" into pid 0, which POSIX
reads as the sender's whole process group. The `kill -0` wait loop had the same shape and
would have spent its full grace period testing the group. All three sites now take a bare
`$EMU_PID` behind one `[ -n ... ] || return 0` guard. boot_emulator's own `kill -0` is
left alone: it runs only after the assignment and cannot reach the group form.

R33 / #42 -- ensure_avd wrote CI's RAM and disk pins to a hardcoded
$HOME/.android/avd/... path and checked nothing. With ANDROID_AVD_HOME (or
ANDROID_USER_HOME, or ANDROID_SDK_HOME) set, the sed failed and the level ran on at
default RAM and userdata, which surfaces much later as "not enough space" and reads as a
device problem. `avd_config_path` now looks in every directory avdmanager honours -- no
precedence is asserted, the existence check decides -- and a level that cannot be found
or written fails instead of running unpinned.

R34 / #43 -- disable_region_sampling's "one blocking wait on the device" did not wait:
`adb shell stop` does not clear sys.boot_completed, so the property still read 1 and the
loop returned at once. Deleted, and the comment now names the service-check loop below it
as the actual wait -- which polls the better thing anyway, since `Can't find service:
package` is the failure it exists to prevent. That loop also says so when it gives up
after 150 s instead of proceeding silently. Deliberately not doing the `setprop
sys.boot_completed 0` variant: the loop tested for an empty value, so a 0 would not have
made it wait either, and the `!= 1` form it would need is an unbounded loop inside `adb
shell` with no timeout.

Checked with `bash -n` and with two stub harnesses in place of a device (shellcheck is
not installed here): one drives the extracted lifecycle functions against fake binaries
and asserts pid 0 really does hit the sender's process group, that an empty EMU_PID now
signals nothing and returns at once, that SIGINT cleans up once and exits 130 within
seconds, that KEEP_AVD survives the trap path, and that an explicit exit status survives
the EXIT trap; the other runs the real script end to end on the boot-failure path, which
stops short of e2e-run.sh, and checks the pins land in config.ini, the created AVD is
removed, a misplaced config.ini fails the level, and `set -u` is not tripped anywhere.
No emulator was booted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 23:06:24 -05:00
JMR-devandClaude Opus 5 792286a2d7 Stop three claims in the API 37 doc outrunning their evidence
Three corrections, all narrowing:

- angle_indirect and swangle_indirect are not two independent renderers here. Both
  logged gles_mode_selected:swangle with the same adapter, differing only in the
  Vulkan backend underneath -- unlike at API 33-36, where angle_indirect resolves to
  ANGLE on llvmpipe. What is 7-for-7 is the host-GLES-versus-not split, not "two
  renderers agree".

- "Disabling SystemUI stops the crashes entirely" was one 180-second measurement on a
  device that had been up twelve minutes. The harness path reproduces a rate collapse,
  not a zero: its own quiet check printed 1 abort in 45 s and 4 across the run. A
  47-second Gradle run survives that; a five-minute one might not.

- "Reproduced twice" conflated two routes. The 49/2/0/2 came back from a hand-driven
  sequence and from the harness, which corroborates the numbers, but the harness path
  itself has one green measurement.

Also records what the doc never said: from 37.1 onward Google ships only 16 KB-page
x86_64 images, so page-size alignment is a prerequisite for that path rather than a
detail. All 20 libraries in the committed FFmpeg AAR are 0x4000-aligned, checked
before the first ps16k boot -- which is why 37.1 reproducing the abort means the
gralloc bug and not a page-size mismatch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 22:10:50 -05:00
JMR-devandClaude Opus 5 739bffa5a0 Re-derive the API 37 emulator failure: it is the renderer, not the image
docs/api-37-emulator-crash.md claimed "Both swiftshader_indirect and host crash...
The crash is in the gralloc mapper, below the renderer." Re-measured, seven runs,
one variable each: that is wrong. The mapper is below the renderer, but whether its
bad path is reached is not.

  -gpu host              gles_mode_selected:host    never boots  (57-71 aborts, looping)
  -gpu swangle_indirect  gles_mode_selected:swangle boots, 85 s  (1 abort)
  -gpu angle_indirect    gles_mode_selected:swangle boots, 112 s (2 aborts)

The old claim rested on two samples of two different things, neither of them ANGLE:
the local swiftshader_indirect sample was void, because on this host every
SwiftShader-GLES launch segfaults the emulator before the guest matters (the
execheap bug in docs/local-emulator.md, not understood when that file was written),
and the CI sample was a single swiftshader_indirect run.

Also re-derived, and null: android-37.1 rev 8 -- a stable REL image the doc's own
"new image revision" trigger was too narrow to catch -- fails identically;
-feature -GLDMA,-GLDMA2,-GLDirectMem is accepted and changes nothing; the image's
advancedFeatures.ini is byte-identical to API 36's but for one camera line; and
there is still no ATD image above API 36.

The mechanism, end to end: SystemUI registers a nav-bar luma-sampling listener,
SurfaceFlinger's RegionSamplingThread locks a GraphicBuffer, Gralloc5 routes into
GoldfishMapper::readFromHost, which asserts, and init SIGKILLs zygote in response --
so the framework restarts under the test run. Disabling SystemUI removes the
listener and the aborts stop dead: 0 in 180 s, against 10-11 per 150 s.

So run-e2e.sh now covers API 37: renderer chosen per level (33-36 need host, 37
must not have it), dotted image labels, SystemUI disabled followed by a deliberate
stop/start, and an abort count printed on every 37 row. The result is 49 tests, 2
failures, 0 errors, 2 skipped, reproduced twice. The two failures are
Media3EngineTest on c2.goldfish.h264.decoder; API 35 under the identical renderer is
49/0/0/2 green, so they are the image and not the renderer.

CI's matrix should still stop at 36, for reasons now written down rather than
assumed. CLAUDE.md is left alone; a replacement bullet is proposed in the doc.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 22:09:19 -05:00