The save test failed an API 35 leg with "No compose hierarchies found in
the app". Dismissing the POST_NOTIFICATIONS dialog presses back and waits
for the permission UI to be gone, but going away and the app being in front
again are not the same moment, and the next Compose query landed in the gap.
Asked of UiAutomator rather than through awaitAppFocus, which is the
opposite of what this class 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.
Two more local API 34 runs at 70/0/0/3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
publish deletes a destination it could not write to -- 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 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 were false in production, D4's fix would be inert and every
existing test would still pass.
It is not false. Measured on an API 34 emulator against the real dialog:
the document SAF hands back is a document URI and reports a size of exactly
zero before anything writes to it. RecordingPublisher reads both at the
moment publish sees them, through the ConversionDependencies seam, then
lets the real copy proceed so the bytes are checked too.
This has to go through the picker, and through the app, and both are
platform constraints rather than choices. E7 in docs/e2e-read-findings.md
records the first: a DocumentsProvider is reachable only through a
picker-issued grant. The second was measured here -- a host Activity in
this source set owning its own CreateDocument launcher cannot be started at
all, because instrumentation runs in the target app's process and
ActivityScenario refuses with "Intent in process org.libremediaconverter
resolved to different process org.libremediaconverter.test". So #226 has no
cheap half, which is what its comment now says.
Three things the flow needed, each measured rather than guessed:
Both taps scroll first. On Ready the screen carries a file card, five
pickers and then the button, so Convert is below the fold; performClick on
an off-screen node dispatches where nothing is and throws nothing, while
assertIsEnabled passes either way. The first version sat waiting for a
Converted that could never come.
The format stays at its default. FixtureDocumentsProvider advertises
video/mp4 so the picker's MIME filter has a mutation with a shape, and
DocumentsUI honours that on the save side too: choosing MP3 makes the
destination audio/mpeg and the fixture root is filtered out of the save
dialog entirely.
The notification dialog is dismissed rather than pre-granted. Convert
converts from the permission callback whichever way the answer goes, so
denying is a real user's path and enough. Granting programmatically did not
take -- GrantPermissionsActivity appeared anyway and swallowed the tap.
The provider gains create, write and delete support, which it needs to be a
save target at all. It carries @FailsOnEmulatorApi37 because anything that
puts DocumentsUI on screen aborts system_server on that image, as #245
established for the other two; baseline 5 -> 6.
Verified on a local API 34 emulator: three full-suite runs at 70/0/0/3, and
a publish that writes no bytes fails it with "array lengths differed,
expected.length=58677 actual.length=0".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This PR's own advisory leg caught it. With five markers and a truncated run it
printed
failed: 4
...
baseline: matches (5 expected, 5 failed)
three lines apart. The match line has always printed the baseline twice, which
was true while `failed` had to equal it to get there -- and the previous commit
removed that requirement for truncated runs without noticing this line depended
on it.
So the truncated spelling says what happened: `matches (5 expected; 4 of 5
failed, on a run the abort truncated -- not compared)`. Pinned by a third case
beside the two from that commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#221 landed while this was in review, adding one instrumented test: 68 -> 69, and
64 on the gating leg. Its own commit was "Move CLAUDE.md's instrumented counts
with the test that changes them", and it still arrived stale -- it says three
markers and 61 tests, both true of the main it was branched from and neither true
of the main it merged into.
That is the third time these two numbers have gone stale in a day, so the
paragraph now says where they come from: a grep for @Test over app/src/androidTest
minus the marker count, cross-checkable against any run's shape, since a leg below
37 reports the first as `expected` and the API 37 gating leg reports the second.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main landed 25 commits while this branch was open, including a third
@FailsOnEmulatorApi37 on Media3EngineTest.cancellingARunningExportStopsIt and a
batch of new instrumented tests. Every number this branch touches moved with them.
Re-derived rather than adjusted, and cross-checked against run 34020234606: the
API 34 leg (no filter) reports 68 tests and the API 37 gating leg 64, which is
68 minus main's four markers. With the picker test marked that is five markers,
baseline 5, and 63 on the gating leg.
The conflict in FailsOnEmulatorApi37.kt is resolved main's way: it had replaced
the hardcoded "grows by two" with a reference to the constant, which is the same
drift this file exists to prevent and a better fix than the number I put there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two results from #223-#230 that belong with the read rather than only in
their own tickets.
E7 re-scoped its own ticket. #226 split into a cheap headless half and an
expensive picker-driven one, on the premise that a real DocumentsProvider
can be reached without DocumentsUI. It cannot: an unprotected one is
refused at install, instrumentation runs in the app's uid so the test APK's
own identity is no help, and adopting shell identity is denied too -- each
denial naming ACTION_OPEN_DOCUMENT as the only way in. Measured three ways.
So #226 is one item at the picker's cost, not two.
The useful half of that distinction is that the input bridge needs no
documents provider at all. getSafParameterForRead opens a descriptor
through the resolver, so any readable content:// URI exercises it, which is
what kept #225 headless.
And that is how the read's one production defect surfaced. #238: joining
files picked through the system picker failed outright on the stream-copy
path, because the concat demuxer whitelists protocols separately from
-safe 0 and ffkitsaf was not on the list. Only STREAM_COPY feeds the
demuxer a list file, and every existing join test passed Uri.fromFile, so
the one broken combination was the only one a user could reach.
Worth stating plainly next to the coverage entry: it was not a missed line
and not an unasserted value, but two covered things no test put together --
the gap shape a coverage number is worst at, and the reason the read
happened.
E4 is marked fixed; #243 made that KDoc name the constant rather than
restate it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two FFmpeg engines were done in ad2a75d and d293646. This is
Media3Engine.transcode's invokeOnCancellation, which posts
transformer.cancel() onto the engine's own HandlerThread because cancel()
has the same single-thread requirement as start().
The assertion is the output file here, where it could not be for FFmpeg.
That side deletes the partial on cancellation, and on POSIX ffmpeg keeps
writing to the unlinked inode, so the path stays gone whether or not the
cancel landed -- it asserts the session's return code instead. Media3Engine
deletes nothing, the partial being ConversionWorker's to clean up, so the
file is the evidence.
A cancelled export reports itself two ways and both mean interrupted: no
video track, or MediaExtractor refusing the file outright with "Failed to
instantiate extractor" because there is no moov atom. The first version
treated only the null as success and the exception failed the test, which
is how that was measured. Only a playable file counts as a miss.
The wait before reading is several times the export's own length, so a
cancel that did not land has certainly finished by then: the failure
direction is "the file became playable", never "we did not wait long
enough". The attempt is retried for the reason the other two engines
measured -- a 3 s 320x240 export outruns a naive cancel on a loaded runner
-- and an export that never wrote a file at all is recorded as
inconclusive rather than allowed to pass as a cancellation.
It carries @FailsOnEmulatorApi37, so FAILS_ON_EMULATOR_API37_BASELINE moves
3 -> 4 in this diff. That file also said removing the marker would grow the
gating leg "by two", which has been wrong since the third marker landed; it
now names the constant instead of restating it.
Verified on a local API 34 emulator: 68 tests, 0 failures, 3 skipped; and
with transformer.cancel() removed all five attempts produce a playable
video/hevc and the test fails, naming each one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reattachment.rank gives RUNNING the highest rank of all -- "live work
outranks a finished result because a running job is holding a foreground
service" -- and no test on either source set had ever produced one.
ReattachOnLaunchTest covers a job that finished, one whose staged file is
gone, an ambiguous pair, one still queued, and one the user cancelled.
ReattachmentTest exercises the ranking as a pure function over fabricated
snapshots. What was missing is a ViewModel meeting a real running job,
which is also the likeliest reattachment there is: the user starts a
conversion, leaves, and comes back while it is still going.
The engine is a fake, deliberately. The job has to still be running when
the ViewModel is built, and every real conversion in this suite finishes in
about a second -- racing that is what made the cancellation tests flaky
enough to need retries. A SoftwareTranscoder that blocks until released
removes the race outright. Nothing about reattachment depends on which
engine is transcoding: the tag query, Reattachment.choose over live
WorkManager state, and observe's mapping to Converting all run identically
whatever is doing the work.
This is what #230 can actually deliver, and the ticket asked for the answer
either way. Process death itself stays device-manual. D3/D13 already record
that am kill refuses a process holding a foreground service, and there is a
more basic obstacle underneath it: instrumentation runs in the app's own
process, so any route that really killed it would take the test runner with
it and leave nothing to assert with. Observing a relaunch needs two
instrumentation runs, which the runner does not provide. So the closest
observable analogue is a fresh ViewModel, with no memory of the work,
meeting a job that is genuinely mid-flight.
The teardown now resets ConversionDependencies. The suite runs without
Android Test Orchestrator, so a BlockingTranscoder left in place would hang
the next class that converts anything.
Verified on a local API 34 emulator: 67 tests, 0 failures, 3 skipped; and
making RUNNING unreattachable in Reattachment.rank fails this test and
nothing else -- which is also the evidence that the JVM ranking test was
not already covering it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Joining files picked through the system picker failed outright whenever
the strategy was stream copy -- the matched-files case the UI advertises
as "joined without re-encoding, no quality loss".
[ffkitsaf @ ...] Protocol 'ffkitsaf' not on whitelist 'file,crypto,data'!
Error opening input file .../joined_from_content.concat_list.txt
JoinScreen picks with OpenMultipleDocuments, so real inputs are always
content://. ConcatEngine maps each through getSafParameterForRead and
FFmpegConcatCommand writes the resulting ffkitsaf: paths into the concat
list file. The demuxer applies its own protocol whitelist, defaulting to
file,crypto,data, and -safe 0 does not touch it: that permits absolute
paths, this permits the scheme they carry. Two separate gates, and only
one was open.
Nothing caught it because the two halves of the bug never met. Only
STREAM_COPY feeds the demuxer a list file -- REENCODE passes each input
with its own -i, where the whitelist does not apply -- so joining over SAF
worked for mismatched clips. And every join test passed Uri.fromFile,
which takes ConcatEngine's uri.path arm instead of the bridge, so
matchingClipsAreJoinedByStreamCopy exercised stream copy with a file:
path and passed. The one broken combination was the one no test produced
and the only one a user can reach.
That is #225's gap: FFmpegKitConfig.getSafParameterForRead is on every
real conversion and join, and was on no passing test -- only on
UnopenableUriTest's failure side, which proves the error message rather
than the bridge. ContentUriInputTest now drives both the convert and join
paths from a real content:// URI.
It uses a plain ContentProvider, because the documents provider cannot be
reached. Measured three ways: a DOCUMENTS_PROVIDER without MANAGE_DOCUMENTS
is refused at install, instrumentation runs in the target app's process so
Instrumentation.getContext() still carries the app's uid and is denied, and
adoptShellPermissionIdentity(MANAGE_DOCUMENTS) is denied identically -- the
denial naming ACTION_OPEN_DOCUMENT as the only way in. The bridge needs no
documents provider: it opens a descriptor through the resolver, so any
readable content:// URI exercises it, and an ordinary provider may be
exported unprotected. The whole class stays headless. Recorded on #226,
which that also settles: its cheap half does not exist.
Verified on a local API 34 emulator: 66 tests, 0 failures, 3 skipped; and
removing the -protocol_whitelist pair reproduces the production failure
verbatim in the join test and nothing else. FFmpegConcatCommandTest pins
the flag on the JVM.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
onInputPicked does not reach Ready on the calling thread. It hops twice --
withContext(pickDispatcher) { InputQuery.describe(...) } and then the probe
-- and pickDispatcher defaults to Dispatchers.IO, a real background thread
Compose's idling knows nothing about. So deliver() returned with the state
still Idle, and asserting immediately was a race the test usually won.
It lost five times on CI in one day, on PRs whose diffs were instrumented
tests and documentation and could not reach it. Two of those failures came
alongside #125's deadlock and could be argued as fallout; three did not.
waitUntil polls through waitForIdle, draining the main looper each time, so
it sees the recomposition the IO hop eventually posts back.
Injecting the dispatcher would be better and is not available here.
pickDispatcher is a constructor parameter precisely so a test can pin it,
but this test composes the real ConverterScreen, which resolves its own
ViewModel through viewModel() -- the seam is one layer below the launcher
edge this class exists to cover, and reaching for it would mean not testing
that edge.
The evidence is the mutation rather than the repetition count, per the note
#218 left: transposing the two launcher callbacks at ConverterScreen.kt:70
and :83 -- the exact defect this test guards -- still fails it, so the wait
did not make it vacuous. A transposed callback leaves the screen in Idle
forever and it fails on the timeout with the meaning it had before.
Supporting evidence, eight consecutive green runs of the class.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both cancellation tests I added in ad2a75d and d293646 wait for
SessionState.RUNNING and then cancel. That is not enough. The conversion
one passed four consecutive local runs and all five CI legs, then failed
the API 34 and 35 legs of the next PR with state=COMPLETED rc=0, on a diff
that could not reach it. On a loaded runner the thread that observed
RUNNING can be descheduled long enough for a short encode to finish before
it calls cancel. A longer timeout does not help: the wait already
succeeded.
Two changes, because neither is sufficient alone.
A slower encode. The conversion test now targets WEBM_VP9 at BEST, the
slowest thing FFmpegCommandBuilder emits -- libvpx-vp9 -crf 31 -b:v 0,
with -deadline realtime added only on FAST. Probed on an API 34 emulator:
that session is still RUNNING at 1 s and finished by 2 s, against well
under a second for x265 -preset medium.
A bounded retry. An attempt whose session finished before the cancel
landed has not tested anything, so it is a miss rather than a failure and
is retried; only exhausting five attempts fails, and the message reports
every attempt's state and return code so a real breakage is distinguishable
from a slow machine.
The retry does not soften the test. With FFmpegKit.cancel removed from
both engines, every attempt ends COMPLETED, so both still fail -- verified,
each listing five [state=COMPLETED rc=0] outcomes. Two clean runs
beforehand at 64/0/0/3.
The join test gets the same treatment. It has not flaked yet, but it is
the same mechanism and the same fragility, and finding out on CI again is
not worth the round trip.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The FFmpegEngine half landed in ad2a75d; this is the same gap in
ConcatEngine. Between them, a real native session being asked to stop is
now covered on both FFmpeg paths.
The assertion is the session's return code again, and here that is not
merely the better choice but close to the only one: ConcatEngine does not
delete its output on cancellation at all. Its invokeOnCancellation is
FFmpegKit.cancel and nothing else, where FFmpegEngine's also deletes the
partial. Whether that asymmetry is deliberate is a separate question, so
this asserts what is true of both engines rather than depending on it.
The cancel triggers on SessionState.RUNNING rather than on progress.
ConcatWorker publishes no progress at all, so there is no callback to hang
it on even in principle -- and the conversion side already measured the
deeper reason, that the committed clips outrun a callback-triggered cancel.
The inputs are the mismatched pair on purpose, so ConcatPlanner chooses
REENCODE. A stream copy of two 2 s clips is close to instantaneous and
would leave nothing to interrupt; re-encoding is also the case where a user
would actually reach for Cancel.
Verified on a local API 34 emulator: three runs green at 64/0/0/3, and
replacing invokeOnCancellation's body with an empty block fails this test
and nothing else, with state=COMPLETED rc=0.
Media3Engine's transformer.cancel() is still uncovered and #224 stays open
for it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every cancel in app/src/androidTest is WorkManager.cancelWorkById against
work that is queued or already finished: ReattachOnLaunchTest cancels a job
carrying a one-hour initial delay, and another immediately after enqueue.
On the JVM, WorkerCancellationTest and HardwareFallbackTest's cancellation
case drive a SoftwareTranscoder double that records the call. No test on
any source set had asked a real native session to stop. That is
docs/defect-audit.md D10's forcing condition.
It is the one path where cancelling wrong is silently expensive rather than
loudly broken: a missed FFmpegKit.cancel leaves the native process encoding
to completion while the UI says the job is cancelled.
Two things were measured rather than assumed, and both changed the test.
The output file cannot be the assertion. invokeOnCancellation deletes the
path, and on POSIX unlinking a file ffmpeg still holds open leaves ffmpeg
writing to the unlinked inode -- so the path stays gone whether or not the
cancel reached the session, and removing FFmpegKit.cancel passes that check
every time. The session's own verdict is what separates them: a cancelled
session ends with the cancel return code, a completed one does not.
Cancelling from the first progress callback loses the race. It was tried
first and failed with state=COMPLETED rc=0: every committed fixture is
2-3 s at 320x240, and the encode finishes before the first statistics
callback is delivered and acted on. FFmpegKit.listSessions shows the
session RUNNING far earlier, so that is what the test waits for.
QualityTier.BEST is deliberate for the same reason -- preset medium leaves
more of the encode ahead of the cancel.
Verified on a local API 34 emulator. Four consecutive runs green at
62/0/0/3, and removing FFmpegKit.cancel while keeping output.delete fails
this test and nothing else, with state=COMPLETED rc=1.
ConcatEngine and Media3Engine carry the same shape and are not covered
here; #224 stays open for them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The verification dispatch of the reworked harness (34011072884) came back
4 expected / 3 received / 3 failed, where the one before it (34008889182) had
been 4/4/4 on the identical configuration. Nothing about the test list changed
between them: the abort landed one test earlier and the picker test never
started.
The baseline check would have called that "one now passes", which is the wrong
reading and the kind of notice #120 is about -- a deviation that is wrong often
enough to teach everyone to skim past deviation notices. So `failed` is compared
only when `completed cleanly` is yes, and `expected` is compared always, because
`Starting N tests` is printed before anything can abort and is what actually
answers "is the marked set the size the baseline says".
Two cases in e2e-report-shape-test.sh, as a pair: a truncated run short by one is
not a deviation, and a CLEAN run short by one still is -- so the first cannot have
bought its quiet by disabling the check.
This is a consequence of adding the fourth marker rather than a pre-existing bug
worth its own ticket: with three, the advisory leg had been completing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ConversionNotifications.build attaches one action, wired to
WorkManager.createCancelPendingIntent(id). Until now
createCancelPendingIntent had no references anywhere outside its own
declaration -- no JVM test, no instrumented test.
That is worth more than an ordinary uncovered line. A conversion runs in a
foreground service and the user is invited to leave the app; once they do,
this action is the only way to stop it. If the PendingIntent carries the
wrong id the button does nothing, the notification stays, and the job runs
to completion, with no error, no log and no screen to look at.
The obvious version of this test reads NotificationManager's active
notifications for id 1001 and taps what it finds. Rejected: the
instrumented suite grants no runtime permissions, so POST_NOTIFICATIONS is
denied throughout, and whether a suppressed foreground-service notification
is returned there is a platform detail that varies. The test would be
asserting something about notification visibility rather than about
cancellation. The PendingIntent is the subject and where it is read from is
incidental, so this builds the notification for a real live work id and
fires its action -- a real dispatch reaching real WorkManager, the same way
on every API level.
The job carries an initial delay so it stays ENQUEUED. A conversion of the
3 s fixture finishes in well under a second on an emulator, so racing a
cancel against a running job would be flaky in the direction that fails,
and cancelWorkById acts on ENQUEUED identically. What is under test is
whether firing the action reaches WorkManager with the right id.
Verified both ways on a local API 34 emulator: 61 tests, 0 failures, 3
skipped as written; and building the PendingIntent from a random UUID
instead of the request's id fails the new test and nothing else.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three commits back I gave `disable_region_sampling` the `adb root` it needed, on
the strength of `Must be root` appearing in every API 37 leg's log. That part was
right and the conclusion drawn from it was not. api37-debug run 34010167885, with
the restart finally real:
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
NOT DISABLED after the restart -- the package state did not survive
three rounds of it, `final state: SystemUI STILL ENABLED`, and the leg reported
`expected: 0, received: 0`. Making the restart work cost the leg every test it had.
Bisected locally on android-37.0: a `stop` 2 s after `pm disable-user` kills
system_server before PackageManager flushes its delayed write, and a 15 s pause
makes the state survive. That repairs the wrong thing. With the package verified
disabled before AND after a clean restart, `com.android.systemui` comes up 3 s
after `system_server` regardless -- and CI's own logcat says the same with no
restart at all: run 34006456986 verifies the package disabled at 02:29:33 and has
SystemUI pid 4275 alive from 02:28:52 for the whole run.
So `pm disable-user` does not stop SystemUI starting on this image, with or without
a restart, and the restart is removed from all three copies rather than repaired.
What is kept is the 45-second window with no new aborts, which is what was always
doing the work: 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. `pm
disable-user` is kept too, because every green leg and every number quoted about
this row was measured with it applied.
The prose the earlier commits got wrong is corrected in place, and one of the
corrections is good news: status_check.yml's caveat that this row runs a
configuration no other leg or Pixel run uses, so nothing depending on system UI
may trust it, describes a state that has never existed. The row is more comparable
to API 33-36 than it has been claiming, not less.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FFmpegEngine derives progress as stats.time / durationMs * 100, and the
statistics callback runs on every conversion in FFmpegEngineTest -- they
all pass durationMs = 3_000. But every call site omits onProgress, so
nothing on any source set had ever looked at the number. Replacing percent
with a constant reddened nothing.
What already existed covers the plumbing downstream and not this: #196
covered the worker's progress lambda with a fake engine that reports
whatever the test tells it to, and ProgressNotificationTest covers the
throttling the same way. The arithmetic was the one part with no reader.
The new test passes 30 s as the duration for a fixture that is exactly
3.000 s, so the conversion still encodes the whole clip and the reported
percentage tops out around 10 rather than 100.
That is what makes it bite. A range check alone is worthless: a constant 0
satisfies both "every value is in 0..100" and "the values never go
backwards", and so does a list of [0, 100]. Pinning the band rejects every
constant, and because the band sits a tenth of the way up it also rejects
an implementation that ignores durationMs, which would report ~100 for the
same run. The bound is loose -- 5..25 for an expected 10 -- because the
last statistics callback can land slightly before the final frame.
Verified on a local API 34 emulator: 61 tests, 0 failures, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third copy of the same defect. tools/local-emulator/run-e2e.sh sends `adb shell
stop` and `start` to /dev/null, so its `Must be root` was never printed and the
framework restart it credits has never happened either.
That matters for what docs/api-37-emulator-crash.md's abort numbers are evidence
of, so the caveat goes next to them rather than in a commit message: on API 37
the image restarts its own framework every minute or so, and a restart landing
after a successful `pm disable-user` brings back a SystemUI-less zygote on its
own. That produces the recorded rate collapse by accident, and it is why the same
code bought nothing on CI's much quieter swiftshader legs, where the logcat shows
SystemUI alive for the whole run.
Also verified, because the previous commit asserted it: the advisory leg really
does run thePickedInputSurvivesARealRotation before the picker test -- run
34008889182 logs the four in the order Media3, Media3, rotation, picker.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
The advisory-leg count (4 expected, 4 received, 4 failed with the new marker) is
api37-debug run 34008889182, dispatched with the annotation as its filter. The
force-stop recovery was made to go red before it was believed: on a local API 36
emulator, with the picker left open and the back presses removed, the test passes
with forceStopThePicker() and fails with exactly the API 37 message without it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The debug workflow's header says it does not fork e2e-run.sh, and it does not --
but it drives the SystemUI disable from its own probe step, so `disable_system_ui`
can be turned off for a dispatch. That is a second copy of the same logic, and run
34008889182 showed it carrying the same defect the real leg had: `Must be root`
twice, and `system_server down after 40 s` printed for a stop that did nothing.
Same fix, and a header note so the next person changing one knows to change both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
encodesFlacLosslessAudio and encodesOpus asserted only that a non-empty
file appeared. Five siblings in the same class check what is in it --
encodesWav reads RIFF two lines away, and GIF, Matroska, MP3, H.264 and
H.265 all assert a container marker or a track MIME.
convert() throws on a non-zero return code, so these two did prove the
command ran. What they could not distinguish is the command running and
producing the wrong thing, which is a failure mode this codebase has
already had once: F1 in docs/coverage-read-findings.md records a live
Vorbis arm in FFmpegCommandBuilder that ContainerCapabilities says cannot
exist. Pointing OutputFormat.FLAC's arm at pcm_s16le left both tests green.
Four bytes each, in the idiom the class already uses. OutputFormat.FLAC is
Container.FLAC, whose ffmpeg format is "flac", so the file opens with the
native stream marker fLaC. OutputFormat.OPUS is Container.OGG -- "ogg" --
so it is an Ogg stream and opens with OggS. Verified against both muxers
directly rather than assumed from the codec name; the marker belongs to the
container, so it does not vary with the ffmpeg build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gating `E2E API 37` leg failed three of the last ten status_check runs. Four
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
`pickingAFileThroughTheSystemPickerFillsInTheFileCard`'s window. Nothing else in
the gating set reaches the mapper.
So that test kills the framework on this image whether it passes or not, and
whether the leg goes red is luck: 34001377499 passed it and lost the leg anyway
with `failed: 0`, 34002313300 passed it 0.6 s after the abort and went green.
That is #108. The test now carries `@FailsOnEmulatorApi37` and the baseline goes
3 -> 4; the marker's own wording widens from "does not pass on this image" to
"cannot be run on this image", because this carrier passes about half the time.
`docs/api-37-emulator-crash.md` had counted those aborts on 2026-08-24, put them
in its table, and then read the pass/fail column alone. The correction is
recorded beside the original rather than replacing it. Probed on the same image
and recorded there too: there is no shell knob for task snapshots -- not in
`getprop`, `settings`, `device_config` or `cmd window` -- so the marker is the
available answer rather than the lazy one.
Two separate defects came out of the same logcats.
`disable_region_sampling` has never restarted the framework on CI. `adb shell
stop` and `start` are root-only and every API 37 leg has printed `Must be root`
for both, so SystemUI stayed up for the whole run -- visible directly as
`WindowManagerShell ... app=com.android.systemui` minutes after "final state:
SystemUI disabled". Both waits also printed their own exhaustion as an elapsed
time, so "system_server down after ~40 s" is what a stop that did nothing looks
like. Measured on the local android-37.0 AVD, same fingerprint as CI: `adb root`
makes `stop` return 0 with `pidof system_server` empty. Root is dropped again
before Gradle runs, and both waits now say whether they observed anything.
And when the picker test does fail, the abort is the coda rather than the cause:
`InputDispatcher: No new touched window at (539.0, 525.0)` is in both reds and
absent from the green, so the tap on the root is discarded, the picker is never
navigated, and all four back presses land on an activity WindowManager says has
not added a window yet. `forceStopThePicker` goes around input entirely so
`pickTheFixture`'s whole-picker retry -- which exists for exactly this -- becomes
reachable. That one is a fix on every API level, not just 37.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four coverage waves have been steered by JaCoCo, which measures
testDebugUnitTest only and cannot see app/src/androidTest at all. So
nothing had ever asked what the 60 device tests pin, only that they were
green. This is that read: a triage, not a test push, in the shape of
docs/coverage-read-findings.md.
Six findings (E1-E6) are things a test would not fix, and five of those
six are prose rather than code — the suite itself is in good condition.
Eight tickets carry the rest (#223-#230), each naming the mutation that
has to go red rather than a coverage delta.
The one that matters is #223. HardwareFallbackTest is the only automated
check of the hardware->software fallback against a real codec failure,
and it has never attempted 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)
because emulators expose no hardware encoder, so the router sends the job
straight to FFmpeg and runMedia3OrFallBack's catch is never entered. Its
two assertions — succeeded, output non-empty — are true anyway. It
finishes in 448 ms, which is not long enough to fail a hardware export
and then software-encode a three-second clip. Deleting that catch reddens
nothing anywhere.
Two things generalise. A test can assert and still not reach, which
neither a coverage number nor a "does it assert something" review can
see; the filter that works is whether the test's premise holds on the
machine running it. And the codebase already knew — ForcedFailureTest
pins DeviceCodecs.PERMISSIVE against this exact hazard and says why, as
does ConversionWorkerTest. Their assertions are about the path, so
without the pin they fail loudly; HardwareFallbackTest's are about the
output, so it passes quietly. That asymmetry is why nobody noticed.
E5 records the structural reason this document is separate: F7 in the
coverage findings calls probeWithExtractor's catch uncovered when
RemuxTest drives it on a device every leg. A JaCoCo-derived document
cannot see androidTest, so it will keep re-deriving that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The suite goes 60 -> 61 and the gating leg 57 -> 58, because the new join-failure
test carries no @FailsOnEmulatorApi37 and so runs on every level including 37.
FAILS_ON_EMULATOR_API37_BASELINE stays at 3, checked rather than assumed:
e2e-report-shape.sh counts lines matching '^[[:space:]]*@FailsOnEmulatorApi37',
which is three annotations -- the fourth occurrence in the tree is the KDoc at
Media3EngineTest.kt:195 saying a test is deliberately NOT marked, and the anchor
excludes it. Nothing here adds a marker.
Carried by this PR rather than the docs one so the file never states a count main
does not have.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#217 closed by noting the join legs had not been run locally. Running them would
not have answered it: nothing on either source set drove a real join *failure*,
so the message unified in #203 was asserted only against values a JVM test hands
to sessionOutcome directly. CI had already run those legs green on the merged
commit, so the outstanding item was a gap in coverage rather than a gap in
execution.
The new case forces a failure with an input that does not exist -- rejected the
same way by every FFmpeg build, unlike malformed media -- and asserts the message
names the operation, carries the return code, and has detail after it.
That last clause is the device-only half. getReturnCode, getFailStackTrace and
getAllLogsAsString are native reads; if the log tail came back empty on a device
the user would see "Joining failed (1): " with nothing after the colon, and every
JVM test would still pass.
It deliberately does not pin which detail source wins. On an ordinary non-zero
return code FFmpegKit reports no fail stack trace, so stack-trace-first and
log-tail-first produce identical text and no assertion here could tell them
apart. SessionOutcomeTest pins that, where both sources can be non-blank at once.
Claiming it here would be a KDoc asserting more than the test checks.
Measured on the local API 33 emulator: 61/61 green with the test, and with both
detail sources nulled in ConcatEngine it fails on the intended assertion --
"the message stopped at the return code and told the user nothing, was:
'Joining failed (1): '" -- so the prefix and return code survive the mutation and
only the device-only claim goes red.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Numbers re-measured on 6004398 rather than inherited: 94.2% line (2234/2372),
87.5% branch (1171/1338), 628 tests in 96 classes, missed 138 lines and 167
branches.
The entry explains its own branch move, because this file has a history of not
doing that. Wave 4's rise is almost entirely numerator -- up 80, with the
denominator moving -4 -- unlike the 2026-08-29 seam work, where the numerator
rose 37 while the denominator fell 70 and much of the gain was scaffolding
leaving the measurement. The line denominator grew 20, which is the seams the
wave cut rather than untested code.
Also records a methodological result worth more than the percentages: #218 fixed
an intermittent flake whose six-runs-per-arm comparison caught nothing either way
and proved nothing, and was settled by a deterministic mutation instead -- then
confirmed when the race reproduced on #217's leg, below the fix.
The JUnit-timeout trap gains the outcome it was waiting for: the jstack watchdog
caught #125's Room/WorkManager deadlock on CI in 10m57s with the hung test named,
against that ticket's prediction of a 60-minute cap and no cause. #125 is closed
as bounded; the inversion is internal to the two libraries and still live at
work-runtime 2.11.2 / room 2.7.0.
Instrumented counts are untouched: #221 is what changes them, so it carries them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JMR-devandClaude Opus 5