main
6
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  JMR-devandClaude Opus 5
|
ba16f5a89b |
Make the fallback test say when it cannot test the fallback (#223)
HardwareFallbackTest is the only automated check of the hardware->software fallback against a real codec failure, and it passed on every CI leg without ever attempting the hardware path. Measured on run 34004304566: the API 33, 34, 35 and 37 legs each log Routing sample_h264_444.mp4 -> ... via FFMPEG (NO_HARDWARE_ENCODER) Emulators expose no hardware encoder, so the router never chooses Media3 and runMedia3OrFallBack's catch is never entered. The test's assertions -- succeeded, output non-empty -- are true of that conversion too. It finished in 448 ms, which is not long enough to fail an export and then software-encode a three-second clip. Deleting the catch reddened nothing. The ticket offered two fixes and left the choice open. Trying the first one answered it, and not the way the ticket expected. Pinning deviceCodecs to PERMISSIVE, as ForcedFailureTest does, makes the router choose Media3 -- and the export then SUCCEEDS. On a local API 34 emulator, MediaCodecInfo logs NoSupport [codec.profileLevel, avc1.F4000C, video/avc] for both c2.goldfish.h264.decoder and c2.android.avc.decoder, and ExoPlayer allocates the goldfish decoder anyway, which decodes the High 4:4:4 fixture regardless of the profile it declares. c2.android.hevc.encoder then encodes it and the job reports MEDIA3. So the class KDoc's "Media3 fails partway through the export on every device" is not true of the emulator images, and no routing pressure makes this fixture force a fallback there. Pinning would also swap in software codecs, which is not the path a real device takes -- it is what made the forced run succeed. That leaves assumeTrue on the production premise as the honest answer, now with a measurement behind it rather than a coin flip. The test skips where it cannot mean anything and runs on the Pixel, where it always could. When it does run the assertion is a pair, because KEY_ENGINE_USED is FFMPEG whether the fallback fired or the router went straight there: the router chose MEDIA3 for this request on this device, AND the worker reported FFMPEG. Together, and only together, that is the fallback. Verified on a local API 34 emulator: the test reports SKIPPED and the level reports skipped=3. ForcedFailureTest still covers the fallback wiring on every leg with a double; what needs a real encoder is two real engines disagreeing about a real file. The third permanent skip is recorded in docs/local-emulator.md and beside SafPickerRoundTripTest's run-shape note. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
225ecdd7e6 |
Split the API 37 leg so the part that works can gate
CI has never run the API level this app targets. The reason it did not was never "API 37 is untestable" -- it was that two tests fail on the emulator image, so one row would be permanently red or permanently allow-listed. This splits that row instead of choosing between those two. E2E API 37 gates. It runs 55 of the suite's 57 instrumented tests and must be green. E2E API 37 Media3 hardware transcode runs the other two, reports, and never blocks (continue-on-error). Both are driven off ONE marker, @FailsOnEmulatorApi37: the gating job passes notAnnotation, the advisory job passes annotation. Two lists would drift, and drift is silent in both directions -- a test that ends up in neither job reads as green. Excluding by class was not an option either: Media3EngineTest has four tests and two of them pass here, so notClass would have thrown away real coverage. The advisory job is named for what it runs, not for what we think is wrong. Both its tests drive a full H.264 -> H.265 hardware transcode, which is what distinguishes them from the two Media3EngineTest cases that pass -- those never decode video. The goldfish-decoder theory sits in a comment inside the job, where it can be corrected without renaming a check people have learned to look for; docs/api-37-emulator-crash.md keeps measurement and inference apart. The SystemUI disable moves into .github/scripts/e2e-run.sh behind E2E_DISABLE_SYSTEM_UI, unset everywhere but the two API 37 jobs, so the other four legs run byte-identical commands -- the same shape as E2E_EXTRA_GRADLE_ARGS. It runs BEFORE the streamed logcat starts, deliberately: `adb shell stop` would end that logcat and nothing restarts it, so a disable placed after it would cost the leg its diagnostics for the part of the run that matters. The body is probe v2 from api37-debug.yml -- the version measured 4/4 -- not the older one-round form: three rounds, waits for system_server to actually be gone, verifies against `pm list packages -d`, and requires a 45 s window with zero new aborts. The weaker probe reported success on a run that then started SystemUI eight more times. The caveat is written next to the row rather than left implicit: this leg runs with SystemUI disabled and the framework restarted under it, a device configuration no other leg and no Pixel run uses. Anything that touches system UI must not trust it, and the Pixel check before each release is still the only API 37 run with SystemUI intact. docs/api-37-emulator-crash.md's "So should CI take API 37?" said no on three reasons. Two were claims about CI that had never been measured; the section now carries the eight runs that measured them, and the third reason is what the split answers. docs/local-emulator.md and api37-debug.yml's header carried the same "the matrix stops at 36" claim and are corrected with it. CLAUDE.md is left alone deliberately -- its "CI's matrix therefore stops at API 36" clause is now false, and that correction is parked in the doc's existing "Correction owed to CLAUDE.md" section, where two others are already waiting. Making E2E API 37 an actually-required check is a repository-settings change and must come after this is on main: adding a required context that does not exist on the default branch blocks every PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
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 --
|
||
|
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 |
||
|
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> |
||
|
JMR-devandClaude Opus 5
|
22c7914395 |
Find out why the emulators segfault, and make them run
CLAUDE.md has said "Emulators segfault on this host -- qemu dies on every AVD" since the E2E matrix landed, and the PR that introduced it called the failure "exit 139 across three AVDs and both GPU backends, environmental". That is accurate about the symptom and wrong about the cause, and the cost of being wrong was the whole instrumented suite being unrunnable here. SwiftShader's Reactor JIT writes generated GLES shader code onto the heap and mprotects it executable. Fedora's SELinux policy denies that -- execheap is not granted to unconfined_t and selinuxuser_execheap is off -- so the mprotect fails and the emulator takes SIGSEGV the moment it calls the routine it just generated. The AVC denial and the core are the same event, one second apart. The predictor is mechanical and held 7 for 7 across every -gpu mode: a run crashes if and only if it dlopens gles_swiftshader/libGLESv2.so. host, angle_indirect and swangle_indirect boot. auto, off, guest and swiftshader_indirect crash -- and auto is the default, which is why the failure looked universal rather than renderer-specific. tools/local-emulator/run-e2e.sh picks a renderer that works and refuses the ones that do not. It reuses .github/scripts/e2e-run.sh rather than forking it, so the local and CI diagnostics cannot drift; the one change there adds an optional E2E_EXTRA_GRADLE_ARGS that is unset in CI, so CI runs byte-identical commands. The API 33-36 sweep has now been run and is written down. All four levels are green on a local emulator and match the physical Pixel 10 Pro XL baseline exactly: 49 tests, 0 failures, 0 errors, 2 skipped, every level. Those counts come from the result XML, not the UTP console counter, which double-counts skips and reported "Finished 51 tests" on all four. No boot log dlopens SwiftShader GLES and the sweep window holds no AVC denial and no qemu core -- which is confirmation of the mode matrix's first row rather than new coverage, since every one of these runs is -gpu host. The table is still seven modes measured once each. Two things the sweep surfaced that the doc now records: pre-build before sweeping, or a fresh checkout spends API 33's 20-minute wrapper budget compiling and wedges before a test runs; and the device pinning is untested by this run, because the Pixel dropped off USB five seconds before it started. Still offered for review rather than applied: the CLAUDE.md correction the doc drafts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |