fix/launcher-wiring-waits-for-the-pick
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 JMR-dev
|
d01a46a708 |
Stop counting the run this page calls inconclusive
R29 found the discriminator claimed "exact across all seven" while r07 is recorded lower down as "inconclusive rather than ruled out, because no evidence came back from it". A row this page calls inconclusive cannot also be counted as evidence for the conclusion. Checking it turned up a second instance of the same over-count, which R29 did not name. The abort-cadence section said "Measured across the seven runs above" -- but the table records r07's aborts as **not readable**, because adb wedged before a crash buffer could be taken. Six runs contributed gaps, not seven. Both now say six, and both say why. The discriminator paragraph also says what excluding r07 costs, which is nothing: it is a `host` row, so the discriminator predicts it would not boot, and confirming a prediction with the one run whose evidence did not come back adds no information in either direction. That is the point R29 made -- claiming six does not weaken the conclusion -- and it is worth stating in the document rather than only in the ticket, because the next reader will otherwise wonder whether a run was quietly dropped. Deliberately left: "four of the seven runs show the directory creation itself is broken during the loop". That is a count of how many runs showed something, not a claim that all seven were readable for it, so it survives. Checked rather than assumed, and named here so the next pass does not re-audit it. R29's other half -- "state how r07's boot outcome was read" -- is not taken, because I do not know and inventing a source would be worse than narrowing the claim. Narrowing is the option R29 offered and the one that can be honest. Closes #38. |
||
 JMR-devandClaude Opus 5
