Compare commits

...
Author SHA1 Message Date
JMR-dev 7b578c1ccf Merge branch 'main' into docs/instrumented-tests-correction 2026-08-24 17:10:27 -05:00
Jason Ross 9e7f80feaa Merge pull request #72 from JMR-dev/test/r38-2-filecard
Say in tests what the file card says when it does not know
2026-08-24 17:10:06 -05:00
JMR-dev 3c5a37fd3c Merge branch 'main' into test/r38-2-filecard 2026-08-24 17:02:19 -05:00
Jason Ross af13155c27 Merge pull request #71 from JMR-dev/test/r38-4-advanced-picker
Hold the Advanced panel's gate, and the error card outside it
2026-08-24 17:01:33 -05:00
JMR-dev 4ea5afefe1 Merge branch 'main' into test/r38-4-advanced-picker 2026-08-24 16:54:10 -05:00
Jason Ross 7e4f22322b Merge pull request #69 from JMR-dev/tools/file-issue-script
Check the shell, and stop one-off issues falling off the board
2026-08-24 16:52:16 -05:00
Jason Ross 2ee97e30b7 Merge branch 'main' into tools/file-issue-script 2026-08-24 16:43:41 -05:00
Jason Ross d8f1590d2b Merge pull request #67 from JMR-dev/test/r38-3-pickers
Hold the three pickers to the constant they hand back
2026-08-24 16:43:16 -05:00
JMR-dev 3d51fefeff Say where instrumented tests run, instead of where they used to not run
CLAUDE.md carried three claims about instrumented tests. All three were false, one of
them contradicted a paragraph forty lines below it in the same file, and a subagent
working on #58 hit the contradiction and had to stop and flag it rather than trust the
project's own instructions. That is the cost being paid here: this file is what every
contributor and every agent reads first.

  "Instrumented tests do not run locally"  -- they do, API 33-36, since 22c7914.
  "Emulators segfault on this host"        -- solved 2026-08-22; it was SwiftShader's
                                              Reactor JIT meeting SELinux execheap, not a
                                              broken machine, and another renderer avoids
                                              it. docs/local-emulator.md is titled
                                              "Emulators do run on this host".
  "CI's matrix therefore stops at API 36"  -- the matrix has been 33/34/35/36/37 since
                                              #56 merged, with a gating API 37 leg.

The contradiction was the worst of it. The testing-norm section added in #51 says "E2E is
runnable locally now", so the file simultaneously told you the emulator works and that it
segfaults on every AVD. A reader has no way to tell which half is current, and the wrong
half is the one that stops work: an agent that believes emulators are impossible here does
not try, and the local e2e half of the definition-of-done in #51 quietly stops being
enforceable.

The replacement says what is true now and names what is still true and why -- API 37 still
needs the manual Pixel check before a release, because the two advisory tests are the one
thing CI cannot answer for. It also states plainly that the advisory job is red on every
PR by design, which is the other thing agents keep rediscovering the hard way: three
separate subagents have now flagged that failure as possibly theirs.

The norm bullet now points at the section rather than re-arguing it, so there is one place
to correct next time rather than two that can drift apart again.

Same defect class as R14, R15, R20 and R25, all of which were documentation claims this
repo's own review falsified. The pattern is not that the docs were careless; it is that
they were written at a moment and the moment moved.
2026-08-24 16:26:47 -05:00
JMR-dev 6cd17f25aa Pin shellcheck, because the unpinned one disagreed with the local run
The step added in the previous commit went red on its own PR, and the reason is the one
CLAUDE.md already gives for pinning ktlint, detekt and JaCoCo: "a new rule in a linter
makes files nobody touched stop passing, so CI goes red on a PR whose diff cannot explain
it." Here it was not even a new rule, just a different version of the same tool.

The runner's ambient shellcheck is 0.9.0. The container used to check locally was 0.11.0.
They disagree about how to report `on_signal`, which is installed as the INT and TERM trap
eleven lines below its declaration and so is never called by name:

  0.11.0  SC2329, once, on the function declaration -- "never invoked"
  0.9.0   SC2317, seven times, one per command in the body -- "appears to be unreachable"

The disable directive named SC2329, so 0.11.0 was silent and 0.9.0 reported seven findings.
Nothing about the script was wrong; the local check simply was not the check CI ran.

Two changes, because either alone still leaves a way to be surprised:

  - CI runs shellcheck from an image pinned by digest, so an upgrade is a line in this
    file that someone chose, not something that arrives on a Tuesday. The version is
    still printed, so a finding out of nowhere can be tied to that line.

  - The directive names SC2317 and SC2329 both, so a contributor whose distro ships 0.9.0
    gets the same answer locally as CI gives. Verified against both images: clean under
    0.9.0 and clean under 0.11.0.

CLAUDE.md now says to check with the pinned digest rather than with whatever is installed,
which is what would have caught this before the push.
2026-08-24 16:13:05 -05:00
JMR-devandClaude Opus 5 fea88a281f Say in tests what the file card says when it does not know
"Size unknown" is the line a stream fixing D5 reported as untestable. It is two
assertTextEquals calls, and it needed two rather than one: the size line renders
independently of the probe, so it is asserted with a probe and without one. That
independence is the contract, and a test of the probed case alone would leave the
branch a user hits first -- the card is on screen before the probe finishes --
unguarded.

The rest of the card degrades in words the same way, and none of it was covered:
the four InputKind branches, "No video track", "No audio track", describeVideo's
"Unknown", and the two `> 0` guards that drop the dimension and length rows
rather than printing 0 and 0:00. Each guard gets a case on both sides, because
the present side alone stays green when the guard is deleted -- what deleting it
produces is "Size: 0x0" and "Length: 0:00", the same invented-measurement defect
as "0 B".

The four pure helpers go in a plain JVM class beside it, with formatBytes pinned
at each threshold and one byte below it. A `>=` quietly becoming a `>` is only
visible from a value sitting exactly on the boundary.

Two things the issue could not have known:

- Its second acceptance criterion, "delete the return@Column and watch the
  Reading... test go red", cannot happen -- it does not compile. The early return
  is what smart-casts `probe` non-null, so ten uses below it fail with "Only safe
  (?.) or non-null asserted (!!.) calls are allowed on a nullable receiver". The
  exit is enforced by the compiler, not by a test. Both compilable regressions
  someone would land instead are covered and were run red.
- CodecNames.describeAudio has no UNPARSEABLE arm, unlike describeVideo, so it
  answers the raw sentinel rather than "Unrecognised". Unreachable today, because
  the UNPARSEABLE kind renders the explanatory line instead of rows. Left alone;
  recorded on the PR for R38.5.

The divider's absence is not asserted and cannot be: Material 3 renders it as a
Box with no semantics modifier, so it contributes no node. What is asserted is
everything it precedes, plus the card's child count. The class KDoc says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 16:10:38 -05:00
JMR-devandClaude Opus 5 7c69d0699a Hold the Advanced panel's gate, and the error card outside it
`AdvancedPicker` is the one leaf on the converter screen that carries its own
state, and `ValidationError` is deliberately invoked after the
`AnimatedVisibility` that gates the chip rows -- so an invalid spec explains
itself and offers one-tap fixes while the section is collapsed. That is the
only route out of an invalid spec for a user who never opened Advanced, it was
completely untested, and folding the two `if` blocks into one is a plausible
tidy-up that compiles.

`AdvancedPickerTest` covers the gate in both directions, clicks each of the
four colliding chip labels through its own row tag, and does every assertion
about the error card with the toggle untouched.

`AdvancedPanelSavedStateTest` is the `DestinationSaverTest` split for
`expanded`: `StateRestorationTester` saves into an in-memory map, so it proves
`rememberSaveable` is in use and nothing about the representation. Driving a
real `SaveableStateRegistry` shows the picker saves the `MutableState` itself
rather than the `Boolean`, which only survives a rotation because
`mutableStateOf` on Android returns a `Parcelable` one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 16:06:00 -05:00
JMR-dev baaaa934e0 Check the shell, and stop one-off issues falling off the board
Two gaps, both found the same way -- by something going wrong quietly.

`gh issue create` does not touch the project board. The issue is created, carries its
labels, and is invisible in the Kanban, which looks exactly like a ticket nobody filed.
On 2026-08-24 eight issues filed as a scripted batch all reached the board and one filed
as a one-off minutes later did not; it surfaced only because someone went looking for it.
A batch carries the board step inside its loop. One-offs are where it slips, so
tools/github/file-issue.sh is for one-offs.

Three things it does that a two-command shell snippet would not:

  - Resolves the project, Status field and option ids BY NAME, every run. Caching them
    is the obvious optimisation and the wrong one -- a renamed or reordered column would
    then have this writing a stale id into the board with no error anywhere.

  - Reads the item back. A mutation returning 200 says the request was accepted, not that
    the board shows what was asked for; the read-back is the only step that checks the
    claim this script exists to make. It is a GraphQL query because REST cannot do it --
    the `fields` array REST returns on a project item carries Title and nothing else, so
    a REST-only check reports every item's Status as unset.

  - Exits 3, loudly, with the issue number on a line of its own, when the issue was
    created but the board step failed. That exact combination is the failure being
    prevented; it must never be the quiet path.

Shell was the other language here with nothing checking it -- four scripts, one of them
the CI entry point. shellcheck now runs in the Static analysis job over
`git ls-files '*.sh'`, so a script added later is covered without editing the workflow,
and it runs at full severity with `info` included.

That raises two findings today and both are the tool being wrong, so both are answered
with a targeted `disable` carrying its reason rather than by lowering the severity:
run-e2e.sh's `on_signal` is reported as never invoked when it is installed as the INT and
TERM trap eleven lines below it, and the `$names` inside file-issue.sh's queries are
GraphQL variables that must not expand -- expanding them would send the shell's idea of
$owner to the API instead of declaring a parameter. A blanket --severity=warning would
have hidden both, and the next real finding with them.

The gradle step gains `if: !cancelled()` so a shellcheck failure cannot cost the
ktlint/detekt/lint lists -- the same reason that step already passes --continue.

Not covered, deliberately: shellcheck here reads .sh files, not the inline `run:` blocks
in the workflows, where a good deal of this repo's bash actually lives. actionlint does
read them, and finds one pre-existing info-level issue in build.yml. Wiring it in means
pinning a container digest, because every action here is pinned by SHA and actionlint's
usual installer is a curl-pipe-bash off a moving branch. Its own ticket, not this commit.
2026-08-24 16:03:14 -05:00
JMR-devandClaude Opus 5 2bed40d080 Hold the three pickers to the constant they hand back
The format, quality and engine pickers are the same dozen lines with a
different enum substituted, and both ways they can go wrong are silent.
An onClick that closes over the picker's `selected` parameter instead of
the chip's own entry returns one constant for every chip; an inverted
`entry == selected` lights every chip but the right one. Neither throws,
neither changes the labels on screen, and a test that only asserted the
callback ran would pass over the first of them.

So each click test presses every chip in the row and compares the whole
recorded list against `entries`, which makes the constant load-bearing
rather than the click count, and each selection test asserts over every
chip rather than only the one that should be lit.

Verified by mutation, not by the suite going green:

- `onSelect(format)` -> `onSelect(OutputFormat.MP4_H264)` fails with
  `expected:<[MP4_H264, MP4_H265, WEBM_VP9, ...]> but was:<[MP4_H264,
  MP4_H264, MP4_H264, ...]>`
- `format == selected` -> `format != selected` fails both format
  selection tests on `Selected = 'true'` for a chip that should not be
- `onSelect(preference)` -> `{}` fails with `expected:<[AUTO,
  PREFER_HARDWARE, FORCE_SOFTWARE]> but was:<[]>`
- `selected.description` -> `QualityTier.FAST.description` fails the
  quality prose test on the missing BEST line

Labels are read off the enums so a reword cannot redden this file for
the wrong reason. `EnginePreference` has no label of its own, so the
screen's own `label()` supplies that set. The one display literal with
no symbol behind it, the custom-spec line, was copied out of the source
byte for byte because it holds a U+2014 that would fail silently if
retyped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 15:58:48 -05:00
Jason Ross 175472ae88 Merge pull request #65 from JMR-dev/test/r38-1-screen-test-seam
Make the screen leaves nameable from a test, and give tests a tag vocabulary
2026-08-24 15:53:38 -05:00
JMR-devandClaude Opus 5 df2e42a2b3 Name the screen leaves, and give the tests a tag vocabulary
Kotlin `private` on a top-level declaration is file-scoped, so every leaf
composable in the two screens was invisible even to the JVM test source set,
which is a friend of main. The only three declarations src/test could name were
ConverterScreen, JoinScreen and AppRoot -- there was nothing to write a test
against, which is why #52 could not be started as filed.

`internal` is the same choice MainActivity already documents for Destination:
the unit tests can name it, and it stays invisible to anything outside the
module. Eleven declarations in ConverterScreen and FileRow in JoinScreen.

The tag table is the other half. Tests reference a symbol rather than a literal,
which is what keeps the five children that follow independent: "Cancel", "Start
over" and "Save file" are each rendered by both screens and by more than one
state branch, so rewording one would otherwise redden several PRs at once and no
diff would explain why. There were zero testTag, semantics or contentDescription
calls anywhere in main before this.

Every tag is applied inside main. A tag a test hands down as a Modifier proves
only that the test set it -- it would survive the affordance losing its own tag
entirely, which is the vacuous shape CLAUDE.md records nine of in one review.
That is why FileRow and DetailRow derive theirs from data they already hold
rather than taking an index from the call site.

TestTags is public rather than internal, and R38.8 is the reason. It reads the
table from androidTest, and whether that is a friend source set of main under
AGP 9 had no in-tree answer -- nothing referenced a main internal from there.
Settled by compiling one: it is a friend, so internal would work today. Public
anyway, because that friendship is AGP wiring rather than something this project
states, and the KDoc now carries the measurement so nobody has to repeat it.

Smoke tests cover each leaf: it renders, and its tag resolves to exactly one
node. Counting rather than asserting existence is deliberate, since a duplicated
tag fails differently depending on which finder a later test happens to use. The
state-branch tags -- Convert, Cancel, Save file, Start over, the progress bars --
have no bite yet: reaching a branch needs the state seam R38.5 extracts, and the
state matrix is R38.6/R38.7 by design.

Five mutations, all red on the named test alone: FORMAT_CHIPS deleted,
FILE_CARD_NAME deleted, detailRow's tag no longer derived from its label,
FileRow's no longer derived from its name, and JOIN_MORE given JOIN's value --
the last caught only by the uniqueness check, which is what it is for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 15:30:52 -05:00
JMR-devandClaude Opus 5 64c1a60a97 Drain the escaped coroutine error before the next test starts
`ConversionViewModelProbeFailureTest.an OutOfMemoryError is not swallowed`
deliberately lets a real error escape `viewModelScope.launch`, which has no
exception handler by design -- the ViewModel's KDoc says an OOM raised in the
probe should reach the thread's handler and take the process down.

On the JVM something else takes it. kotlinx-coroutines-test installs a
process-wide collector for uncaught coroutine errors, keeps whatever it catches,
and hands the backlog to the next `runTest` that starts, which throws
UncaughtExceptionsBeforeTest. Every Compose test is a `runTest`: that is how
`createComposeRule` runs a composition. So the error lands on an unrelated test
in an unrelated file, and the message names neither the test that caused it nor
the error's origin.

Nothing has hit it yet only because the sole Compose test in the repo happens to
run before the ViewModel one. R38 adds six more Compose classes in exactly the
two packages that surround it, and the first two of them made the suite fail in
two different files on two consecutive runs of identical, green code -- the
throw is on a real Dispatchers.IO thread, delivered after the state assertion
that ends the test responsible, so which class catches it is a race.

Drain it where a Compose rule is built. A @Before cannot: the rule's `runTest`
wraps the statement that calls it, so it has already thrown. @BeforeClass cannot
either, because Robolectric runs it outside the sandbox classloader, where the
collector is a different object. Constructing the rule is early enough, since
JUnit builds a fresh test-class instance -- and every @get:Rule field on it --
before evaluating any rule.

This is containment, not the cure. The cure is a seam: give the probe hop an
injectable dispatcher the way the constructor already does for
cleanupDispatcher, so the error has somewhere to land. That is a production
change and deserves its own commit.

kotlinx-coroutines-test was already on the unit-test classpath through
compose-ui-test-junit4; it is declared now because a file imports it. Pinned,
like robolectric and the linters: org.jetbrains.kotlinx is not one of the groups
the prerelease guard covers, so a float here would be free to take a milestone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 15:30:32 -05:00
Jason Ross 1779f20a03 Merge pull request #56 from JMR-dev/ci/api37-split
Split the API 37 CI leg so the part that works can gate
2026-08-23 17:35:52 -05:00
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>
2026-08-23 17:20:49 -05:00
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>
2026-08-23 17:20:40 -05:00
Jason Ross 577dae998b Merge pull request #55 from JMR-dev/ci/api37-debug
Verify the SystemUI disable instead of trusting what pm reported
2026-08-23 09:49:15 -05:00
JMR-devandClaude Opus 5 acc71bcaee Verify the SystemUI disable instead of trusting what pm reported
Four dispatches of one configuration -- API 37.0, swiftshader_indirect,
SystemUI disabled -- came back three green and one not, and the odd one out
was not a different failure so much as the same run without the fix applied.
In 32646029143 `pm disable-user` reported `new state: disabled-user` and
SystemUI then started eight more times:

  14:41:24 ActivityManager: Start proc 6412:com.android.systemui ... GradientColorWallpaper
  14:45:05 ActivityManager: Start proc 17299:com.android.systemui ... GradientColorWallpaper

with ten more RegionSampling aborts and a surfaceflinger pid that never sat
still (489, 1570, 3524, 4396, 6038, 7987, 9732, 11520, 13208, 15048). The
framework is being SIGKILLed every twenty seconds while this runs, so a
package-state change can go down with the system_server that accepted it.

Two things were wrong, and the second is why the first went unnoticed:

  - one disable attempt was treated as sufficient
  - the wait after `adb shell stop` was not a wait. It asked `service check`
    0.3 s later and got `found` from the system_server that was still on its
    way out, so it never waited for anything. Both the good and the bad run
    printed `services back after 5 s`, which is how a broken fix looked
    identical to a working one.

Now: up to three rounds of disable -> take the framework down and confirm
system_server is actually gone -> bring it back -> verify the package is in
`pm list packages -d` -> require a 45 s window with zero new aborts. Nothing
is believed because a command said so.

Also adds measure_baseline, default true. The 45 s pre-measurement is what
makes the rate comparable with the local figures, but it is 45 s of
crash-looping before the disable has to land, which is a worse starting
point than a real leg would have.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-23 09:48:53 -05:00
Jason Ross b97d7c36a3 Merge pull request #54 from JMR-dev/ci/api37-debug
Resolve adb by path in the API 37 watchdog
2026-08-23 09:28:19 -05:00
JMR-devandClaude Opus 5 93398c4616 Resolve adb by path in the API 37 watchdog
The first three dispatches came back with every watchdog sample reading
`boot=? surfaceflinger=none zygote64=none dma_aborts=0`, on runs where the
device demonstrably booted and the action's own adb was working two steps
away. The watchdog was not measuring anything.

The emulator action puts platform-tools on PATH with core.addPath, which
writes GITHUB_PATH and therefore only affects LATER steps. The watchdog is
started before the action -- that is the whole point of it -- so it inherits
the runner's own PATH, where a bare `adb` is not necessarily anything. Every
call failed into `2>/dev/null` and the sampler dutifully recorded the silence
as zero.

It now resolves adb by path, preferring ANDROID_HOME, re-resolving on every
iteration in case platform-tools arrives later, and echoing the path it
settled on. The launch step prints ANDROID_HOME and `command -v adb` for the
same reason: a repeat of this failure should be one line to spot, not three
runs of quiet zeros.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-23 09:27:57 -05:00
Jason Ross 19a1e66277 Merge pull request #53 from JMR-dev/ci/api37-debug
Add a dispatch-only workflow for the API 37 CI question
2026-08-23 09:15:49 -05:00
JMR-devandClaude Opus 5 8fdad6e20b Add a dispatch-only workflow for the API 37 CI question
status_check.yml stops its E2E matrix at 36 and says the android-37.0 image is
why. That is established locally under -gpu host and under ANGLE, and it is not
established for CI: runners use -gpu swiftshader_indirect, and the one local
measurement of that mode was void for a local reason -- Fedora denies execheap
to SwiftShader's JIT, so the emulator died before the guest mattered. What CI
does at API 37 has therefore never actually been measured.

This is that E2E job with the matrix replaced by workflow_dispatch inputs, so a
hypothesis costs a dispatch rather than a commit: renderer, API level, image
target, channel, SystemUI disable, boot timeout, whether the suite runs at all,
and free-form emulator and Gradle arguments. It triggers on nothing else and
gates nothing.

It calls .github/scripts/e2e-run.sh rather than forking it, and pins the same
disk-size, ram-size, action SHAs and KVM setup as the job it copies, so a run
here measures the renderer and not a different device.

The watchdog is load-bearing rather than decorative. The emulator action calls
killEmulator() from its own catch block, so a run whose emulator never boots is
torn down before any script: line executes and leaves nothing behind -- which is
the exact failure shape API 37 is suspected of. It starts before the action,
samples sys.boot_completed, the surfaceflinger and zygote pids and the
hasReadColorBufferDma abort count every 20 s, and keeps a rolling copy of the
crash buffer so the last read survives the teardown.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-23 09:15:08 -05:00
Jason Ross 742703d360 Merge pull request #51 from JMR-dev/docs/definition-of-done
Write down that testable code is not done until it is tested
2026-08-23 08:52:00 -05:00
JMR-devandClaude Opus 5 6c34fad17c Write down that testable code is not done until it is tested
Stated as a project norm: if a piece is unit testable it gets unit tests, and
if it is e2e testable it gets e2e tests, before it counts as done. Both clauses,
not either/or.

Recorded here rather than left as a habit because the recent review measured
what happens without it. Forty-six mutations were run against a 257-test suite;
thirty-six bit and NINE were vacuous, five of those passing the entire suite
while a reattachment code path sat completely unguarded. That code had shipped,
been reviewed, and looked tested. "The suite is green" was true and meant
nothing.

The convention also names the two things that make it enforceable rather than
aspirational. Unit-testable is broader than it looks, because the pure-seam
pattern converts device-bound logic into a testable function plus a thin edge,
and Robolectric now covers the rest including Compose. And e2e is genuinely
runnable locally since the emulator renderer cause was found -- until last
night, "run the instrumented suite" was not a request anyone could act on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-23 08:50:15 -05:00
Jason Ross 9c4f14202f Merge pull request #50 from JMR-dev/docs/coverage-figure
Measure the coverage figure instead of carrying it forward
2026-08-23 00:51:23 -05:00
JMR-devandClaude Opus 5 2747bb8627 Measure the coverage figure instead of carrying it forward
CLAUDE.md has said "~31% of lines" since the lint/format work landed. Measured
on main today it is 29.8% (629/2113 lines, 408/1424 branches).

The number went DOWN, which is worth stating rather than quietly correcting.
The JVM suite went from 11 test files to 43 over the same period, so the
intuition -- and the review finding that prompted this, which called the
direction certain -- was that coverage must have risen. It did not: main source
grew from 4,114 to 5,715 lines as the fixes added Reattachment, JobSnapshots,
JobTags, InputQuery, StagingNames, StagingSweep, NativeLoadFailure and an
Application class. The denominator outran the numerator.

That is not an argument against the tests. It is an argument against quoting a
coverage percentage from memory, which is exactly how the stale figure survived.
The line now carries the measurement, its date, and the instruction to re-measure.

R30 / #39

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-23 00:22:53 -05:00
Jason Ross 6d6d2189ca Merge pull request #47 from JMR-dev/tools/api-37-emulator
Re-derive the API 37 emulator failure, and harden the local sweep
2026-08-23 00:21:26 -05:00
25 changed files with 2924 additions and 89 deletions
+98
View File
@@ -40,6 +40,104 @@ WEDGE_LOG="$TMP/wedge-diagnostics-api${LABEL}.txt"
# enough under the job's 60-min cap that a genuine wedge still leaves time to capture it.
WEDGE_TIMEOUT=1200
# ---------------------------------------------------------------------------
# API 37 only, and nothing else sets it, so this is inert everywhere it is not wanted --
# the same shape as E2E_EXTRA_GRADLE_ARGS below. The other four E2E legs run byte-identical
# commands with it unset.
#
# WHY IT RUNS HERE, BEFORE THE LOGCAT STREAM: `adb shell stop` ends the `adb logcat` started
# below, and nothing restarts it, so a disable performed after that point would cost this leg
# its whole diagnostic story for the part of the run that matters. Everything this function
# counts comes from `adb logcat -d -b crash`, which is a fresh read each time and independent
# of the stream.
#
# WHAT IT IS FOR: the android-37.x images abort surfaceflinger from RegionSamplingThread inside
# their own gralloc 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
# because SystemUI registers a nav-bar luma-sampling listener, so removing the package removes
# the whole chain. Measured cadence of those kills: 20-90 s apart, median 60-70 s, three to five
# in a four-minute window -- fast enough that install and instrumentation start-up do not fit
# inside one gap.
#
# 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
# started SystemUI eight more times, with ten more aborts. `pm disable-user` can be accepted by
# a system_server that is SIGKILLed before the state is written, and `pm disable-user` does not
# 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'; }
systemui_disabled() { adb shell pm list packages -d 2> /dev/null | grep -q 'com.android.systemui'; }
disable_region_sampling() {
local round=1 i out before after
while [ "$round" -le 3 ]; do
echo "--- SystemUI disable, round $round ---"
for i in $(seq 1 10); do
out="$(adb shell pm disable-user --user 0 com.android.systemui 2>&1 | tr -d '\r')"
echo " pm attempt $i: $out"
case "$out" in *"new state: disabled"*) break ;; esac
sleep 5
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
echo " verified: com.android.systemui is in pm list packages -d"
else
echo " NOT DISABLED after the restart -- the package state did not survive"
round=$((round + 1))
continue
fi
before="$(count_aborts)"
sleep 45
after="$(count_aborts)"
echo " abort rate, SystemUI disabled: $((after - before)) new in 45 s (total ${after:-0})"
[ "$((after - before))" -eq 0 ] && break
echo " still aborting after round $round"
round=$((round + 1))
done
# 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
# buffer and the diagnostics attached, which is more useful than dying here with none of it.
if systemui_disabled; then
echo " final state: SystemUI disabled"
else
echo "::warning::E2E api${LABEL}: SystemUI is still enabled -- expect INSTRUMENTATION_ABORTED"
fi
return 0
}
if [ "${E2E_DISABLE_SYSTEM_UI:-}" = "1" ]; then
echo "::group::E2E api${LABEL} -- removing the region-sampling listener"
disable_region_sampling
echo "::endgroup::"
fi
# Stream logcat from now until the step ends, into a file that survives to the artifact upload.
# Without this, a failure that happens on-device leaves nothing behind: `adb logcat -d` at the
# end only has whatever is still in the ring buffer, and a chatty test run evicts the cause.
+413
View File
@@ -0,0 +1,413 @@
name: API 37 debug
# ---------------------------------------------------------------------------
# WHAT THIS IS FOR, AND WHY IT IS SEPARATE
#
# The android-37.x emulator images abort surfaceflinger inside their own gralloc
# mapper. That was established locally, under -gpu host and under ANGLE
# (docs/api-37-emulator-crash.md), but NOT on a GitHub runner: CI runs
# -gpu swiftshader_indirect, and the one local measurement of that mode was void for a
# purely local reason (Fedora's SELinux denies execheap to SwiftShader's Reactor JIT --
# docs/local-emulator.md). What CI actually does at API 37 was an open question, and
# this workflow is the instrument that answered it.
#
# IT IS STILL THE INSTRUMENT. status_check.yml now carries API 37 -- a gating leg that
# disables SystemUI first, and an advisory one for the two @FailsOnEmulatorApi37 tests
# -- so this file's job is no longer to decide that, but to test a change to it for one
# dispatch instead of one commit. The next questions it exists for are written down
# under "When to revisit" in docs/api-37-emulator-crash.md: a new API 37.x image, or an
# ATD image for 37, either of which could retire the whole workaround.
#
# It is a copy of that E2E job with the matrix replaced by workflow_dispatch inputs,
# so one hypothesis costs one dispatch rather than one commit. It triggers on nothing
# else: no push, no pull_request, no schedule. Nothing depends on it and it gates
# nothing.
#
# TWO THINGS THIS DELIBERATELY DOES NOT DO:
#
# - 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
# exercises every day. This calls it, exactly as status_check.yml does.
# - 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 WATCHDOG IS THE POINT, not a nicety. reactivecircus/android-emulator-runner calls
# killEmulator() from its own catch block, so a run whose emulator never boots is torn
# down before a single `script:` line executes -- no probe, no e2e-run.sh, no artifacts,
# nothing to read afterwards. That is precisely the failure shape API 37 is suspected of.
# The watchdog therefore starts BEFORE the action, from outside it, and samples the device
# on its own clock.
# ---------------------------------------------------------------------------
on:
workflow_dispatch:
inputs:
api_level:
description: 'API level, as the SDK spells it. 37.0, 37.1, 37.2-beta3, 36 ... A bare 37 does not exist and fails during SDK setup.'
type: string
default: '37.0'
target:
description: 'System image target. android-37.1 and 37.2-beta* ship ONLY as google_apis_ps16k -- there is no plain google_apis above 37.0.'
type: string
default: 'google_apis'
channel:
description: 'SDK channel. beta is required for any 37.2-beta* image.'
type: choice
options: ['stable', 'beta', 'dev', 'canary']
default: 'stable'
gpu_mode:
description: 'The -gpu argument. swiftshader_indirect is what status_check.yml uses today; swangle_indirect is what works locally on API 37.'
type: choice
options:
- swiftshader_indirect
- swangle_indirect
- angle_indirect
- host
- auto
- guest
- 'off'
default: 'swiftshader_indirect'
disable_system_ui:
description: 'Take SystemUI out before the suite runs, which is what stops SurfaceFlinger RegionSamplingThread reaching the mapper bug. Restarts the framework.'
type: boolean
default: false
run_tests:
description: 'Run the instrumented suite. false boots, probes and stops -- the cheap loop when the question is only whether it boots and at what abort rate.'
type: boolean
default: true
emulator_boot_timeout:
description: 'Seconds the action waits for sys.boot_completed. Do not lower this for a software renderer: a slow boot would be misreported as a failed one.'
type: string
default: '600'
emulator_extra_options:
description: 'Appended verbatim to the emulator command line -- e.g. "-verbose", or "-feature -GLDMA,-GLDMA2". The action interpolates it into a sh -c, so "| tee $RUNNER_TEMP/emulator.log" also works and is uploaded.'
type: string
default: ''
disable_animations:
description: 'The action settings-puts three animation scales after boot. Each is an adb call that throws if the framework is mid-restart, which would kill the run before the probe. false removes three of those calls.'
type: boolean
default: true
gradle_extra_args:
description: 'Passed to e2e-run.sh as E2E_EXTRA_GRADLE_ARGS, its existing hook -- e.g. "--rerun", or -Pandroid.testInstrumentationRunnerArguments.class=... to run one class instead of the suite.'
type: string
default: ''
measure_baseline:
description: 'Measure the abort rate for 45 s BEFORE disabling SystemUI. Answers "how fast is it aborting"; costs 45 s of crash-looping first, which is a worse starting point for the disable.'
type: boolean
default: true
run-name: >-
api ${{ inputs.api_level }}/${{ inputs.target }} · gpu ${{ inputs.gpu_mode }} ·
systemui ${{ inputs.disable_system_ui && 'disabled' || 'running' }} ·
tests ${{ inputs.run_tests && 'yes' || 'no' }}
# No `concurrency` block, unlike status_check.yml. Every dispatch here runs on the same
# ref (main), so a group keyed on github.ref with cancel-in-progress would make two
# parallel experiments cancel each other -- which is the opposite of what this is for.
permissions:
contents: read
env:
GRADLE_CACHE_PATHS: |
~/.gradle/caches
~/.gradle/wrapper
jobs:
e2e-api37:
name: E2E API ${{ inputs.api_level }} (${{ inputs.gpu_mode }})
runs-on: ubuntu-latest
timeout-minutes: 60
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0
with:
distribution: temurin
java-version: '25' # Matches the daemon JVM pinned in gradle/gradle-daemon-jvm.properties
- uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: ${{ env.GRADLE_CACHE_PATHS }}
key: gradle-${{ runner.os }}-${{ hashFiles('**/*.gradle.kts', 'gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties') }}
restore-keys: gradle-${{ runner.os }}-
# Without this the emulator falls back to software rendering and takes minutes
# longer to boot, when it boots at all.
- name: Enable KVM
run: |
echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' \
| sudo tee /etc/udev/rules.d/99-kvm4all.rules
sudo udevadm control --reload-rules
sudo udevadm trigger --name-match=kvm
# Both helpers live in RUNNER_TEMP rather than in the repository: they are debug
# instrumentation for this workflow only, and writing them here keeps the whole
# experiment in one file that can be read top to bottom.
- name: Write the watchdog and the probe
env:
LABEL: ${{ inputs.api_level }}
run: |
cat > "$RUNNER_TEMP/watchdog.sh" <<'WATCHDOG'
#!/usr/bin/env bash
# Samples the device from outside the emulator action, because the action tears the
# emulator down on a boot timeout before any script: line runs. Everything here is
# `timeout`-wrapped: a wedged adb must not stall the sampler, and no probe may fail.
SERIAL="emulator-5554"
# adb is resolved by path, not by name. The emulator action puts platform-tools on
# PATH with core.addPath, which only affects LATER steps -- this one already exists
# by then, so a bare `adb` here is not the runner's adb and may be nothing at all.
# The first version of this file assumed otherwise and every sample came back
# boot=? dma_aborts=0 while the action's own adb was working fine two steps away.
# Re-resolved every iteration because platform-tools may be installed after this
# starts, and echoed to stdout so a repeat of that failure is visible immediately.
ADB=""
resolve_adb() {
for c in "$ADB" "${ANDROID_HOME:-}/platform-tools/adb" "${ANDROID_SDK_ROOT:-}/platform-tools/adb" "$(command -v adb 2> /dev/null)"; do
if [ -n "$c" ] && [ -x "$c" ]; then
[ "$c" = "$ADB" ] || echo "watchdog: adb resolved to $c"
ADB="$c"
return 0
fi
done
return 1
}
OUT="$RUNNER_TEMP/watchdog-api$LABEL.txt"
CRASH="$RUNNER_TEMP/crash-buffer-api$LABEL.txt"
GUESTLOG="$RUNNER_TEMP/watchdog-logcat-api$LABEL.txt"
STOP="$RUNNER_TEMP/watchdog.stop"
# A continuous guest logcat, restarted whenever the device goes away. During a
# surfaceflinger crash loop the framework restarts every few seconds and adb goes
# with it, so a single `adb logcat` would end at the first restart.
(
while [ ! -f "$STOP" ]; do
if resolve_adb; then
timeout 120 "$ADB" -s "$SERIAL" wait-for-device > /dev/null 2>&1 \
&& timeout 3000 "$ADB" -s "$SERIAL" logcat -v time >> "$GUESTLOG" 2>&1
fi
sleep 3
done
) &
echo "watchdog started $(date -u +%FT%TZ) -- serial $SERIAL" >> "$OUT"
i=0
while [ "$i" -lt 300 ]; do
i=$((i + 1))
[ -f "$STOP" ] && break
if ! resolve_adb; then
echo "$(date -u +%T) no adb yet" >> "$OUT"
sleep 20
continue
fi
boot="$(timeout 20 "$ADB" -s "$SERIAL" shell getprop sys.boot_completed 2> /dev/null | tr -d '\r\n')"
sf="$(timeout 20 "$ADB" -s "$SERIAL" shell pidof surfaceflinger 2> /dev/null | tr -d '\r\n')"
zy="$(timeout 20 "$ADB" -s "$SERIAL" shell pidof zygote64 2> /dev/null | tr -d '\r\n')"
# Kept as a file rather than a variable so the last successful read survives the
# action killing the emulator -- which is when it is most worth having.
if timeout 30 "$ADB" -s "$SERIAL" logcat -d -b crash > "$CRASH.new" 2> /dev/null; then
mv "$CRASH.new" "$CRASH"
fi
dma="$(grep -c 'hasReadColorBufferDma' "$CRASH" 2> /dev/null || true)"
sigabrt="$(grep -c 'signal 6' "$CRASH" 2> /dev/null || true)"
printf '%s boot=%-4s surfaceflinger=%-8s zygote64=%-8s dma_aborts=%-5s sigabrt=%s\n' \
"$(date -u +%T)" "${boot:-?}" "${sf:-none}" "${zy:-none}" "${dma:-0}" "${sigabrt:-0}" >> "$OUT"
# One shot, the first time the device is up: which GLES implementation the guest
# actually got. This is the guest-side answer to the same question the emulator's
# own gles_mode_selected line answers host-side.
if [ "$boot" = "1" ] && [ ! -f "$RUNNER_TEMP/renderer-api$LABEL.txt" ]; then
{
echo "=== booted at $(date -u +%FT%TZ), watchdog sample $i ==="
timeout 30 "$ADB" -s "$SERIAL" shell dumpsys SurfaceFlinger 2>&1 | head -40
echo "--- getprop ---"
timeout 20 "$ADB" -s "$SERIAL" shell getprop 2>&1 | grep -Ei 'egl|gles|gpu|ranchu|gfxstream' || true
} > "$RUNNER_TEMP/renderer-api$LABEL.txt" 2>&1
fi
sleep 20
done
echo "watchdog finished $(date -u +%FT%TZ) after $i samples" >> "$OUT"
WATCHDOG
cat > "$RUNNER_TEMP/probe.sh" <<'PROBE'
#!/usr/bin/env bash
# Runs on the booted device, before the suite. Two jobs: record what the guest got,
# and measure the gralloc abort RATE -- which is the number that decides whether a
# five-minute test run can survive, and the one comparable with the local figures in
# docs/api-37-emulator-crash.md (10-11 per 150 s idle under ANGLE with SystemUI up).
#
# Never exits non-zero. The action runs script: lines in one try/catch, so a failing
# probe would skip e2e-run.sh entirely and the run would measure nothing.
exec > >(tee -a "$RUNNER_TEMP/probe-api$LABEL.txt") 2>&1
echo "===== probe api$LABEL -- $(date -u +%FT%TZ) ====="
adb shell getprop sys.boot_completed
adb shell getprop ro.build.fingerprint
adb shell getprop ro.build.version.sdk
echo "--- SurfaceFlinger (the GLES line names the renderer the guest is on) ---"
adb shell dumpsys SurfaceFlinger 2>&1 | head -30
echo "--- binder services ---"
for s in package activity window; do adb shell service check "$s" 2>&1; done
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'; }
if [ "${MEASURE_BASELINE:-true}" = "true" ]; then
before="$(count_aborts)"
sleep 45
after="$(count_aborts)"
echo "--- abort rate, SystemUI running: $((after - before)) new in 45 s (total ${after:-0}) ---"
else
# Skipped on purpose when the question is reliability rather than rate: every
# second spent measuring is a second of crash-looping, and the disable is what
# has to land. A real CI leg would disable as early as it can, so measure that.
echo "--- baseline window skipped (MEASURE_BASELINE=false) ---"
fi
if [ "${DISABLE_SYSTEM_UI:-false}" = "true" ]; then
# Three rounds, because ONE round is not reliable and the failure is silent.
# Measured: of four runs of the same configuration, three came back with the
# suite running and one (32646029143) had SystemUI restarting throughout --
# `ActivityManager: Start proc N:com.android.systemui ... GradientColorWallpaper`
# eight more times after a `pm disable-user` that had reported
# `new state: disabled-user`, and ten more RegionSampling aborts with it. The
# framework is being SIGKILLed under this loop, so a package-state change can be
# lost with the system_server that accepted it. (An earlier version of this comment
# said "every ~20 s". That was the watchdog's SAMPLING interval, not the cadence.
# Measured: 20-90 s between aborts, median 60-70 s, 3-5 in a four-minute window --
# docs/api-37-emulator-crash.md, "Abort cadence, corrected".)
#
# Nothing here trusts a command's own report. Each round: disable, take the
# framework down and confirm it is DOWN before bringing it back (the previous
# version 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), then verify
# the package is really disabled and that no abort lands in a quiet window.
round=1
while [ "$round" -le 3 ]; do
echo "--- disable round $round ---"
for i in $(seq 1 10); do
out="$(adb shell pm disable-user --user 0 com.android.systemui 2>&1 | tr -d '\r')"
echo " pm attempt $i: $out"
case "$out" in *"new state: disabled"*) break ;; esac
sleep 5
done
# pm disable-user does not retract SystemUI's existing region-sampling
# registration -- by the time boot completes it has already registered. Only a
# framework restart brings back a SystemUI-less SurfaceFlinger. See
# disable_region_sampling in tools/local-emulator/run-e2e.sh.
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
echo " verified: com.android.systemui is in pm list packages -d"
else
echo " NOT DISABLED after the restart -- the package state did not survive"
round=$((round + 1))
continue
fi
before="$(count_aborts)"
sleep 45
after="$(count_aborts)"
echo "--- abort rate, SystemUI disabled: $((after - before)) new in 45 s (total ${after:-0}) ---"
[ "$((after - before))" -eq 0 ] && break
echo " still aborting after round $round"
round=$((round + 1))
done
systemui_disabled && echo "final state: SystemUI disabled" || echo "final state: SystemUI STILL ENABLED -- expect Starting 0 tests"
fi
echo "--- crash buffer (tail 60) ---"
adb logcat -d -b crash 2>&1 | tail -60
echo "===== probe done ====="
exit 0
PROBE
chmod +x "$RUNNER_TEMP/watchdog.sh" "$RUNNER_TEMP/probe.sh"
echo "helpers written to $RUNNER_TEMP"
- name: Start the watchdog
env:
LABEL: ${{ inputs.api_level }}
run: |
echo "ANDROID_HOME=${ANDROID_HOME:-<unset>} ANDROID_SDK_ROOT=${ANDROID_SDK_ROOT:-<unset>}"
echo "adb on PATH: $(command -v adb || echo '<none -- the watchdog will fall back to ANDROID_HOME>')"
nohup bash "$RUNNER_TEMP/watchdog.sh" > "$RUNNER_TEMP/watchdog-stdout.txt" 2>&1 < /dev/null &
disown
echo "watchdog pid $!"
- name: Instrumented tests
uses: reactivecircus/android-emulator-runner@a421e43855164a8197daf9d8d40fe71c6996bb0d # v2.38.0
env:
LABEL: ${{ inputs.api_level }}
DISABLE_SYSTEM_UI: ${{ inputs.disable_system_ui }}
MEASURE_BASELINE: ${{ inputs.measure_baseline }}
# e2e-run.sh's own hook, unset in CI's real workflow and therefore inert there.
E2E_EXTRA_GRADLE_ARGS: ${{ inputs.gradle_extra_args }}
with:
api-level: ${{ inputs.api_level }}
target: ${{ inputs.target }}
channel: ${{ inputs.channel }}
arch: x86_64
profile: pixel_6
emulator-boot-timeout: ${{ inputs.emulator_boot_timeout }}
emulator-options: -no-window -gpu ${{ inputs.gpu_mode }} -noaudio -no-boot-anim -camera-back none ${{ inputs.emulator_extra_options }}
disable-animations: ${{ inputs.disable_animations }}
# disk-size, ram-size: kept exactly as status_check.yml pins them, so this
# measures the renderer and not a different device. 8G because the APK plus
# FFmpeg does not fit the default userdata partition; 2560M because the
# emulator's own RAM floor varies by API level and 2560M is the highest of them.
disk-size: 8G
ram-size: 2560M
# Two lines, because the action splits script: on newlines and runs each as its
# own `sh -c`. The first is this workflow's own probe; the second is CI's real
# harness, invoked unmodified. run_tests: false replaces it with an echo rather
# than a second copy of the job.
script: |
bash ${{ runner.temp }}/probe.sh
${{ inputs.run_tests && format('bash .github/scripts/e2e-run.sh {0}', inputs.api_level) || 'echo "run_tests=false -- suite skipped, boot and probe only"' }}
- name: Stop the watchdog
if: always()
run: |
touch "$RUNNER_TEMP/watchdog.stop"
echo "----- watchdog samples -----"
cat "$RUNNER_TEMP/watchdog-api${{ inputs.api_level }}.txt" 2>/dev/null || echo "(no watchdog output)"
echo "----- renderer -----"
cat "$RUNNER_TEMP/renderer-api${{ inputs.api_level }}.txt" 2>/dev/null || echo "(never booted, or dumpsys unavailable)"
echo "----- crash buffer, last read before teardown (tail 80) -----"
tail -80 "$RUNNER_TEMP/crash-buffer-api${{ inputs.api_level }}.txt" 2>/dev/null || echo "(none)"
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
if: always()
with:
name: api37-debug-run${{ github.run_number }}
path: |
${{ runner.temp }}/watchdog-api*.txt
${{ runner.temp }}/watchdog-logcat-api*.txt
${{ runner.temp }}/watchdog-stdout.txt
${{ runner.temp }}/crash-buffer-api*.txt
${{ runner.temp }}/renderer-api*.txt
${{ runner.temp }}/probe-api*.txt
${{ runner.temp }}/emulator*.log
${{ runner.temp }}/logcat-api*.txt
${{ runner.temp }}/diagnostics-api*.txt
${{ runner.temp }}/wedge-diagnostics-api*.txt
app/build/reports/androidTests/
app/build/outputs/androidTest-results/
if-no-files-found: warn
+169 -9
View File
@@ -156,7 +156,37 @@ jobs:
key: gradle-${{ runner.os }}-${{ hashFiles('**/*.gradle.kts', 'gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties') }}
restore-keys: gradle-${{ runner.os }}-
# Shell is the other language in this repo -- four scripts, one of them the CI
# entry point itself -- and nothing was checking it. `git ls-files` rather than a
# fixed list, so a script added later is covered without editing this workflow.
#
# Full severity, `info` included. The findings it raises today are answered with
# targeted `disable` directives carrying their reason, the same way
# config/detekt/detekt.yml carries only the rules this codebase legitimately
# breaks. A blanket --severity=warning would have hidden them and the next real
# one alike.
#
# PINNED BY DIGEST, for the reason CLAUDE.md already gives for pinning ktlint,
# detekt and JaCoCo: a new rule in a linter makes files nobody touched stop
# passing, so CI goes red on a PR whose diff cannot explain it. That is not
# hypothetical here. The first cut of this step used the runner's ambient
# shellcheck, which is 0.9.0, and 0.9.0 reports a trap handler as seven
# unreachable commands (SC2317) where 0.11.0 reports it once on the declaration
# (SC2329) -- same script, same directive, different answer, and a red build on
# the PR that introduced the step. The version is printed so a finding that
# appears out of nowhere can be tied to a bump of this line.
- name: shellcheck
env:
SHELLCHECK: koalaman/shellcheck@sha256:61862eba1fcf09a484ebcc6feea46f1782532571a34ed51fedf90dd25f925a8d
run: |
docker run --rm "$SHELLCHECK" --version
git ls-files -z '*.sh' | xargs -0 -r docker run --rm -v "$PWD:/mnt" "$SHELLCHECK"
# `!cancelled()` rather than a plain sequence: a shellcheck failure above must not
# cost the ktlint/detekt/lint lists. Same reason this step passes --continue -- one
# round trip should produce every list, not stop at the first.
- name: ktlint, detekt and Android lint
if: '!cancelled()'
run: ./gradlew :app:ktlintCheck :app:detekt :app:lintDebug --continue --stacktrace
# The XML matters as much as the HTML: it is the one that can be diffed between
@@ -178,8 +208,9 @@ jobs:
# across it -- none below 34, dataSync at 34, mediaProcessing from 35. Testing a
# single level would leave two thirds of that branch unexercised.
#
# It stops at 36 rather than targetSdk 37 because the android-37.0 emulator image
# is broken, not because 37 does not matter. See docs/api-37-emulator-crash.md.
# It reaches targetSdk 37, but the API 37 row is not like the other four and the
# comment on it says how. Two tests are excluded there and run in their own
# advisory job below. See docs/api-37-emulator-crash.md.
#
# FFmpeg is not built here. The AAR is committed under bin/, so a red run means the
# code is broken rather than that a cross-compile hiccuped.
@@ -204,13 +235,28 @@ jobs:
api-level: "35"
- label: "36"
api-level: "36"
# No API 37 row. targetSdk is 37, but the android-37.0 emulator image
# crash-loops surfaceflinger inside its own gralloc mapper, so every test
# fails there no matter what this app does. Ruling that in took four CI
# rounds, so the evidence and the ruled-out fixes are written down rather
# than left to be rediscovered: docs/api-37-emulator-crash.md. That file
# also records what to try first when re-adding it -- note that the row
# needs api-level "37.0", since a bare 37 fails during SDK setup.
# 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
# SystemUI disabled and the framework restarted under it. No other leg
# and no Pixel run uses that configuration. It is defensible only because
# nothing in this suite touches system UI -- these are Media3, FFmpeg and
# WorkManager tests -- and because the alternative is no CI coverage of
# the level this app targets. **Anything that ever does depend on system
# UI must not trust this row.** E2E_DISABLE_SYSTEM_UI is what does it;
# .github/scripts/e2e-run.sh explains the mechanism and why every step of
# it is verified rather than assumed.
#
# api-level must be "37.0". A bare 37 is not an SDK package and fails
# during setup, which cost a run to discover.
#
# notAnnotation removes the two tests that do not pass on this image; they
# run in the advisory job below, off the same marker so they cannot end up
# in both or neither. docs/api-37-emulator-crash.md has the measurements.
- label: "37"
api-level: "37.0"
disable-system-ui: "1"
gradle-args: "-Pandroid.testInstrumentationRunnerArguments.notAnnotation=org.libremediaconverter.FailsOnEmulatorApi37"
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
@@ -236,6 +282,12 @@ jobs:
- name: Instrumented tests
uses: reactivecircus/android-emulator-runner@a421e43855164a8197daf9d8d40fe71c6996bb0d # v2.38.0
# Both of these are empty on every row but 37, and both are read with a
# `:-` default in e2e-run.sh, so the four legs below 37 run the identical
# gradle command they always have.
env:
E2E_DISABLE_SYSTEM_UI: ${{ matrix.disable-system-ui }}
E2E_EXTRA_GRADLE_ARGS: ${{ matrix.gradle-args }}
with:
api-level: ${{ matrix.api-level }}
target: google_apis
@@ -297,3 +349,111 @@ jobs:
name: e2e-wedge-api${{ matrix.label }}
path: ${{ runner.temp }}/wedge-diagnostics-api${{ matrix.label }}.txt
if-no-files-found: ignore
# ---------------------------------------------------------------------------
# The two API 37 tests the gating row above excludes, run on their own so they
# stay visible instead of disappearing behind a notAnnotation.
#
# continue-on-error: it reports, it never blocks. That is the whole reason it is
# a separate job rather than a sixth matrix row: a row would share the gating
# job's `E2E API <label>` name, and a check cannot be both required and advisory
# under one name.
#
# It is named for WHAT IT RUNS, deliberately. Both tests drive a full H.264 ->
# H.265 hardware transcode through Media3Engine -- which is exactly what
# separates them from the two Media3EngineTest cases that pass here, since those
# two never decode video. The current theory about why they fail is in the next
# paragraph, where it can be corrected without renaming a check that people have
# already learned to look for.
#
# THEORY, NOT SETTLED: the exception surfaces at `dequeueOutputBuffer` on
# `c2.goldfish.h264.decoder`, the emulator's own codec, which gets its frames out
# of a host-side colour buffer -- the same readback machinery that aborts
# surfaceflinger on this image. What is MEASURED is narrower: these two fail on
# the API 37 emulator image; pass at API 36 on this runner under the same renderer
# AND the same SystemUI-disable path; pass at API 33-36 without that path at all,
# since nothing below 37 needs it; and pass on a physical Pixel 10 Pro XL at 37. That the decoder is the culprit rather than something else
# the decode path touches is inference. docs/api-37-emulator-crash.md separates
# the two, and the images also differ on the encoder side, which is why "broken
# h264 decoder" is not written into this job's name.
#
# WHEN THIS GOES GREEN, delete the annotation rather than this job: the gating
# row picks the tests back up automatically, and this job goes empty and can go
# with it.
# ---------------------------------------------------------------------------
e2e-api37-advisory:
name: E2E API 37 Media3 hardware transcode (advisory)
runs-on: ubuntu-latest
needs: ffmpeg
timeout-minutes: 60
continue-on-error: true
env:
E2E_LABEL: "37-media3-transcode"
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0
with:
distribution: temurin
java-version: '25'
- uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: ${{ env.GRADLE_CACHE_PATHS }}
key: gradle-${{ runner.os }}-${{ hashFiles('**/*.gradle.kts', 'gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties') }}
restore-keys: gradle-${{ runner.os }}-
- name: Enable KVM
run: |
echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' \
| sudo tee /etc/udev/rules.d/99-kvm4all.rules
sudo udevadm control --reload-rules
sudo udevadm trigger --name-match=kvm
- name: Instrumented tests
uses: reactivecircus/android-emulator-runner@a421e43855164a8197daf9d8d40fe71c6996bb0d # v2.38.0
env:
E2E_DISABLE_SYSTEM_UI: "1"
# The complement of the gating row's notAnnotation, off the same marker,
# so a test can never be excluded from both jobs or run in both.
E2E_EXTRA_GRADLE_ARGS: "-Pandroid.testInstrumentationRunnerArguments.annotation=org.libremediaconverter.FailsOnEmulatorApi37"
with:
# Every device pin below matches the gating row exactly, so a difference
# between the two jobs is the test selection and nothing else.
api-level: "37.0"
target: google_apis
arch: x86_64
profile: pixel_6
emulator-options: -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
disable-animations: true
disk-size: 8G
ram-size: 2560M
script: bash .github/scripts/e2e-run.sh ${{ env.E2E_LABEL }}
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
if: always()
with:
name: e2e-report-api${{ env.E2E_LABEL }}
path: |
app/build/reports/androidTests/
app/build/outputs/androidTest-results/
if-no-files-found: warn
# Uploaded always, and here it matters more than anywhere else in this file:
# this job is EXPECTED to be red, so the logcat is the only thing that says
# whether it is red for the known reason or for a new one.
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
if: always()
with:
name: e2e-diagnostics-api${{ env.E2E_LABEL }}
path: |
${{ runner.temp }}/logcat-api${{ env.E2E_LABEL }}.txt
${{ runner.temp }}/diagnostics-api${{ env.E2E_LABEL }}.txt
if-no-files-found: warn
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
if: always()
with:
name: e2e-wedge-api${{ env.E2E_LABEL }}
path: ${{ runner.temp }}/wedge-diagnostics-api${{ env.E2E_LABEL }}.txt
if-no-files-found: ignore
+81 -10
View File
@@ -64,16 +64,28 @@ of the first one that fails.
on this machine (below), so without it an androidTest compile error is not discovered until CI.
ktlint and detekt also cover the `test`/`androidTest` source sets that `lintDebug` skips.
## Instrumented tests do not run locally
## Instrumented tests: where they actually run
Two independent reasons, so do not spend time on either:
This section said the opposite until 2026-08-24, and both of its claims had been false for two
days. Read it as the current answer, and see the git history if you need the old one.
- **Emulators segfault on this host.** qemu dies on every AVD. Instrumented tests run on CI or on
the physical Pixel, never in a local emulator.
- **The API 37 image is broken.** `android-37.0` crash-loops surfaceflinger inside its own gralloc
mapper, so every test fails there regardless of this app. `docs/api-37-emulator-crash.md` records
the evidence and the ruled-out fixes; CI's matrix therefore stops at API 36 even though targetSdk
is 37. **API 37 needs a manual check on the Pixel 10 Pro XL before each release.**
- **Local emulators work, for API 33-36.** `tools/local-emulator/run-e2e.sh` runs them on this
host. The segfault that made this look impossible was not a broken machine: SwiftShader's Reactor
JIT writes generated shader code onto the heap and executes it, Fedora's SELinux policy denies
`execheap`, and qemu dies. Choosing a different renderer avoids it entirely — `-gpu host`,
`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
table.
- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. Two Media3 hardware-transcode
tests fail inside the emulator's own `c2.goldfish.h264.decoder` rather than on anything this app
does; they carry `@FailsOnEmulatorApi37` and run in a separate `continue-on-error` job,
`E2E API 37 Media3 hardware transcode (advisory)`. The gating leg runs the other 55.
**That advisory job 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 two tests pass.
`docs/api-37-emulator-crash.md` has the measurements.
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.** The advisory pair is the one thing CI cannot answer for.
On a device or emulator, build only the ABI it can execute:
@@ -96,10 +108,69 @@ install for code that can never run — and on API 37 the full APK does not fit
- The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
decision layer, where one branch is one documented user-visible outcome and the metric counts
answers rather than complexity. Every other rule still applies there.
- **Coverage is reported, not gated** — currently ~31% of lines. A floor needs a baseline that has
settled first.
- **Coverage is reported, not gated** — **29.8% of lines (629/2113), 28.7% of branches**, measured
on `main` 2026-08-23 with `./gradlew :app:jacocoTestReport`. A floor needs a baseline that has
settled first, and this one has not: the figure **fell** from the ~31% recorded earlier even
though the JVM suite went from 11 test files to 43. Main source grew 4,114 -> 5,715 lines over
the same period, so the denominator outran the numerator. Re-measure before quoting it; do not
assume more tests means a higher percentage here.
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply —
a change that is both needs both.
Three things make that a real bar rather than a slogan here:
- **Unit-testable is broader than it looks.** The pure-seam pattern — `work/FailureOutcome.kt`
documents the reasoning — turns "needs a device" into "a pure function plus a thin edge".
Robolectric is in the JVM source set, `compose-ui-test-junit4` with it, so Compose screens are
unit testable too. Reach for the seam before concluding something cannot be unit tested.
- **E2E is runnable locally**, API 33-36, via `tools/local-emulator/run-e2e.sh` — see
"Instrumented tests: where they actually run" above. That was believed impossible until the
SELinux/renderer cause was found, and it is what makes the e2e half of this norm enforceable.
- **A test has to bite.** Revert the line it covers, confirm it goes red, restore. A review of
this codebase ran 46 mutations against a 257-test suite and **9 were vacuous** — five of them
passing the whole suite over a completely unguarded code path. Green is not evidence.
Name what you did not cover and why. Genuine exemptions exist; implied coverage is the problem.
- `kotlin.code.style=official`. Gradle stays Kotlin DSL.
- **File one-off issues with `tools/github/file-issue.sh`, not `gh issue create`.** `gh issue
create` does not touch the project board, so the issue exists, carries its labels, and is
invisible in the Kanban — indistinguishable from never having been filed. Measured 2026-08-24:
eight issues filed as a scripted batch all reached the board; one filed as a one-off minutes
later did not. A batch carries the board step in its loop; **one-offs are where it slips**, which
is what the script is for. It resolves the project and Status ids by name rather than caching
them, and it **reads the item back** — a mutation returning 200 is not evidence the board shows
what was asked for. Exit 3 means the issue was created but did not reach the board, and prints
the number so it cannot be lost quietly.
`above-cut` and `backlog` are **labels from the 2026-08-22 triage pass** — "worked autonomously
overnight" and "held for manual review". They are not board columns. Status carries board state;
do not put a cut label on a newly filed ticket.
- **shellcheck runs in CI**, inside the Static analysis job, over `git ls-files '*.sh'` so a new
script is covered without editing the workflow. It runs at full severity, `info` included: the
two findings that raises today are answered with targeted `disable` directives carrying their
reason, exactly as `config/detekt/detekt.yml` carries only the rules this codebase legitimately
breaks. Do not silence it with `--severity=warning` — that hides the next real finding too.
**It is pinned by image digest, and joins ktlint/detekt/JaCoCo in the "Dependency versions"
rule above** — for exactly the reason stated there, demonstrated the day it was added. The first
cut used the runner's ambient shellcheck. That is **0.9.0**, while the container used to check
locally was 0.11.0, and the two disagree about how to report a trap handler: 0.11.0 says
`SC2329` once on the declaration, 0.9.0 says `SC2317` on each of seven lines in the body. Same
script, same directive, one green and one red. Directives that must survive both name both codes.
Locally, use the same pin rather than whatever is installed:
`podman run --rm -v "$PWD:/mnt:z" docker.io/koalaman/shellcheck@sha256:61862eba... <files>`
(the digest is in `status_check.yml`; there is no shellcheck system package on this host).
**It does not cover inline `run:` blocks in the workflows**, and a good deal of this repo's bash
lives there. `actionlint` does cover them — it runs shellcheck over each `run:` — and reports one
pre-existing `info` finding in `build.yml`. It is not wired in because every action here is
pinned by SHA, and actionlint's usual installer is a `curl | bash` off a moving branch; doing it
properly means pinning a container digest. Tracked separately rather than bolted on.
## Dependency versions
Libraries **float on minor + patch** (`coreKtx = "1.+"`). Three groups deliberately do not:
+4
View File
@@ -306,6 +306,10 @@ dependencies {
// the tests stay green.
testImplementation(platform(libs.compose.bom))
testImplementation(libs.compose.ui.test.junit4)
// For `runTest` alone, in EscapedCoroutineErrors.kt. It arrives transitively with the
// rule above anyway; declared because a test file imports it directly, and an import of
// something nobody asked for breaks the day the library that pulled it in stops.
testImplementation(libs.kotlinx.coroutines.test)
androidTestImplementation(platform(libs.compose.bom))
androidTestImplementation(libs.androidx.junit)
@@ -0,0 +1,24 @@
package org.libremediaconverter
/**
* Marks an instrumented test that does not pass 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
* `notAnnotation` to build the gating API 37 leg, and once with `annotation` to build the advisory
* one — so a test carrying it runs in exactly one of the two and can never fall through both.
* 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).
*
* 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,
* so this must never be read as "this test is allowed to fail at API 37" — only as "the API 37
* emulator image cannot currently answer this one". `docs/api-37-emulator-crash.md` has the
* measurements and the one bullet in them that is still inference.
*
* Removing it is the goal, and the trigger is written down: a new API 37.x system image, or an
* ATD image for 37. Delete the annotation from the tests, and the advisory job goes empty and
* the gating one grows by two.
*/
@Retention(AnnotationRetention.RUNTIME)
@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION)
annotation class FailsOnEmulatorApi37
@@ -16,6 +16,7 @@ import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.FailsOnEmulatorApi37
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.OutputFormat
import java.io.File
@@ -57,6 +58,7 @@ class Media3EngineTest {
}
@Test
@FailsOnEmulatorApi37
fun transcodesH264ToH265AndReportsProgress(): Unit = runBlocking {
val seen = mutableListOf<Int>()
@@ -119,6 +121,7 @@ class Media3EngineTest {
* HandlerThread indirection holds before any of that lands in Phase 2.
*/
@Test
@FailsOnEmulatorApi37
fun runsFromAThreadWithNoLooper() {
val pool = Executors.newSingleThreadExecutor()
try {
@@ -33,6 +33,7 @@ import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.dp
import androidx.lifecycle.compose.collectAsStateWithLifecycle
@@ -51,6 +52,7 @@ import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.ui.PrimaryButtonHeight
import org.libremediaconverter.ui.ScreenPaddingHorizontal
import org.libremediaconverter.ui.ScreenPaddingVertical
import org.libremediaconverter.ui.TestTags
import java.util.Locale
@UnstableApi
@@ -116,7 +118,8 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
onClick = { pickInput.launch(arrayOf("*/*")) },
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight),
.height(PrimaryButtonHeight)
.testTag(TestTags.Converter.CHOOSE_FILE),
) { Text("Choose file") }
}
@@ -147,11 +150,16 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
// The Advanced picker lets an impossible combination be selected on
// purpose, so this is what stops it from being run.
enabled = validation.isValid,
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.Converter.CONVERT),
) { Text("Convert") }
OutlinedButton(
onClick = { pickInput.launch(arrayOf("*/*")) },
modifier = Modifier.fillMaxWidth(),
modifier = Modifier
.fillMaxWidth()
.testTag(TestTags.Converter.CHOOSE_DIFFERENT_FILE),
) { Text("Choose a different file") }
}
@@ -160,11 +168,13 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
Text("Converting… ${s.percent}%")
LinearProgressIndicator(
progress = { s.percent / 100f },
modifier = Modifier.fillMaxWidth(),
modifier = Modifier
.fillMaxWidth()
.testTag(TestTags.Converter.PROGRESS),
)
OutlinedButton(
onClick = viewModel::cancel,
modifier = Modifier.fillMaxWidth(),
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
) { Text("Cancel") }
}
@@ -183,7 +193,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
)
OutlinedButton(
onClick = viewModel::cancel,
modifier = Modifier.fillMaxWidth(),
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
) { Text("Cancel") }
}
@@ -202,11 +212,14 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
}
Button(
onClick = { chooseDestination.launch(s.suggestedName) },
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.SAVE_FILE),
) { Text("Save file") }
OutlinedButton(
onClick = viewModel::reset,
modifier = Modifier.fillMaxWidth(),
modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
) { Text("Start over") }
}
@@ -214,7 +227,10 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
Text("Saved ${s.displayName}.", style = MaterialTheme.typography.bodyLarge)
Button(
onClick = viewModel::reset,
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.Converter.CONVERT_ANOTHER),
) { Text("Convert another") }
}
@@ -226,7 +242,10 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
)
Button(
onClick = viewModel::reset,
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.START_OVER),
) { Text("Start over") }
}
}
@@ -237,9 +256,12 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
@OptIn(ExperimentalLayoutApi::class)
@Composable
private fun FormatPicker(selected: OutputFormat?, onSelect: (OutputFormat) -> Unit) {
internal fun FormatPicker(selected: OutputFormat?, onSelect: (OutputFormat) -> Unit) {
Text("Output format", style = MaterialTheme.typography.titleSmall)
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
FlowRow(
horizontalArrangement = Arrangement.spacedBy(8.dp),
modifier = Modifier.testTag(TestTags.Converter.FORMAT_CHIPS),
) {
OutputFormat.entries.forEach { format ->
FilterChip(
selected = format == selected,
@@ -267,7 +289,7 @@ private fun FormatPicker(selected: OutputFormat?, onSelect: (OutputFormat) -> Un
*/
@OptIn(ExperimentalLayoutApi::class)
@Composable
private fun AdvancedPicker(
internal fun AdvancedPicker(
spec: OutputSpec,
validation: Validation,
onContainer: (Container) -> Unit,
@@ -277,14 +299,23 @@ private fun AdvancedPicker(
) {
var expanded by rememberSaveable { mutableStateOf(false) }
TextButton(onClick = { expanded = !expanded }) {
TextButton(
onClick = { expanded = !expanded },
modifier = Modifier.testTag(TestTags.Converter.ADVANCED_TOGGLE),
) {
Text(if (expanded) "Hide advanced" else "Advanced")
}
AnimatedVisibility(visible = expanded) {
Column(verticalArrangement = Arrangement.spacedBy(12.dp)) {
Column(
verticalArrangement = Arrangement.spacedBy(12.dp),
modifier = Modifier.testTag(TestTags.Converter.ADVANCED_PANEL),
) {
Text("Container", style = MaterialTheme.typography.titleSmall)
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
FlowRow(
horizontalArrangement = Arrangement.spacedBy(8.dp),
modifier = Modifier.testTag(TestTags.Converter.ADVANCED_CONTAINER_CHIPS),
) {
Container.entries.forEach { container ->
FilterChip(
selected = container == spec.container,
@@ -295,7 +326,10 @@ private fun AdvancedPicker(
}
Text("Video", style = MaterialTheme.typography.titleSmall)
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
FlowRow(
horizontalArrangement = Arrangement.spacedBy(8.dp),
modifier = Modifier.testTag(TestTags.Converter.ADVANCED_VIDEO_CHIPS),
) {
VideoCodec.entries.forEach { codec ->
FilterChip(
selected = codec == spec.videoCodec,
@@ -306,7 +340,10 @@ private fun AdvancedPicker(
}
Text("Audio", style = MaterialTheme.typography.titleSmall)
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
FlowRow(
horizontalArrangement = Arrangement.spacedBy(8.dp),
modifier = Modifier.testTag(TestTags.Converter.ADVANCED_AUDIO_CHIPS),
) {
AudioCodec.entries.forEach { codec ->
FilterChip(
selected = codec == spec.audioCodec,
@@ -331,9 +368,11 @@ private fun AdvancedPicker(
@OptIn(ExperimentalLayoutApi::class)
@Composable
private fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSpec) -> Unit) {
internal fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSpec) -> Unit) {
Card(
modifier = Modifier.fillMaxWidth(),
modifier = Modifier
.fillMaxWidth()
.testTag(TestTags.Converter.VALIDATION_ERROR),
colors = CardDefaults.cardColors(
containerColor = MaterialTheme.colorScheme.errorContainer,
contentColor = MaterialTheme.colorScheme.onErrorContainer,
@@ -347,10 +386,11 @@ private fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSp
if (invalid.suggestions.isNotEmpty()) {
Text("Try instead:", style = MaterialTheme.typography.labelMedium)
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
invalid.suggestions.forEach { suggestion ->
invalid.suggestions.forEachIndexed { index, suggestion ->
AssistChip(
onClick = { onSuggestion(suggestion) },
label = { Text(describe(suggestion)) },
modifier = Modifier.testTag(TestTags.Converter.suggestion(index)),
)
}
}
@@ -359,7 +399,7 @@ private fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSp
}
}
private fun describe(spec: OutputSpec): String {
internal fun describe(spec: OutputSpec): String {
val video = when (spec.videoCodec) {
VideoCodec.NONE -> null
else -> spec.videoCodec.label
@@ -374,9 +414,12 @@ private fun describe(spec: OutputSpec): String {
@OptIn(ExperimentalLayoutApi::class)
@Composable
private fun QualityPicker(selected: QualityTier, onSelect: (QualityTier) -> Unit) {
internal fun QualityPicker(selected: QualityTier, onSelect: (QualityTier) -> Unit) {
Text("Quality", style = MaterialTheme.typography.titleSmall)
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
FlowRow(
horizontalArrangement = Arrangement.spacedBy(8.dp),
modifier = Modifier.testTag(TestTags.Converter.QUALITY_CHIPS),
) {
QualityTier.entries.forEach { tier ->
FilterChip(
selected = tier == selected,
@@ -390,9 +433,12 @@ private fun QualityPicker(selected: QualityTier, onSelect: (QualityTier) -> Unit
@OptIn(ExperimentalLayoutApi::class)
@Composable
private fun EnginePicker(selected: EnginePreference, onSelect: (EnginePreference) -> Unit) {
internal fun EnginePicker(selected: EnginePreference, onSelect: (EnginePreference) -> Unit) {
Text("Engine", style = MaterialTheme.typography.titleSmall)
FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
FlowRow(
horizontalArrangement = Arrangement.spacedBy(8.dp),
modifier = Modifier.testTag(TestTags.Converter.ENGINE_CHIPS),
) {
EnginePreference.entries.forEach { preference ->
FilterChip(
selected = preference == selected,
@@ -403,7 +449,7 @@ private fun EnginePicker(selected: EnginePreference, onSelect: (EnginePreference
}
}
private fun EnginePreference.label(): String = when (this) {
internal fun EnginePreference.label(): String = when (this) {
EnginePreference.AUTO -> "Automatic"
EnginePreference.PREFER_HARDWARE -> "Prefer hardware"
EnginePreference.FORCE_SOFTWARE -> "Force software"
@@ -418,21 +464,34 @@ private fun EnginePreference.label(): String = when (this) {
* pretending it has an unknown codec.
*/
@Composable
private fun FileCard(input: InputFile) {
Card(modifier = Modifier.fillMaxWidth()) {
internal fun FileCard(input: InputFile) {
Card(
modifier = Modifier
.fillMaxWidth()
.testTag(TestTags.Converter.FILE_CARD),
) {
Column(modifier = Modifier.padding(16.dp)) {
Text(input.displayName, style = MaterialTheme.typography.titleMedium)
Text(
input.displayName,
style = MaterialTheme.typography.titleMedium,
modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_NAME),
)
// The null is handled here rather than inside formatBytes, because "no provider would
// say" is not a number and a formatter that invented one -- "0 B" -- is the defect
// this card would be showing. It degrades in words, like the codec rows below it.
Text(
input.sizeBytes?.let(::formatBytes) ?: "Size unknown",
style = MaterialTheme.typography.bodySmall,
modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_BYTES),
)
val probe = input.probe
if (probe == null) {
Text("Reading…", style = MaterialTheme.typography.bodySmall)
Text(
"Reading…",
style = MaterialTheme.typography.bodySmall,
modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_NOTE),
)
return@Column
}
@@ -442,6 +501,7 @@ private fun FileCard(input: InputFile) {
InputKind.UNPARSEABLE -> Text(
"Could not identify this file. It will be converted with FFmpeg.",
style = MaterialTheme.typography.bodySmall,
modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_NOTE),
)
InputKind.IMAGE -> {
@@ -481,22 +541,23 @@ private fun FileCard(input: InputFile) {
}
@Composable
private fun DetailRow(label: String, value: String) {
internal fun DetailRow(label: String, value: String) {
Text(
"$label: $value",
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onSurfaceVariant,
modifier = Modifier.testTag(TestTags.Converter.detailRow(label)),
)
}
private fun formatDuration(ms: Long): String {
internal fun formatDuration(ms: Long): String {
val totalSeconds = ms / 1000
val minutes = totalSeconds / 60
val seconds = totalSeconds % 60
return String.format(Locale.US, "%d:%02d", minutes, seconds)
}
private fun formatBytes(bytes: Long): String = when {
internal fun formatBytes(bytes: Long): String = when {
bytes >= 1_000_000_000 -> String.format(Locale.US, "%.1f GB", bytes / 1e9)
bytes >= 1_000_000 -> String.format(Locale.US, "%.1f MB", bytes / 1e6)
bytes >= 1_000 -> String.format(Locale.US, "%.0f kB", bytes / 1e3)
@@ -21,6 +21,7 @@ import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.dp
import androidx.lifecycle.compose.collectAsStateWithLifecycle
@@ -31,6 +32,7 @@ import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.ui.PrimaryButtonHeight
import org.libremediaconverter.ui.ScreenPaddingHorizontal
import org.libremediaconverter.ui.ScreenPaddingVertical
import org.libremediaconverter.ui.TestTags
import org.libremediaconverter.work.ConcatWorker
@UnstableApi
@@ -82,7 +84,8 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
onClick = { pickInputs.launch(arrayOf("video/*")) },
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight),
.height(PrimaryButtonHeight)
.testTag(TestTags.Join.CHOOSE_FILES),
) { Text("Choose files") }
}
@@ -97,11 +100,16 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
s.inputs.forEach { FileRow(it) }
Button(
onClick = viewModel::join,
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.Join.JOIN),
) { Text("Join ${s.inputs.size} files") }
OutlinedButton(
onClick = { pickInputs.launch(arrayOf("video/*")) },
modifier = Modifier.fillMaxWidth(),
modifier = Modifier
.fillMaxWidth()
.testTag(TestTags.Join.CHOOSE_DIFFERENT_FILES),
) { Text("Choose different files") }
}
@@ -110,10 +118,14 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
// Indeterminate on purpose: FFmpeg reports progress against a
// single input's duration, which means nothing across a
// concatenation. A fabricated percentage would be worse than none.
LinearProgressIndicator(modifier = Modifier.fillMaxWidth())
LinearProgressIndicator(
modifier = Modifier
.fillMaxWidth()
.testTag(TestTags.Join.PROGRESS),
)
OutlinedButton(
onClick = viewModel::cancel,
modifier = Modifier.fillMaxWidth(),
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
) { Text("Cancel") }
}
@@ -127,7 +139,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
)
OutlinedButton(
onClick = viewModel::cancel,
modifier = Modifier.fillMaxWidth(),
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
) { Text("Cancel") }
}
@@ -146,11 +158,14 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
)
Button(
onClick = { chooseDestination.launch(s.suggestedName) },
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.SAVE_FILE),
) { Text("Save file") }
OutlinedButton(
onClick = viewModel::reset,
modifier = Modifier.fillMaxWidth(),
modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
) { Text("Start over") }
}
@@ -158,7 +173,10 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
Text("Saved ${s.displayName}.", style = MaterialTheme.typography.bodyLarge)
Button(
onClick = viewModel::reset,
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.Join.JOIN_MORE),
) { Text("Join more") }
}
@@ -170,7 +188,10 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
)
Button(
onClick = viewModel::reset,
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.START_OVER),
) { Text("Start over") }
}
}
@@ -180,8 +201,12 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
}
@Composable
private fun FileRow(input: InputFile) {
Card(modifier = Modifier.fillMaxWidth()) {
internal fun FileRow(input: InputFile) {
Card(
modifier = Modifier
.fillMaxWidth()
.testTag(TestTags.Join.fileRow(input.displayName)),
) {
Column(modifier = Modifier.padding(12.dp)) {
Text(input.displayName, style = MaterialTheme.typography.bodyMedium)
}
@@ -0,0 +1,142 @@
package org.libremediaconverter.ui
/**
* Where a test finds each affordance on the two screens.
*
* Every button, picker and card in `ConverterScreen` and `JoinScreen` carries one of these through
* `Modifier.testTag`, so a test names a symbol and never a literal. That is the whole reason the
* table exists: `"Cancel"`, `"Start over"` and `"Save file"` are each rendered by both screens and
* by more than one state branch, so rewording one of them would otherwise redden several
* independent test files at once, and none of those diffs would explain why.
*
* Tags are applied inside `main`, never handed in by the caller. A tag a test passes down as a
* `Modifier` proves only that the test set it -- it would stay green with the affordance's own tag
* deleted, which is exactly the vacuous test `CLAUDE.md` records nine of.
*
* ### Public rather than `internal`, deliberately
*
* `androidTest` **is** a friend source set of `main` here: an `androidTest` file referencing the
* `internal` `Destination.CONVERT` compiles clean through `:app:compileDebugAndroidTestKotlin`
* under AGP 9.3.1 (measured 2026-08-24 -- nothing in the repo referenced a main `internal` from
* `androidTest`, so the question had no in-tree answer until then). `internal` would compile today.
*
* It is public anyway. That friendship is AGP wiring rather than something this project states, and
* this table is a contract read from three source sets: `main` applies the tags, `src/test` and
* `src/androidTest` name them. Public buys no external exposure in an application module -- nothing
* consumes it from outside -- so the durable answer costs nothing here.
*
* ### Invariants
*
* Values are distinct, which `TagTableUniquenessTest` asserts. Two affordances sharing a tag would
* break the "resolves to exactly one node" assertion in a file nobody had touched.
*/
object TestTags {
/**
* Affordances both screens render, under one name each.
*
* Shared rather than per-screen because only one screen is composed at a time -- the shell
* swaps them -- so a tag can only ever resolve within the screen under test.
*/
const val CANCEL: String = "action.cancel"
/** Rendered by `Converted`/`Joined` and again by `Failed` on both screens. */
const val START_OVER: String = "action.startOver"
const val SAVE_FILE: String = "action.saveFile"
/** `ConverterScreen`. */
object Converter {
const val CHOOSE_FILE: String = "converter.chooseFile"
const val CONVERT: String = "converter.convert"
const val CHOOSE_DIFFERENT_FILE: String = "converter.chooseDifferentFile"
const val CONVERT_ANOTHER: String = "converter.convertAnother"
/** The determinate bar in `Converting`. It carries no text, so nothing else can find it. */
const val PROGRESS: String = "converter.progress"
const val FILE_CARD: String = "converter.fileCard"
const val FILE_CARD_NAME: String = "converter.fileCard.name"
/**
* The byte size, or `"Size unknown"`.
*
* Named for bytes rather than "size" because the `IMAGE` branch also renders a row labelled
* `Size` -- pixel dimensions -- through [detailRow], and the two mean different things.
*/
const val FILE_CARD_BYTES: String = "converter.fileCard.bytes"
/**
* The one-line explanation that stands in for the detail rows: `"Reading…"` while the probe
* is still running, or the unreadable-file line once it has finished and found nothing.
* The two are mutually exclusive, so one tag covers both.
*/
const val FILE_CARD_NOTE: String = "converter.fileCard.note"
/**
* The chip rows, not the pickers around them.
*
* Each tag sits on the `FlowRow` of chips, so the prose a picker renders beside it -- the
* `"Custom — set below."` line under the formats, the tier description under the quality
* chips -- is outside the tagged node. Tagging the picker as a whole would mean wrapping
* three sibling emissions in a layout that does not exist today.
*/
const val FORMAT_CHIPS: String = "converter.formatChips"
const val QUALITY_CHIPS: String = "converter.qualityChips"
const val ENGINE_CHIPS: String = "converter.engineChips"
/** The `Advanced` / `Hide advanced` toggle. Present whether or not the panel is open. */
const val ADVANCED_TOGGLE: String = "converter.advanced.toggle"
/** The panel the toggle gates. Absent from the tree while collapsed. */
const val ADVANCED_PANEL: String = "converter.advanced.panel"
/**
* The three chip rows inside the panel, separately.
*
* Separately because their labels collide: `"Copy"` and `"None"` are both a `VideoCodec`
* and an `AudioCodec`, and `"MP3"` and `"FLAC"` are both a `Container` and an `AudioCodec`,
* so a text matcher over the open panel is ambiguous for four of the chips.
*/
const val ADVANCED_CONTAINER_CHIPS: String = "converter.advanced.containerChips"
const val ADVANCED_VIDEO_CHIPS: String = "converter.advanced.videoChips"
const val ADVANCED_AUDIO_CHIPS: String = "converter.advanced.audioChips"
/** The error card. Rendered outside the panel, so it is reachable while collapsed. */
const val VALIDATION_ERROR: String = "converter.validationError"
/** One detail line of the file card, by the label it renders: `Container`, `Video`, ... */
fun detailRow(label: String): String = "converter.fileCard.row:$label"
/**
* One suggested output on the validation card, by position.
*
* By position rather than by the text of the suggestion, because that text comes from
* `describe`, which is itself under test -- a tag derived from it would move whenever the
* thing it is meant to locate changed.
*/
fun suggestion(index: Int): String = "converter.validationError.suggestion:$index"
}
/** `JoinScreen`. */
object Join {
const val CHOOSE_FILES: String = "join.chooseFiles"
const val JOIN: String = "join.join"
const val CHOOSE_DIFFERENT_FILES: String = "join.chooseDifferentFiles"
const val JOIN_MORE: String = "join.joinMore"
/** The indeterminate bar in `Joining`. */
const val PROGRESS: String = "join.progress"
/**
* One picked input, by the name it displays.
*
* By name rather than by position, so the tag is derived from data the row already holds
* and can stay inside `FileRow`. Passing an index down would mean the call site owned the
* tag, and a test that supplies its own tag asserts nothing about the screen.
*/
fun fileRow(displayName: String): String = "join.fileRow:$displayName"
}
}
@@ -8,7 +8,6 @@ import androidx.compose.runtime.setValue
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.test.assertIsSelected
import androidx.compose.ui.test.junit4.StateRestorationTester
import androidx.compose.ui.test.junit4.v2.createComposeRule
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
@@ -42,8 +41,11 @@ import org.robolectric.RobolectricTestRunner
@RunWith(RobolectricTestRunner::class)
class AppRootRestorationTest {
// Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors]. Every Compose test
// class in this source set starts there, whether or not it is the one that happens to be
// running when another test's escaped coroutine error is delivered.
@get:Rule
val composeRule = createComposeRule()
val composeRule = createDrainedComposeRule()
private val restoration = StateRestorationTester(composeRule)
@@ -0,0 +1,53 @@
package org.libremediaconverter
import androidx.compose.ui.test.junit4.v2.createComposeRule
import kotlinx.coroutines.test.runTest
/**
* Clears coroutine errors this module's tests deliberately let escape, so they land on the test
* that caused them instead of on the next one to start.
*
* **Every Compose test class in `src/test` has to start here.** `createComposeRule` runs the
* composition inside `runTest`, and `runTest` opens by throwing `UncaughtExceptionsBeforeTest` for
* anything already sitting in kotlinx-coroutines-test's collector -- a process-wide
* `CoroutineExceptionHandler` it installs once and never removes.
*
* There is one deposit into that collector here, and it is not a mistake:
* `ConversionViewModelProbeFailureTest.an OutOfMemoryError is not swallowed` proves an OOM raised
* inside the probe is rethrown rather than reported as an unreadable file. `onInputPicked` runs it
* in `viewModelScope.launch`, which has no exception handler by design -- the ViewModel's own KDoc
* says a real OOM should reach the thread's handler and take the process down. On the JVM the
* collector takes it instead, holds it, and hands it to whichever `runTest` starts next.
*
* It surfaced as two *different* Compose test classes failing on two consecutive runs of the same,
* green, code, with a message naming neither the test nor the error's origin. Which class catches
* it moves because the throw happens on a real `Dispatchers.IO` thread, after the state assertion
* that ends the test that caused it -- so it can be delivered long after that class is done.
*
* A `@Before` method cannot do this: the compose rule's `runTest` wraps the statement that calls
* `@Before`, so it has already thrown. `@BeforeClass` cannot either -- Robolectric runs it outside
* the sandbox classloader, where the collector is a different object. Draining while the rule is
* being *constructed* is early enough, because JUnit builds a fresh test-class instance, and with
* it every `@get:Rule` field, before evaluating any rule.
*
* The real fix is a seam: give the probe hop an injectable dispatcher the way
* `ConversionViewModel`'s constructor already does for `cleanupDispatcher`, and the error would
* have somewhere to land. That is a production change, so it belongs in its own commit.
*/
fun drainEscapedCoroutineErrors() {
// Entering a test scope is what flushes the collector; the flush is reported as this
// throwing, and there is nothing to assert about an error another test already asserted on.
runCatching { runTest {} }
}
/**
* [createComposeRule], with [drainEscapedCoroutineErrors] run first. Use this rather than
* `createComposeRule` directly in `src/test`.
*
* It also keeps the one mixed import in one place: the rule comes from the **v2** package
* (`androidx.compose.ui.test.junit4.v2`) while `StateRestorationTester`, which takes it, does not.
*/
fun createDrainedComposeRule() = run {
drainEscapedCoroutineErrors()
createComposeRule()
}
@@ -0,0 +1,159 @@
package org.libremediaconverter.convert
import android.os.Bundle
import android.os.Parcel
import android.os.Parcelable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.saveable.LocalSaveableStateRegistry
import androidx.compose.runtime.saveable.SaveableStateRegistry
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.performClick
import androidx.media3.common.util.UnstableApi
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.createDrainedComposeRule
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.Validation
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
/**
* What `AdvancedPickerTest`'s restoration test cannot see.
*
* `StateRestorationTester` saves into an **in-memory map**, never a `Bundle`. That is enough to
* discriminate `rememberSaveable` from `remember`, and it is where it stops: the map holds object
* references, so a value the platform could never parcel goes in and comes back out looking green.
* `AppRootRestorationTest` has the same blind spot and `DestinationSaverTest` is the split it
* prompted; this is that split for `expanded`, the only `rememberSaveable` on either screen's
* leaves.
*
* ### The saved representation is not the Boolean
*
* `var expanded by rememberSaveable { mutableStateOf(false) }` passes no `stateSaver`, so
* `autoSaver` saves **the `MutableState` itself**, not the `false` inside it. That works only
* because `mutableStateOf` on Android returns a `Parcelable` implementation -- the same call on a
* plain JVM returns one that is not. So what stands between an open panel and a rotation that
* closes it is a platform-specific detail of a factory function nothing here names directly, and
* an in-memory map cannot tell the two apart.
*
* Pinning it is the move `DestinationSaverTest` makes about names versus ordinals. Passing an
* explicit `stateSaver` would save a bare `Boolean` instead and is a perfectly reasonable edit --
* it is just not the one in the tree, and it should be made on purpose rather than discovered
* after a rotation.
*
* ### Shared bite, stated rather than implied
*
* `rememberSaveable` -> `remember` empties the registry, so it reddens this file *and* the
* restoration test in `AdvancedPickerTest`. Both failures belong in any report of that mutation.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class AdvancedPanelSavedStateTest {
// Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors].
@get:Rule
val composeRule = createDrainedComposeRule()
/**
* `canBeSaved = { true }` deliberately.
*
* A predicate mirroring what a `Bundle` accepts would be a hand-written copy of the thing
* under test, and a `false` from it *drops* the entry silently -- so the test would fail by
* finding nothing saved, which is also how a `remember` regression fails. Two causes, one
* symptom, is not a test. The type is checked on the way out instead.
*/
private val registry = SaveableStateRegistry(restoredValues = null, canBeSaved = { true })
@Test
fun `the panel registers its open state with the registry, and nothing else`() {
setPicker()
// Collapsed is a saved value, not an absent one: `rememberSaveable` registers its provider
// on first composition, whatever the state happens to be. Exactly one, because `expanded`
// is the only saveable in the subtree -- a second would mean something else began saving.
assertEquals(1, savedValues().size)
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
val saved = theOneSavedValue()
assertTrue("saved as ${saved?.javaClass?.name}", saved is MutableState<*>)
assertEquals(true, (saved as MutableState<*>).value)
}
@Test
fun `the open panel survives a real Parcel, not just an in-memory map`() {
setPicker()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
val saved = theOneSavedValue()
// The claim the restoration test cannot make. A `MutableState` that was not `Parcelable`
// would satisfy `StateRestorationTester` and then be dropped by the platform.
assertTrue("saved as ${saved?.javaClass?.name}", saved is Parcelable)
val restored = throughARealBundle(saved as Parcelable)
assertTrue("restored as ${restored.javaClass.name}", restored is MutableState<*>)
assertEquals(true, (restored as MutableState<*>).value)
}
private fun setPicker() {
composeRule.setContent {
CompositionLocalProvider(LocalSaveableStateRegistry provides registry) {
AdvancedPicker(
spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC),
validation = Validation.Valid,
onContainer = {},
onVideoCodec = {},
onAudioCodec = {},
onSuggestion = {},
)
}
}
}
/** Every value the picker hands the host to persist, keys dropped -- they are positional. */
private fun savedValues(): List<Any?> = composeRule.runOnIdle { registry.performSave().values.flatten() }
/**
* The single saved value, asserted rather than assumed.
*
* `single()` on an empty list throws `NoSuchElementException: List is empty`, which names
* neither the panel nor the registry -- and an empty registry is exactly how the
* `rememberSaveable` -> `remember` regression shows up here.
*/
private fun theOneSavedValue(): Any? {
val values = savedValues()
assertEquals("the panel should register exactly one saved value", 1, values.size)
return values.first()
}
/** A write and a read through a real `Parcel`, which is what the tester's map stands in for. */
private fun throughARealBundle(value: Parcelable): Parcelable {
val bundle = Bundle().apply { putParcelable(KEY, value) }
val parcel = Parcel.obtain()
return try {
parcel.writeBundle(bundle)
parcel.setDataPosition(0)
val restored = requireNotNull(parcel.readBundle(javaClass.classLoader)) {
"the Bundle did not survive the Parcel"
}
requireNotNull(restored.getParcelable(KEY, Parcelable::class.java)) {
"the saved state did not survive the Parcel"
}
} finally {
parcel.recycle()
}
}
private companion object {
const val KEY = "expanded"
}
}
@@ -0,0 +1,321 @@
package org.libremediaconverter.convert
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertTextEquals
import androidx.compose.ui.test.hasAnyAncestor
import androidx.compose.ui.test.hasTestTag
import androidx.compose.ui.test.hasText
import androidx.compose.ui.test.junit4.StateRestorationTester
import androidx.compose.ui.test.onAllNodesWithTag
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.media3.common.util.UnstableApi
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.createDrainedComposeRule
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.ContainerCapabilities
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.Validation
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
/**
* The gate over the Advanced chips, and the error card that deliberately sits outside it.
*
* Two defects, and they pull in opposite directions.
*
* The first is the chips escaping the gate, or never being reachable through it. `AdvancedPicker`
* is the one leaf on this screen that is not stateless -- `expanded` is its own `rememberSaveable`
* -- and Container, Video and Audio live inside `AnimatedVisibility(visible = expanded)`. Nothing
* else on the screen hides anything, so a refactor that flattened the panel, or wired the toggle to
* a state nobody reads, would render an app that looks reasonable in a screenshot and is wrong.
*
* The second is the opposite mistake, and it is the one this file exists for: **moving the
* `ValidationError` call inside the `AnimatedVisibility`**. It is invoked after that block, so an
* invalid spec explains itself and offers one-tap fixes *while the section is collapsed*. That is
* the only route out of an invalid spec for a user who never opened Advanced -- and since the only
* way to reach an invalid spec is through Advanced, hiding the way out behind the same toggle looks
* locally sensible and is a trap. Tidying the two `if` blocks into one is a plausible edit, it
* compiles, and until this file existed nothing went red. Every assertion about the error card here
* therefore runs with the toggle untouched, and asserts the panel is absent in the same test, so a
* future `expanded = true` default cannot quietly satisfy it either.
*
* The invalid specs come from [ContainerCapabilities.validate] rather than from a hand-built
* [Validation.Invalid], so the messages and the suggestions are the real pairing. A hand-built one
* would keep passing after `validate` stopped producing anything like it.
*
* Node location is by the three separate chip-row tags, never by text. `"Copy"` and `"None"` are
* each both a [VideoCodec] and an [AudioCodec], and `"MP3"` and `"FLAC"` are each both a
* [Container] and an [AudioCodec], so a text matcher over the open panel is ambiguous for four
* chips -- which is what the separate tags are for.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class AdvancedPickerTest {
// Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors].
@get:Rule
val composeRule = createDrainedComposeRule()
private val restoration = StateRestorationTester(composeRule)
private val containers = mutableListOf<Container>()
private val videoCodecs = mutableListOf<VideoCodec>()
private val audioCodecs = mutableListOf<AudioCodec>()
private val applied = mutableListOf<OutputSpec>()
// --- the expand gate ----------------------------------------------------
@Test
fun `the three chip rows appear only while the panel is expanded`() {
setPicker()
assertPanelHidden()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertExists()
ROW_TAGS.forEach { composeRule.onNodeWithTag(it).assertExists() }
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
// The exit transition outlives the click, so absence has to be waited for rather than
// asserted straight away -- unlike the initial collapsed state, which has no animation
// in flight.
composeRule.waitUntil { nodeCount(TestTags.Converter.ADVANCED_PANEL) == 0 }
assertPanelHidden()
}
/** The toggle is the only affordance the collapsed picker offers, so it has to say so. */
@Test
fun `the toggle names the direction it will move in`() {
setPicker()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).assertTextEquals("Advanced")
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE)
.assertTextEquals("Hide advanced")
}
/**
* The four colliding labels, one per row.
*
* `"Copy"` is a video codec *and* an audio codec; `"MP3"` is a container *and* an audio codec.
* Clicking each through its own row is what proves the rows are wired to different callbacks
* -- a picker that handed every chip to `onAudioCodec` would look identical on screen.
*/
@Test
fun `each chip row reports to its own callback, including the labels that collide`() {
setPicker()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
chipIn(TestTags.Converter.ADVANCED_VIDEO_CHIPS, "Copy").performClick()
assertEquals(listOf(VideoCodec.COPY), videoCodecs)
assertEquals(emptyList<AudioCodec>(), audioCodecs)
chipIn(TestTags.Converter.ADVANCED_AUDIO_CHIPS, "Copy").performClick()
assertEquals(listOf(AudioCodec.COPY), audioCodecs)
chipIn(TestTags.Converter.ADVANCED_CONTAINER_CHIPS, "MP3").performClick()
assertEquals(listOf(Container.MP3), containers)
// Still only the one audio click. `MP3` is an AudioCodec label too, and the container row
// must not be reporting through that callback.
assertEquals(listOf(AudioCodec.COPY), audioCodecs)
}
// --- the error card, which is outside the gate --------------------------
/**
* The headline case. Dropping both tracks is reachable from the collapsed screen -- the
* `None`/`None` pair is set inside Advanced, but the user can close it again -- and the
* explanation has to still be there.
*/
@Test
fun `an empty output explains itself while the section is collapsed`() {
val spec = OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE)
val invalid = invalidFor(spec)
assertEquals("This would produce an empty file — keep at least one track.", invalid.message)
setPicker(spec, invalid)
assertPanelHidden()
composeRule.onNodeWithTag(TestTags.Converter.VALIDATION_ERROR).assertExists()
composeRule.onNodeWithText(invalid.message).assertIsDisplayed()
}
@Test
fun `a codec the container cannot hold explains itself while the section is collapsed`() {
val spec = OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.OPUS)
val invalid = invalidFor(spec)
assertEquals("WebM cannot hold H.264 video.", invalid.message)
setPicker(spec, invalid)
assertPanelHidden()
composeRule.onNodeWithText(invalid.message).assertIsDisplayed()
}
/**
* Clicking a suggestion, with the toggle never touched.
*
* The second suggestion rather than the first, and its count pinned first: with one suggestion
* a picker that handed every chip `suggestions[0]` would pass, and `onNodeWithTag` on a
* suggestion index that no longer exists reports an unhelpful matcher failure rather than
* saying the list shrank.
*/
@Test
fun `a suggestion chip applies its own spec without the section ever being opened`() {
val spec = OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.OPUS)
val invalid = invalidFor(spec)
assertEquals(2, invalid.suggestions.size)
val second = invalid.suggestions[1]
setPicker(spec, invalid)
assertPanelHidden()
composeRule.onNodeWithTag(TestTags.Converter.suggestion(1)).assertTextEquals(describe(second))
composeRule.onNodeWithTag(TestTags.Converter.suggestion(1)).performClick()
assertEquals(listOf(second), applied)
// What the chips offer is what `validate` said would work, not a repair of the test's own.
assertTrue(
"suggestion $second should itself validate",
ContainerCapabilities.validate(second, PROBE).isValid,
)
}
/** A valid spec has nothing to say, collapsed or not. */
@Test
fun `a valid spec renders no error card`() {
setPicker()
composeRule.onNodeWithTag(TestTags.Converter.VALIDATION_ERROR).assertDoesNotExist()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
composeRule.onNodeWithTag(TestTags.Converter.VALIDATION_ERROR).assertDoesNotExist()
}
// --- recreation ---------------------------------------------------------
/**
* `expanded` is the only `rememberSaveable` on either screen's leaves.
*
* `MainActivity` declares no `configChanges`, so a rotation destroys and rebuilds the whole
* composition. A panel the user opened, set three chips in, and left open must not close
* itself on the way back. `remember` would.
*
* What this cannot see is the saved *representation* -- `StateRestorationTester` saves into an
* in-memory map rather than a `Bundle`. `AdvancedPanelSavedStateTest` covers that half.
*/
@Test
fun `an open panel is still open after recreation`() {
restoration.setContent {
AdvancedPicker(
spec = VALID_SPEC,
validation = Validation.Valid,
onContainer = {},
onVideoCodec = {},
onAudioCodec = {},
onSuggestion = {},
)
}
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertExists()
restoration.emulateSavedInstanceStateRestore()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertExists()
ROW_TAGS.forEach { composeRule.onNodeWithTag(it).assertExists() }
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE)
.assertTextEquals("Hide advanced")
}
/** The default has to survive too, or the panel would spring open on every rotation. */
@Test
fun `a collapsed panel is still collapsed after recreation`() {
restoration.setContent {
AdvancedPicker(
spec = VALID_SPEC,
validation = Validation.Valid,
onContainer = {},
onVideoCodec = {},
onAudioCodec = {},
onSuggestion = {},
)
}
assertPanelHidden()
restoration.emulateSavedInstanceStateRestore()
assertPanelHidden()
}
// --- helpers ------------------------------------------------------------
private fun setPicker(spec: OutputSpec = VALID_SPEC, validation: Validation = Validation.Valid) {
composeRule.setContent {
AdvancedPicker(
spec = spec,
validation = validation,
onContainer = { containers += it },
onVideoCodec = { videoCodecs += it },
onAudioCodec = { audioCodecs += it },
onSuggestion = { applied += it },
)
}
}
/** The whole panel, by every tag it owns, so a partial escape counts as a failure. */
private fun assertPanelHidden() {
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertDoesNotExist()
ROW_TAGS.forEach { composeRule.onNodeWithTag(it).assertDoesNotExist() }
}
private fun nodeCount(tag: String) = composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().size
private fun chipIn(rowTag: String, label: String) =
composeRule.onNode(hasText(label) and hasAnyAncestor(hasTestTag(rowTag)))
private fun invalidFor(spec: OutputSpec): Validation.Invalid {
val validation = ContainerCapabilities.validate(spec, PROBE)
return validation as? Validation.Invalid
?: throw AssertionError("$spec was expected to be invalid, but validate said $validation")
}
private companion object {
val ROW_TAGS = listOf(
TestTags.Converter.ADVANCED_CONTAINER_CHIPS,
TestTags.Converter.ADVANCED_VIDEO_CHIPS,
TestTags.Converter.ADVANCED_AUDIO_CHIPS,
)
val VALID_SPEC = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC)
/** An ordinary H.264/AAC MP4, so the suggestions have a real source to repair towards. */
val PROBE = InputProbe(
videoCodec = "h264",
audioCodec = "aac",
durationMs = 90_000,
container = Container.MP4,
)
}
}
@@ -0,0 +1,152 @@
package org.libremediaconverter.convert
import org.junit.Assert.assertEquals
import org.junit.Test
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.VideoCodec
/**
* The four pure helpers behind the converter screen's prose, pinned at the points where they
* change what they say.
*
* No Compose rule and no Robolectric: these are `String` in, `String` out, and running them under a
* device sandbox would buy nothing while hiding the boundaries in a rendered tree.
*
* The defect each group bites on:
*
* - **[formatBytes] picks a unit by comparing against three thresholds.** Every one of them is a
* `>=`, and a `>` would move a file sitting exactly on a boundary into the unit below -- `1 GB`
* shown as `1000.0 MB`. Only a value *on* the threshold can tell the two apart, so each of the
* three is asserted at the boundary and one below it. The unit prefixes are decimal, matching
* what the file manager and the provider report, not powers of two.
* - **[formatDuration] has no hours field.** An hour-long recording reads `60:00`, and that is the
* contract rather than an oversight -- the row is a length, not a clock. Pinned so that adding
* hours is a deliberate change with a red test in front of it instead of a silent reformat.
* - **[describe] builds the suggestion-chip label out of up to three parts**, and the parts are
* conditional: [VideoCodec.NONE] and [AudioCodec.NONE] drop out entirely, so an image output
* with neither track has to render as the container alone rather than as a container followed
* by a dangling separator.
* - **[EnginePreference] carries no `label` property**, unlike every other enum the screen
* renders; its three display strings live in a `when` in the screen file. Adding a constant is
* caught by the compiler because that `when` is exhaustive, but nothing stops two constants
* being given the same string, which is what the distinctness assertion is for.
*/
class ConverterFormattersTest {
@Test
fun `bytes below a kilobyte are counted exactly`() {
assertEquals("0 B", formatBytes(0))
assertEquals("1 B", formatBytes(1))
assertEquals("999 B", formatBytes(999))
}
@Test
fun `each unit starts exactly on its threshold rather than one byte past it`() {
assertEquals("1 kB", formatBytes(1_000))
assertEquals("1.0 MB", formatBytes(1_000_000))
assertEquals("1.0 GB", formatBytes(1_000_000_000))
}
/**
* One byte below each threshold, which is the half a `>=` to `>` change leaves alone. Both
* halves are needed: the boundary values alone would still pass if the comparison let
* everything through.
*/
@Test
fun `a value just below a threshold stays in the smaller unit`() {
assertEquals("999 B", formatBytes(999))
assertEquals("1000 kB", formatBytes(999_999))
assertEquals("1000.0 MB", formatBytes(999_999_999))
}
@Test
fun `a real file size reads as one decimal place`() {
assertEquals("12.3 MB", formatBytes(12_345_678))
assertEquals("1.5 GB", formatBytes(1_500_000_000))
}
@Test
fun `a duration is minutes and zero-padded seconds`() {
assertEquals("0:00", formatDuration(0))
assertEquals("0:01", formatDuration(1_000))
assertEquals("0:59", formatDuration(59_000))
assertEquals("1:00", formatDuration(60_000))
assertEquals("1:30", formatDuration(90_000))
}
/** Sub-second remainders are dropped rather than rounded up into the next second. */
@Test
fun `a partial second does not become a whole one`() {
assertEquals("0:00", formatDuration(999))
assertEquals("0:59", formatDuration(59_999))
}
/** No hours field, deliberately: an hour is `60:00` and two hours are `120:00`. */
@Test
fun `an hour and beyond keeps counting in minutes`() {
assertEquals("60:00", formatDuration(3_600_000))
assertEquals("61:01", formatDuration(3_661_000))
assertEquals("120:00", formatDuration(7_200_000))
}
@Test
fun `a spec with both tracks names the container and joins the two codecs`() {
assertEquals(
"MP4 · H.264 + AAC",
describe(OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC)),
)
}
@Test
fun `a track set to none is left out instead of being named none`() {
assertEquals(
"MP3 · MP3",
describe(OutputSpec(Container.MP3, VideoCodec.NONE, AudioCodec.MP3)),
)
assertEquals(
"MP4 · H.264",
describe(OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.NONE)),
)
}
/** An image output has neither track, so there is nothing for the separator to separate. */
@Test
fun `a spec with no tracks at all is the container alone, with no trailing separator`() {
assertEquals("GIF", describe(OutputSpec(Container.GIF, VideoCodec.NONE, AudioCodec.NONE)))
assertEquals(
"PNG frames",
describe(OutputSpec(Container.IMAGE_SEQUENCE, VideoCodec.NONE, AudioCodec.NONE)),
)
}
/** `Copy` is a codec here, not the absence of one, so a remux describes both tracks. */
@Test
fun `a remux names copy on both tracks rather than dropping them`() {
assertEquals(
"Matroska · Copy + Copy",
describe(OutputSpec(Container.MKV, VideoCodec.COPY, AudioCodec.COPY)),
)
}
@Test
fun `each engine preference has the wording the chips show`() {
assertEquals("Automatic", EnginePreference.AUTO.label())
assertEquals("Prefer hardware", EnginePreference.PREFER_HARDWARE.label())
assertEquals("Force software", EnginePreference.FORCE_SOFTWARE.label())
}
/**
* Two constants sharing a label would render as two identical chips, one of which the user
* could not choose deliberately. The exhaustive `when` cannot catch that; this does.
*/
@Test
fun `no two engine preferences render the same chip`() {
val labels = EnginePreference.entries.map { it.label() }
assertEquals(EnginePreference.entries.size, labels.toSet().size)
assertEquals(emptyList<String>(), labels.filter { it.isBlank() })
}
}
@@ -0,0 +1,196 @@
package org.libremediaconverter.convert
import android.net.Uri
import androidx.compose.ui.test.assertCountEquals
import androidx.compose.ui.test.onAllNodesWithTag
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.performClick
import androidx.media3.common.util.UnstableApi
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.createDrainedComposeRule
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.InputKind
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.QualityTier
import org.libremediaconverter.model.Validation
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
/**
* Each leaf of the converter screen renders, and each tag it claims resolves to exactly one node.
*
* The defect this bites on is a tag that is not where the table says it is: dropped by a refactor
* that rewrote a `Modifier` chain, applied to the wrong one of two siblings, or duplicated onto a
* leaf that is rendered twice. None of that is visible at compile time -- a `testTag` is a string
* handed to a modifier -- and none of it shows up in the app either, because nothing but a test
* ever reads one.
*
* It has to be caught here rather than by the children that consume the tags. R38.2, R38.3 and
* R38.4 all *begin* by locating a node through one of these, so a tag that had quietly moved would
* surface as three unrelated PRs failing on a line their own diffs do not touch. Counting the nodes
* rather than asserting existence is deliberate: `onNodeWithTag` on two matches throws about
* ambiguity in one place and passes in another, so "exactly one" is the property worth pinning.
*
* Deliberately *not* the state matrix. Which affordances each `ConversionState` renders is R38.6,
* and it needs the state seam R38.5 extracts -- the branch buttons tagged in this change (Convert,
* Cancel, Save file, Start over, ...) therefore have no bite yet, which the PR body records.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class ConverterLeafTagsTest {
@get:Rule
val composeRule = createDrainedComposeRule()
private fun assertResolvesToOneNode(tag: String) {
composeRule.onAllNodesWithTag(tag).assertCountEquals(1)
}
private fun input(sizeBytes: Long? = 12_345_678L, probe: InputProbe? = VIDEO_PROBE) = InputFile(
uri = Uri.parse("content://test/clip.mkv"),
displayName = "clip.mkv",
sizeBytes = sizeBytes,
probe = probe,
)
@Test
fun `the format picker tags its chip row`() {
composeRule.setContent { FormatPicker(OutputFormat.MP4_H264) {} }
assertResolvesToOneNode(TestTags.Converter.FORMAT_CHIPS)
}
@Test
fun `the quality picker tags its chip row`() {
composeRule.setContent { QualityPicker(QualityTier.FAST) {} }
assertResolvesToOneNode(TestTags.Converter.QUALITY_CHIPS)
}
@Test
fun `the engine picker tags its chip row`() {
composeRule.setContent { EnginePicker(EnginePreference.AUTO) {} }
assertResolvesToOneNode(TestTags.Converter.ENGINE_CHIPS)
}
@Test
fun `the advanced picker tags its toggle, which is all it renders while collapsed`() {
setAdvancedPicker()
assertResolvesToOneNode(TestTags.Converter.ADVANCED_TOGGLE)
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertDoesNotExist()
}
/**
* The panel and its three rows only exist once the toggle has been clicked, which is R38.4's
* subject. Expanding is the only way to reach the tags at all, so the smoke test has to do it.
*/
@Test
fun `expanding the advanced picker tags the panel and each of its three chip rows`() {
setAdvancedPicker()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
assertResolvesToOneNode(TestTags.Converter.ADVANCED_PANEL)
assertResolvesToOneNode(TestTags.Converter.ADVANCED_CONTAINER_CHIPS)
assertResolvesToOneNode(TestTags.Converter.ADVANCED_VIDEO_CHIPS)
assertResolvesToOneNode(TestTags.Converter.ADVANCED_AUDIO_CHIPS)
}
@Test
fun `the validation card tags itself and every suggestion on it`() {
composeRule.setContent {
ValidationError(
Validation.Invalid(
message = "WebM cannot hold H.264 video.",
suggestions = listOf(
OutputSpec(Container.MKV, VideoCodec.H264, AudioCodec.AAC),
OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.OPUS),
),
),
) {}
}
assertResolvesToOneNode(TestTags.Converter.VALIDATION_ERROR)
assertResolvesToOneNode(TestTags.Converter.suggestion(0))
assertResolvesToOneNode(TestTags.Converter.suggestion(1))
}
@Test
fun `the file card tags itself, its name and its size line`() {
composeRule.setContent { FileCard(input()) }
assertResolvesToOneNode(TestTags.Converter.FILE_CARD)
assertResolvesToOneNode(TestTags.Converter.FILE_CARD_NAME)
assertResolvesToOneNode(TestTags.Converter.FILE_CARD_BYTES)
}
/**
* Both writers of the note line get their own case. They are two separate `Text` calls in two
* branches that share one tag, so a test of either alone would leave the other unguarded.
*/
@Test
fun `the file card tags the note it shows while the probe is still running`() {
composeRule.setContent { FileCard(input(probe = null)) }
assertResolvesToOneNode(TestTags.Converter.FILE_CARD_NOTE)
}
@Test
fun `the file card tags the note it shows when nothing could read the file`() {
composeRule.setContent { FileCard(input(probe = InputProbe(kind = InputKind.UNPARSEABLE))) }
assertResolvesToOneNode(TestTags.Converter.FILE_CARD_NOTE)
}
@Test
fun `a detail row tags itself with the label it renders`() {
composeRule.setContent { DetailRow("Container", "Matroska") }
assertResolvesToOneNode(TestTags.Converter.detailRow("Container"))
}
/** The rows the file card builds carry the same per-label tags, one per row it renders. */
@Test
fun `the file card's detail rows are each tagged by their own label`() {
composeRule.setContent { FileCard(input()) }
assertResolvesToOneNode(TestTags.Converter.detailRow("Container"))
assertResolvesToOneNode(TestTags.Converter.detailRow("Video"))
assertResolvesToOneNode(TestTags.Converter.detailRow("Audio"))
assertResolvesToOneNode(TestTags.Converter.detailRow("Length"))
}
private fun setAdvancedPicker() {
composeRule.setContent {
AdvancedPicker(
spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC),
validation = Validation.Valid,
onContainer = {},
onVideoCodec = {},
onAudioCodec = {},
onSuggestion = {},
)
}
}
private companion object {
val VIDEO_PROBE = InputProbe(
videoCodec = "video/avc",
audioCodec = "audio/mp4a-latm",
durationMs = 90_000,
kind = InputKind.VIDEO,
container = Container.MKV,
width = 1920,
height = 1080,
)
}
}
@@ -0,0 +1,169 @@
package org.libremediaconverter.convert
import androidx.compose.ui.test.SemanticsNodeInteraction
import androidx.compose.ui.test.assertIsNotSelected
import androidx.compose.ui.test.assertIsSelected
import androidx.compose.ui.test.hasAnyAncestor
import androidx.compose.ui.test.hasTestTag
import androidx.compose.ui.test.hasText
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.media3.common.util.UnstableApi
import org.junit.Assert.assertEquals
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.createDrainedComposeRule
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.model.QualityTier
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
/**
* Each picker lights the chip it was handed and reports the constant that was pressed.
*
* The defect this bites on is a picker that renders perfectly and answers wrongly. All three are
* the same dozen lines with a different enum substituted, so the failure mode is a copy-paste that
* survives review: an `onClick` that closes over the picker's `selected` parameter instead of the
* chip's own entry hands back one constant no matter which chip was tapped, and an inverted
* `entry == selected` lights every chip except the right one. Neither throws, neither changes the
* set of labels on screen, and a test that only asserted "the callback ran" would pass over both.
*
* Clicking every chip in turn and comparing the whole recorded list against `entries` is what makes
* the constant load-bearing rather than the click count -- a hardcoded `onSelect` fires the same
* number of times as a correct one. Selection is asserted over every chip for the same reason: the
* one that should be lit proves nothing on its own, because `!=` lights it too whenever the enum
* has exactly one entry, and lights all its siblings whenever it has more.
*
* Labels come from `OutputFormat.label` and `QualityTier.label`; [label], which the screen owns
* because `EnginePreference` carries no label of its own, supplies the third set. Retyping any of
* them here would turn a rename into a red test that named the wrong cause.
*
* Not covered, deliberately: the `"Output format"`, `"Quality"` and `"Engine"` headings, which are
* untagged `Text` calls with no enum behind them and no behaviour to bite on.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class ConverterPickerSelectionTest {
@get:Rule
val composeRule = createDrainedComposeRule()
/**
* The chip carrying [label] inside the row tagged [rowTag].
*
* By ancestor rather than by direct child: how many semantics nodes Material 3 puts between a
* `FlowRow` and its chips is that library's business, and a matcher that assumed "one" would
* break on an upgrade that changed nothing this test is about.
*/
private fun chipIn(rowTag: String, label: String): SemanticsNodeInteraction =
composeRule.onNode(hasAnyAncestor(hasTestTag(rowTag)) and hasText(label))
private fun assertOnlySelected(rowTag: String, labels: List<String>, selected: String?) {
labels.forEach { label ->
val chip = chipIn(rowTag, label)
if (label == selected) chip.assertIsSelected() else chip.assertIsNotSelected()
}
}
@Test
fun `the format picker lights the selected format and no other`() {
composeRule.setContent { FormatPicker(OutputFormat.WEBM_VP9) {} }
assertOnlySelected(
rowTag = TestTags.Converter.FORMAT_CHIPS,
labels = OutputFormat.entries.map { it.label },
selected = OutputFormat.WEBM_VP9.label,
)
}
/** A spec no preset can express lights nothing, which is what the custom line stands in for. */
@Test
fun `the format picker lights nothing when the spec is custom`() {
composeRule.setContent { FormatPicker(null) {} }
assertOnlySelected(
rowTag = TestTags.Converter.FORMAT_CHIPS,
labels = OutputFormat.entries.map { it.label },
selected = null,
)
composeRule.onNodeWithText(CUSTOM_SPEC_NOTE).assertExists()
}
@Test
fun `a selected format hides the custom line`() {
composeRule.setContent { FormatPicker(OutputFormat.MP3) {} }
composeRule.onNodeWithText(CUSTOM_SPEC_NOTE).assertDoesNotExist()
}
@Test
fun `clicking a format chip reports that format`() {
val picked = mutableListOf<OutputFormat>()
composeRule.setContent { FormatPicker(null) { picked += it } }
OutputFormat.entries.forEach { chipIn(TestTags.Converter.FORMAT_CHIPS, it.label).performClick() }
assertEquals(OutputFormat.entries.toList(), picked)
}
@Test
fun `the quality picker lights the selected tier and no other`() {
composeRule.setContent { QualityPicker(QualityTier.BEST) {} }
assertOnlySelected(
rowTag = TestTags.Converter.QUALITY_CHIPS,
labels = QualityTier.entries.map { it.label },
selected = QualityTier.BEST.label,
)
}
/** The line under the chips describes what was chosen, not whichever tier was written first. */
@Test
fun `the quality picker explains the tier that is selected`() {
composeRule.setContent { QualityPicker(QualityTier.BEST) {} }
composeRule.onNodeWithText(QualityTier.BEST.description).assertExists()
composeRule.onNodeWithText(QualityTier.FAST.description).assertDoesNotExist()
}
@Test
fun `clicking a quality chip reports that tier`() {
val picked = mutableListOf<QualityTier>()
composeRule.setContent { QualityPicker(QualityTier.FAST) { picked += it } }
QualityTier.entries.forEach { chipIn(TestTags.Converter.QUALITY_CHIPS, it.label).performClick() }
assertEquals(QualityTier.entries.toList(), picked)
}
@Test
fun `the engine picker lights the selected preference and no other`() {
composeRule.setContent { EnginePicker(EnginePreference.FORCE_SOFTWARE) {} }
assertOnlySelected(
rowTag = TestTags.Converter.ENGINE_CHIPS,
labels = EnginePreference.entries.map { it.label() },
selected = EnginePreference.FORCE_SOFTWARE.label(),
)
}
@Test
fun `clicking an engine chip reports that preference`() {
val picked = mutableListOf<EnginePreference>()
composeRule.setContent { EnginePicker(EnginePreference.AUTO) { picked += it } }
EnginePreference.entries.forEach { chipIn(TestTags.Converter.ENGINE_CHIPS, it.label()).performClick() }
assertEquals(EnginePreference.entries.toList(), picked)
}
private companion object {
/**
* Copied byte for byte out of `ConverterScreen.kt` -- it holds a U+2014 em dash, which
* retyped as ASCII would match nothing and fail as "no node found" rather than as a reword.
*/
const val CUSTOM_SPEC_NOTE: String = "Custom — set below."
}
}
@@ -0,0 +1,254 @@
package org.libremediaconverter.convert
import android.net.Uri
import androidx.compose.ui.test.assertCountEquals
import androidx.compose.ui.test.assertTextEquals
import androidx.compose.ui.test.onChildren
import androidx.compose.ui.test.onNodeWithTag
import androidx.media3.common.util.UnstableApi
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.createDrainedComposeRule
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.InputKind
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
/**
* What the source-info card says when it does not know something.
*
* The defect is a card that invents an answer instead of admitting it has none. Two of them are
* live here and neither had a test before this file:
*
* - **`InputFile.sizeBytes` is nullable and the card is the reader that has to say so in words.**
* `sizeBytes` used to be `0L` for "nobody told me", and [UnknownInputSizeTest] records what that
* cost at the space check. The card is the other reader, and its failure mode is the mirror
* image: hand the null to `formatBytes` and it renders `"0 B"` -- a measurement, shown to the
* user, that no provider ever made. It renders **independently of the probe**, which is why the
* same assertion appears twice below, with the probe present and absent. That independence is
* the contract; a test covering only the probed case would leave the branch a user actually hits
* first -- the card is on screen before the probe finishes -- unguarded.
* - **The codec rows degrade in words too.** `CodecNames.describeVideo`/`describeAudio` answer
* `"Unknown"` for a codec nothing named, the `VIDEO` branch answers `"No audio track"` for a file
* with no audio, and the two `> 0` guards drop the dimension and length rows rather than printing
* `0` and `0:00`. Each of those has a case below on **both** sides of the guard, because a test
* of the present side alone stays green with the guard deleted.
*
* ### What cannot be asserted here, so that it is a decision rather than an omission
*
* The `probe == null` branch exits before `HorizontalDivider`, and **the divider's absence is not
* observable from a test**: Material 3 renders it as a `Box` with no semantics modifier, so it
* contributes no node to the semantics tree at all. What is asserted instead is everything the
* divider precedes -- no detail row for any label the four kind branches can emit -- plus the
* card's child count, which pins "these three texts and nothing else" without having to enumerate.
*
* The early exit itself is enforced by the compiler rather than by this file, which the PR body
* records: deleting `return@Column` un-smart-casts `probe`, and the `probe.kind` below it stops
* compiling. The mutation that reddens the test here is the compilable form of that regression --
* defaulting the null away with `?: InputProbe()` and letting the kind rows render.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class FileCardTest {
@get:Rule
val composeRule = createDrainedComposeRule()
@Test
fun `a file no provider could measure says so in words rather than showing a zero`() {
setFileCard(input(sizeBytes = null, probe = VIDEO_PROBE))
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES)
.assertTextEquals("Size unknown")
}
/**
* The same line, with no probe at all. Separate from the case above rather than folded into
* it because `setContent` may only be called once per rule, and because two independent reds
* are the evidence that the size line does not depend on the probe.
*/
@Test
fun `the size line says the same thing while the probe is still running`() {
setFileCard(input(sizeBytes = null, probe = null))
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES)
.assertTextEquals("Size unknown")
}
@Test
fun `a size that was reported is formatted rather than replaced by the unknown line`() {
setFileCard(input(sizeBytes = 12_345_678L, probe = VIDEO_PROBE))
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME).assertTextEquals("clip.mkv")
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES).assertTextEquals("12.3 MB")
}
/**
* The note and the emptiness are one behaviour, so they are one test: a regression that keeps
* the note but renders the rows anyway would leave a note-only test green.
*/
@Test
fun `while the probe is still running the card shows the reading note and nothing else`() {
setFileCard(input(probe = null))
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NOTE)
.assertTextEquals("Reading…")
assertNoDetailRows()
// Name, size, note. Catches a row whose label is not in EVERY_ROW_LABEL as well.
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD).onChildren().assertCountEquals(3)
}
@Test
fun `a file nothing could read gets the explanatory line instead of unknown codecs`() {
setFileCard(input(probe = InputProbe(kind = InputKind.UNPARSEABLE)))
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NOTE)
.assertTextEquals("Could not identify this file. It will be converted with FFmpeg.")
assertNoDetailRows()
}
@Test
fun `an image gets its type and its pixel dimensions`() {
setFileCard(input(probe = InputProbe(kind = InputKind.IMAGE, width = 1920, height = 1080)))
assertRow("Type", "Image")
assertRow("Size", "1920×1080")
}
/** The `width > 0` guard, from the side that would print `0×0` if it were dropped. */
@Test
fun `an image whose dimensions nothing reported gets the type row alone`() {
setFileCard(input(probe = InputProbe(kind = InputKind.IMAGE)))
assertRow("Type", "Image")
assertNoRow("Size")
}
@Test
fun `an audio-only file says it has no video track rather than leaving the row blank`() {
setFileCard(
input(
probe = InputProbe(
audioCodec = "aac",
hasVideo = false,
durationMs = 90_000,
kind = InputKind.AUDIO_ONLY,
container = Container.MP3,
),
),
)
assertRow("Container", Container.MP3.label)
assertRow("Video", "No video track")
assertRow("Audio", AudioCodec.AAC.label)
assertRow("Length", "1:30")
assertNoRow("Type")
assertNoRow("Size")
}
/**
* Everything the audio branch can fail to know, at once: no container, no codec name, no
* duration. Each degrades in its own words, and the length row disappears rather than
* claiming `0:00`.
*/
@Test
fun `an audio-only file nothing else could describe degrades one row at a time`() {
setFileCard(input(probe = InputProbe(hasVideo = false, kind = InputKind.AUDIO_ONLY)))
assertRow("Container", "Unknown")
assertRow("Video", "No video track")
assertRow("Audio", "Unknown")
assertNoRow("Length")
}
@Test
fun `a video file composes its codec with its dimensions on one row`() {
setFileCard(input(probe = VIDEO_PROBE))
assertRow("Container", Container.MP4.label)
assertRow("Video", "${VideoCodec.H264.label} · 1920×1080")
assertRow("Audio", AudioCodec.AAC.label)
assertRow("Length", "1:30")
}
/**
* `"No audio track"` rather than `describeAudio(null)`'s `"Unknown"`. The video branch knows
* the difference between a track it could not name and a track that is not there; the audio
* branch above cannot, because a file with no audio is not audio-only.
*/
@Test
fun `a video file with no audio track says so instead of naming an unknown codec`() {
setFileCard(input(probe = VIDEO_PROBE.copy(audioCodec = null)))
assertRow("Audio", "No audio track")
}
/** Both `> 0` guards on the video branch, plus the codec name nothing supplied. */
@Test
fun `a video file missing its codec, dimensions and duration omits them rather than faking them`() {
setFileCard(
input(
probe = VIDEO_PROBE.copy(
videoCodec = null,
width = 0,
height = 0,
durationMs = 0,
),
),
)
assertRow("Video", "Unknown")
assertNoRow("Length")
}
/**
* The row is one node, not a label node beside a value node. A test matching on `"Container"`
* alone would pass against either shape.
*/
@Test
fun `a detail row renders its label and its value as a single node`() {
composeRule.setContent { DetailRow("Container", "Matroska") }
composeRule.onNodeWithTag(TestTags.Converter.detailRow("Container"))
.assertTextEquals("Container: Matroska")
}
private fun setFileCard(input: InputFile) = composeRule.setContent { FileCard(input) }
private fun input(sizeBytes: Long? = 12_345_678L, probe: InputProbe? = VIDEO_PROBE) = InputFile(
uri = Uri.parse("content://test/clip.mkv"),
displayName = "clip.mkv",
sizeBytes = sizeBytes,
probe = probe,
)
private fun assertRow(label: String, value: String) {
composeRule.onNodeWithTag(TestTags.Converter.detailRow(label))
.assertTextEquals("$label: $value")
}
private fun assertNoRow(label: String) {
composeRule.onNodeWithTag(TestTags.Converter.detailRow(label)).assertDoesNotExist()
}
private fun assertNoDetailRows() = EVERY_ROW_LABEL.forEach(::assertNoRow)
private companion object {
/** Every label the four kind branches can emit, so absence can be asserted exhaustively. */
val EVERY_ROW_LABEL = listOf("Container", "Video", "Audio", "Length", "Type", "Size")
val VIDEO_PROBE = InputProbe(
videoCodec = "h264",
audioCodec = "aac",
durationMs = 90_000,
kind = InputKind.VIDEO,
container = Container.MP4,
width = 1920,
height = 1080,
)
}
}
@@ -0,0 +1,53 @@
package org.libremediaconverter.join
import android.net.Uri
import androidx.compose.ui.test.assertCountEquals
import androidx.compose.ui.test.onAllNodesWithTag
import androidx.media3.common.util.UnstableApi
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.InputFile
import org.libremediaconverter.createDrainedComposeRule
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
/**
* The join screen's one leaf renders, and tags itself with the file it is showing.
*
* `FileRow` is the only place on either screen where the same leaf is rendered more than once at a
* time -- one row per picked input -- so it is the only tag that cannot be a constant. It is
* derived from `displayName`, inside `FileRow` itself, and that is the part worth a test: a row
* that took its tag from the call site would let R38.7 pass a tag in and assert nothing, which is
* the vacuous shape `CLAUDE.md` records nine of in one review.
*
* Two rows are rendered here rather than one, because a tag derived from the wrong thing -- a
* constant, an index the row does not have -- would still resolve to one node with a single input
* on screen.
*
* Deliberately not the state matrix: which affordances each `JoinState` renders is R38.7.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class JoinLeafTagsTest {
@get:Rule
val composeRule = createDrainedComposeRule()
private fun input(displayName: String) = InputFile(
uri = Uri.parse("content://test/$displayName"),
displayName = displayName,
sizeBytes = 4_000_000L,
)
@Test
fun `each file row is tagged with the name it displays`() {
composeRule.setContent {
FileRow(input("first.mp4"))
FileRow(input("second.mp4"))
}
composeRule.onAllNodesWithTag(TestTags.Join.fileRow("first.mp4")).assertCountEquals(1)
composeRule.onAllNodesWithTag(TestTags.Join.fileRow("second.mp4")).assertCountEquals(1)
}
}
@@ -0,0 +1,40 @@
package org.libremediaconverter.ui
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Test
/**
* No two entries of [TestTags] may share a value.
*
* A duplicated value is the one mistake this table invites -- the constants are added in blocks of
* near-identical lines, and a copy-paste that keeps the old string still compiles, still reads
* correctly at the call site, and still passes every test in the file that placed it. It surfaces
* later, in someone else's PR, as an affordance that "resolves to exactly one node" finding two,
* with nothing in that diff to explain it.
*
* Read by reflection rather than from a hand-written list, because a hand-written list would be a
* second copy of the table with the same copy-paste failure in it.
*/
class TagTableUniquenessTest {
private fun tagsIn(vararg holders: Class<*>): List<String> = holders.flatMap { holder ->
holder.declaredFields
.filter { it.type == String::class.java }
.map { it.get(null) as String }
}
@Test
fun `every tag constant has its own value`() {
val tags = tagsIn(
TestTags::class.java,
TestTags.Converter::class.java,
TestTags.Join::class.java,
)
// Without this the check would pass on an empty list, which is what a reflection call
// that stopped finding the constants would hand it.
assertTrue("reflection found only ${tags.size} tag constants, so it is not reading the table", tags.size > 20)
assertEquals(emptyList<String>(), tags.groupBy { it }.filterValues { it.size > 1 }.keys.toList())
}
}
+183 -20
View File
@@ -7,8 +7,8 @@ boots, disabling SystemUI collapses the crash rate far enough to run a suite —
**2 failures, 0 errors and the two by-design skips** (measured 49 / 2 / 0 / 2 at `22c7914`, where
the suite was 49 tests — [Reading these totals](#reading-these-totals) before comparing any total
with another). The crashes do not stop outright, and the two failures are real; both are quantified
below. CI's matrix should still stop at 36 — see
[So should CI take API 37?](#so-should-ci-take-api-37).
below. CI now takes API 37 as two jobs — a gating leg and an advisory one for those two
failures — see [So should CI take API 37?](#so-should-ci-take-api-37).
**Last verified:** 2026-08-22, emulator `37.1.11.0` (build 15917651), Fedora 44,
against system images `android-37.0` rev 6 **and** `android-37.1` rev 8.
@@ -339,38 +339,193 @@ Caused by: android.media.MediaCodec$CodecException:
```
Three measurements say this is the emulator image and not this app, and not the software
renderer:
renderer. A fourth bullet offers a mechanism, and is inference rather than measurement:
- **Control at API 35 under the identical renderer.** `GPU_MODE=swangle_indirect
tools/local-emulator/run-e2e.sh 35` → **49 / 0 / 0 / 2** at `22c7914`, green.
`c2.goldfish.h264.decoder` is perfectly happy under ANGLE one API level down, so the renderer is
not what breaks it.
- **Real API 37 hardware passes**, see below. There is no `c2.goldfish.*` codec on a Pixel.
- **API 36 against API 37 on CI, back to back, everything else held.** Same two tests, same
`-gpu swiftshader_indirect`, same SystemUI-disable path — `pm disable-user`, `stop`, wait for
`system_server` to actually be gone, `start`, then verify against `pm list packages -d`. Both
runs were narrowed to the two failing tests:
```
-Pandroid.testInstrumentationRunnerArguments.class=\
org.libremediaconverter.convert.Media3EngineTest#transcodesH264ToH265AndReportsProgress,\
org.libremediaconverter.convert.Media3EngineTest#runsFromAThreadWithNoLooper
```
and the filter is confirmed three independent ways: `tests="2"` in the XML, `Expected 2 tests`
in the abort message, and `run started: 2 tests` in the guest logcat.
| run | api | result XML |
|---|---|---|
| [32660148155](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32660148155) | 37.0 | `tests="2" failures="2" errors="0" skipped="0"` |
| [32660152961](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32660152961) | 36 | `tests="2" failures="0" errors="0" skipped="0" time="4.603"` |
API 37 fails with the signature above — `name=c2.goldfish.h264.decoder`,
`MediaCodec$CodecException` at `dequeueOutputBuffer(MediaCodec.java:4274)`. API 36 passes both in
4.603 s, and `c2.goldfish.h264.decoder` is in *its* logcat too (44 mentions), so the two runs are
not being served by different decoder names. **What this falsifies is "the stripped
configuration is what breaks these tests"** — a reading none of the other measurements
addresses, because they all compare against a device that still had SystemUI. Here SystemUI is
absent and the framework has been restarted on both sides, and the healthy image is green anyway.
Two things it does **not** control, which is why it narrows the claim rather than closing it:
- **The restarts were not performed under equal conditions.** API 36 did its `stop`/`start` with
`dma_aborts=0`; API 37's did the same restart with two aborts already logged. "A framework
restart performed while the abort loop is running" therefore remains uncontrolled.
- **The images differ on the encoder side.** These tests transcode H.264 → H.265. The API 37
logcat carries `c2.goldfish.hevc.decoder` (16 mentions in the control run) where API 36 carries
`c2.android.hevc.encoder` (32). The pipeline is not identical end to end, which is a second
reason "the image ships a broken h264 decoder" is the wrong *shape* of claim: what is measured
is that these two tests fail on the API 37 image, pass at API 36 under the same renderer *and*
the same disable path, and pass at 33–36 without needing that path at all — because nothing
below 37 has the bug it works around.
- The failing call is `dequeueOutputBuffer` on the *goldfish* decoder — the emulator's own codec,
which like `RegionSamplingThread` gets its frames out of a host-side colour buffer. Same
readback machinery, one layer down. This is inference rather than a measurement, and is flagged
as such; what is measured is the first two bullets.
as such; what is measured is the first three bullets.
**Do not try `-feature -HardwareDecoder`.** It is the obvious next idea and it is much worse:
forcing the guest onto software decoders took the run from 2 failures to **46**, across
`RemuxTest`, `ForcedFailureTest`, `HardwareFallbackTest` and `UnopenableUriTest` as well. The
suite depends on those decoders existing.
### The intact-SystemUI counterfactual cannot be measured on CI
The control the block above still lacks is the obvious one: run those same two tests at API 37
with SystemUI **left running**. Passing would put the failure on the disable rather than on the
image; failing on the decoder would make the decoder attribution direct instead of inferred.
**Seven dispatches of `api37-debug.yml`, zero verdicts.** Not bad luck — a mechanism, which is why
this is written down rather than left as a gap for the next person to spend seven runs on:
| arm | run | result XML | what actually happened |
|---|---|---|---|
| E1 | [32660528355](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32660528355) | `tests="1" failures="1"`, `<failure>` body empty | `Expected 2 tests, received 0. INSTRUMENTATION_ABORTED: System has crashed.` |
| E2 | [32660533845](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32660533845) | `tests="0"` | never installed: `Failed to commit install session ... Failure calling service package: Broken pipe (32)` |
| E3 | [32660539259](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32660539259) | `tests="0"` | `Test run failed to complete. No test results.` |
| E4 | [32661117237](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32661117237) | `tests="2" failures="2"` | both failed in `@Before`, never reached MediaCodec |
| E5 | [32661121972](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32661121972) | `tests="2" failures="2"` | same |
| S1 | [32661127224](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32661127224) | `tests="1" failures="1"` | same, single-test arm |
| S2 | [32661132024](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32661132024) | `tests="1" failures="1"` | same |
While the framework is crash-looping, the guest cannot reliably create per-user private
directories. An app installed during the loop has no cache directory — and `Media3EngineTest`
copies its H.264 fixture into `context.cacheDir` in `@Before`, so it dies there, **before any
MediaCodec exists**:
```
W/ContextImpl( 8216): Failed to ensure /data/user/0/org.libremediaconverter/cache
I/TestRunner( 8216): run started: 1 tests
E/TestRunner( 8216): failed: transcodesH264ToH265AndReportsProgress(...)
E/TestRunner( 8216): java.io.FileNotFoundException:
/data/user/0/org.libremediaconverter/cache/sample_h264.mp4: open failed: ENOENT
at org.libremediaconverter.convert.Media3EngineTest.setUp(Media3EngineTest.kt:47)
```
Not app-specific: `com.google.android.googlesdksetup` and `com.google.android.apps.nexuslauncher`
hit the same `Failed to ensure /data/user/0/<pkg>/cache` in the same logcats.
**The result XML masks this, and reading only the report gets you the wrong bug.** What E4, E5, S1
and S2 report is
```
<failure>kotlin.UninitializedPropertyAccessException: lateinit property output has not been initialized
at org.libremediaconverter.convert.Media3EngineTest.tearDown(Media3EngineTest.kt:56)
```
— `tearDown` failing because `setUp` threw before it assigned `output`. That looks like a
teardown defect in this repository and is not one; the cause is only in the guest logcat.
So the obstacle is structural: install, data-directory creation and instrumentation start-up do
not fit between framework kills, and four of the seven runs show the directory creation itself is
broken during the loop. More dispatches of this shape would repeat these outcomes. The
counterfactual is still open on the **Pixel 10 Pro XL**, the one API 37 device here that is not an
emulator — but a Pixel has no `c2.goldfish.*` codec at all, so it answers "does the app work at
API 37", not "is that codec broken".
#### Abort cadence, corrected
`.github/workflows/api37-debug.yml` carried "roughly every 20 s" for the kill cycle in its own
comments. That number was the watchdog's **sampling** interval, not the cadence, and the two got
conflated. Measured across the seven runs above, gaps between successive `hasReadColorBufferDma`
aborts run **20 s to 90 s, median 60–70 s — three to five aborts in a four-minute window**.
Slower than assumed, and still not slow enough: install, data-directory creation and
instrumentation start-up do not fit inside one gap.
`sys.boot_completed` held at `1` throughout every one of those test windows. The device reports
itself booted while zygote is being killed under it, which is why no boot-state check catches
this and why `stop`/`start` waits must poll `pidof system_server` and `service check` instead
(see `disable_region_sampling` in `tools/local-emulator/run-e2e.sh`).
### So should CI take API 37?
**No, and the matrix should still stop at 36.** Three reasons, in order of weight:
**Yes, as two jobs: a gating `E2E API 37` and an advisory leg carrying the two tests that do not
pass.** That reverses the answer this section gave, and the reversal is measured rather than
argued — two of its three reasons were claims *about CI*, and CI had never been measured. The
instrument that measured it is [`.github/workflows/api37-debug.yml`](../.github/workflows/api37-debug.yml),
dispatch-only, a copy of the E2E job with the matrix replaced by inputs.
1. CI runs `swiftshader_indirect` on a GPU-less runner. The aborts happen under ANGLE too — they
are merely sparser — so nothing here says a runner would be stable.
2. The working configuration needs SystemUI disabled and a framework restart mid-job. That is a
lot of bespoke device surgery to put behind a merge gate, and it silently weakens what the leg
proves.
3. Even at its best two tests fail, so the leg would be permanently red or permanently
allow-listed. Neither is a gate worth having.
Every run below is `ubuntu-latest`, KVM on, `pixel_6`, x86_64, disk 8G, RAM 2560M, emulator
`37.1.11.0` build 15917651 — the same emulator build the local investigation used. Every **API
37** row is `system-images;android-37.0;google_apis;x86_64`; c2 is the API 36 control and runs
that level's own image, which is the whole point of it.
What has changed is the *local* story: API 37 is no longer a level nobody can look at. A
regression that shows up at 37 and not at 36 can now be reproduced on this workstation in about
four minutes, which is what the missing matrix row was really costing.
| # | run | api | `-gpu` | SystemUI | suite | verdict |
|---|---|---|---|---|---|---|
| c1 | [32644947334](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32644947334) | 37.0 | swiftshader_indirect | running | `Starting 0 tests` | FAIL |
| c2 | [32644965828](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32644965828) | **36** | swiftshader_indirect | running | 57 tests, BUILD SUCCESSFUL | green control |
| c3 | [32644970240](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32644970240) | 37.0 | swangle_indirect | running | `Starting 0 tests` | FAIL |
| c5 | [32645543238](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32645543238) | 37.0 | swangle_indirect | disabled | 57 / 2 / 0 / 2 | suite ran |
| c6 | [32646029143](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32646029143) | 37.0 | swiftshader_indirect | one-shot disable, **did not hold** | `Starting 0 tests` | FAIL |
| c8 | [32646611485](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32646611485) | 37.0 | swiftshader_indirect | disabled, verified | 57 / 2 / 0 / 2 | suite ran |
| c9 | [32646615706](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32646615706) | 37.0 | swiftshader_indirect | disabled, verified | 57 / 2 / 0 / 2 | suite ran |
| c10 | [32646619472](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32646619472) | 37.0 | swiftshader_indirect | disabled, verified | 57 / 2 / 0 / 2 | suite ran |
| c11 | [32647138060](https://github.com/JMR-dev/LibreMediaConverter/actions/runs/32647138060) | 37.0 | swiftshader_indirect | disabled, verified | 57 / 2 / 0 / 2 | suite ran |
57 is that checkout's own `@Test` count at `acc71bc`, so those are whole-suite runs and not
truncated ones — see [Reading these totals](#reading-these-totals). Taking the three old reasons
in turn:
1. **"Nothing says a runner would be stable" — measured, and it is.** `-gpu swiftshader_indirect`
on a GPU-less runner resolves to `gles_mode_selected:swiftshader`, a third renderer that
locally never survives to say anything (Fedora's SELinux denies `execheap` to SwiftShader's
Reactor JIT — see [`local-emulator.md`](local-emulator.md)). It boots `android-37.0` in about
60 s. The local discriminator — fatal iff `gles_mode_selected:host` — holds, and a runner with
no GPU can never select host, so CI was never in the fatal class. Switching CI's `-gpu` changes
nothing either way: c1 and c3 both fail with SystemUI up, under swiftshader and swangle
respectively, and c5 and c8–c11 show the suite running under either once SystemUI is gone.
2. **"Bespoke device surgery" — still true, and now a written caveat rather than a reason to skip
the level.** It is one env flag, `E2E_DISABLE_SYSTEM_UI`, read by `.github/scripts/e2e-run.sh`
and unset on every other leg. What it costs is stated where it can be read from the failing
check: the API 37 row runs a device configuration no other leg and no Pixel run uses. What
makes it dependable is verification, not repetition — c6 is the counter-case, a one-shot
`pm disable-user` that reported `new state: disabled-user` and then started SystemUI eight more
times. The verified form is 4/4; the unverified form was 3/4.
3. **"Permanently red or permanently allow-listed" — this was the real objection, and it is the
one the split answers.** The two failures are marked `@FailsOnEmulatorApi37` in
`app/src/androidTest`. The gating job runs `notAnnotation` on that marker and must be green;
the advisory job runs `annotation` on the *same* marker, reports, and never blocks. One marker
rather than two lists, so a test cannot silently end up in neither job — which would read as
green.
The cost is about three minutes on the API 37 leg — the `stop`/`start` plus a 45 s quiet window,
and another round when the first does not verify. Measured wall clock for the whole job, boot
included: 6–7 minutes at API 37 against ~6 at API 36.
Two things this does **not** buy. The advisory job is expected red, so a *third* failure there is
the signal and the run's logcat is the only thing that distinguishes it — which is why that job
uploads diagnostics unconditionally. And a green `E2E API 37` still does not replace the release
check on the Pixel: the emulator leg runs without SystemUI, and the Pixel does not.
The *local* story changed at the same time and independently: API 37 is no longer a level nobody
can look at. A regression that shows up at 37 and not at 36 can be reproduced on this workstation
in about four minutes.
## Verified on real API 37 hardware
@@ -492,6 +647,12 @@ though a new API level shipped. Watch for these instead:
which is what drives `RegionSamplingThread`, so one would very likely sidestep the bug
entirely. Try it before anything else here.
- **the upstream issue being marked fixed.**
- **`E2E API 37 Media3 hardware transcode (advisory)` going green.** Nothing announces this: the
job is `continue-on-error`, so it fixing itself looks exactly like a check nobody reads
quietly ceasing to be red. It is listed here because that makes it the *least* likely of these
triggers to be noticed, not the most. When it happens, delete `@FailsOnEmulatorApi37` from the
two tests rather than the job — the gating leg picks them back up on its own, and the advisory
job then runs nothing and can go.
## Correction owed to `CLAUDE.md`
@@ -504,8 +665,9 @@ though a new API level shipped. Watch for these instead:
The first sentence is right, and now under-specified in one direction and over-specified in the
other: it is not only `android-37.0` (it is `37.1` too), and it does not crash-loop under every
renderer. Proposed replacement, offered for review rather than applied here — `CLAUDE.md` is left
alone deliberately, because several branches touch it:
renderer. **The last clause is now simply false: CI's matrix does not stop at API 36 any more.**
Proposed replacement, offered for review rather than applied here — `CLAUDE.md` is left alone
deliberately, because several branches touch it:
> - **The API 37 images crash-loop surfaceflinger under the host GL renderer.** Both
> `android-37.0` and `android-37.1` abort inside their own gralloc mapper
@@ -514,6 +676,7 @@ alone deliberately, because several branches touch it:
> boots at all; under `-gpu swangle_indirect` it boots and the aborts merely become
> intermittent. `docs/api-37-emulator-crash.md` has the seven-run matrix and the ruled-out
> list, and `tools/local-emulator/run-e2e.sh` picks the working renderer per API level.
> CI's matrix stops at 36 because CI runs `swiftshader_indirect` on a GPU-less runner and the
> aborts continue there too. **API 37 still needs a manual check on the Pixel 10 Pro XL before
> each release.**
> CI takes API 37 as two jobs: a gating `E2E API 37` that disables SystemUI first, and an
> advisory leg carrying the two `@FailsOnEmulatorApi37` tests. The gating leg therefore runs a
> device configuration nothing else does. **API 37 still needs a manual check on the Pixel 10
> Pro XL before each release** — it is the only API 37 run with SystemUI intact.
+2 -2
View File
@@ -469,8 +469,8 @@ Proposed replacement for the section, offered for review rather than applied her
> `android-37.1` abort surfaceflinger inside their own gralloc mapper, and init SIGKILLs
> zygote each time. Under `-gpu host` they never boot; under `-gpu swangle_indirect` they
> boot, and disabling SystemUI removes the trigger. `run-e2e.sh` does all of that per level,
> and the local API 37 result is two failures and the two usual skips, not a clean run. CI's
> matrix still stops at 36.
> and the local API 37 result is two failures and the two usual skips, not a clean run. CI
> takes API 37 as a gating leg plus an advisory one carrying those two tests.
> `docs/api-37-emulator-crash.md` has the matrix and the reasoning. **API 37 needs a manual
> check on the Pixel 10 Pro XL before each release.**
+15
View File
@@ -80,6 +80,17 @@ jacoco = "0.8.15"
# 4.16.1 is the newest RELEASED version; the 4.17 line is beta-only at the time of writing.
robolectric = "4.16.1"
# kotlinx-coroutines-test. Already on the unit-test classpath transitively, through
# compose-ui-test-junit4 -- declared here because a source file now imports it, and a direct
# import of a transitive is a dependency nobody chose.
#
# PINNED, for the same reason as robolectric above: org.jetbrains.kotlinx is not one of the
# groups in the prerelease guard's `floatedGroupPrefixes`, so a "1.+" here would be free to
# resolve to a milestone build. This value is what the Compose BOM already resolves it to, so
# stating it changes nothing in the graph today; if the BOM moves ahead, Gradle takes the
# higher version and this stays a floor rather than a conflict.
coroutinesTest = "1.9.0"
[libraries]
androidx-core-ktx = { group = "androidx.core", name = "core-ktx", version.ref = "coreKtx" }
androidx-activity-compose = { group = "androidx.activity", name = "activity-compose", version.ref = "activityCompose" }
@@ -134,6 +145,10 @@ androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-co
# a TDD loop anyone here can execute.
robolectric = { group = "org.robolectric", name = "robolectric", version.ref = "robolectric" }
# Only for its `runTest`, and only to drain the collector kotlinx-coroutines-test installs
# process-wide. See EscapedCoroutineErrors.kt in the JVM test source set.
kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "coroutinesTest" }
[plugins]
# com.android.application and org.jetbrains.kotlin.plugin.compose are deliberately absent.
# They come from the root buildscript classpath (see build.gradle.kts) so that a newer KGP
+253
View File
@@ -0,0 +1,253 @@
#!/usr/bin/env bash
#
# Files a GitHub issue AND puts it on the project board, as one operation.
#
# Usage: tools/github/file-issue.sh --title TITLE (--body TEXT | --body-file PATH) [options]
#
# --status NAME board column, matched case-insensitively against the board's own
# options; a miss lists what is available. Default: Backlog
# --label NAME repeatable. Passed through to `gh issue create` unchanged.
# --project N project number. Default: $ISSUE_PROJECT_NUMBER, else 6
# --repo OWNER/NAME default: whatever `gh repo view` resolves in the working directory
# --dry-run resolve and validate everything, create nothing
#
# EXIT CODE: 0 only when the issue exists, is on the board, AND reads back carrying the
# Status that was asked for. 2 for a usage or validation error, before anything is created.
# **3 means the issue was created but did not reach the board** -- the number is printed on
# a line of its own, because that combination is the entire failure this script exists to
# prevent and it must never be quiet.
#
# WHY THIS EXISTS
#
# `gh issue create` does not touch the project board. The issue is created, carries its
# labels, and is invisible in the Kanban -- which looks exactly like a ticket nobody filed.
# Measured 2026-08-24: eight issues filed as a scripted batch all reached the board; one
# filed as a one-off a few minutes later did not, and was caught only because someone went
# looking. A batch carries the board step inside its loop. One-offs are where it slips, so
# one-offs are what this is for.
#
# Adding an item and setting a field value are GraphQL-only. REST can list project items
# and field definitions, but the `fields` array it returns on an item carries Title and
# nothing else -- a REST-only check reports every item's Status as unset, which is why the
# read-back at the end is a GraphQL query rather than the cheaper REST one.
#
# WHAT IT DELIBERATELY DOES NOT DO
#
# It does not cache the project, field or option ids. Resolving them by name costs one
# GraphQL query per run, and it means a renamed or reordered column cannot make this write
# a stale id. The ids are the fragile part; the names are what people actually use.
#
# It does not apply triage labels for you. `above-cut` and `backlog` are labels from one
# specific 2026-08-22 triage pass -- they mean "worked autonomously overnight" and "held for
# manual review", not "this is in the Backlog column". Status carries board state. Pass
# --label only for things that are true about the issue itself.
#
# It does not create the project, the Status field, or a missing option. Anything absent is
# an error to report, not to invent.
set -euo pipefail
readonly EXIT_USAGE=2
readonly EXIT_ORPHANED=3
die() {
printf 'file-issue: %s\n' "$1" >&2
exit "${2:-$EXIT_USAGE}"
}
title=""
body=""
body_file=""
status="Backlog"
project="${ISSUE_PROJECT_NUMBER:-6}"
repo=""
dry_run=0
labels=()
while [ $# -gt 0 ]; do
case "$1" in
--title) [ $# -ge 2 ] || die "--title needs a value"; title="$2"; shift 2 ;;
--body) [ $# -ge 2 ] || die "--body needs a value"; body="$2"; shift 2 ;;
--body-file) [ $# -ge 2 ] || die "--body-file needs a path"; body_file="$2"; shift 2 ;;
--status) [ $# -ge 2 ] || die "--status needs a value"; status="$2"; shift 2 ;;
--label) [ $# -ge 2 ] || die "--label needs a value"; labels+=("$2"); shift 2 ;;
--project) [ $# -ge 2 ] || die "--project needs a number"; project="$2"; shift 2 ;;
--repo) [ $# -ge 2 ] || die "--repo needs OWNER/NAME"; repo="$2"; shift 2 ;;
--dry-run) dry_run=1; shift ;;
-h|--help) awk 'NR > 1 && /^#/ { sub(/^# ?/, ""); print; next } NR > 1 { exit }' "$0"
exit 0 ;;
*) die "unknown argument: $1" ;;
esac
done
[ -n "$title" ] || die "--title is required"
if [ -n "$body" ] && [ -n "$body_file" ]; then
die "pass --body or --body-file, not both"
fi
[ -n "$body" ] || [ -n "$body_file" ] || die "one of --body or --body-file is required"
if [ -n "$body_file" ] && [ ! -r "$body_file" ]; then
die "--body-file is not readable: $body_file"
fi
case "$project" in
''|*[!0-9]*) die "--project must be a number, got: $project" ;;
esac
command -v gh >/dev/null 2>&1 || die "gh is not on PATH"
if [ -z "$repo" ]; then
repo=$(gh repo view --json nameWithOwner --jq '.nameWithOwner') \
|| die "could not resolve the repository; pass --repo OWNER/NAME"
fi
owner="${repo%%/*}"
[ -n "$owner" ] || die "could not read an owner out of: $repo"
# ---------------------------------------------------------------------------
# Resolve the board by NAME. Every id below is read fresh; none is hardcoded.
# ---------------------------------------------------------------------------
# The $names in the query are GraphQL variables, declared by the query and bound by the
# -f flags. Expanding them in the shell would send this shell's idea of $owner to the
# API instead of declaring a parameter -- which is why every query here is single-quoted.
# shellcheck disable=SC2016
board=$(gh api graphql \
-f query='
query($owner: String!, $number: Int!) {
user(login: $owner) {
projectV2(number: $number) {
id
title
field(name: "Status") {
... on ProjectV2SingleSelectField { id options { id name } }
}
}
}
}' \
-f owner="$owner" -F number="$project" 2>&1) \
|| die "could not read project $project for $owner. A 403 naming scopes means gh is
missing 'project'; a 403 naming a rate limit is the GraphQL budget, not permissions. The
API said: $board"
project_id=$(printf '%s' "$board" | jq -r '.data.user.projectV2.id // empty')
field_id=$(printf '%s' "$board" | jq -r '.data.user.projectV2.field.id // empty')
project_title=$(printf '%s' "$board" | jq -r '.data.user.projectV2.title // empty')
[ -n "$project_id" ] || die "no project number $project under user $owner"
[ -n "$field_id" ] || die "project $project has no single-select field named 'Status'"
# Case-insensitive match, so "backlog" and "Backlog" both work. The canonical name is
# what gets reported back, so a sloppy argument still produces an exact log line.
option=$(printf '%s' "$board" | jq -r --arg want "$status" '
.data.user.projectV2.field.options[]
| select((.name | ascii_downcase) == ($want | ascii_downcase))
| "\(.id)\t\(.name)"' | head -n 1)
if [ -z "$option" ]; then
printf 'file-issue: no Status option named %s. Available:\n' "$status" >&2
printf '%s' "$board" | jq -r '.data.user.projectV2.field.options[] | " " + .name' >&2
exit "$EXIT_USAGE"
fi
option_id="${option%%$'\t'*}"
status_canonical="${option#*$'\t'}"
printf 'repo %s\n' "$repo"
printf 'board %s (project %s)\n' "$project_title" "$project"
printf 'status %s\n' "$status_canonical"
printf 'labels %s\n' "${labels[*]:-(none)}"
printf 'title %s\n' "$title"
if [ "$dry_run" -eq 1 ]; then
printf '\ndry run: everything above resolved; nothing was created.\n'
exit 0
fi
# ---------------------------------------------------------------------------
# Create. Past this line a failure can leave an issue off the board, so every
# error path prints the number.
# ---------------------------------------------------------------------------
create_args=(--repo "$repo" --title "$title")
if [ -n "$body_file" ]; then
create_args+=(--body-file "$body_file")
else
create_args+=(--body "$body")
fi
for label in ${labels[@]+"${labels[@]}"}; do
create_args+=(--label "$label")
done
issue_url=$(gh issue create "${create_args[@]}") || die "gh issue create failed; nothing was filed"
issue_number="${issue_url##*/}"
case "$issue_number" in
''|*[!0-9]*) die "could not read an issue number out of: $issue_url" ;;
esac
orphaned() {
printf 'file-issue: %s\n' "$1" >&2
printf 'file-issue: THE ISSUE EXISTS BUT IS NOT ON THE BOARD. Fix it by hand:\n' >&2
printf '%s\n' "$issue_url" >&2
exit "$EXIT_ORPHANED"
}
content_id=$(gh api "/repos/$repo/issues/$issue_number" --jq '.node_id') \
|| orphaned "could not read the node id for #$issue_number"
# shellcheck disable=SC2016 # GraphQL variables, as above
item_id=$(gh api graphql \
-f query='
mutation($project: ID!, $content: ID!) {
addProjectV2ItemById(input: {projectId: $project, contentId: $content}) {
item { id }
}
}' \
-f project="$project_id" -f content="$content_id" \
--jq '.data.addProjectV2ItemById.item.id') \
|| orphaned "could not add #$issue_number to the board"
[ -n "$item_id" ] || orphaned "the board add returned no item id for #$issue_number"
# shellcheck disable=SC2016 # GraphQL variables, as above
gh api graphql \
-f query='
mutation($project: ID!, $item: ID!, $field: ID!, $option: String!) {
updateProjectV2ItemFieldValue(input: {
projectId: $project, itemId: $item, fieldId: $field,
value: {singleSelectOptionId: $option}
}) { projectV2Item { id } }
}' \
-f project="$project_id" -f item="$item_id" -f field="$field_id" -f option="$option_id" \
>/dev/null \
|| orphaned "#$issue_number is on the board but its Status could not be set"
# ---------------------------------------------------------------------------
# Read back. A mutation returning 200 is not evidence the board shows what was
# asked for -- this is the only check that is.
# ---------------------------------------------------------------------------
# shellcheck disable=SC2016 # GraphQL variables, as above
readback=$(gh api graphql \
-f query='
query($item: ID!) {
node(id: $item) {
... on ProjectV2Item {
content { ... on Issue { number } }
fieldValueByName(name: "Status") {
... on ProjectV2ItemFieldSingleSelectValue { name }
}
}
}
}' \
-f item="$item_id") \
|| orphaned "#$issue_number was written but could not be read back"
seen_number=$(printf '%s' "$readback" | jq -r '.data.node.content.number // empty')
seen_status=$(printf '%s' "$readback" | jq -r '.data.node.fieldValueByName.name // empty')
if [ "$seen_number" != "$issue_number" ]; then
orphaned "read-back names issue #${seen_number:-<none>}, expected #$issue_number"
fi
if [ "$seen_status" != "$status_canonical" ]; then
orphaned "read-back Status is ${seen_status:-<unset>}, expected $status_canonical"
fi
printf '\n#%s on %s as %s -- verified by read-back\n' \
"$issue_number" "$project_title" "$seen_status"
printf '%s\n' "$issue_url"
+4
View File
@@ -456,6 +456,10 @@ cleanup() {
# bash runs a trap only between commands, so this starts when whatever was in the foreground
# returns -- which for Ctrl-C is immediately, because the same interrupt reached that command
# too. `kill -INT` aimed at this script alone waits for the foreground command to finish.
# Invoked indirectly -- installed as the INT and TERM trap a few lines below. Both codes,
# because shellcheck 0.9.0 reports this as unreachable commands (SC2317) and 0.11.0 as an
# uninvoked function (SC2329); CI pins 0.11.0 but a local install may be either.
# shellcheck disable=SC2317,SC2329
on_signal() {
echo
echo "interrupted (SIG$1) -- stopping the emulator and removing the AVDs this run created"