|
3925f1aa9f |
Re-find the picker node when it goes stale, and re-measure API 37
CI found a flake this workstation could not, and fixing it overturned half of what
the previous commit recorded about API 37.
THE FLAKE. UiObject2 caches the AccessibilityNodeInfo it was found with, and
DocumentsUI is still settling when a node first appears -- its list rebinds, the
roots strip lays out, a window animates. If the node is replaced in that gap,
click() throws against the handle rather than missing the target:
androidx.test.uiautomator.StaleObjectException
at androidx.test.uiautomator.UiObject2.getAccessibilityNodeInfo(UiObject2.java:1042)
at androidx.test.uiautomator.UiObject2.click(UiObject2.java:526)
at SafPickerRoundTripTest.pickTheFixture(SafPickerRoundTripTest.kt:223)
It is not intermittent on a COLD emulator -- CI hit it on API 33, 34 and 35, every
one of them, on the first run. It never appeared here because the local emulator had
been warm for an hour. tapPickerNode now re-finds the node and taps again, three
attempts. That retries acquiring a handle to a node that has to be there anyway:
every attempt still goes through awaitPickerNode, which fails outright if it is
absent, so the MIME mutation's bite is untouched. Verified with `pm clear
com.google.android.documentsui` between runs, five for five green on API 34.
AND THE CORRECTION IT FORCED. The previous commit marked the whole class
@FailsOnEmulatorApi37 on the strength of two measured failures. One of them was
this bug. Re-measured with the fix, one method per fresh android-37.0 emulator:
thePickedInputSurvivesARealRotation INSTRUMENTATION_ABORTED:
System has crashed.
pickingAFileThroughTheSystemPickerFillsInTheFileCard PASSED
So a rotation, which rebuilds every surface at once, is what the gralloc mapper does
not survive; starting another app's activity is not. The marker moves to the one
method that earned it, and the picker test runs on the gating API 37 leg like
anything else. The workflow comment, run-e2e.sh and the doc all say that now.
The lesson is worth more than the measurement, and the doc keeps it: an annotation
is a claim about an IMAGE, and a broken test makes every image look broken. Both a
framework abort and a stale node read as "the run fell over". Re-measure after
fixing a test before deciding what the platform did.
Also measured rather than assumed, since it is what keeps the gating leg green: the
runner's annotation filter honours a class-level marker, expanding it to every
method. On API 34, `annotation=` selected exactly 4 tests (2 Media3EngineTest + 2
here) and `notAnnotation=` selected 55 with neither of these in it. CI's own gating
API 37 leg then reported 55 / 0 on the previous push. That is why moving the marker
to a single method is a narrowing rather than a repair.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
a3c835b7c9 |
Keep the picker test off the API 37 gating leg, having measured why
The API 37 emulator images abort surfaceflinger inside the guest's Gralloc5 mapper,
init SIGKILLs zygote with it, and the framework restarts under the run. run-e2e.sh
and the CI leg disable SystemUI to remove the trigger -- but that removes the IDLE
one, RegionSamplingThread's nav-bar luma sampling. Driving DocumentsUI and rotating
the display are not idle. They are the first things in this suite that generate
surface traffic of their own.
Both tests were measured on android-37.0 under swangle_indirect with SystemUI
disabled and verified quiet, and measured SEPARATELY -- inferring the second from
the first is the mistake docs/api-37-emulator-crash.md opens by correcting. They
fail in the two shapes a framework restart produces:
thePickedInputSurvivesARealRotation
INSTRUMENTATION_ABORTED: System has crashed.
Expected 59 tests, received 50
(5 hasReadColorBufferDma aborts; the framework dies DURING the test, so six
later tests never run and the XML carries a failure with no text at all)
pickingAFileThroughTheSystemPickerFillsInTheFileCard
androidx.test.uiautomator.StaleObjectException
at androidx.test.uiautomator.UiObject2.click(UiObject2.java:526)
(3 aborts; the picker's root node was rebuilt between finding it and tapping it)
Both pass on API 33 and API 36 locally -- whole suite, 59/0/0/2 on each -- which is
the same evidence pattern that made the Media3EngineTest pair the image rather than
the app.
So the class carries @FailsOnEmulatorApi37 and runs on the advisory leg.
THREE PLACES SAID "nothing in this suite touches system UI", and that is what makes
the SystemUI-disable deviation defensible. It is no longer true of the suite, and all
three are corrected rather than left to rot -- the workflow comment, run-e2e.sh's
header, and the doc. The rule they state is being APPLIED, not broken: the thing that
depends on system UI is excluded from the leg that cannot be trusted for it.
Two consequences stated rather than left to be discovered:
- run-e2e.sh applies no annotation filter, unlike CI, so a local `run-e2e.sh 37`
reports these two on top of the Media3 pair AND DOES NOT FINISH. Its totals come
back short and which later tests ran is arbitrary. The summary row now says so;
it previously promised "exactly two failures", which would have read as a
regression in someone else's diff.
- The advisory job is still named "E2E API 37 Media3 hardware transcode", and half
of what it now runs is neither. Renaming a check touches branch protection, so it
is deliberately not done here; the doc records the staleness and the revisit
trigger now says the marker covers two unrelated bugs that can go green apart.
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
|
b3a705e3da |
Measure the API 36 control and record what CI cannot measure
Three additions to docs/api-37-emulator-crash.md, all from a CI investigation run through .github/workflows/api37-debug.yml. A third measured bullet: API 36 against API 37, back to back, same two tests, same renderer, same SystemUI-disable path. 37.0 fails both on c2.goldfish.h264.decoder (32660148155); 36 passes both in 4.603 s with the same decoder in its logcat (32660152961). That falsifies "the stripped configuration is what breaks these tests" -- a reading the other measurements never addressed, because they all compare against a device that still had SystemUI. It carries its two uncontrolled variables rather than dropping them: API 36's framework restart happened with zero aborts logged where API 37's had two, so a restart under an active abort loop is still uncontrolled; and the images differ on the encoder side, which is a second reason "broken h264 decoder" is the wrong shape of claim. The decoder-mechanism bullet is unchanged and still labelled inference. This adds a measurement next to it; it does not retract anything. The intact-SystemUI counterfactual is unmeasurable on a GitHub runner, and now says why. Seven dispatches, zero verdicts, with a mechanism rather than bad luck: while the framework crash-loops the guest cannot reliably create per-user private directories, so an app installed during the loop has no cache dir and the fixture copy dies in @Before before any codec exists. googlesdksetup and nexuslauncher hit the same thing. The result XML masks it behind an UninitializedPropertyAccessException in tearDown, which reads as a defect in this repository and is not one. Abort cadence corrected. "Roughly every 20 s" was the watchdog's sampling interval, not the cadence: measured gaps are 20-90 s, median 60-70 s, three to five per run, with sys.boot_completed held at 1 throughout. The wrong figure lived in api37-debug.yml's own comments, so that line is corrected too. 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
|
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> |
||
|
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
|
edd6385bf7 |
Record that the suite passes on real API 37 hardware
The doc reasoned that the WorkManager and lateinit failures in CI were downstream of the broken framework rather than real defects, but said so as inference and flagged that only a healthy API 37 device could settle it. One was available. The full instrumented suite runs green on a Pixel 10 Pro XL on Android 17 -- a release build, not a preview -- with 40 tests, 0 failures, 2 skipped, both skips being benchmarks that assume sample files present. ConversionWorkerTest and ConcatWorkerTest drive a real WorkManager round trip and are among the tests that failed that way in CI; they pass on hardware. So the bug is confined to the emulator image, and the gap left by the missing matrix row is automated coverage rather than confidence in the app. Noted that the suite should be run on a physical API 37 device before each release while the row is absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
4e6fe6b75a |
Drop API 37 from the E2E matrix and write down why
The android-37.0 emulator image crash-loops surfaceflinger inside its own gralloc mapper: RegionSamplingThread calls GraphicBuffer::lock, which reaches GoldfishMapper::readFromHost, which asserts that the host has not negotiated ReadColorBufferDma. It has, so surfaceflinger aborts, restarts, and aborts again. Nothing this app does can survive that, and it reproduces on a GitHub runner under swiftshader_indirect and on a workstation under -gpu host alike. There is no ATD image at android-37.0 to fall back to, and -feature -GLDMA is accepted by the emulator but does not prevent the assertion. Correcting the previous commit, which is already pushed so its message stands: ram-size was not the cause of that failure. Setting it did move the job from failing at install to failing during the test run, which is how the real crash became visible, but at 2560M the guest had 1.5 GB free when it died. The setting is kept because the emulator's own floor varies by API level -- 2048M at 33, 2560M at 34 to 36 -- and pinning it makes the matrix uniform. Also corrected: a comment claiming this could not be reproduced locally. It can, and the local crash was the same one all along. Dropped the dmesg probe. adb shell is not root, so klogctl is denied and it only ever printed a permission error -- which a later reader would reasonably misread as "no OOM kills". docs/api-37-emulator-crash.md carries the evidence, the ruled-out fixes, the reproduction, and how to file it upstream, so re-adding the row later starts from what is already known rather than from scratch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |