Compare commits
47
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
240528facb | ||
|
|
f98e49942f | ||
|
|
25f162923c | ||
|
|
d0b9745220 | ||
|
|
b36d56c932 | ||
|
|
a0b6a3dde8 | ||
|
|
ba27b8306b | ||
|
|
bda5abea6c | ||
|
|
8bd5fedcc8 | ||
|
|
21eeb6f3f8 | ||
|
|
a83cb60c61 | ||
|
|
2063fe06aa | ||
|
|
47a423413b | ||
|
|
dab28d5f44 | ||
|
|
aed4d83e70 | ||
|
|
4aba3bbd2e | ||
|
|
dbba213c51 | ||
|
|
8ac6e2b1c2 | ||
|
|
4ff44be1d7 | ||
|
|
7f951baf8f | ||
|
|
5ec2bba64b | ||
|
|
fd2bb1d889 | ||
|
|
ad28293b72 | ||
|
|
b9abe85580 | ||
|
|
4375a377bc | ||
|
|
3925f1aa9f | ||
|
|
a3c835b7c9 | ||
|
|
650ca8fca3 | ||
|
|
b18f45def7 | ||
|
|
d674bc4848 | ||
|
|
02555ceb91 | ||
|
|
1b1d5c6d04 | ||
|
|
2f3f461cc1 | ||
|
|
6166763f24 | ||
|
|
46ad95350b | ||
|
|
e968deb5a2 | ||
|
|
3d55004286 | ||
|
|
e06b0826a0 | ||
|
|
7b578c1ccf | ||
|
|
9e7f80feaa | ||
|
|
3c5a37fd3c | ||
|
|
af13155c27 | ||
|
|
4ea5afefe1 | ||
|
|
7e4f22322b | ||
|
|
3d51fefeff | ||
|
|
fea88a281f | ||
|
|
7c69d0699a |
@@ -240,17 +240,33 @@ jobs:
|
||||
# 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
|
||||
# nothing THIS LEG RUNS touches system UI -- 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.
|
||||
#
|
||||
# "this leg" and not "this suite", since 2026-08-24, and the difference is
|
||||
# now load-bearing: SafPickerRoundTripTest DOES touch system UI. It drives
|
||||
# DocumentsUI and rotates the display, and both reach the gralloc mapper
|
||||
# this image aborts in -- disabling SystemUI removes the IDLE trigger, not
|
||||
# those. Measured per method on android-37.0: the ROTATION test takes the
|
||||
# framework down (INSTRUMENTATION_ABORTED) and carries
|
||||
# @FailsOnEmulatorApi37, so notAnnotation below keeps it off this row; the
|
||||
# PICKER test passes and runs here like anything else. A rotation rebuilds
|
||||
# every surface at once, and starting another app's activity does not.
|
||||
#
|
||||
# So this row does now run one test that depends on system UI, and the
|
||||
# caveat above still applies to it: a green here is not evidence the picker
|
||||
# works on a device with SystemUI running -- the Pixel release check is.
|
||||
# docs/api-37-emulator-crash.md has the per-method measurements, and the
|
||||
# correction that produced them.
|
||||
#
|
||||
# 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
|
||||
# notAnnotation removes the three 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"
|
||||
@@ -351,7 +367,7 @@ jobs:
|
||||
if-no-files-found: ignore
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The two API 37 tests the gating row above excludes, run on their own so they
|
||||
# The three 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
|
||||
@@ -359,17 +375,29 @@ jobs:
|
||||
# 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.
|
||||
# It was named for WHAT IT RUNS, and that is now APPROXIMATE rather than exact.
|
||||
# When this job was created it held two tests, both driving a full H.264 -> H.265
|
||||
# hardware transcode through Media3Engine -- which is exactly what separated them
|
||||
# from the two Media3EngineTest cases that pass here, since those two never decode
|
||||
# video. Since 2026-08-25 it also holds SafPickerRoundTripTest's rotation case,
|
||||
# which drives no transcode at all: a real rotation rebuilds every surface at once
|
||||
# and takes the framework down on this image (INSTRUMENTATION_ABORTED), which is a
|
||||
# different failure from the decoder one below.
|
||||
#
|
||||
# The name is kept anyway, and that is a decision rather than an oversight. This is
|
||||
# not a required context, it is red on every PR by design, and it is one people
|
||||
# have learned to look for -- renaming a check costs more than the imprecision
|
||||
# does. **The marker is the definition, not the name:** what this job holds is the
|
||||
# tests that cannot pass on the API 37 emulator image, whatever their subject. The
|
||||
# theory about the Media3 pair is in the next paragraph, where it can be corrected
|
||||
# without touching the name.
|
||||
#
|
||||
# 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
|
||||
# surfaceflinger on this image. It is about the MEDIA3 PAIR only; the rotation
|
||||
# case above fails for its own reason. What is MEASURED is narrower: those 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
|
||||
|
||||
@@ -64,16 +64,34 @@ 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. **Three** of the 59 instrumented
|
||||
tests cannot pass on that image, for two unrelated reasons: two Media3 hardware transcodes fail
|
||||
inside the emulator's own `c2.goldfish.h264.decoder`, and one SAF test takes the framework down
|
||||
when it rotates the display. All three carry `@FailsOnEmulatorApi37` and run in a separate
|
||||
`continue-on-error` job; the gating leg runs the other 56.
|
||||
|
||||
That job is still called `E2E API 37 Media3 hardware transcode (advisory)`, which no longer
|
||||
describes everything in it. The name is kept deliberately — it is not a required context and
|
||||
people have learned to look for it — so **read the marker, not the name**, for what it holds.
|
||||
**It 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 three 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.** Those three tests are the one thing CI cannot answer
|
||||
for.
|
||||
|
||||
On a device or emulator, build only the ABI it can execute:
|
||||
|
||||
@@ -96,12 +114,26 @@ 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** — **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.
|
||||
- **Coverage is reported, not gated** — **69.2% of lines (1519/2194), 53.2% of branches**,
|
||||
measured 2026-08-24 with `./gradlew :app:jacocoTestReport`.
|
||||
|
||||
**Every figure this file carried before that date was an artifact, roughly half the real one.**
|
||||
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
|
||||
skips no-location classes by default, and nothing told it otherwise — so **not one Robolectric
|
||||
test counted**, and Robolectric is what exercises the framework edge here. The
|
||||
`isIncludeNoLocationClasses` block in `app/build.gradle.kts` is what fixes it; **do not delete
|
||||
it as stray config**, and re-run the numbers if you ever touch it. Same commit, same 335 tests:
|
||||
29.7% -> 69.2% with that block alone.
|
||||
|
||||
The old entry also explained the wrong thing. It said coverage **fell** as the suite grew from 11
|
||||
test files to 43 because "the denominator outran the numerator" on framework-edge code "the JVM
|
||||
cannot reach". The JVM reaches that code fine. What actually happened is that the new tests were
|
||||
disproportionately Robolectric, so each one added denominator and no numerator — the measurement
|
||||
was punishing exactly the tests that were hardest to write.
|
||||
|
||||
Two things still hold. A floor needs a baseline that has settled, and this one has now moved by
|
||||
39 points in a single build change, so it has not. And **re-measure before quoting** — that
|
||||
instruction is the only reason this was caught.
|
||||
- **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.
|
||||
@@ -112,9 +144,9 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
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 now.** `tools/local-emulator/run-e2e.sh` runs API 33-36 on this
|
||||
machine; see `docs/local-emulator.md`. That was believed impossible until the SELinux/renderer
|
||||
cause was found, and it is what makes the e2e half of this norm enforceable.
|
||||
- **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.
|
||||
|
||||
@@ -113,7 +113,14 @@ container × codec matrix — including combinations that cannot work, which it
|
||||
offers alternatives for rather than hiding.
|
||||
|
||||
Conversions run as durable background work, so they survive leaving the app and are
|
||||
restored after a restart.
|
||||
restored when you reopen it.
|
||||
|
||||
One case is not restored, and it is worth knowing about. If Android refuses to let a job
|
||||
restart in the background, it is retried on an exponential backoff for about eight and a
|
||||
half hours and then given up on — and a job that has been given up on does not come back
|
||||
when you reopen the app. Reopening the app is what grants permission to run, so a
|
||||
conversion that has stalled this way is best started again from the app rather than waited
|
||||
on.
|
||||
|
||||
## Building
|
||||
|
||||
|
||||
+38
-5
@@ -188,6 +188,30 @@ detekt {
|
||||
}
|
||||
|
||||
// Pin the coverage agent rather than inheriting whatever Gradle bundles.
|
||||
// Robolectric loads every class it touches through its own sandbox classloader, and those
|
||||
// classes arrive with no source location. JaCoCo skips no-location classes by default, so
|
||||
// without this block **not one Robolectric test counts** -- and Robolectric is what exercises
|
||||
// the framework edge here: the workers, the publisher, both ViewModels, every Compose screen.
|
||||
//
|
||||
// Measured on e06b082, same 335 tests, same 0 failures, only this block added:
|
||||
//
|
||||
// LINE 29.7% -> 69.2% OutputPublisher 0.0% -> 97.5%
|
||||
// BRANCH 29.8% -> 53.2% ConversionViewModel 0.0% -> 85.4%
|
||||
//
|
||||
// The discriminator, if this ever looks like superstition: inside ConverterScreenKt, `describe`
|
||||
// is the one non-Composable and is exercised by a plain JVM test -- it reported 8/8 covered while
|
||||
// every @Composable in the same class reported 0, including ones whose mutations demonstrably
|
||||
// failed the build when reverted.
|
||||
//
|
||||
// `excludes` is not optional. Without it JaCoCo walks JDK-internal classes that Robolectric has
|
||||
// no location for either, and the test JVM dies rather than reporting a number.
|
||||
tasks.withType<Test>().configureEach {
|
||||
extensions.configure<JacocoTaskExtension> {
|
||||
isIncludeNoLocationClasses = true
|
||||
excludes = listOf("jdk.internal.*")
|
||||
}
|
||||
}
|
||||
|
||||
jacoco {
|
||||
toolVersion = libs.versions.jacoco.get()
|
||||
}
|
||||
@@ -211,8 +235,11 @@ val jacocoGeneratedExcludes = listOf(
|
||||
)
|
||||
|
||||
// AGP 9 compiles Kotlin through its built-in compiler, which writes here rather than to the
|
||||
// classic `tmp/kotlin-classes/debug`. All hand-written code in this module is Kotlin, so the
|
||||
// javac output (BuildConfig and R only) is not read at all.
|
||||
// classic `tmp/kotlin-classes/debug`. All hand-written code in the MAIN source set is Kotlin, so
|
||||
// the javac output (BuildConfig and R only) is not read at all. There is now one hand-written
|
||||
// Java file in the module -- androidTest's FixtureDocumentsProvider, which cannot be Kotlin
|
||||
// because the process it runs in has no Kotlin stdlib; its own header explains why. It is in
|
||||
// androidTest, so it is not in this task's classDirectories and this stays accurate.
|
||||
val jacocoDebugKotlinClasses = layout.buildDirectory.dir(
|
||||
"intermediates/built_in_kotlinc/debug/compileDebugKotlin/classes",
|
||||
)
|
||||
@@ -306,9 +333,9 @@ 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.
|
||||
// For `runTest` alone, in ConversionViewModelProbeFailureTest. 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))
|
||||
@@ -316,5 +343,11 @@ dependencies {
|
||||
androidTestImplementation(libs.androidx.espresso.core)
|
||||
androidTestImplementation(libs.compose.ui.test.junit4)
|
||||
androidTestImplementation(libs.androidx.work.testing)
|
||||
// androidTest only, and it has to be: UiAutomator drives the whole device, including
|
||||
// windows belonging to other packages. The system file picker is one -- DocumentsUI runs
|
||||
// in its own process, so Compose's matchers cannot see it and Espresso's cannot either
|
||||
// (both are scoped to this process's view hierarchy). Nothing on the JVM has a device to
|
||||
// drive, so there is no unit-test counterpart to add it to.
|
||||
androidTestImplementation(libs.androidx.uiautomator)
|
||||
debugImplementation(libs.compose.ui.test.manifest)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
<?xml version="1.0" encoding="utf-8"?>
|
||||
<!--
|
||||
The first manifest this source set has ever had, and it exists for one component.
|
||||
|
||||
SafPickerRoundTripTest drives the real system file picker. DocumentsUI only shows what a
|
||||
DocumentsProvider offers it, so a test that picks a file needs a provider to pick from, and
|
||||
that provider has to be declared: a ContentProvider is instantiated by the system from a
|
||||
manifest entry and cannot be registered from test code.
|
||||
|
||||
It is declared HERE rather than in src/debug on purpose. src/debug would put a fake storage
|
||||
root inside the shipped debug APK, where it would show up in every developer's own file
|
||||
picker and in every other app's; this way it is installed only by the instrumentation APK,
|
||||
alongside the test that needs it, and is gone the moment that APK is uninstalled.
|
||||
|
||||
The four attributes are not decoration. Each one is required for the picker to see it:
|
||||
|
||||
exported DocumentsUI is another app; an unexported provider is invisible to it.
|
||||
permission MANAGE_DOCUMENTS is held by DocumentsUI and essentially nothing else,
|
||||
so this is what stops any installed app from reading the fixture. The
|
||||
provider is exported to the *picker*, not to the world.
|
||||
grantUriPermissions How the app under test ends up able to read the URI it was handed. The
|
||||
picker returns the document URI with FLAG_GRANT_READ_URI_PERMISSION,
|
||||
and that flag does nothing unless the provider allows grants. Without
|
||||
it the pick "succeeds" and every read of the result fails.
|
||||
DOCUMENTS_PROVIDER The action DocumentsUI queries the package manager for. No filter, no
|
||||
root in the drawer.
|
||||
|
||||
The authority carries the .test suffix because this component belongs to the instrumentation
|
||||
package (org.libremediaconverter.test), not to the app. Authorities are global to the device:
|
||||
reusing the app's would collide with the app on any device where both are installed.
|
||||
-->
|
||||
<manifest xmlns:android="http://schemas.android.com/apk/res/android">
|
||||
|
||||
<application>
|
||||
<provider
|
||||
android:name="org.libremediaconverter.saf.FixtureDocumentsProvider"
|
||||
android:authorities="org.libremediaconverter.test.fixtures"
|
||||
android:exported="true"
|
||||
android:grantUriPermissions="true"
|
||||
android:permission="android.permission.MANAGE_DOCUMENTS">
|
||||
<intent-filter>
|
||||
<action android:name="android.content.action.DOCUMENTS_PROVIDER" />
|
||||
</intent-filter>
|
||||
</provider>
|
||||
</application>
|
||||
|
||||
</manifest>
|
||||
@@ -0,0 +1,234 @@
|
||||
package org.libremediaconverter.saf;
|
||||
|
||||
import android.database.Cursor;
|
||||
import android.database.MatrixCursor;
|
||||
import android.os.CancellationSignal;
|
||||
import android.os.ParcelFileDescriptor;
|
||||
import android.provider.DocumentsContract.Document;
|
||||
import android.provider.DocumentsContract.Root;
|
||||
import android.provider.DocumentsProvider;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileNotFoundException;
|
||||
import java.io.FileOutputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.io.OutputStream;
|
||||
|
||||
/**
|
||||
* One file, offered to the system file picker, so that picking one can be tested at all.
|
||||
*
|
||||
* <p>DocumentsUI does not browse a filesystem: it lists what {@link DocumentsProvider}s hand it.
|
||||
* So a test that drives the real picker has to supply the thing being picked, and it has to
|
||||
* supply it as a manifest-declared component, because a {@code ContentProvider} is instantiated
|
||||
* by the system and cannot be registered from test code. {@code
|
||||
* app/src/androidTest/AndroidManifest.xml} is that declaration and says why each of its
|
||||
* attributes is load-bearing.
|
||||
*
|
||||
* <h2>The only Java file in this module, and it has to be</h2>
|
||||
*
|
||||
* <p>Everything else here is Kotlin. This cannot be: <b>the Kotlin standard library is not on
|
||||
* this class's classpath at runtime.</b>
|
||||
*
|
||||
* <p>Instrumentation code normally never notices. The test APK's dex is loaded into the app's
|
||||
* process, where the app APK supplies {@code kotlin.jvm.internal.Intrinsics} — so the test APK is
|
||||
* built without it, deliberately, since packaging a second copy is what {@code
|
||||
* checkDebugAndroidTestDuplicateClasses} exists to prevent. A provider is different. It is a
|
||||
* component of the instrumentation <i>package</i>, so when DocumentsUI queries it the system
|
||||
* starts a plain {@code org.libremediaconverter.test} process with only the test APK on its dex
|
||||
* path, and no app APK anywhere. The Kotlin version of this file crashed there on its first
|
||||
* query, before returning a single row:
|
||||
*
|
||||
* <pre>
|
||||
* FATAL EXCEPTION: binder:6369_2
|
||||
* Process: org.libremediaconverter.test
|
||||
* java.lang.NoClassDefFoundError: Failed resolution of: Lkotlin/jvm/internal/Intrinsics;
|
||||
* at org.libremediaconverter.saf.FixtureDocumentsProvider.queryDocument
|
||||
* </pre>
|
||||
*
|
||||
* <p>The compiler emits that reference for the null checks on almost every function, so there is
|
||||
* no Kotlin dialect that avoids it. For the same reason nothing here imports {@code androidx.*}:
|
||||
* those classes are absent from this process for exactly the same reason. Framework and JDK only.
|
||||
*
|
||||
* <h2>Why a provider rather than a file in Downloads</h2>
|
||||
*
|
||||
* <p>That would have worked, and it would have tested less. Two properties are what {@code
|
||||
* SafPickerRoundTripTest} actually needs:
|
||||
*
|
||||
* <ul>
|
||||
* <li><b>The root declares {@link Root#COLUMN_MIME_TYPES}, and DocumentsUI filters by it.</b>
|
||||
* That is what gives the screen's MIME filter a mutation with a shape: ask for a type this
|
||||
* root does not offer and the root itself is not in the picker, so the failure reads as
|
||||
* "the fixture root is not there" rather than "one file among the hundreds in Downloads was
|
||||
* not listed".
|
||||
* <li><b>The contents are exactly this and nothing else.</b> A shared directory accumulates
|
||||
* whatever earlier runs and other tests left in it, and a picker test that finds the wrong
|
||||
* file passes.
|
||||
* </ul>
|
||||
*
|
||||
* <p>The descriptor is opened on a real file rather than served through a pipe, deliberately.
|
||||
* {@code InputQuery.sizeOf} falls back to {@code ParcelFileDescriptor.statSize} when a provider
|
||||
* omits {@code OpenableColumns.SIZE}, and a pipe's {@code statSize} is {@code -1} — an unknown
|
||||
* size, which is a different case with a screen of its own. This fixture is meant to be an
|
||||
* ordinary, fully described file, so that the one thing under test is the round trip.
|
||||
*/
|
||||
public final class FixtureDocumentsProvider extends DocumentsProvider {
|
||||
|
||||
/**
|
||||
* What the picker calls this root.
|
||||
*
|
||||
* <p>Deliberately not a word any other root uses. The picker's own landing screen already
|
||||
* offers "Images", "Audio", "Videos" and "Documents", and a UiAutomator selector that could
|
||||
* match two things is not a selector.
|
||||
*/
|
||||
public static final String ROOT_TITLE = "LMC R38 fixtures";
|
||||
|
||||
/**
|
||||
* What the file card has to end up showing.
|
||||
*
|
||||
* <p>The same string reaches the assertion two ways — as the picker row UiAutomator taps, and
|
||||
* as {@code OpenableColumns.DISPLAY_NAME} on the URI the app is handed — which is exactly the
|
||||
* round trip under test.
|
||||
*/
|
||||
public static final String FIXTURE_DISPLAY_NAME = "lmc-r38-fixture.mp4";
|
||||
|
||||
/**
|
||||
* The type the root advertises, and the one the MIME mutation has to stop matching.
|
||||
*
|
||||
* <p>A real type rather than something invented, so the wildcard filter the screen passes
|
||||
* today is not the only filter under which this test could pass.
|
||||
*/
|
||||
public static final String FIXTURE_MIME_TYPE = "video/mp4";
|
||||
|
||||
private static final String ROOT_ID = "lmc-r38-root";
|
||||
private static final String ROOT_DOCUMENT_ID = "root";
|
||||
private static final String FIXTURE_DOCUMENT_ID = "root/" + FIXTURE_DISPLAY_NAME;
|
||||
|
||||
/** Already in this source set, and already a real H.264 MP4 the engines can open. */
|
||||
private static final String FIXTURE_ASSET = "sample_h264.mp4";
|
||||
|
||||
private static final String[] DEFAULT_ROOT_PROJECTION = {
|
||||
Root.COLUMN_ROOT_ID,
|
||||
Root.COLUMN_DOCUMENT_ID,
|
||||
Root.COLUMN_TITLE,
|
||||
Root.COLUMN_SUMMARY,
|
||||
Root.COLUMN_MIME_TYPES,
|
||||
Root.COLUMN_FLAGS,
|
||||
Root.COLUMN_ICON,
|
||||
};
|
||||
|
||||
private static final String[] DEFAULT_DOCUMENT_PROJECTION = {
|
||||
Document.COLUMN_DOCUMENT_ID,
|
||||
Document.COLUMN_DISPLAY_NAME,
|
||||
Document.COLUMN_MIME_TYPE,
|
||||
Document.COLUMN_FLAGS,
|
||||
Document.COLUMN_SIZE,
|
||||
Document.COLUMN_LAST_MODIFIED,
|
||||
};
|
||||
|
||||
@Override
|
||||
public boolean onCreate() {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* The single root.
|
||||
*
|
||||
* <p>{@link Root#COLUMN_MIME_TYPES} is the important column. Left null it would mean "this
|
||||
* root supports everything", the picker would list it whatever was asked for, and the MIME
|
||||
* mutation would have nothing to bite on.
|
||||
*/
|
||||
@Override
|
||||
public Cursor queryRoots(String[] projection) {
|
||||
MatrixCursor cursor = new MatrixCursor(projection != null ? projection : DEFAULT_ROOT_PROJECTION);
|
||||
cursor.newRow()
|
||||
.add(Root.COLUMN_ROOT_ID, ROOT_ID)
|
||||
.add(Root.COLUMN_DOCUMENT_ID, ROOT_DOCUMENT_ID)
|
||||
.add(Root.COLUMN_TITLE, ROOT_TITLE)
|
||||
.add(Root.COLUMN_SUMMARY, "Instrumentation fixture")
|
||||
.add(Root.COLUMN_MIME_TYPES, FIXTURE_MIME_TYPE)
|
||||
.add(Root.COLUMN_FLAGS, Root.FLAG_LOCAL_ONLY)
|
||||
.add(Root.COLUMN_ICON, android.R.drawable.ic_menu_gallery);
|
||||
return cursor;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Cursor queryDocument(String documentId, String[] projection) throws FileNotFoundException {
|
||||
MatrixCursor cursor = new MatrixCursor(projection != null ? projection : DEFAULT_DOCUMENT_PROJECTION);
|
||||
if (ROOT_DOCUMENT_ID.equals(documentId)) {
|
||||
addDirectoryRow(cursor);
|
||||
} else if (FIXTURE_DOCUMENT_ID.equals(documentId)) {
|
||||
addFixtureRow(cursor);
|
||||
} else {
|
||||
throw new FileNotFoundException("no such document: " + documentId);
|
||||
}
|
||||
return cursor;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Cursor queryChildDocuments(String parentDocumentId, String[] projection, String sortOrder)
|
||||
throws FileNotFoundException {
|
||||
MatrixCursor cursor = new MatrixCursor(projection != null ? projection : DEFAULT_DOCUMENT_PROJECTION);
|
||||
if (ROOT_DOCUMENT_ID.equals(parentDocumentId)) {
|
||||
addFixtureRow(cursor);
|
||||
}
|
||||
return cursor;
|
||||
}
|
||||
|
||||
@Override
|
||||
public ParcelFileDescriptor openDocument(String documentId, String mode, CancellationSignal signal)
|
||||
throws FileNotFoundException {
|
||||
if (!FIXTURE_DOCUMENT_ID.equals(documentId)) {
|
||||
throw new FileNotFoundException("no such document: " + documentId);
|
||||
}
|
||||
return ParcelFileDescriptor.open(fixtureFile(), ParcelFileDescriptor.MODE_READ_ONLY);
|
||||
}
|
||||
|
||||
private void addDirectoryRow(MatrixCursor cursor) {
|
||||
cursor.newRow()
|
||||
.add(Document.COLUMN_DOCUMENT_ID, ROOT_DOCUMENT_ID)
|
||||
.add(Document.COLUMN_DISPLAY_NAME, ROOT_TITLE)
|
||||
.add(Document.COLUMN_MIME_TYPE, Document.MIME_TYPE_DIR)
|
||||
.add(Document.COLUMN_FLAGS, 0)
|
||||
.add(Document.COLUMN_SIZE, null);
|
||||
}
|
||||
|
||||
private void addFixtureRow(MatrixCursor cursor) throws FileNotFoundException {
|
||||
File file = fixtureFile();
|
||||
cursor.newRow()
|
||||
.add(Document.COLUMN_DOCUMENT_ID, FIXTURE_DOCUMENT_ID)
|
||||
.add(Document.COLUMN_DISPLAY_NAME, FIXTURE_DISPLAY_NAME)
|
||||
.add(Document.COLUMN_MIME_TYPE, FIXTURE_MIME_TYPE)
|
||||
.add(Document.COLUMN_FLAGS, 0)
|
||||
.add(Document.COLUMN_SIZE, file.length())
|
||||
.add(Document.COLUMN_LAST_MODIFIED, file.lastModified());
|
||||
}
|
||||
|
||||
/**
|
||||
* The fixture on disk, unpacked from this APK's own assets the first time anything asks.
|
||||
*
|
||||
* <p>On demand rather than seeded once in {@link #onCreate()}, because this process is started
|
||||
* by whoever queries the provider and can be killed between two queries of the same test.
|
||||
*
|
||||
* <p>A failure here is reported as {@link FileNotFoundException} rather than swallowed. A
|
||||
* provider that answers with a zero-byte file would put the test on the "Size unknown" screen
|
||||
* with nothing saying why.
|
||||
*/
|
||||
private File fixtureFile() throws FileNotFoundException {
|
||||
File file = new File(getContext().getFilesDir(), FIXTURE_DISPLAY_NAME);
|
||||
if (file.length() > 0L) {
|
||||
return file;
|
||||
}
|
||||
try (InputStream source = getContext().getAssets().open(FIXTURE_ASSET);
|
||||
OutputStream sink = new FileOutputStream(file)) {
|
||||
byte[] buffer = new byte[8192];
|
||||
int read;
|
||||
while ((read = source.read(buffer)) != -1) {
|
||||
sink.write(buffer, 0, read);
|
||||
}
|
||||
} catch (IOException e) {
|
||||
throw new FileNotFoundException("could not unpack " + FIXTURE_ASSET + ": " + e);
|
||||
}
|
||||
return file;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,786 @@
|
||||
package org.libremediaconverter.saf
|
||||
|
||||
import android.app.UiAutomation
|
||||
import androidx.compose.ui.test.ComposeTimeoutException
|
||||
import androidx.compose.ui.test.assertTextEquals
|
||||
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
||||
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 androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import androidx.test.platform.app.InstrumentationRegistry
|
||||
import androidx.test.uiautomator.By
|
||||
import androidx.test.uiautomator.BySelector
|
||||
import androidx.test.uiautomator.Configurator
|
||||
import androidx.test.uiautomator.StaleObjectException
|
||||
import androidx.test.uiautomator.UiDevice
|
||||
import androidx.test.uiautomator.Until
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertNotEquals
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.FailsOnEmulatorApi37
|
||||
import org.libremediaconverter.MainActivity
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
|
||||
/**
|
||||
* Choosing a file, through the real system picker, and still having it after a rotation.
|
||||
*
|
||||
* Two defects, and neither is reachable from anywhere else in this repo.
|
||||
*
|
||||
* **The picker is opened with a filter, and a filter can hide the user's file.** `ConverterScreen`
|
||||
* launches `ActivityResultContracts.OpenDocument` with a MIME array; DocumentsUI hides every root
|
||||
* and every document that array does not match. Narrow it and the app still compiles, still
|
||||
* renders, still passes every JVM test — and the user taps "Choose file" and is shown an empty
|
||||
* picker. Nothing in either source set drove SAF **as a picker** before this: the only SAF coverage
|
||||
* is the publish side, in `OutputPublisherPublishTest`, against hand-written `ContentProvider`
|
||||
* fakes. The launcher wiring, the filter, and the read grant that comes back had never been
|
||||
* executed by a test.
|
||||
*
|
||||
* **The picked file has to survive a rotation.** `MainActivity` declares no `configChanges`, so
|
||||
* every rotation destroys and recreates it, and `ConversionViewModel` holds the picked file in a
|
||||
* plain `MutableStateFlow` with no `SavedStateHandle` behind it. The only thing that carries it
|
||||
* across is the retained `ViewModelStore` the Activity gets from resolving the ViewModel through
|
||||
* `LocalViewModelStoreOwner`. Scope it to the composition instead and the file is gone.
|
||||
*
|
||||
* ### Why these two are one test class
|
||||
*
|
||||
* A rotation test alone has no bite of its own. `AppRootRestorationTest` already catches
|
||||
* `rememberSaveable` -> `remember` on the JVM, and a second test whose only mutation is one an
|
||||
* existing test catches is the vacuous test this whole decomposition exists to prevent. So the
|
||||
* rotation here runs **from a real picked input**, which is a state no JVM test can produce:
|
||||
* `AppRootRestorationTest` injects a stub `content` lambda specifically to avoid standing up
|
||||
* either ViewModel, and `StateRestorationTester` saves into an in-memory map rather than a
|
||||
* `Bundle`.
|
||||
*
|
||||
* ### #93: what actually failed was reading the screen, not the picker
|
||||
*
|
||||
* Ninety minutes after this class landed it started failing on gating legs at API 33, 34, 35 and
|
||||
* 37 — on diffs that were two KDoc comments, a MIME lookup table and a README paragraph (#93).
|
||||
* Every failure named the fixture root, so it read as a root-discovery race, and the ticket was
|
||||
* filed on that reading. It was not one, and it was not the `StaleObjectException` #80 had fixed
|
||||
* an hour earlier either.
|
||||
*
|
||||
* **DocumentsUI was fine.** On the API 34 leg of run 32806342548 its own
|
||||
* `ProvidersAccess: Matched roots` names
|
||||
* `content://org.libremediaconverter.test.fixtures/root/lmc-r38-root` five times inside the sixty
|
||||
* seconds the test spent failing, `ActivityTaskManager` logged the `PickActivity` as `Displayed`,
|
||||
* and the provider process started on cue.
|
||||
*
|
||||
* **This process could not read any window at all.** Two counts settle it. Across that whole leg
|
||||
* UiAutomator logged `Retrieving node with selector` 1095 times and `Node not found with selector`
|
||||
* 1095 times — not one selector ever matched, from the first query of the run. The green leg of
|
||||
* the same job asked 7 times and found 5. `UiDevice.getWindowRoots` builds its search set from
|
||||
* `UiAutomation.getWindows()` and, on API 21 and up, from nothing else; an empty list there makes
|
||||
* every selector unfindable and says nothing whatever about SAF. The corroborating detail is that
|
||||
* `By.desc("Show roots")` — the toolbar button, present on that screen whether the roots list is
|
||||
* stale or not — was also not found, 28 s after the picker was displayed.
|
||||
*
|
||||
* **A fresh picker is not the repair, and this was measured rather than assumed.** The same leg
|
||||
* opened a *second* `PickActivity` for the second test, in the same DocumentsUI process
|
||||
* (pid 3299), and read exactly as little from it. So whatever was broken outlived one window.
|
||||
* [requireAReadableScreen] is the part aimed at that: it asks whether this process can see the
|
||||
* app's own window *before* the picker is opened, and [rebuildUiAutomation] tears the connection
|
||||
* down and builds another if it cannot.
|
||||
*
|
||||
* **The check has since caught the real thing, in CI, and the connection rebuild did not repair
|
||||
* it.** Run 32811493607, API 35 and API 37 legs, both tests, 12 s each instead of 60:
|
||||
*
|
||||
* ```
|
||||
* java.lang.AssertionError: UiAutomator cannot see this app's own window, so it could not have
|
||||
* seen the picker's either. This is not a SAF failure.
|
||||
* at SafPickerRoundTripTest.requireAReadableScreen
|
||||
* ```
|
||||
*
|
||||
* That is the diagnosis this class could not previously give, and it moves the question off SAF
|
||||
* for good.
|
||||
*
|
||||
* ### What the window list said, and why nothing here can fix it
|
||||
*
|
||||
* [describeWindows] was added to that failure so the next occurrence would close the question
|
||||
* rather than reopen it. It did — on the API 34 leg of run 32812248131 and again, character for
|
||||
* character, on the API 33 leg of run 32812892103:
|
||||
*
|
||||
* ```
|
||||
* ... Waking the device, dismissing the keyguard and rebuilding the UiAutomation connection all
|
||||
* failed to make it readable. What it could see: com.android.systemui[type=3], android[type=3]
|
||||
* ```
|
||||
*
|
||||
* `type=3` is `AccessibilityWindowInfo.TYPE_SYSTEM`. The list is **not** empty — it holds the
|
||||
* system windows and **not one `TYPE_APPLICATION` window**, on a device where the framework had
|
||||
* already logged `Displayed org.libremediaconverter/.MainActivity`. So the application layer
|
||||
* never reaches accessibility on those boots, and every selector in this class, the picker's and
|
||||
* the app's alike, is unfindable for the whole instrumentation run.
|
||||
*
|
||||
* Three CI runs on this branch caught the fault, at API 33, 34, 35 and 37, and every one of them
|
||||
* printed that same list. It is not one level's quirk.
|
||||
*
|
||||
* ### And that list is what identified the occluder
|
||||
*
|
||||
* `android[type=3]` is `system_server`, and what it was holding is in the same logcat, minutes
|
||||
* before this class ever ran:
|
||||
*
|
||||
* ```
|
||||
* ANR in com.google.android.apps.nexuslauncher (com.google.android.apps.nexuslauncher/.NexusLauncherActivity)
|
||||
* Reason: Input dispatching timed out (Application does not have a focused window)
|
||||
* Window{4ed8414 u0 Application Not Responding: com.google.android.apps.nexuslauncher}
|
||||
* ```
|
||||
*
|
||||
* **The launcher ANRs on a loaded runner emulator, and the dialog it leaves behind never goes
|
||||
* away.** It is opaque and fullscreen, so `AccessibilityWindowManager` drops every application
|
||||
* window beneath it — which is how the app can be `Displayed` and unreadable at once, the
|
||||
* contradiction that made #93 look like a SAF bug for six PRs. It is present on both legs
|
||||
* examined, at API 33 and 34, at the failure timestamp.
|
||||
*
|
||||
* So [dismissASystemErrorDialog] is tried first, and it is the remedy with a mechanism behind it.
|
||||
* The other two are kept behind it and are **measured as not the cause**: [unlockTheDevice] (the
|
||||
* keyguard theory, from `KeyguardViewMediator` reporting an unprovisioned device — dismissing it
|
||||
* changed nothing) and [rebuildUiAutomation]. A second `PickActivity` is not a remedy for this
|
||||
* either, and that was measured too: the first failing leg opened one and read as little from it.
|
||||
*
|
||||
* **What is honest about the dialog remedy: it has been shown to do no harm, not to work.** It
|
||||
* was forced on with no dialog present and the suite stayed green, which is the way a blind
|
||||
* `click()` could have broken a healthy run. Dismissing a real ANR dialog has not been observed,
|
||||
* because the fault has never been reproduced locally — not on six warm runs, not on cold
|
||||
* full-suite runs at API 34 and 35 on freshly created AVDs under `swangle_indirect` at two cores,
|
||||
* not under host load. If it recurs, the message now names the dialog and the window list, so the
|
||||
* next step is a measurement rather than another theory.
|
||||
*
|
||||
* ### The whole pick is retried, which is a separate and smaller claim
|
||||
*
|
||||
* [pickTheFixture] also backs out and asks for another picker when the walk comes up short. That
|
||||
* is not the answer to the paragraph above; it is the answer to a picker whose *lists* were built
|
||||
* before their data arrived, which is a real thing DocumentsUI does and which
|
||||
* [tapPickerNode]'s re-find cannot reach either — it re-acquires a handle inside the one picker.
|
||||
*
|
||||
* One API 37 run failed a step deeper than the rest: the root appeared and
|
||||
* `[TEXT='\Qlmc-r38-fixture.mp4\E']` did not. **That shape has not been reproduced or
|
||||
* diagnosed.** It is covered here only because a fresh pick re-walks from Recent, and that is
|
||||
* worth writing down rather than letting the retry read as a fix for something nobody measured.
|
||||
*
|
||||
* ### The mutations, and what they printed
|
||||
*
|
||||
* Both were run, not asserted. Narrowing the wildcard array `ConverterScreen.kt` passes to
|
||||
* `pickInput.launch` — to `arrayOf("application/x-lmc-no-such-type")` — empties the picker of the
|
||||
* fixture root entirely, and both tests fail on the assertion that names it. **Re-run after the
|
||||
* #93 retry landed**, because a retry that tolerated an absent root would have made this mutation
|
||||
* vacuous, which is the one thing that must not happen here:
|
||||
*
|
||||
* ```
|
||||
* java.lang.AssertionError: the system picker never showed BySelector [TEXT='\QLMC R38 fixtures\E'],
|
||||
* in 3 separate pickers (the last one left org.libremediaconverter in front)
|
||||
* at org.libremediaconverter.saf.SafPickerRoundTripTest.pickTheFixture(SafPickerRoundTripTest.kt:268)
|
||||
* ```
|
||||
*
|
||||
* The root is absent from all three pickers, so all three report it, and the cost of saying so is
|
||||
* bounded: 126 s and 127 s for the two tests, against the 1200 s wrapper timeout in
|
||||
* `.github/scripts/e2e-run.sh`. The clause about what was left in front is not decoration either
|
||||
* — it is what says the retry really did get back to the app between attempts rather than tapping
|
||||
* behind a picker that never closed.
|
||||
*
|
||||
* **That mutation only shows the retry failing correctly.** Showing it *recovering* needs a
|
||||
* failure that goes away, so one was injected: a field making the first
|
||||
* [walkThePickerToTheFixture] of each test return a selector nothing matches. Both tests then
|
||||
* passed, with `ActivityTaskManager` logging four `OPEN_DOCUMENT` starts for the two of them —
|
||||
* two pickers each. That is the run which says the reopened pick completes: that
|
||||
* `pickInput.launch` is not refused from the re-resumed Activity, and that the second test's
|
||||
* reopen, which lands in the last-accessed stack rather than on Recent, still walks to the file.
|
||||
* Making the ViewModel composition-scoped leaves the picker test alone and fails
|
||||
* [thePickedInputSurvivesARealRotation], with `:app:testDebugUnitTest` still BUILD SUCCESSFUL —
|
||||
* which is the divergence this ticket was filed to establish, and which was doubted on it. It is
|
||||
* `viewModel()` -> `viewModel(viewModelStoreOwner = remember { <a plain ViewModelStoreOwner> })`,
|
||||
* **plus** `factory = ViewModelProvider.AndroidViewModelFactory()` and a `MutableCreationExtras`
|
||||
* carrying `APPLICATION_KEY`. The factory half is not decoration: an owner that is not a
|
||||
* `HasDefaultViewModelProviderFactory` contributes no creation extras, and the default factory
|
||||
* cannot construct an `AndroidViewModel` without them — so the owner swap alone crashes on
|
||||
* construction instead of demonstrating the scope. The PR body quotes both failures verbatim.
|
||||
*
|
||||
* ### It has to be an unlocked emulator
|
||||
*
|
||||
* The Pixel 10 Pro XL is secure-locked and cannot be unlocked from a shell, so the picker cannot be
|
||||
* driven there at all. That is why this gap survived as long as it did.
|
||||
* `tools/local-emulator/run-e2e.sh` runs API 33-36 on the development host, and both tests pass
|
||||
* there: **59 / 0 / 0 / 2 at API 33 and again at API 36**, whole suite, 2026-08-24.
|
||||
*
|
||||
* ### Why only the rotation test carries [FailsOnEmulatorApi37]
|
||||
*
|
||||
* This class is the first thing in the suite that touches system UI, and the android-37.x images
|
||||
* are where that stops being free: surfaceflinger aborts inside the guest's Gralloc5 mapper, init
|
||||
* SIGKILLs zygote with it, and the framework restarts underneath the run. Disabling SystemUI --
|
||||
* the deviation the API 37 leg already makes -- removes the *idle* trigger, not this one.
|
||||
*
|
||||
* The marker is on one method and not on the class, because that is what was measured, one method
|
||||
* per fresh emulator, on `android-37.0` under `swangle_indirect`:
|
||||
*
|
||||
* ```
|
||||
* thePickedInputSurvivesARealRotation INSTRUMENTATION_ABORTED: System has crashed.
|
||||
* Expected 1 tests, received 0
|
||||
* pickingAFileThroughTheSystemPickerFillsInTheFileCard PASSED
|
||||
* ```
|
||||
*
|
||||
* A rotation rebuilds every surface on screen at once, which the mapper does not survive; merely
|
||||
* starting DocumentsUI does not.
|
||||
*
|
||||
* **The first version of this said the class, and it was wrong.** The picker test had failed at
|
||||
* API 37 too -- with a `StaleObjectException` that turned out to be this file's own bug rather
|
||||
* than the image's, and which CI then reproduced deterministically at API 33, 34 and 35. Fixing
|
||||
* it ([tapPickerNode]) and re-measuring is what separated the two. An annotation is a claim about
|
||||
* an image, and a broken test makes every image look broken; **re-measure after fixing a test
|
||||
* before deciding what the platform did.**
|
||||
*
|
||||
* The annotation says only that, and CI reads it twice, so the rotation test runs on the advisory
|
||||
* API 37 leg and not the gating one. **Do not read it as "a rotation is allowed to lose the
|
||||
* file".** That is what API 33 through 36 are for, and they answer it.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class SafPickerRoundTripTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createAndroidComposeRule<MainActivity>()
|
||||
|
||||
private val device: UiDevice =
|
||||
UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
|
||||
|
||||
/** The app under test, whose own window is what [requireAReadableScreen] asks for. */
|
||||
private val appPackage: String =
|
||||
InstrumentationRegistry.getInstrumentation().targetContext.packageName
|
||||
|
||||
/** Set by the one test that rotates, read by [restoreOrientation]. See its KDoc. */
|
||||
private var rotated = false
|
||||
|
||||
/**
|
||||
* Leave the device the way it was found — and only if this test moved it.
|
||||
*
|
||||
* Two things are deliberate here, and both are about the *other* tests on the device rather
|
||||
* than about these two.
|
||||
*
|
||||
* The flag, because this runs after every test in the class, not only the one that rotated. An
|
||||
* unconditional restore issues a WindowManager rotation request after the picker test as well,
|
||||
* which has nothing to undo; JUnit does not promise method order, so that is an interaction
|
||||
* between two tests that no single-class run would ever show. Tracked as a flag rather than
|
||||
* read back off `isNaturalOrientation`, because a device whose *natural* orientation is
|
||||
* landscape would answer that question the wrong way round.
|
||||
*
|
||||
* And `unfreezeRotation`, because `setOrientationNatural` does not merely rotate: it freezes
|
||||
* the rotation there. A run that stopped after it would hand the next test a device that
|
||||
* cannot rotate at all.
|
||||
*/
|
||||
@After
|
||||
fun restoreOrientation() {
|
||||
if (!rotated) return
|
||||
device.setOrientationNatural()
|
||||
device.unfreezeRotation()
|
||||
device.waitForIdle()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun pickingAFileThroughTheSystemPickerFillsInTheFileCard() {
|
||||
pickTheFixture()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME)
|
||||
.assertTextEquals(FixtureDocumentsProvider.FIXTURE_DISPLAY_NAME)
|
||||
|
||||
// Not the same assertion twice. The name above comes from a metadata query, which a URI
|
||||
// with no read grant answers just as well; this line only appears once something has
|
||||
// opened the file and read its header. It is what says the picker handed back a URI the
|
||||
// app can actually USE -- delete grantUriPermissions from the fixture's manifest entry and
|
||||
// the name still arrives while this goes red.
|
||||
//
|
||||
// The whole "Container: MP4" and not "MP4": DetailRow renders the label and the value as
|
||||
// one semantics node.
|
||||
awaitNode(TestTags.Converter.detailRow(CONTAINER_LABEL))
|
||||
composeRule.onNodeWithTag(TestTags.Converter.detailRow(CONTAINER_LABEL))
|
||||
.assertTextEquals("$CONTAINER_LABEL: MP4")
|
||||
}
|
||||
|
||||
@Test
|
||||
@FailsOnEmulatorApi37
|
||||
fun thePickedInputSurvivesARealRotation() {
|
||||
pickTheFixture()
|
||||
// The identity hash rather than the Activity itself, so nothing here keeps a destroyed
|
||||
// Activity reachable across the recreation it is being used to detect.
|
||||
val before = System.identityHashCode(composeRule.activity)
|
||||
|
||||
device.setOrientationLandscape()
|
||||
rotated = true
|
||||
composeRule.waitForIdle()
|
||||
|
||||
// Two guards before the assertion that matters, because both of the ways this test could
|
||||
// pass while proving nothing are silent ones.
|
||||
//
|
||||
// A device that ignored the rotation request would leave the app exactly as it was, and
|
||||
// "the file is still there" would then be a statement about a screen nothing happened to.
|
||||
assertNotEquals(
|
||||
"the device did not actually rotate, so nothing below is about a rotation",
|
||||
NATURAL_ROTATION,
|
||||
device.displayRotation,
|
||||
)
|
||||
// And a rotation that did NOT recreate the Activity -- a configChanges attribute added to
|
||||
// the manifest, an aspect-ratio or orientation lock -- would make this a recomposition
|
||||
// test. The retained ViewModelStore is only interesting because the Activity around it
|
||||
// really was destroyed and rebuilt.
|
||||
assertNotEquals(
|
||||
"the rotation did not recreate MainActivity, so the retained ViewModelStore was never used",
|
||||
before,
|
||||
System.identityHashCode(composeRule.activity),
|
||||
)
|
||||
|
||||
awaitNode(TestTags.Converter.FILE_CARD_NAME)
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME)
|
||||
.assertTextEquals(FixtureDocumentsProvider.FIXTURE_DISPLAY_NAME)
|
||||
}
|
||||
|
||||
// --- driving the picker ---------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Taps "Choose file", walks the system picker to the fixture, and returns once the app has it.
|
||||
*
|
||||
* Everything between the first tap and the last belongs to `com.google.android.documentsui`,
|
||||
* which is why UiAutomator is here at all: Compose's matchers stop at this process's
|
||||
* composition and Espresso's at its view hierarchy, and the picker is neither.
|
||||
*
|
||||
* **What is retried here is the whole pick.** [tapPickerNode]'s re-find re-acquires a handle
|
||||
* to a node inside the picker that is already open, so it cannot reach a list that was built
|
||||
* before its data arrived. Backing out and tapping "Choose file" again gets a *second*
|
||||
* `PickActivity`, which rebuilds every list in it — and is what a user does when a picker
|
||||
* comes up wrong. It is **not** the answer to the unreadable-screen failure in the class
|
||||
* KDoc; [requireAReadableScreen], one line above, is the part aimed at that.
|
||||
*
|
||||
* The first attempt keeps the full [PICKER_TIMEOUT_MS]; the later ones use
|
||||
* [REOPENED_TIMEOUT_MS], because by then the picker's process, its provider and its root cache
|
||||
* are all warm and the only thing being waited on is one screen. That is what keeps the cost
|
||||
* of a genuinely absent root bounded — see the class KDoc.
|
||||
*/
|
||||
private fun pickTheFixture() {
|
||||
var missing: BySelector? = null
|
||||
repeat(PICK_ATTEMPTS) { attempt ->
|
||||
requireAReadableScreen()
|
||||
openThePicker()
|
||||
missing = walkThePickerToTheFixture(
|
||||
if (attempt == 0) PICKER_TIMEOUT_MS else REOPENED_TIMEOUT_MS,
|
||||
)
|
||||
if (missing == null) {
|
||||
awaitNode(TestTags.Converter.FILE_CARD_NAME)
|
||||
return
|
||||
}
|
||||
dismissThePicker()
|
||||
}
|
||||
throw AssertionError(
|
||||
"the system picker never showed $missing, in $PICK_ATTEMPTS separate pickers " +
|
||||
"(the last one left ${device.currentPackageName} in front)",
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Refuses to go near the picker until this process can read a window it already knows is there.
|
||||
*
|
||||
* **This is the check that would have answered #93 outright**, instead of leaving six PRs to
|
||||
* infer a SAF fault from a picker that was never the problem. It is here because of what the
|
||||
* failing logcat counts. Across the whole API 34 leg UiAutomator
|
||||
* asked for a node 1095 times and logged `Node not found` 1095 times — it never read anything,
|
||||
* from the first query of the run onwards. The green leg of the same job asked 7 times and
|
||||
* found 5. So the window list `UiDevice` searches, `UiAutomation.getWindows()`, was empty for
|
||||
* that entire instrumentation run; on API 21 and up that list is the *only* place
|
||||
* `getWindowRoots` looks, so an empty one makes every selector unfindable and says nothing
|
||||
* about the app, the picker or the fixture.
|
||||
*
|
||||
* The probe is deliberately the app's **own** window, asked while the app is in front and
|
||||
* before anything is tapped. It is the one window that must be readable for any of the rest to
|
||||
* mean anything, so a failure here is unambiguous — where "the picker never showed the root"
|
||||
* was not, and is what sent #93 looking at package installation and root caches.
|
||||
*
|
||||
* The repair is [rebuildUiAutomation]. It has been forced on and measured — a rebuilt
|
||||
* connection still reads windows, which is the way it could have been worse than nothing —
|
||||
* but it has **never been run against the real fault**, because the fault has never been
|
||||
* reproduced on demand. See the class KDoc. What is certain is that a fresh picker is *not*
|
||||
* the repair: the failing leg opened a second `PickActivity` for the second test, in the
|
||||
* same DocumentsUI process, and read exactly as little from it.
|
||||
*/
|
||||
private fun requireAReadableScreen() {
|
||||
val app = By.pkg(appPackage)
|
||||
if (device.wait(Until.hasObject(app), READABLE_TIMEOUT_MS) == true) return
|
||||
dismissASystemErrorDialog()
|
||||
if (device.wait(Until.hasObject(app), READABLE_TIMEOUT_MS) == true) return
|
||||
unlockTheDevice()
|
||||
if (device.wait(Until.hasObject(app), READABLE_TIMEOUT_MS) == true) return
|
||||
rebuildUiAutomation()
|
||||
if (device.wait(Until.hasObject(app), READABLE_TIMEOUT_MS) != true) {
|
||||
throw AssertionError(
|
||||
"UiAutomator cannot see this app's own window, so it could not have seen the " +
|
||||
"picker's either. This is not a SAF failure. Closing a system error dialog, " +
|
||||
"waking the device, dismissing the keyguard and rebuilding the UiAutomation " +
|
||||
"connection all failed to make it readable. What it could see: " +
|
||||
describeWindows(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Closes a system "isn't responding" dialog, if that is what is on top of the app.
|
||||
*
|
||||
* **This is the occluder #93 turned out to have**, and it took the window list in the failure
|
||||
* message to find it. `AppNotRespondingDialog` belongs to `system_server`, so it is the
|
||||
* `android[type=3]` in `com.android.systemui[type=3], android[type=3]` — and it is opaque and
|
||||
* fullscreen, so `AccessibilityWindowManager` drops every application window beneath it. The
|
||||
* app is `Displayed` and unreadable at the same time, which is exactly the contradiction this
|
||||
* class spent #93 failing to explain. It is not even this app's dialog:
|
||||
*
|
||||
* ```
|
||||
* ANR in com.google.android.apps.nexuslauncher (com.google.android.apps.nexuslauncher/.NexusLauncherActivity)
|
||||
* Reason: Input dispatching timed out (Application does not have a focused window)
|
||||
* Window{4ed8414 u0 Application Not Responding: com.google.android.apps.nexuslauncher}
|
||||
* ```
|
||||
*
|
||||
* The launcher ANRs on a loaded runner emulator minutes before this class runs, and the dialog
|
||||
* it leaves behind never goes away on its own.
|
||||
*
|
||||
* Dismissed by resource id rather than by button text, because the text is localised and the
|
||||
* ids are not, and by id rather than by "the first button in the system window", because that
|
||||
* would click whatever system window happened to be there. `aerr_wait` first: it dismisses the
|
||||
* dialog and leaves the offending app alone, which is the polite answer when the app is not
|
||||
* ours. Back is not tried — `BaseErrorDialog` swallows key events.
|
||||
*/
|
||||
private fun dismissASystemErrorDialog() {
|
||||
for (id in ERROR_DIALOG_BUTTONS) {
|
||||
val button = device.findObject(By.res(id)) ?: continue
|
||||
button.click()
|
||||
device.waitForIdle()
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Wakes the display and asks the keyguard to go away.
|
||||
*
|
||||
* The cheapest explanation for "this process cannot see the app's own window" is that
|
||||
* something is in front of it, and on a runner emulator that something is the lock screen:
|
||||
* these images come up unprovisioned, and `KeyguardViewMediator` says so in as many words --
|
||||
* `we need to show the keyguard since the device isn't provisioned yet`. An occluded window is
|
||||
* not in the accessibility window list, which is the same symptom as a broken connection and
|
||||
* has a far more ordinary cause.
|
||||
*
|
||||
* `wm dismiss-keyguard` rather than a swipe, because it is a request to the window manager
|
||||
* rather than a gesture that has to land somewhere this process cannot see. It is only
|
||||
* attempted on the failure path -- a device that was readable never reaches here -- so a run
|
||||
* where the keyguard was never up pays nothing and is not altered.
|
||||
*/
|
||||
private fun unlockTheDevice() {
|
||||
device.wakeUp()
|
||||
device.executeShellCommand("wm dismiss-keyguard")
|
||||
device.waitForIdle()
|
||||
}
|
||||
|
||||
/** The accessibility window list, for a failure message that says what was actually there. */
|
||||
private fun describeWindows(): String {
|
||||
val windows = InstrumentationRegistry.getInstrumentation().uiAutomation.windows
|
||||
if (windows.isEmpty()) return "no windows at all (UiAutomation.getWindows() is empty)"
|
||||
return windows.joinToString(", ") { "${it.root?.packageName ?: "?"}[type=${it.type}]" }
|
||||
}
|
||||
|
||||
/**
|
||||
* Tears down this run's `UiAutomation` connection and establishes a new one.
|
||||
*
|
||||
* `Instrumentation.getUiAutomation` hands back the existing connection unless the flags differ
|
||||
* from the ones it was created with, in which case it destroys it and builds another — so
|
||||
* asking for different flags and then for the original ones back is how a test reaches the
|
||||
* connection at all. `UiDevice` re-reads the flags from `Configurator` on every call rather
|
||||
* than caching an instance, so the next selector goes through the new connection.
|
||||
*
|
||||
* `FLAG_DONT_SUPPRESS_ACCESSIBILITY_SERVICES` is toggled rather than chosen: it is only being
|
||||
* used as a value that differs from whatever is configured, and it is put back.
|
||||
*
|
||||
* **Forced on and measured, because the obvious way for this to be worse than nothing is
|
||||
* silent.** `UiDevice` puts `FLAG_RETRIEVE_INTERACTIVE_WINDOWS` on the service info during its
|
||||
* own initialisation, and `getWindows()` is empty without it — so a rebuilt connection that
|
||||
* did not get the flag back would cause exactly the emptiness this is meant to cure, on the
|
||||
* one path where it is the last hope. Run unconditionally on every attempt, on a cold API 34
|
||||
* emulator, both tests passed, and logcat shows the connection really being replaced rather
|
||||
* than handed back: `Init UiAutomation[id=2, flags=0]`, then `id=4, flags=1`, then
|
||||
* `id=6, flags=0`, with `Registering UiTestAutomationService` between each.
|
||||
*/
|
||||
private fun rebuildUiAutomation() {
|
||||
val configurator = Configurator.getInstance()
|
||||
val flags = configurator.uiAutomationFlags
|
||||
val instrumentation = InstrumentationRegistry.getInstrumentation()
|
||||
configurator.uiAutomationFlags = flags xor UiAutomation.FLAG_DONT_SUPPRESS_ACCESSIBILITY_SERVICES
|
||||
instrumentation.getUiAutomation(configurator.uiAutomationFlags)
|
||||
configurator.uiAutomationFlags = flags
|
||||
instrumentation.getUiAutomation(flags)
|
||||
}
|
||||
|
||||
/** Waits for the app to be showing its own screen again, then asks for a picker. */
|
||||
private fun openThePicker() {
|
||||
awaitNode(TestTags.Converter.CHOOSE_FILE)
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).performClick()
|
||||
}
|
||||
|
||||
/**
|
||||
* Null once the fixture URI is with the app, or the selector whose list never carried it.
|
||||
*
|
||||
* Three things have to be there, in order, and the `when` names them in that order so that a
|
||||
* failure says which one was missing rather than "the picker did not work".
|
||||
*
|
||||
* **The first branch is what tells an unreadable picker from an absent root.** In #93 neither
|
||||
* the root *nor the toolbar's "Show roots" button* could be found for sixty seconds, and a
|
||||
* stale roots list would have left the toolbar findable. Both arrived as one message. Asking
|
||||
* for the picker's package on its own separates them: `never showed BySelector [PKG=...]`
|
||||
* means the picker was not readable, and the root selector means the root was not offered.
|
||||
*
|
||||
* The second is the line the MIME filter mutation fails on: DocumentsUI matches the requested
|
||||
* types against `Root.COLUMN_MIME_TYPES` and drops the roots that cannot answer, so a filter
|
||||
* the fixture root does not satisfy takes the root out of the picker altogether — along with
|
||||
* "Images", "Audio", "Videos" and "Documents", measured on API 34.
|
||||
*
|
||||
* **The third takes no recovery action of its own, and that is deliberate rather than an
|
||||
* oversight.** [openTheRootsDrawer] exists because a root has a *second* place it can be
|
||||
* shown; a document in a directory listing has no second place, so there is nothing an
|
||||
* in-picker action could do. Its recovery is the outer loop: a fresh picker re-walks from
|
||||
* Recent into the root, which rebuilds the directory listing as well as the roots strip.
|
||||
*/
|
||||
private fun walkThePickerToTheFixture(timeoutMs: Long): BySelector? {
|
||||
val picker = By.pkg(DOCUMENTS_UI_PACKAGE)
|
||||
val root = By.text(FixtureDocumentsProvider.ROOT_TITLE)
|
||||
val fixture = By.text(FixtureDocumentsProvider.FIXTURE_DISPLAY_NAME)
|
||||
return when {
|
||||
device.wait(Until.hasObject(picker), timeoutMs) != true -> picker
|
||||
!tapPickerNode(root, timeoutMs, ifAbsent = ::openTheRootsDrawer) -> root
|
||||
!tapPickerNode(fixture, timeoutMs) -> fixture
|
||||
else -> null
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The picker's own drawer, opened only when the root was not on the screen it landed on.
|
||||
*
|
||||
* **In practice it never runs, and #80 was right to say so.** A hierarchy dump taken on a
|
||||
* cold API 34 emulator while this test was passing has the fixture root on the landing
|
||||
* screen — `text="LMC R38 fixtures"` at `android:id/title`, under a `BROWSE FILES IN OTHER
|
||||
* APPS` header — with the drawer shut (`Show roots` present, `Hide roots` absent). So the
|
||||
* roots strip is the normal path and the drawer is a widening, kept because a device with a
|
||||
* populated Recent may push the strip off screen. Looking in a second place widens where the
|
||||
* root is searched for; it does not weaken what has to be found, which is still this root.
|
||||
*/
|
||||
private fun openTheRootsDrawer() {
|
||||
device.findObject(By.desc(SHOW_ROOTS_DESCRIPTION))?.click()
|
||||
}
|
||||
|
||||
/**
|
||||
* Backs out of the picker until the app has the window focus again.
|
||||
*
|
||||
* **The focus is asked of the Activity, not of UiAutomator, and that is not a stylistic
|
||||
* choice.** The failure this retry exists for is a picker window UiAutomator cannot see, so a
|
||||
* probe that went through the same accessibility window list would cheerfully report "the
|
||||
* picker is gone" about the window that is still in front — and the reopened pick would then
|
||||
* tap "Choose file" behind it. `Activity.hasWindowFocus` comes from the framework instead, and
|
||||
* answers about the app rather than about the picker.
|
||||
*
|
||||
* It is also why this counts backs rather than pressing a fixed number of them. One back is
|
||||
* enough from Recent and two are needed from inside the root, but a third from Recent would
|
||||
* finish `MainActivity` and take the rest of the test with it.
|
||||
*/
|
||||
private fun dismissThePicker() {
|
||||
repeat(BACK_PRESSES) {
|
||||
if (awaitAppFocus()) return
|
||||
// Before the back press, not instead of it: an app-error dialog swallows key events,
|
||||
// so a back aimed at the picker lands on the dialog and nothing moves. Measured --
|
||||
// API 34 of run 32813885120 exhausted all four presses with `android` in front, which
|
||||
// is that dialog, while the launcher it belonged to went on ANRing behind everything.
|
||||
dismissASystemErrorDialog()
|
||||
device.pressBack()
|
||||
}
|
||||
// The check after the last press, and not a spare one: `repeat` presses on its final
|
||||
// iteration too, so without this a dismissal that worked on the last press would still be
|
||||
// reported as a failure to close.
|
||||
if (!awaitAppFocus()) {
|
||||
throw AssertionError(
|
||||
"the system picker would not close: after $BACK_PRESSES back presses the app " +
|
||||
"still does not have the window focus, and ${device.currentPackageName} is " +
|
||||
"in front. What could be seen: " + describeWindows(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/** True once [MainActivity] has the window focus, false if it does not take it in time. */
|
||||
private fun awaitAppFocus(): Boolean = try {
|
||||
composeRule.waitUntil("the app has the window focus back", FOCUS_TIMEOUT_MS) {
|
||||
composeRule.activity.hasWindowFocus()
|
||||
}
|
||||
true
|
||||
} catch (_: ComposeTimeoutException) {
|
||||
false
|
||||
}
|
||||
|
||||
/**
|
||||
* Finds the picker node [selector] names and taps it, re-finding it if it goes stale.
|
||||
*
|
||||
* **The re-finding is not padding, and this is not a retry of the assertion.** A `UiObject2`
|
||||
* holds an `AccessibilityNodeInfo` captured when it was found, and DocumentsUI is still
|
||||
* settling when the node first appears — its list rebinds, the roots strip lays out, a window
|
||||
* animates. If the node is replaced in that gap, `click()` throws `StaleObjectException`
|
||||
* against the handle rather than missing the target. Measured on a cold API 34 emulator:
|
||||
*
|
||||
* ```
|
||||
* androidx.test.uiautomator.StaleObjectException
|
||||
* at androidx.test.uiautomator.UiObject2.getAccessibilityNodeInfo(UiObject2.java:1042)
|
||||
* at androidx.test.uiautomator.UiObject2.click(UiObject2.java:526)
|
||||
* ```
|
||||
*
|
||||
* So what is retried is *acquiring a handle to a node that has to be there anyway*. **A node
|
||||
* that is simply not in this picker is reported rather than retried here** — it comes back as
|
||||
* `false`, and [pickTheFixture] answers it with a whole new picker, which is the only thing
|
||||
* that rebuilds a list or a window. The MIME mutation's bite is untouched either way: a root
|
||||
* that is not in the picker is not found on any attempt or in any picker, and the failure is
|
||||
* still "the system picker never showed" rather than a stale one.
|
||||
*/
|
||||
private fun tapPickerNode(selector: BySelector, timeoutMs: Long, ifAbsent: () -> Unit = {}): Boolean {
|
||||
var stale: StaleObjectException? = null
|
||||
repeat(TAP_ATTEMPTS) { attempt ->
|
||||
// ifAbsent only on the first attempt: it navigates, and re-navigating from a screen it
|
||||
// already reached would walk away from the node.
|
||||
val node = awaitPickerNode(selector, timeoutMs, if (attempt == 0) ifAbsent else ({}))
|
||||
?: return false
|
||||
device.waitForIdle()
|
||||
try {
|
||||
node.click()
|
||||
return true
|
||||
} catch (e: StaleObjectException) {
|
||||
stale = e
|
||||
}
|
||||
}
|
||||
throw AssertionError("$selector kept going stale between finding it and tapping it", stale)
|
||||
}
|
||||
|
||||
/**
|
||||
* The picker node [selector] names, or null if this picker never showed it.
|
||||
*
|
||||
* [ifAbsent] runs once, after the first wait comes up empty, and then the wait is repeated. A
|
||||
* null return from `findObject` is deliberately not an error there: it is the "already on the
|
||||
* right screen" case.
|
||||
*/
|
||||
private fun awaitPickerNode(selector: BySelector, timeoutMs: Long, ifAbsent: () -> Unit) =
|
||||
device.wait(Until.findObject(selector), timeoutMs)
|
||||
?: run {
|
||||
ifAbsent()
|
||||
device.wait(Until.findObject(selector), timeoutMs)
|
||||
}
|
||||
|
||||
/**
|
||||
* Blocks until [tag] is in the composition, so an assertion cannot race the picker's result.
|
||||
*
|
||||
* The described overload of `waitUntil`, not the bare one. A timeout is how both of this
|
||||
* class's mutations report themselves, and the bare overload's message is
|
||||
* `Condition still not satisfied after 30000 ms` — which names neither the node nor the test.
|
||||
* With the description it says which affordance never arrived, which is the whole finding.
|
||||
*/
|
||||
private fun awaitNode(tag: String) {
|
||||
composeRule.waitUntil("a node tagged $tag exists", APP_TIMEOUT_MS) {
|
||||
composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().isNotEmpty()
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
|
||||
/**
|
||||
* Generous on purpose. This waits on another app being started, and on FFprobe spawning a
|
||||
* native process over a `content://` URI; a timeout that merely usually passes is a flaky
|
||||
* gating leg on five API levels, which costs far more than the seconds it saves.
|
||||
*/
|
||||
const val PICKER_TIMEOUT_MS = 30_000L
|
||||
const val APP_TIMEOUT_MS = 30_000L
|
||||
|
||||
/**
|
||||
* The same wait once a picker has already come and gone, and shorter for a reason.
|
||||
*
|
||||
* What [PICKER_TIMEOUT_MS] is generous about is a cold start: DocumentsUI's process, the
|
||||
* fixture's provider process, the root cache. By the second attempt all three are warm and
|
||||
* the only thing left to wait on is one screen being laid out — measured at 2.7 to 3.4 s
|
||||
* from the picker starting, on cold CI emulators at API 33, 34 and 35. Ten seconds is
|
||||
* three times the worst of those, and it is what keeps a genuinely absent root — the MIME
|
||||
* mutation — from costing three full-length attempts.
|
||||
*/
|
||||
const val REOPENED_TIMEOUT_MS = 10_000L
|
||||
|
||||
/**
|
||||
* How long the app is given to take the window focus back after a back press.
|
||||
*
|
||||
* Short, because this is asked once per back press and the first one is always asked while
|
||||
* the picker is still in front, where it is *expected* to time out.
|
||||
*/
|
||||
const val FOCUS_TIMEOUT_MS = 3_000L
|
||||
|
||||
/**
|
||||
* How long this process is given to be able to read the screen at all.
|
||||
*
|
||||
* Short, and it is not waiting on anything being drawn: the app is already in front
|
||||
* when this is asked. It is waiting only on the accessibility window list existing,
|
||||
* which either does within a poll or two or -- as in #93 -- not at all.
|
||||
*/
|
||||
const val READABLE_TIMEOUT_MS = 5_000L
|
||||
|
||||
/** `Surface.ROTATION_0`, named rather than `0` so the comparison reads. */
|
||||
const val NATURAL_ROTATION = 0
|
||||
|
||||
/**
|
||||
* How many pickers the fixture may fail to appear in before that is the finding.
|
||||
*
|
||||
* Three. Each one is a fresh `PickActivity` -- a fresh window, a fresh accessibility
|
||||
* registration, a fresh roots query and a fresh directory load -- so this bounds the thing
|
||||
* #93 measured, which is a picker that came up unreadable *once*. A root that is genuinely
|
||||
* not offered is absent from all three, which is what keeps #64's MIME mutation red.
|
||||
*/
|
||||
const val PICK_ATTEMPTS = 3
|
||||
|
||||
/**
|
||||
* How many back presses may be spent getting out of a picker.
|
||||
*
|
||||
* One is enough from Recent, two from inside the fixture's own directory. Four leaves room
|
||||
* for a picker that has been navigated deeper than this test ever navigates it, and stops
|
||||
* well short of the count that would start finishing `MainActivity` instead.
|
||||
*/
|
||||
const val BACK_PRESSES = 4
|
||||
|
||||
/**
|
||||
* The package the system picker runs in.
|
||||
*
|
||||
* Named rather than resolved: `PackageManager.resolveActivity` is deprecated from API 33
|
||||
* and its replacement is a lint argument this test does not need to have. A wrong value
|
||||
* here cannot pass silently -- it is the first thing [walkThePickerToTheFixture] looks
|
||||
* for, so the failure would read `never showed BySelector [PKG='...']` on every device.
|
||||
* It is `com.google.android.documentsui` on every `google_apis` emulator image the CI
|
||||
* matrix uses and on the Pixel 10 Pro XL.
|
||||
*/
|
||||
const val DOCUMENTS_UI_PACKAGE = "com.google.android.documentsui"
|
||||
|
||||
/**
|
||||
* How many times a picker node may be re-found before its staleness is the finding.
|
||||
*
|
||||
* Three, not "until the timeout". Each attempt already waits up to [PICKER_TIMEOUT_MS] for
|
||||
* the node to exist, so this bounds only the settling window after it does; a node that is
|
||||
* still being replaced after three of those is telling you something about the device, and
|
||||
* a loop that hid it would be the flake rather than the fix.
|
||||
*/
|
||||
const val TAP_ATTEMPTS = 3
|
||||
|
||||
/**
|
||||
* The buttons on the framework's app-error dialogs, by resource id.
|
||||
*
|
||||
* `aerr_wait` is first because it dismisses the dialog without killing the app under it,
|
||||
* and the app under it is usually the launcher rather than anything this suite owns.
|
||||
* `button1` catches the plainer `BaseErrorDialog` shapes that have no `aerr_` ids.
|
||||
*/
|
||||
val ERROR_DIALOG_BUTTONS = listOf(
|
||||
"android:id/aerr_wait",
|
||||
"android:id/aerr_close",
|
||||
"android:id/button1",
|
||||
)
|
||||
|
||||
/** DocumentsUI's drawer button. It carries no text, only this description. */
|
||||
const val SHOW_ROOTS_DESCRIPTION = "Show roots"
|
||||
|
||||
/** The detail row `MediaProbe` fills in for anything it could open and identify. */
|
||||
const val CONTAINER_LABEL = "Container"
|
||||
}
|
||||
}
|
||||
@@ -75,7 +75,13 @@ class AndroidDeviceCodecs private constructor(
|
||||
return AndroidDeviceCodecs(encoders, decoders)
|
||||
}
|
||||
|
||||
private fun mimeFor(codec: VideoCodec): String? = when (codec) {
|
||||
/**
|
||||
* `internal` rather than `private` so the cross-check test can ask what a [VideoCodec]
|
||||
* means here and compare it with what [NAME_TO_MIME] says the same codec's names mean.
|
||||
* The JVM test source set is a friend of `main`, so this stays invisible outside the
|
||||
* module — the precedent is `MainActivity`'s `Destination`.
|
||||
*/
|
||||
internal fun mimeFor(codec: VideoCodec): String? = when (codec) {
|
||||
VideoCodec.H264 -> MediaFormat.MIMETYPE_VIDEO_AVC
|
||||
VideoCodec.H265 -> MediaFormat.MIMETYPE_VIDEO_HEVC
|
||||
VideoCodec.VP8 -> MediaFormat.MIMETYPE_VIDEO_VP8
|
||||
@@ -87,20 +93,62 @@ class AndroidDeviceCodecs private constructor(
|
||||
VideoCodec.COPY, VideoCodec.NONE -> null
|
||||
}
|
||||
|
||||
/** Maps an FFprobe-style codec name onto a MediaFormat MIME type. */
|
||||
private fun mimeForCodecName(name: String): String? = when (name.lowercase()) {
|
||||
"h264", "avc", "avc1" -> MediaFormat.MIMETYPE_VIDEO_AVC
|
||||
"hevc", "h265", "hvc1" -> MediaFormat.MIMETYPE_VIDEO_HEVC
|
||||
"vp8" -> MediaFormat.MIMETYPE_VIDEO_VP8
|
||||
"vp9" -> MediaFormat.MIMETYPE_VIDEO_VP9
|
||||
"av1", "av01" -> MediaFormat.MIMETYPE_VIDEO_AV1
|
||||
"mpeg4" -> MediaFormat.MIMETYPE_VIDEO_MPEG4
|
||||
// Unknown to us: assume the platform can handle it and let a failed export
|
||||
// trigger the FFmpeg fallback, rather than pre-emptively refusing hardware.
|
||||
else -> null
|
||||
}
|
||||
/**
|
||||
* FFprobe-style codec names, and the MediaFormat MIME type each one asks about.
|
||||
*
|
||||
* This is the same vocabulary `CodecNames.VIDEO_ALIASES` holds, written out a second time
|
||||
* because this side has to answer in platform MIME types and `model` does not depend on
|
||||
* Android. Two copies of one vocabulary drift, and these had: `x264`, `hev1`, `x265` and
|
||||
* `vp09` resolved for display and routing and fell through to null here, so the app ran
|
||||
* the capability check blind on inputs it had already identified (#87). They are listed
|
||||
* now, which **changes behaviour** for those four names — see [mimeForCodecName].
|
||||
*
|
||||
* A map rather than a `when` because a `when` cannot be enumerated, and `CodecVocabularyTest`
|
||||
* has to walk both key sets to notice the next divergence.
|
||||
*/
|
||||
internal val NAME_TO_MIME: Map<String, String> = mapOf(
|
||||
"h264" to MediaFormat.MIMETYPE_VIDEO_AVC,
|
||||
"avc" to MediaFormat.MIMETYPE_VIDEO_AVC,
|
||||
"avc1" to MediaFormat.MIMETYPE_VIDEO_AVC,
|
||||
"x264" to MediaFormat.MIMETYPE_VIDEO_AVC,
|
||||
"hevc" to MediaFormat.MIMETYPE_VIDEO_HEVC,
|
||||
"h265" to MediaFormat.MIMETYPE_VIDEO_HEVC,
|
||||
"hvc1" to MediaFormat.MIMETYPE_VIDEO_HEVC,
|
||||
"hev1" to MediaFormat.MIMETYPE_VIDEO_HEVC,
|
||||
"x265" to MediaFormat.MIMETYPE_VIDEO_HEVC,
|
||||
"vp8" to MediaFormat.MIMETYPE_VIDEO_VP8,
|
||||
"vp9" to MediaFormat.MIMETYPE_VIDEO_VP9,
|
||||
"vp09" to MediaFormat.MIMETYPE_VIDEO_VP9,
|
||||
"av1" to MediaFormat.MIMETYPE_VIDEO_AV1,
|
||||
"av01" to MediaFormat.MIMETYPE_VIDEO_AV1,
|
||||
"mpeg4" to MediaFormat.MIMETYPE_VIDEO_MPEG4,
|
||||
)
|
||||
|
||||
/** Test seam: lets instrumented tests build a probe from explicit sets. */
|
||||
/**
|
||||
* The names in [NAME_TO_MIME] that no [VideoCodec] member spells, and why.
|
||||
*
|
||||
* MPEG-4 Part 2 is decodable input the app never targets, so there is no enum for it and
|
||||
* `CodecNames` is right not to carry it. That makes it the one place the two tables
|
||||
* legitimately differ. It is listed rather than implied so the cross-check can tell a
|
||||
* documented asymmetry from a fresh drift — and so the list itself is checked: a name here
|
||||
* that `CodecNames` does resolve is a divergence being waved through, and the test fails on
|
||||
* it.
|
||||
*/
|
||||
internal val DECODE_ONLY_NAMES: Set<String> = setOf("mpeg4")
|
||||
|
||||
/**
|
||||
* Maps an FFprobe-style codec name onto a MediaFormat MIME type.
|
||||
*
|
||||
* Null keeps its documented meaning — unknown to us: assume the platform can handle it and
|
||||
* let a failed export trigger the FFmpeg fallback, rather than pre-emptively refusing
|
||||
* hardware. What changed with #87 is which names are unknown. Four that FFmpeg genuinely
|
||||
* emits used to land here and be treated as unknown while the rest of the app knew exactly
|
||||
* what they were; a device without the matching decoder now routes them to FFmpeg up front
|
||||
* instead of spending a doomed hardware attempt to find out.
|
||||
*/
|
||||
internal fun mimeForCodecName(name: String): String? = NAME_TO_MIME[name.lowercase()]
|
||||
|
||||
/** Test seam: lets a test build a probe from explicit sets, on a device or on the JVM. */
|
||||
fun forTesting(encoders: Set<String>, decoders: Set<String>) = AndroidDeviceCodecs(encoders, decoders)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -120,6 +120,29 @@ class ConversionViewModel @JvmOverloads constructor(
|
||||
* the first screen.
|
||||
*/
|
||||
private val cleanupDispatcher: CoroutineDispatcher = Dispatchers.IO,
|
||||
/**
|
||||
* Where the two blocking hops behind a pick run — the metadata query and the probe.
|
||||
*
|
||||
* A seam for the probe above all, because that is the one call in this class that throws
|
||||
* on purpose. [probeOrUnreadable] rethrows anything that is not a native load failure, and
|
||||
* the `launch` it runs in has no exception handler by design: on a device the error reaches
|
||||
* the thread's default handler and takes the process down, which is what an
|
||||
* [OutOfMemoryError] should do.
|
||||
*
|
||||
* On the JVM there is no such handler. kotlinx-coroutines-test installs a process-wide
|
||||
* collector, once and for the life of the classloader, that keeps an escaped error and
|
||||
* hands it to whichever `runTest` starts next — so it failed a Compose test class that had
|
||||
* nothing to do with it, and *which* class moved between runs of identical code. Naming the
|
||||
* dispatcher is what lets a test keep the throw inside its own window, where it fails the
|
||||
* test that caused it and is consumed rather than collected.
|
||||
*
|
||||
* Both hops rather than the probe alone, which is where this differs from the seam issue #66
|
||||
* proposed: leaving the metadata query on a real [Dispatchers.IO] makes the coroutine resume
|
||||
* on a main looper that Robolectric leaves paused, and that bounce is precisely the
|
||||
* asynchrony that made delivery unpredictable. One dispatcher covers a whole pick, and
|
||||
* leaves nothing about it to timing.
|
||||
*/
|
||||
private val pickDispatcher: CoroutineDispatcher = Dispatchers.IO,
|
||||
) : AndroidViewModel(app) {
|
||||
|
||||
private val workManager = WorkManager.getInstance(app)
|
||||
@@ -241,13 +264,13 @@ class ConversionViewModel @JvmOverloads constructor(
|
||||
viewModelScope.launch {
|
||||
// Both the metadata query and the probe touch disk, and the probe spawns FFprobe.
|
||||
// Neither belongs on the main thread.
|
||||
val file = withContext(Dispatchers.IO) { InputQuery.describe(getApplication(), uri) }
|
||||
val file = withContext(pickDispatcher) { InputQuery.describe(getApplication(), uri) }
|
||||
// Show the file as soon as its name and size are known. Probing now runs FFprobe on
|
||||
// every pick, which is a native process spawn, and making the whole screen wait on it
|
||||
// would read as the app having ignored the tap.
|
||||
_state.value = ConversionState.Ready(file)
|
||||
|
||||
val probe = withContext(Dispatchers.IO) { probeOrUnreadable(uri) }
|
||||
val probe = withContext(pickDispatcher) { probeOrUnreadable(uri) }
|
||||
// Only fill in the probe if the user has not moved on in the meantime.
|
||||
_state.update { current ->
|
||||
if (current is ConversionState.Ready && current.input.uri == uri) {
|
||||
|
||||
@@ -87,6 +87,89 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
ActivityResultContracts.RequestPermission(),
|
||||
) { viewModel.convert() }
|
||||
|
||||
ConverterScreenContent(
|
||||
state = state,
|
||||
settings = settings,
|
||||
validation = validation,
|
||||
actions = ConverterActions(
|
||||
onPickInput = { pickInput.launch(arrayOf("*/*")) },
|
||||
onPreset = viewModel::setPreset,
|
||||
onContainer = viewModel::setContainer,
|
||||
onVideoCodec = viewModel::setVideoCodec,
|
||||
onAudioCodec = viewModel::setAudioCodec,
|
||||
onSuggestion = viewModel::applySuggestion,
|
||||
onQuality = viewModel::setQuality,
|
||||
onEnginePreference = viewModel::setEnginePreference,
|
||||
onConvert = { requestNotifications.launch(Manifest.permission.POST_NOTIFICATIONS) },
|
||||
onCancel = viewModel::cancel,
|
||||
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
||||
onReset = viewModel::reset,
|
||||
),
|
||||
modifier = modifier,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Everything [ConverterScreenContent] can ask for, in one value.
|
||||
*
|
||||
* A holder rather than twelve parameters because detekt's `LongParameterList` sits at its default
|
||||
* threshold of six and `config/detekt/detekt.yml` does not relax it for `@Composable` the way it
|
||||
* relaxes `LongMethod` and `CyclomaticComplexMethod` -- `AdvancedPicker` already sits exactly on
|
||||
* that threshold. The rule exempts data classes, so the callbacks travel together.
|
||||
*
|
||||
* In production every one of these is a launcher or a `ConversionViewModel` call. Naming them here
|
||||
* instead of handing the content a ViewModel is the whole point of the seam: a test can render a
|
||||
* [ConversionState] no ViewModel can be driven into, since `Waiting` needs a denied foreground
|
||||
* start and `Converted` needs a worker run that has already succeeded.
|
||||
*/
|
||||
internal data class ConverterActions(
|
||||
/** Open the document picker. The `Idle` and `Ready` branches both offer it. */
|
||||
val onPickInput: () -> Unit,
|
||||
val onPreset: (OutputFormat) -> Unit,
|
||||
val onContainer: (Container) -> Unit,
|
||||
val onVideoCodec: (VideoCodec) -> Unit,
|
||||
val onAudioCodec: (AudioCodec) -> Unit,
|
||||
val onSuggestion: (OutputSpec) -> Unit,
|
||||
val onQuality: (QualityTier) -> Unit,
|
||||
val onEnginePreference: (EnginePreference) -> Unit,
|
||||
/**
|
||||
* Start the job. It asks for the notification permission first, which is why the screen never
|
||||
* calls `convert` directly -- the launcher's result callback does, whichever way it went.
|
||||
*/
|
||||
val onConvert: () -> Unit,
|
||||
val onCancel: () -> Unit,
|
||||
/**
|
||||
* Open the save dialog for the finished output.
|
||||
*
|
||||
* Takes the suggested name rather than reading it back off the state, because the name comes
|
||||
* from the job -- see `ConversionWorker.KEY_SUGGESTED_NAME` -- and the branch that renders the
|
||||
* button is the only place that has it.
|
||||
*/
|
||||
val onSave: (suggestedName: String) -> Unit,
|
||||
val onReset: () -> Unit,
|
||||
)
|
||||
|
||||
/**
|
||||
* The converter screen, with its state handed in.
|
||||
*
|
||||
* Split from [ConverterScreen] so that state has somewhere to come from other than a live
|
||||
* `ConversionViewModel`. Driving the screen through a real one needs a `WorkManager` and a media
|
||||
* probe in the constructor, and even then two of the six states are unreachable: `Waiting` follows
|
||||
* a denied foreground start and `Converted` follows a completed worker.
|
||||
*
|
||||
* `internal` rather than private, because `src/test` is a friend of `main` and this is what the
|
||||
* state tests compose. The leaves below stay exactly where they were -- this function is a move,
|
||||
* not a redesign, and the tests that already pin those leaves are what says so.
|
||||
*/
|
||||
@UnstableApi
|
||||
@Composable
|
||||
internal fun ConverterScreenContent(
|
||||
state: ConversionState,
|
||||
settings: ConversionSettings,
|
||||
validation: Validation,
|
||||
actions: ConverterActions,
|
||||
modifier: Modifier = Modifier,
|
||||
) {
|
||||
Column(
|
||||
modifier = modifier
|
||||
.fillMaxSize()
|
||||
@@ -115,7 +198,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
modifier = Modifier.padding(bottom = 16.dp),
|
||||
)
|
||||
Button(
|
||||
onClick = { pickInput.launch(arrayOf("*/*")) },
|
||||
onClick = actions.onPickInput,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
@@ -132,21 +215,19 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
|
||||
is ConversionState.Ready -> {
|
||||
FileCard(s.input)
|
||||
FormatPicker(settings.matchingPreset, viewModel::setPreset)
|
||||
FormatPicker(settings.matchingPreset, actions.onPreset)
|
||||
AdvancedPicker(
|
||||
spec = settings.spec,
|
||||
validation = validation,
|
||||
onContainer = viewModel::setContainer,
|
||||
onVideoCodec = viewModel::setVideoCodec,
|
||||
onAudioCodec = viewModel::setAudioCodec,
|
||||
onSuggestion = viewModel::applySuggestion,
|
||||
onContainer = actions.onContainer,
|
||||
onVideoCodec = actions.onVideoCodec,
|
||||
onAudioCodec = actions.onAudioCodec,
|
||||
onSuggestion = actions.onSuggestion,
|
||||
)
|
||||
QualityPicker(settings.quality, viewModel::setQuality)
|
||||
EnginePicker(settings.enginePreference, viewModel::setEnginePreference)
|
||||
QualityPicker(settings.quality, actions.onQuality)
|
||||
EnginePicker(settings.enginePreference, actions.onEnginePreference)
|
||||
Button(
|
||||
onClick = {
|
||||
requestNotifications.launch(Manifest.permission.POST_NOTIFICATIONS)
|
||||
},
|
||||
onClick = actions.onConvert,
|
||||
// The Advanced picker lets an impossible combination be selected on
|
||||
// purpose, so this is what stops it from being run.
|
||||
enabled = validation.isValid,
|
||||
@@ -156,7 +237,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
.testTag(TestTags.Converter.CONVERT),
|
||||
) { Text("Convert") }
|
||||
OutlinedButton(
|
||||
onClick = { pickInput.launch(arrayOf("*/*")) },
|
||||
onClick = actions.onPickInput,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Converter.CHOOSE_DIFFERENT_FILE),
|
||||
@@ -173,7 +254,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
.testTag(TestTags.Converter.PROGRESS),
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
onClick = actions.onCancel,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
@@ -192,7 +273,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
style = MaterialTheme.typography.bodyMedium,
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
onClick = actions.onCancel,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
@@ -208,17 +289,21 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
// explains why a job was slow, makes the software fallback
|
||||
// visible, and is how the user learns a remux happened rather
|
||||
// than a re-encode.
|
||||
AssistChip(onClick = {}, label = { Text(s.routeReason) })
|
||||
AssistChip(
|
||||
onClick = {},
|
||||
label = { Text(s.routeReason) },
|
||||
modifier = Modifier.testTag(TestTags.Converter.ROUTE_REASON),
|
||||
)
|
||||
}
|
||||
Button(
|
||||
onClick = { chooseDestination.launch(s.suggestedName) },
|
||||
onClick = { actions.onSave(s.suggestedName) },
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.SAVE_FILE),
|
||||
) { Text("Save file") }
|
||||
OutlinedButton(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
|
||||
) { Text("Start over") }
|
||||
}
|
||||
@@ -226,7 +311,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
is ConversionState.Saved -> {
|
||||
Text("Saved ${s.displayName}.", style = MaterialTheme.typography.bodyLarge)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
@@ -241,7 +326,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
style = MaterialTheme.typography.bodyMedium,
|
||||
)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
|
||||
@@ -138,31 +138,6 @@ class Media3Engine(private val context: Context) : HardwareTranscoder {
|
||||
.build()
|
||||
}
|
||||
|
||||
/**
|
||||
* Media3 encodes only H.264 and H.265 of the codecs this app offers.
|
||||
*
|
||||
* VP8/VP9/AV1 targets never reach here — the router sends them to FFmpeg because
|
||||
* `Transformer.setVideoMimeType` rejects them — so anything unexpected returns null and lets
|
||||
* Transformer pick, rather than silently substituting H.265 the way the old mapping did.
|
||||
*/
|
||||
private fun videoMimeTypeFor(codec: VideoCodec): String? = when (codec) {
|
||||
VideoCodec.H264 -> MimeTypes.VIDEO_H264
|
||||
VideoCodec.H265 -> MimeTypes.VIDEO_H265
|
||||
// Never reached: only an Encode plan consults this, and COPY/NONE are not Encode.
|
||||
VideoCodec.COPY, VideoCodec.NONE -> null
|
||||
VideoCodec.VP8, VideoCodec.VP9, VideoCodec.AV1 -> null
|
||||
}
|
||||
|
||||
private fun audioMimeTypeFor(codec: AudioCodec): String? = when (codec) {
|
||||
AudioCodec.AAC -> MimeTypes.AUDIO_AAC
|
||||
AudioCodec.OPUS -> MimeTypes.AUDIO_OPUS
|
||||
AudioCodec.VORBIS -> MimeTypes.AUDIO_VORBIS
|
||||
AudioCodec.PCM -> MimeTypes.AUDIO_RAW
|
||||
AudioCodec.COPY, AudioCodec.NONE -> null
|
||||
// MP3 and FLAC have no Android encoder; the router routes them to FFmpeg.
|
||||
AudioCodec.MP3, AudioCodec.FLAC -> null
|
||||
}
|
||||
|
||||
/**
|
||||
* Polls export progress on the Transformer's own thread.
|
||||
*
|
||||
@@ -192,7 +167,57 @@ class Media3Engine(private val context: Context) : HardwareTranscoder {
|
||||
thread.quitSafely()
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/**
|
||||
* The progress interval, and the two enum-to-MIME tables.
|
||||
*
|
||||
* The tables are pure functions of a codec enum, so they sit here rather than on the instance:
|
||||
* a JVM test can then exercise every arm without constructing an engine, which would start a
|
||||
* real [HandlerThread] to answer a lookup. `internal` rather than `private` for the reason
|
||||
* `MainActivity`'s `Destination` records — the JVM test source set is a friend of `main`, so
|
||||
* these stay invisible to anything outside the module.
|
||||
*/
|
||||
internal companion object {
|
||||
const val PROGRESS_INTERVAL_MS = 250L
|
||||
|
||||
/**
|
||||
* Media3 encodes only H.264 and H.265 of the codecs this app offers.
|
||||
*
|
||||
* VP8/VP9/AV1 targets never reach here — the router sends them to FFmpeg because
|
||||
* `Transformer.setVideoMimeType` rejects them — so anything unexpected returns null and
|
||||
* lets Transformer pick, rather than silently substituting H.265 as the old mapping did.
|
||||
*/
|
||||
internal fun videoMimeTypeFor(codec: VideoCodec): String? = when (codec) {
|
||||
VideoCodec.H264 -> MimeTypes.VIDEO_H264
|
||||
VideoCodec.H265 -> MimeTypes.VIDEO_H265
|
||||
// Never reached, and no longer only asserted: `Media3EngineMimeTypesTest` drives
|
||||
// `CopyPlanner` over every spec it can be handed and shows that no Encode plan carries
|
||||
// either, which is what turns "COPY/NONE are not Encode" into a checked claim.
|
||||
VideoCodec.COPY, VideoCodec.NONE -> null
|
||||
VideoCodec.VP8, VideoCodec.VP9, VideoCodec.AV1 -> null
|
||||
}
|
||||
|
||||
/**
|
||||
* Media3 encodes AAC, Opus and PCM. Three arms below are dead, not two.
|
||||
*
|
||||
* The comment this replaces named MP3 and FLAC as the exceptions, which reads as though
|
||||
* every other arm were live. **Vorbis is not.** A single router rule diverts every audio
|
||||
* codec outside {AAC, Opus, PCM} to FFmpeg, and Vorbis is outside it, so
|
||||
* `VORBIS -> AUDIO_VORBIS` names a MIME type Transformer is never actually asked for.
|
||||
*
|
||||
* The arm stays because the mapping is correct — deleting a right answer out of
|
||||
* unreachable code buys nothing — but it is an entry waiting on a routing change rather
|
||||
* than a live one. `Media3EngineMimeTypesTest` routes all six encodable codecs and asserts
|
||||
* which three arrive, so if that set moves, the disagreement fails rather than surprises.
|
||||
*/
|
||||
internal fun audioMimeTypeFor(codec: AudioCodec): String? = when (codec) {
|
||||
AudioCodec.AAC -> MimeTypes.AUDIO_AAC
|
||||
AudioCodec.OPUS -> MimeTypes.AUDIO_OPUS
|
||||
AudioCodec.VORBIS -> MimeTypes.AUDIO_VORBIS
|
||||
AudioCodec.PCM -> MimeTypes.AUDIO_RAW
|
||||
AudioCodec.COPY, AudioCodec.NONE -> null
|
||||
// MP3 and FLAC have no Android encoder at any API level, so the router sends them to
|
||||
// FFmpeg before an encoder is ever asked for.
|
||||
AudioCodec.MP3, AudioCodec.FLAC -> null
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -242,8 +242,20 @@ object MediaProbe {
|
||||
else -> Container.MKV
|
||||
}
|
||||
|
||||
/** FFprobe describes still images through the image demuxers rather than a media container. */
|
||||
private fun isImageFormat(formatName: String): Boolean {
|
||||
/**
|
||||
* FFprobe describes still images through the image demuxers rather than a media container.
|
||||
*
|
||||
* The two halves of the rule are not interchangeable. `image2` is a whole name — what FFprobe
|
||||
* reports for a numbered image sequence — while `_pipe` has to be a *suffix* test, because the
|
||||
* piped demuxers are named one per image codec: `png_pipe`, `jpeg_pipe`, `webp_pipe`, and
|
||||
* thirty more. Relaxing that suffix to a substring would swallow `yuv4mpegpipe`, which is raw
|
||||
* video, and `classify` checks this before anything else — so a false positive makes the
|
||||
* source-info card describe a video as an image.
|
||||
*
|
||||
* `internal` so the unit tests can name both halves; the JVM test source set is a friend of
|
||||
* `main`, so this stays invisible outside the module.
|
||||
*/
|
||||
internal fun isImageFormat(formatName: String): Boolean {
|
||||
val names = formatName.split(',').map { it.trim().lowercase() }
|
||||
return names.any { it == "image2" || it.endsWith("_pipe") }
|
||||
}
|
||||
@@ -284,11 +296,35 @@ object MediaProbe {
|
||||
}
|
||||
}
|
||||
|
||||
private fun MediaFormat.intOr(key: String, fallback: Int = 0): Int =
|
||||
/**
|
||||
* One track property as an Int, or [fallback] when the format has no Int to give.
|
||||
*
|
||||
* `containsKey` alone is not enough, because `MediaFormat` is a heterogeneous map: a key it
|
||||
* holds as a Float answers `getInteger` with a `ClassCastException` rather than a coercion, and
|
||||
* `KEY_FRAME_RATE` — which [probeForConcat] reads — is legitimately set either way. The
|
||||
* `runCatching` is therefore load-bearing rather than defensive. Without it a single
|
||||
* oddly-typed field throws past the whole track loop, and the catch there answers with an empty
|
||||
* [ConcatInput], discarding the codec and dimensions that had already been read.
|
||||
*
|
||||
* `internal` for the unit tests, as [shortName].
|
||||
*/
|
||||
internal fun MediaFormat.intOr(key: String, fallback: Int = 0): Int =
|
||||
if (containsKey(key)) runCatching { getInteger(key) }.getOrDefault(fallback) else fallback
|
||||
|
||||
/** MediaFormat MIME -> the short codec names the router and FFmpeg both speak. */
|
||||
private fun shortName(mime: String): String = when (mime) {
|
||||
/**
|
||||
* MediaFormat MIME -> the short codec names the router and FFmpeg both speak.
|
||||
*
|
||||
* A lookup table over platform constants is the shape that rots quietly. Most of these arms are
|
||||
* translations rather than trimming — `video/avc` is `h264`, `audio/mp4a-latm` is `aac`,
|
||||
* `video/x-vnd.on2.vp9` is `vp9` — so a dropped arm does not fail. It falls through to
|
||||
* `substringAfter('/')` and reports a different, plausible-looking string that
|
||||
* `CodecNames` may or may not still recognise, and an unrecognised codec is how a
|
||||
* stream-copyable file quietly becomes a re-encode.
|
||||
*
|
||||
* `internal` so the unit tests can name every arm; the JVM test source set is a friend of
|
||||
* `main`, so this stays invisible outside the module.
|
||||
*/
|
||||
internal fun shortName(mime: String): String = when (mime) {
|
||||
MediaFormat.MIMETYPE_VIDEO_AVC -> "h264"
|
||||
MediaFormat.MIMETYPE_VIDEO_HEVC -> "hevc"
|
||||
MediaFormat.MIMETYPE_VIDEO_VP8 -> "vp8"
|
||||
|
||||
@@ -53,6 +53,46 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
remember(destinationMime) { ActivityResultContracts.CreateDocument(destinationMime) },
|
||||
) { uri -> uri?.let(viewModel::save) }
|
||||
|
||||
JoinScreenContent(
|
||||
state = state,
|
||||
actions = JoinActions(
|
||||
onPickInputs = { pickInputs.launch(arrayOf("video/*")) },
|
||||
onJoin = viewModel::join,
|
||||
onCancel = viewModel::cancel,
|
||||
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
||||
onReset = viewModel::reset,
|
||||
),
|
||||
modifier = modifier,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Everything [JoinScreenContent] can ask for, in one value.
|
||||
*
|
||||
* Five callbacks would fit under detekt's `LongParameterList` threshold, unlike the converter's
|
||||
* twelve. It is a holder anyway, so both screens present the same shape to the state tests and
|
||||
* neither one has to be reworked the first time a branch grows a button.
|
||||
*/
|
||||
internal data class JoinActions(
|
||||
/** Open the multi-document picker. The `Idle` and `Ready` branches both offer it. */
|
||||
val onPickInputs: () -> Unit,
|
||||
val onJoin: () -> Unit,
|
||||
val onCancel: () -> Unit,
|
||||
/** Open the save dialog. Takes the name the job chose -- see `ConcatWorker.KEY_SUGGESTED_NAME`. */
|
||||
val onSave: (suggestedName: String) -> Unit,
|
||||
val onReset: () -> Unit,
|
||||
)
|
||||
|
||||
/**
|
||||
* The join screen, with its state handed in.
|
||||
*
|
||||
* The same split as [org.libremediaconverter.convert.ConverterScreenContent], for the same reason:
|
||||
* `JoinState.Waiting` follows a denied foreground start and `JoinState.Joined` follows a completed
|
||||
* concatenation, so neither is reachable by driving a real `JoinViewModel`.
|
||||
*/
|
||||
@UnstableApi
|
||||
@Composable
|
||||
internal fun JoinScreenContent(state: JoinState, actions: JoinActions, modifier: Modifier = Modifier) {
|
||||
Column(
|
||||
modifier = modifier
|
||||
.fillMaxSize()
|
||||
@@ -81,7 +121,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
modifier = Modifier.padding(bottom = 16.dp),
|
||||
)
|
||||
Button(
|
||||
onClick = { pickInputs.launch(arrayOf("video/*")) },
|
||||
onClick = actions.onPickInputs,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
@@ -99,14 +139,14 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
is JoinState.Ready -> {
|
||||
s.inputs.forEach { FileRow(it) }
|
||||
Button(
|
||||
onClick = viewModel::join,
|
||||
onClick = actions.onJoin,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.Join.JOIN),
|
||||
) { Text("Join ${s.inputs.size} files") }
|
||||
OutlinedButton(
|
||||
onClick = { pickInputs.launch(arrayOf("video/*")) },
|
||||
onClick = actions.onPickInputs,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Join.CHOOSE_DIFFERENT_FILES),
|
||||
@@ -124,7 +164,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
.testTag(TestTags.Join.PROGRESS),
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
onClick = actions.onCancel,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
@@ -138,7 +178,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
style = MaterialTheme.typography.bodyMedium,
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
onClick = actions.onCancel,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
@@ -157,14 +197,14 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
)
|
||||
Button(
|
||||
onClick = { chooseDestination.launch(s.suggestedName) },
|
||||
onClick = { actions.onSave(s.suggestedName) },
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.SAVE_FILE),
|
||||
) { Text("Save file") }
|
||||
OutlinedButton(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
|
||||
) { Text("Start over") }
|
||||
}
|
||||
@@ -172,7 +212,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
is JoinState.Saved -> {
|
||||
Text("Saved ${s.displayName}.", style = MaterialTheme.typography.bodyLarge)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
@@ -187,7 +227,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
style = MaterialTheme.typography.bodyMedium,
|
||||
)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
|
||||
@@ -15,36 +15,97 @@ package org.libremediaconverter.model
|
||||
*/
|
||||
object CodecNames {
|
||||
|
||||
fun videoFromName(name: String?): VideoCodec? = when (name?.lowercase()) {
|
||||
null, InputProbe.UNPARSEABLE -> null
|
||||
"h264", "avc", "avc1", "x264" -> VideoCodec.H264
|
||||
"hevc", "h265", "hvc1", "hev1", "x265" -> VideoCodec.H265
|
||||
"vp8" -> VideoCodec.VP8
|
||||
"vp9", "vp09" -> VideoCodec.VP9
|
||||
"av1", "av01" -> VideoCodec.AV1
|
||||
else -> null
|
||||
}
|
||||
/**
|
||||
* The video vocabulary, as data rather than a `when`.
|
||||
*
|
||||
* This is not the only place the app spells these names. `AndroidDeviceCodecs` reads the same
|
||||
* FFprobe strings to decide what the device can decode, and answers in platform MIME types,
|
||||
* which `model` cannot name without depending on Android. The two copies drifted apart:
|
||||
* `x264`, `hev1`, `x265` and `vp09` resolved here and returned null there, so the app
|
||||
* identified the codec for display and routing and then ran the device check blind, attempting
|
||||
* a hardware path it had enough information to skip (#87).
|
||||
*
|
||||
* The reason this is a map is that **a `when` cannot be enumerated**, so nothing could compare
|
||||
* the two tables. `CodecVocabularyTest` walks both key sets, so a name added to or removed
|
||||
* from one side alone now fails the build rather than waiting for a wasted transcode to show
|
||||
* it.
|
||||
*
|
||||
* Keys are lowercase; [videoFromName] lowercases before looking one up.
|
||||
*/
|
||||
internal val VIDEO_ALIASES: Map<String, VideoCodec> = mapOf(
|
||||
"h264" to VideoCodec.H264,
|
||||
"avc" to VideoCodec.H264,
|
||||
"avc1" to VideoCodec.H264,
|
||||
"x264" to VideoCodec.H264,
|
||||
"hevc" to VideoCodec.H265,
|
||||
"h265" to VideoCodec.H265,
|
||||
"hvc1" to VideoCodec.H265,
|
||||
"hev1" to VideoCodec.H265,
|
||||
"x265" to VideoCodec.H265,
|
||||
"vp8" to VideoCodec.VP8,
|
||||
"vp9" to VideoCodec.VP9,
|
||||
"vp09" to VideoCodec.VP9,
|
||||
"av1" to VideoCodec.AV1,
|
||||
"av01" to VideoCodec.AV1,
|
||||
)
|
||||
|
||||
fun audioFromName(name: String?): AudioCodec? = when (name?.lowercase()) {
|
||||
null -> null
|
||||
"aac", "mp4a", "aac_latm" -> AudioCodec.AAC
|
||||
"opus" -> AudioCodec.OPUS
|
||||
"vorbis" -> AudioCodec.VORBIS
|
||||
"mp3", "mp3float", "mpga" -> AudioCodec.MP3
|
||||
"flac" -> AudioCodec.FLAC
|
||||
"pcm", "raw", "pcm_s16le", "pcm_s24le", "pcm_f32le" -> AudioCodec.PCM
|
||||
else -> null
|
||||
}
|
||||
/**
|
||||
* The audio vocabulary, data for the same reason.
|
||||
*
|
||||
* Nothing cross-checks this one yet, and that is a gap rather than a decision: the device
|
||||
* capability check is video-only, so this module holds no second audio table to compare it
|
||||
* against. `Media3Engine.audioMimeTypeFor` is the other half, and #85 owns that file.
|
||||
*/
|
||||
internal val AUDIO_ALIASES: Map<String, AudioCodec> = mapOf(
|
||||
"aac" to AudioCodec.AAC,
|
||||
"mp4a" to AudioCodec.AAC,
|
||||
"aac_latm" to AudioCodec.AAC,
|
||||
"opus" to AudioCodec.OPUS,
|
||||
"vorbis" to AudioCodec.VORBIS,
|
||||
"mp3" to AudioCodec.MP3,
|
||||
"mp3float" to AudioCodec.MP3,
|
||||
"mpga" to AudioCodec.MP3,
|
||||
"flac" to AudioCodec.FLAC,
|
||||
"pcm" to AudioCodec.PCM,
|
||||
"raw" to AudioCodec.PCM,
|
||||
"pcm_s16le" to AudioCodec.PCM,
|
||||
"pcm_s24le" to AudioCodec.PCM,
|
||||
"pcm_f32le" to AudioCodec.PCM,
|
||||
)
|
||||
|
||||
fun videoFromName(name: String?): VideoCodec? = asCodecName(name)?.let(VIDEO_ALIASES::get)
|
||||
|
||||
fun audioFromName(name: String?): AudioCodec? = asCodecName(name)?.let(AUDIO_ALIASES::get)
|
||||
|
||||
/** Human-readable name for the source-info card. Falls back to the raw probe string. */
|
||||
fun describeVideo(name: String?): String = when {
|
||||
fun describeVideo(name: String?): String = describe(name) { videoFromName(it)?.label }
|
||||
|
||||
fun describeAudio(name: String?): String = describe(name) { audioFromName(it)?.label }
|
||||
|
||||
/**
|
||||
* Lowercases a probe string, and answers null for the two inputs that are not codec names at
|
||||
* all: absent, and the [InputProbe.UNPARSEABLE] sentinel.
|
||||
*
|
||||
* The sentinel would miss every key anyway, so naming it changes no answer. Naming it is still
|
||||
* the point: `videoFromName` excluded it explicitly and `audioFromName` did not, which read as
|
||||
* though the two disagreed about what the sentinel means — the same asymmetry as #74 one
|
||||
* function further up.
|
||||
*/
|
||||
private fun asCodecName(name: String?): String? =
|
||||
if (name == null || name == InputProbe.UNPARSEABLE) null else name.lowercase()
|
||||
|
||||
/**
|
||||
* The shared body of [describeVideo] and [describeAudio].
|
||||
*
|
||||
* They are one function apiece over one vocabulary, and they had stopped matching:
|
||||
* `describeVideo` answered "Unrecognised" for [InputProbe.UNPARSEABLE] and `describeAudio` fell
|
||||
* through to `?: name` instead. The sentinel opens with a NUL, so that fallback would have put
|
||||
* a U+0000 into a `Text` on the source-info card (#74). Sharing the arms is what stops the next
|
||||
* one being added to one side only.
|
||||
*/
|
||||
private fun describe(name: String?, label: (String) -> String?): String = when {
|
||||
name == null -> "Unknown"
|
||||
name == InputProbe.UNPARSEABLE -> "Unrecognised"
|
||||
else -> videoFromName(name)?.label ?: name
|
||||
}
|
||||
|
||||
fun describeAudio(name: String?): String = when {
|
||||
name == null -> "Unknown"
|
||||
else -> audioFromName(name)?.label ?: name
|
||||
else -> label(name) ?: name
|
||||
}
|
||||
}
|
||||
|
||||
@@ -55,6 +55,15 @@ object TestTags {
|
||||
/** The determinate bar in `Converting`. It carries no text, so nothing else can find it. */
|
||||
const val PROGRESS: String = "converter.progress"
|
||||
|
||||
/**
|
||||
* The chip on `Converted` that says which engine ran the job and why.
|
||||
*
|
||||
* Conditional on `routeReason` being non-blank, and that condition is what the tag is for:
|
||||
* its text comes from the finished job, so a text matcher looking for it would have to
|
||||
* name a routing explanation the screen does not own.
|
||||
*/
|
||||
const val ROUTE_REASON: String = "converter.routeReason"
|
||||
|
||||
const val FILE_CARD: String = "converter.fileCard"
|
||||
const val FILE_CARD_NAME: String = "converter.fileCard.name"
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ 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
|
||||
@@ -33,19 +34,22 @@ import org.robolectric.RobolectricTestRunner
|
||||
* representation survives a `Bundle` round trip. A JVM round-trip test on the
|
||||
* saver covers the representation.
|
||||
*
|
||||
* Robolectric rather than the instrumented suite, deliberately. The instrumented tests
|
||||
* cannot run on the development host at all (see CLAUDE.md), and a red test nobody can
|
||||
* execute is not a loop anyone can work in.
|
||||
* Robolectric rather than the instrumented suite, deliberately -- but not because the
|
||||
* instrumented suite is unavailable. It runs on this host for API 33-36
|
||||
* (`tools/local-emulator/run-e2e.sh`), and CI runs 33-37. The reason is cost: this test
|
||||
* needs a composition and a saved-state round trip, nothing a device supplies, and it runs
|
||||
* in the same `./gradlew` invocation as every other JVM test instead of booting an
|
||||
* emulator. A loop measured in seconds is a loop people stay inside.
|
||||
*/
|
||||
@UnstableApi
|
||||
@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.
|
||||
// The rule is the **v2** one (`androidx.compose.ui.test.junit4.v2`) while
|
||||
// [StateRestorationTester], which takes it below, is not. The mismatched imports are
|
||||
// deliberate: the v2 package has no tester of its own and the two do interoperate.
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
private val restoration = StateRestorationTester(composeRule)
|
||||
|
||||
|
||||
@@ -1,53 +0,0 @@
|
||||
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,143 @@
|
||||
package org.libremediaconverter.codec
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNotNull
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import org.libremediaconverter.model.CodecNames
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
|
||||
/**
|
||||
* Bites on #87: two tables read one codec vocabulary and had stopped agreeing.
|
||||
*
|
||||
* `CodecNames.VIDEO_ALIASES` answers "which enum is this FFprobe name", for the source-info card
|
||||
* and for routing. `AndroidDeviceCodecs.NAME_TO_MIME` answers "which MIME do I ask this device
|
||||
* about", for the capability check. On `ad28293` five names lived in one and not the other: `x264`,
|
||||
* `hev1`, `x265` and `vp09` were identified for display and then fell through the device check as
|
||||
* unknown, so the app attempted a hardware path it had enough information to skip; `mpeg4` ran the
|
||||
* other way and rendered as a raw name on the card.
|
||||
*
|
||||
* Per-table arm tests would have passed on both tables and encoded the disagreement, which is why
|
||||
* these walk the key sets instead. A name added to — or removed from — one side alone fails here.
|
||||
*/
|
||||
class CodecVocabularyTest {
|
||||
|
||||
private val aliases = CodecNames.VIDEO_ALIASES
|
||||
private val mimes = AndroidDeviceCodecs.NAME_TO_MIME
|
||||
private val decodeOnly = AndroidDeviceCodecs.DECODE_ONLY_NAMES
|
||||
|
||||
@Test
|
||||
fun `no video codec name resolves for display without also resolving for the device check`() {
|
||||
assertEquals(
|
||||
"resolve in CodecNames but return null from mimeForCodecName, so the device check runs blind",
|
||||
emptySet<String>(),
|
||||
aliases.keys - mimes.keys,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `no video codec name resolves for the device check without being a name the app can label`() {
|
||||
assertEquals(
|
||||
"resolve in AndroidDeviceCodecs but not in CodecNames, and are not listed as decode-only",
|
||||
emptySet<String>(),
|
||||
mimes.keys - aliases.keys - decodeOnly,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Membership is not enough: `"x265" to MIMETYPE_VIDEO_AVC` would satisfy both key sets and
|
||||
* still ask the device about the wrong codec.
|
||||
*/
|
||||
@Test
|
||||
fun `the two tables agree on what each name means, not merely that they know it`() {
|
||||
aliases.forEach { (name, codec) ->
|
||||
val expected = AndroidDeviceCodecs.mimeFor(codec)
|
||||
assertNotNull("$name maps to $codec, which has no MIME to ask about", expected)
|
||||
assertEquals("$name is $codec in CodecNames", expected, mimes[name])
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The exception list is the escape hatch: any future divergence could be waved through by
|
||||
* adding the name to it. Guard both directions so it cannot be.
|
||||
*/
|
||||
@Test
|
||||
fun `the decode-only names are genuinely decode-only`() {
|
||||
decodeOnly.forEach { name ->
|
||||
assertNotNull("$name is listed as decode-only but the device check cannot resolve it", mimes[name])
|
||||
assertNull(
|
||||
"$name is listed as decode-only, but CodecNames does resolve it — that is a divergence " +
|
||||
"being waved through rather than a documented exception",
|
||||
CodecNames.videoFromName(name),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The five names #87 measured, pinned by name so the specific regression cannot come back
|
||||
* quietly even if someone rewrites the tables above.
|
||||
*/
|
||||
@Test
|
||||
fun `the names that used to resolve on one side only resolve on both`() {
|
||||
mapOf(
|
||||
"x264" to VideoCodec.H264,
|
||||
"hev1" to VideoCodec.H265,
|
||||
"x265" to VideoCodec.H265,
|
||||
"vp09" to VideoCodec.VP9,
|
||||
).forEach { (name, codec) ->
|
||||
assertEquals("$name is a name FFmpeg emits", codec, CodecNames.videoFromName(name))
|
||||
assertEquals(
|
||||
"$name has to reach the device check too, or the app identifies it and then asks blind",
|
||||
AndroidDeviceCodecs.mimeFor(codec),
|
||||
AndroidDeviceCodecs.mimeForCodecName(name),
|
||||
)
|
||||
}
|
||||
// The one that runs the other way: decodable input with no enum to name it.
|
||||
assertNull("mpeg4 is not an output the app can target", CodecNames.videoFromName("mpeg4"))
|
||||
assertNotNull("mpeg4 is still decodable input", AndroidDeviceCodecs.mimeForCodecName("mpeg4"))
|
||||
}
|
||||
|
||||
/**
|
||||
* Without this the agreement test above could pass on two nulls.
|
||||
*
|
||||
* `MediaFormat.MIMETYPE_VIDEO_AVC` is a Java compile-time constant, so it is inlined and the
|
||||
* unit-test classpath's stubbed `android.jar` never has to supply it. If that ever stops being
|
||||
* true, every MIME comparison here would be `null == null` and green — the vacuous-mutation
|
||||
* failure this repo has counted before. Assert one literal so the stub fails loudly instead.
|
||||
*/
|
||||
@Test
|
||||
fun `the MIME constants are real strings rather than stubs`() {
|
||||
assertEquals("video/avc", AndroidDeviceCodecs.mimeForCodecName("h264"))
|
||||
assertEquals("video/hevc", AndroidDeviceCodecs.mimeForCodecName("hevc"))
|
||||
assertEquals("video/avc", AndroidDeviceCodecs.mimeFor(VideoCodec.H264))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `codec names are matched case-insensitively on both sides`() {
|
||||
assertEquals(VideoCodec.H265, CodecNames.videoFromName("HEV1"))
|
||||
assertEquals("video/hevc", AndroidDeviceCodecs.mimeForCodecName("HEV1"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a name neither table knows still resolves to nothing`() {
|
||||
assertNull(CodecNames.videoFromName("cinepak"))
|
||||
assertNull(AndroidDeviceCodecs.mimeForCodecName("cinepak"))
|
||||
}
|
||||
|
||||
/**
|
||||
* The behaviour #87 actually changes, at the seam that uses it.
|
||||
*
|
||||
* `canDecode` treats an unresolved name as "assume the platform copes". Before the alias
|
||||
* landed, a device with no HEVC decoder answered true for `x265` and Media3 was handed a job it
|
||||
* could not do; now the router sends it to FFmpeg without spending the attempt.
|
||||
*/
|
||||
@Test
|
||||
fun `a device without the decoder now says so for the aliases it used to wave through`() {
|
||||
val hevcOnly = AndroidDeviceCodecs.forTesting(encoders = emptySet(), decoders = setOf("video/hevc"))
|
||||
assertTrue("x265 is HEVC by another name", hevcOnly.canDecode("x265"))
|
||||
assertFalse("this device has no AVC decoder, and x264 is AVC", hevcOnly.canDecode("x264"))
|
||||
assertTrue("a name nobody knows keeps the permissive answer", hevcOnly.canDecode("cinepak"))
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,158 @@
|
||||
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.junit4.v2.createComposeRule
|
||||
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.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 {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
/**
|
||||
* `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,323 @@
|
||||
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.junit4.v2.createComposeRule
|
||||
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.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 {
|
||||
|
||||
// The rule is the **v2** one (`androidx.compose.ui.test.junit4.v2`) while
|
||||
// [StateRestorationTester], which takes it below, is not. The mismatched imports are
|
||||
// deliberate: the v2 package has no tester of its own and the two do interoperate.
|
||||
@get:Rule
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
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,
|
||||
)
|
||||
}
|
||||
}
|
||||
+25
-26
@@ -2,14 +2,15 @@ package org.libremediaconverter.convert
|
||||
|
||||
import android.app.Application
|
||||
import android.net.Uri
|
||||
import android.os.Looper
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.workDataOf
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNotNull
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertThrows
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
@@ -18,8 +19,6 @@ import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.work.ConversionWorker
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import org.robolectric.Shadows.shadowOf
|
||||
import java.util.concurrent.TimeUnit
|
||||
|
||||
/**
|
||||
* That a probe which throws leaves a screen the user can act on, not a dead coroutine.
|
||||
@@ -87,19 +86,35 @@ class ConversionViewModelProbeFailureTest {
|
||||
* is out of memory" into "this video looks unreadable" and let the app carry on in a
|
||||
* state it cannot honour — which is the regression a blanket `catch (Throwable)` would
|
||||
* have introduced, and the reason this defect was left open rather than fixed carelessly.
|
||||
*
|
||||
* **The error itself is what is asserted here, and that is what the `pickDispatcher` seam
|
||||
* bought.** With the hop hard-coded to `Dispatchers.IO` this was impossible: the throw
|
||||
* happened on a pool thread some time after this method had returned, so all a test could do
|
||||
* was infer it from a card that never filled in — which is also what a probe returning null
|
||||
* would look like. Worse, the escaped error went into kotlinx-coroutines-test's process-wide
|
||||
* collector and was rethrown at whichever `runTest` started next, which is a *different*
|
||||
* Compose class between runs of identical code. Putting the pick on [Dispatchers.Unconfined]
|
||||
* runs it inline, inside a `runTest` whose scope owns the collector's callback: the error is
|
||||
* handed to this test and consumed, rather than stored for a stranger.
|
||||
*
|
||||
* Note where it surfaces — at the end of `runTest`, not inside `onInputPicked`. `launch`
|
||||
* gives an escaped error to the handler chain and never to its caller, so nothing can catch
|
||||
* it at the call itself. This is as close as the coroutine machinery allows, and unlike the
|
||||
* old assertion it is the real [OutOfMemoryError] instance.
|
||||
*/
|
||||
@Test
|
||||
fun `an OutOfMemoryError is not swallowed`() {
|
||||
ConversionDependencies.probe = { _, _ -> throw OutOfMemoryError("Failed to allocate 512 MB") }
|
||||
// Unconfined for the pick, so the whole of onInputPicked runs inline on this thread and
|
||||
// has thrown before runTest can leave the scope that has to receive the error.
|
||||
val viewModel = ConversionViewModel(app, Dispatchers.Unconfined, Dispatchers.Unconfined)
|
||||
|
||||
val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
|
||||
viewModel.onInputPicked(INPUT)
|
||||
val escaped = assertThrows(OutOfMemoryError::class.java) { runTest { viewModel.onInputPicked(INPUT) } }
|
||||
|
||||
// The observable difference, and the reason this is asserted on state rather than on a
|
||||
// caught throwable: the probe hop is on Dispatchers.IO, so an error that escapes lands
|
||||
// on that thread's handler rather than at this call. What must not happen is the card
|
||||
// filling in with an "unreadable" verdict the app would then act on.
|
||||
val settled = settle(viewModel)
|
||||
assertEquals("Failed to allocate 512 MB", escaped.message)
|
||||
// The other half of the contract, unchanged: an OOM is about the process, so the card is
|
||||
// left as it was rather than filled in with a verdict the app would then act on.
|
||||
val settled = viewModel.state.value
|
||||
// `sizeBytes = null`, not `0L`: no provider is registered for this authority, so the
|
||||
// metadata query returns nothing and the descriptor cannot be opened either. That is the
|
||||
// unknown, and it stopped being spelled the same way as "empty" -- see [InputQuery].
|
||||
@@ -132,23 +147,7 @@ class ConversionViewModelProbeFailureTest {
|
||||
return (ready as ConversionState.Ready).input.probe
|
||||
}
|
||||
|
||||
/**
|
||||
* Pumps the looper the way [awaitState] does, but for a fixed span and without requiring
|
||||
* anything to happen — here "the pick never came back" is the expected outcome, so there
|
||||
* is no predicate to wait on.
|
||||
*/
|
||||
private fun settle(viewModel: ConversionViewModel): ConversionState {
|
||||
val deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(SETTLE_MS)
|
||||
while (System.nanoTime() < deadline) {
|
||||
shadowOf(Looper.getMainLooper()).idle()
|
||||
Thread.sleep(POLL_MS)
|
||||
}
|
||||
return viewModel.state.value
|
||||
}
|
||||
|
||||
private companion object {
|
||||
val INPUT: Uri = Uri.parse("content://test/holiday.mp4")
|
||||
const val SETTLE_MS = 500L
|
||||
const val POLL_MS = 5L
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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() })
|
||||
}
|
||||
}
|
||||
@@ -2,6 +2,7 @@ package org.libremediaconverter.convert
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.compose.ui.test.assertCountEquals
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
import androidx.compose.ui.test.onAllNodesWithTag
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performClick
|
||||
@@ -9,7 +10,6 @@ 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
|
||||
@@ -47,7 +47,7 @@ import org.robolectric.RobolectricTestRunner
|
||||
class ConverterLeafTagsTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
private fun assertResolvesToOneNode(tag: String) {
|
||||
composeRule.onAllNodesWithTag(tag).assertCountEquals(1)
|
||||
|
||||
@@ -6,6 +6,7 @@ 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.junit4.v2.createComposeRule
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
@@ -13,7 +14,6 @@ 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
|
||||
@@ -48,7 +48,7 @@ import org.robolectric.RobolectricTestRunner
|
||||
class ConverterPickerSelectionTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
/**
|
||||
* The chip carrying [label] inside the row tagged [rowTag].
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.compose.ui.test.performScrollTo
|
||||
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.model.Validation
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* The seam carries a `ConversionState` in and an action back out.
|
||||
*
|
||||
* The defect this bites on is the extraction having quietly stopped being an extraction: a
|
||||
* `ConverterScreenContent` that ignores the `state` it was handed, or renders the finished job's
|
||||
* affordances without wiring them to the callbacks the entry point supplies. Neither shows up at
|
||||
* compile time -- an unread parameter compiles, and a `Button` whose `onClick` does nothing is a
|
||||
* valid `Button` -- and neither is visible from the leaf tests, which compose `FileCard`,
|
||||
* `AdvancedPicker` and the pickers directly and never see a state at all.
|
||||
*
|
||||
* **Both assertions were unreachable before R38.5**, which is the point of the ticket rather than
|
||||
* a remark about it. `ConversionState.Converted` is produced only by a `ConversionWorker` run that
|
||||
* has already succeeded, so no test can drive a real `ConversionViewModel` into it: it would need
|
||||
* a `WorkManager`, a media probe, a staged output file and a completed job. Handing the state in
|
||||
* is the only way to ask what the screen does with it.
|
||||
*
|
||||
* Deliberately not the state matrix. Which affordances each of the six `ConversionState`s renders
|
||||
* is R38.6 (#62); this file asserts only that the injection point exists and works in both
|
||||
* directions, so the two PRs cannot collide over the same cases.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class ConverterScreenContentTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
/** What the screen asked to save, in the order it asked. Empty until Save is tapped. */
|
||||
private val savedAs = mutableListOf<String>()
|
||||
|
||||
@Test
|
||||
fun `a converted job renders the save button`() {
|
||||
setContent(converted())
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).assertExists()
|
||||
}
|
||||
|
||||
/**
|
||||
* The direction that did not exist before this change.
|
||||
*
|
||||
* Asserting the *name* rather than just that something was called: the suggested name comes
|
||||
* from the job -- `ConversionWorker.KEY_SUGGESTED_NAME` -- and is what the save dialog opens
|
||||
* with, so a Save button wired to the wrong branch's state would hand over the wrong one and
|
||||
* a bare "was called" check would stay green.
|
||||
*/
|
||||
@Test
|
||||
fun `tapping save hands back the name the finished job chose`() {
|
||||
setContent(converted())
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("holiday.mp4"), savedAs)
|
||||
}
|
||||
|
||||
/**
|
||||
* `staged` names a file that does not exist, on purpose.
|
||||
*
|
||||
* The branch renders `formatBytes(s.staged.length())`, and `length()` answers `0L` for a
|
||||
* missing path rather than throwing, so the size line reads `0 B` and no temporary folder is
|
||||
* needed. `routeReason` stays blank, which is what keeps the routing chip out of the tree --
|
||||
* that chip is R38.6's case, not this file's.
|
||||
*/
|
||||
private fun converted() = ConversionState.Converted(
|
||||
input = InputFile(
|
||||
uri = Uri.parse("content://test/holiday.mkv"),
|
||||
displayName = "holiday.mkv",
|
||||
sizeBytes = 12_345_678L,
|
||||
),
|
||||
staged = File("no-such-staged-output.mp4"),
|
||||
suggestedName = "holiday.mp4",
|
||||
mimeType = "video/mp4",
|
||||
)
|
||||
|
||||
private fun setContent(state: ConversionState) {
|
||||
composeRule.setContent {
|
||||
ConverterScreenContent(
|
||||
state = state,
|
||||
settings = ConversionSettings(),
|
||||
validation = Validation.Valid,
|
||||
actions = ConverterActions(
|
||||
onPickInput = {},
|
||||
onPreset = {},
|
||||
onContainer = {},
|
||||
onVideoCodec = {},
|
||||
onAudioCodec = {},
|
||||
onSuggestion = {},
|
||||
onQuality = {},
|
||||
onEnginePreference = {},
|
||||
onConvert = {},
|
||||
onCancel = {},
|
||||
onSave = { suggestedName -> savedAs += suggestedName },
|
||||
onReset = {},
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,402 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.compose.ui.semantics.ProgressBarRangeInfo
|
||||
import androidx.compose.ui.test.assertIsEnabled
|
||||
import androidx.compose.ui.test.assertIsNotEnabled
|
||||
import androidx.compose.ui.test.assertRangeInfoEquals
|
||||
import androidx.compose.ui.test.assertTextEquals
|
||||
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
|
||||
import androidx.compose.ui.test.performScrollTo
|
||||
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.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
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* Every `ConversionState` renders its own affordances, and only its own.
|
||||
*
|
||||
* The defect this bites on is a `when` arm that has drifted from the state it names: a button
|
||||
* offered in a state where it cannot work, a state's own data never reaching the node that is
|
||||
* supposed to display it, or an affordance wired to the wrong callback. None of that is a compile
|
||||
* error -- every arm of the `when` returns `Unit`, so an arm can render anything at all -- and none
|
||||
* of it is visible from the leaf tests, which compose `FileCard`, `AdvancedPicker` and the three
|
||||
* pickers directly and never see a `ConversionState`.
|
||||
*
|
||||
* The arm most worth guarding is `Ready`'s `enabled = validation.isValid`. The Advanced picker
|
||||
* deliberately lets an impossible container / codec combination be selected -- `AdvancedPicker`'s
|
||||
* KDoc says teaching the constraint beats hiding it -- so that single expression is the only thing
|
||||
* standing between an invalid spec and a job that cannot succeed. `enabled = true` compiles, renders
|
||||
* an identical screen apart from one colour, and passes every other test in this suite.
|
||||
*
|
||||
* Callbacks are asserted by **identity, over the whole log**: [fired] records all twelve of them and
|
||||
* each assertion compares the complete list against one expected entry. A bare "the callback ran"
|
||||
* check stays green when an arm fires the right callback for the wrong reason, and a check on one
|
||||
* callback alone stays green when an arm fires two.
|
||||
*
|
||||
* ### Not asserted here, so that each is a decision rather than an omission
|
||||
*
|
||||
* - **`Failed`'s error colour.** #62's table asks for the message "in the error colour". Compose
|
||||
* publishes no text colour to the semantics tree -- there is no `SemanticsProperties` entry for
|
||||
* it -- so it is unobservable from a JVM test, the same limit `FileCardTest` records for
|
||||
* `HorizontalDivider`. The message text itself is asserted; the colour would need a screenshot.
|
||||
* - **The three `assertDoesNotExist` checks on [TestTags.Converter.FILE_CARD] are compile-guarded,
|
||||
* not guarded by this file.** `Idle` is a `data object`, and `Saved` and `Failed` carry only a
|
||||
* `displayName` and a `message`; none of the three has an `input`, so `FileCard(s.input)` does not
|
||||
* compile in those arms. The lines stay because they state the intent cheaply, but they are not
|
||||
* what stops a `FileCard` appearing there and this file does not claim they are.
|
||||
* - **Which constant each chip hands back** belongs to `ConverterPickerSelectionTest`, and **what
|
||||
* the file card says about an unknown size** to `FileCardTest`. This file asserts that `Ready`
|
||||
* puts those leaves on screen at all, not what they then do.
|
||||
* - **The suggested name `Converted` hands to the save dialog** is pinned by
|
||||
* `ConverterScreenContentTest`; repeating it here would be a second copy of one assertion.
|
||||
* - **`ConverterScreen`'s permission dance.** `requestNotifications` calls `convert()` on both grant
|
||||
* and deny, deliberately -- the KDoc explains that the foreground service runs either way -- and
|
||||
* it lives in the entry point, above the seam this file composes.
|
||||
* - **`is ConversionState.Idle -> Unit` in the nested `when`.** The outer `when` peels `Idle` off
|
||||
* first, so that arm is permanently unreachable and no test can reach it.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class ConverterStateAffordancesTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
/**
|
||||
* Every callback the screen fired, in order, tagged with the value it carried.
|
||||
*
|
||||
* All twelve are recorded rather than only the one under test, so an assertion can be
|
||||
* `assertEquals(listOf("cancel"), fired)` -- which says "this one and nothing else".
|
||||
*/
|
||||
private val fired = mutableListOf<String>()
|
||||
|
||||
// -------------------------------------------------------------------- Idle
|
||||
|
||||
@Test
|
||||
fun `an idle screen offers the prompt and the picker, and nothing to act on yet`() {
|
||||
setContent(ConversionState.Idle)
|
||||
|
||||
composeRule.onNodeWithText("Pick a file to convert.").assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CONVERT).assertDoesNotExist()
|
||||
composeRule.onNodeWithTag(TestTags.CANCEL).assertDoesNotExist()
|
||||
// Compile-guarded rather than guarded here -- `Idle` has no `input`. See the class KDoc.
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD).assertDoesNotExist()
|
||||
}
|
||||
|
||||
/**
|
||||
* No [performScrollTo] on this one, unlike every other click below. `Idle` is the centred
|
||||
* branch outside the `verticalScroll` column, so it has no scrollable ancestor to scroll in.
|
||||
*/
|
||||
@Test
|
||||
fun `tapping choose file on an idle screen asks for a file and does nothing else`() {
|
||||
setContent(ConversionState.Idle)
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).performClick()
|
||||
|
||||
assertEquals(listOf("pickInput"), fired)
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------- Ready
|
||||
|
||||
/**
|
||||
* All four pickers, the card above them and both buttons below, in one assertion each.
|
||||
*
|
||||
* A superset of #62's "all five pickers": which four or five of these count as a picker is not
|
||||
* worth arguing about, so the case names everything the arm emits.
|
||||
*/
|
||||
@Test
|
||||
fun `a picked file offers its card, all four pickers and both buttons`() {
|
||||
setContent(ConversionState.Ready(input()))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FORMAT_CHIPS).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.QUALITY_CHIPS).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ENGINE_CHIPS).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CONVERT).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_DIFFERENT_FILE).assertExists()
|
||||
}
|
||||
|
||||
/** The card is handed `s.input`, so the name on it is how the state is shown to have arrived. */
|
||||
@Test
|
||||
fun `the file card on a picked file names the file that was picked`() {
|
||||
setContent(ConversionState.Ready(input()))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME).assertTextEquals("holiday.mkv")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `convert is offered for a spec that can be produced`() {
|
||||
setContent(ConversionState.Ready(input()), validation = Validation.Valid)
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CONVERT).assertIsEnabled()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `tapping convert starts the job and does nothing else`() {
|
||||
setContent(ConversionState.Ready(input()), validation = Validation.Valid)
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CONVERT).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("convert"), fired)
|
||||
}
|
||||
|
||||
/**
|
||||
* The bite named in #62. Reverting `enabled = validation.isValid` to `enabled = true` reddens
|
||||
* exactly this case, and nothing else in the repository.
|
||||
*/
|
||||
@Test
|
||||
fun `convert is withheld for a spec that cannot be produced`() {
|
||||
setContent(ConversionState.Ready(input()), validation = INVALID)
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CONVERT).assertIsNotEnabled()
|
||||
}
|
||||
|
||||
/** The other button on the arm goes back to the picker rather than starting anything. */
|
||||
@Test
|
||||
fun `tapping choose a different file asks for a file rather than converting`() {
|
||||
setContent(ConversionState.Ready(input()))
|
||||
|
||||
composeRule
|
||||
.onNodeWithTag(TestTags.Converter.CHOOSE_DIFFERENT_FILE)
|
||||
.performScrollTo()
|
||||
.performClick()
|
||||
|
||||
assertEquals(listOf("pickInput"), fired)
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------- Converting
|
||||
|
||||
/**
|
||||
* Two independent readings of the same `percent`, on purpose.
|
||||
*
|
||||
* The heading is a string and the bar is a float, and the arm computes them from the state
|
||||
* separately -- `"${s.percent}%"` against `s.percent / 100f`. A hardcoded bar and a hardcoded
|
||||
* heading are different mistakes, so neither assertion covers the other.
|
||||
*/
|
||||
@Test
|
||||
fun `a running job reports how far it has got, in words and on the bar`() {
|
||||
setContent(ConversionState.Converting(input(), percent = 42))
|
||||
|
||||
composeRule.onNodeWithText("Converting… 42%").assertExists()
|
||||
composeRule
|
||||
.onNodeWithTag(TestTags.Converter.PROGRESS)
|
||||
.assertRangeInfoEquals(ProgressBarRangeInfo(0.42f, 0f..1f))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a running job offers cancel and not start over`() {
|
||||
setContent(ConversionState.Converting(input(), percent = 42))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.CANCEL).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.START_OVER).assertDoesNotExist()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `tapping cancel on a running job cancels it and does nothing else`() {
|
||||
setContent(ConversionState.Converting(input(), percent = 42))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.CANCEL).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("cancel"), fired)
|
||||
}
|
||||
|
||||
// ----------------------------------------------------------------- Waiting
|
||||
|
||||
/**
|
||||
* The second bite named in #62. Deleting the `Cancel` button from the `Waiting` arm reddens
|
||||
* this case and the one below it.
|
||||
*
|
||||
* The paragraph is asserted in full rather than by a fragment because it is the only thing the
|
||||
* arm renders besides the card and the button, and because its wording is the arm's whole
|
||||
* job -- `FailureOutcome` records that two different causes land here and the state cannot tell
|
||||
* them apart, so the text has to cover both. A reword should redden one test, and this is it.
|
||||
*/
|
||||
@Test
|
||||
fun `a paused job explains why and still offers cancel`() {
|
||||
setContent(ConversionState.Waiting(input()))
|
||||
|
||||
composeRule.onNodeWithText(PAUSED_PARAGRAPH).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.CANCEL).assertExists()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `tapping cancel on a paused job cancels it and does nothing else`() {
|
||||
setContent(ConversionState.Waiting(input()))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.CANCEL).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("cancel"), fired)
|
||||
}
|
||||
|
||||
// --------------------------------------------------------------- Converted
|
||||
|
||||
@Test
|
||||
fun `a finished job offers save and start over, and no longer offers cancel`() {
|
||||
setContent(converted())
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.START_OVER).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.CANCEL).assertDoesNotExist()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `tapping start over on a finished job resets and does not save`() {
|
||||
setContent(converted())
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.START_OVER).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("reset"), fired)
|
||||
}
|
||||
|
||||
/**
|
||||
* The chip carries the job's own explanation, so its text is the assertion rather than its
|
||||
* presence: a chip showing the engine name, or the previous job's reason, would still exist.
|
||||
*/
|
||||
@Test
|
||||
fun `a finished job shows the routing decision the job reported`() {
|
||||
setContent(converted(routeReason = "Software — the MKV input needed a re-encode"))
|
||||
|
||||
composeRule
|
||||
.onNodeWithTag(TestTags.Converter.ROUTE_REASON)
|
||||
.assertTextEquals("Software — the MKV input needed a re-encode")
|
||||
}
|
||||
|
||||
/** The other side of the `isNotBlank` guard, which is unguarded without a case of its own. */
|
||||
@Test
|
||||
fun `a finished job that reported no routing decision shows no chip`() {
|
||||
setContent(converted(routeReason = ""))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.ROUTE_REASON).assertDoesNotExist()
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------- Saved
|
||||
|
||||
@Test
|
||||
fun `a saved file names itself and offers another conversion`() {
|
||||
setContent(ConversionState.Saved(displayName = "holiday.mp4"))
|
||||
|
||||
composeRule.onNodeWithText("Saved holiday.mp4.").assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CONVERT_ANOTHER).assertExists()
|
||||
// Compile-guarded rather than guarded here -- `Saved` has no `input`. See the class KDoc.
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD).assertDoesNotExist()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `tapping convert another after a save resets and does nothing else`() {
|
||||
setContent(ConversionState.Saved(displayName = "holiday.mp4"))
|
||||
|
||||
composeRule
|
||||
.onNodeWithTag(TestTags.Converter.CONVERT_ANOTHER)
|
||||
.performScrollTo()
|
||||
.performClick()
|
||||
|
||||
assertEquals(listOf("reset"), fired)
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------ Failed
|
||||
|
||||
/**
|
||||
* The message is the arm's only output that carries information, and it comes from the state.
|
||||
* An arm rendering a fixed apology would look right and say nothing, which is why the assertion
|
||||
* is on the text handed in rather than on a node existing.
|
||||
*/
|
||||
@Test
|
||||
fun `a failed job renders the reason it was given and offers a restart`() {
|
||||
setContent(ConversionState.Failed(message = "Ran out of space while writing the output."))
|
||||
|
||||
composeRule.onNodeWithText("Ran out of space while writing the output.").assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.START_OVER).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).assertDoesNotExist()
|
||||
// Compile-guarded rather than guarded here -- `Failed` has no `input`. See the class KDoc.
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD).assertDoesNotExist()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `tapping start over after a failure resets and does nothing else`() {
|
||||
setContent(ConversionState.Failed(message = "Ran out of space while writing the output."))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.START_OVER).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("reset"), fired)
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------ Harness
|
||||
|
||||
private fun input() = InputFile(
|
||||
uri = Uri.parse("content://test/holiday.mkv"),
|
||||
displayName = "holiday.mkv",
|
||||
sizeBytes = 12_345_678L,
|
||||
)
|
||||
|
||||
/**
|
||||
* `staged` names a path that does not exist, deliberately: `File.length()` answers `0L` for a
|
||||
* missing file rather than throwing, so the size line reads `0 B` and no temporary folder is
|
||||
* needed to render the arm.
|
||||
*/
|
||||
private fun converted(routeReason: String = "") = ConversionState.Converted(
|
||||
input = input(),
|
||||
staged = File("no-such-staged-output.mp4"),
|
||||
routeReason = routeReason,
|
||||
suggestedName = "holiday.mp4",
|
||||
mimeType = "video/mp4",
|
||||
)
|
||||
|
||||
private fun setContent(state: ConversionState, validation: Validation = Validation.Valid) {
|
||||
composeRule.setContent {
|
||||
ConverterScreenContent(
|
||||
state = state,
|
||||
settings = ConversionSettings(),
|
||||
validation = validation,
|
||||
actions = ConverterActions(
|
||||
onPickInput = { fired += "pickInput" },
|
||||
onPreset = { fired += "preset:$it" },
|
||||
onContainer = { fired += "container:$it" },
|
||||
onVideoCodec = { fired += "videoCodec:$it" },
|
||||
onAudioCodec = { fired += "audioCodec:$it" },
|
||||
onSuggestion = { fired += "suggestion:$it" },
|
||||
onQuality = { fired += "quality:$it" },
|
||||
onEnginePreference = { fired += "engine:$it" },
|
||||
onConvert = { fired += "convert" },
|
||||
onCancel = { fired += "cancel" },
|
||||
onSave = { fired += "save:$it" },
|
||||
onReset = { fired += "reset" },
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
|
||||
/**
|
||||
* A spec no container can hold, with somewhere to go instead.
|
||||
*
|
||||
* Built here rather than run through `ContainerCapabilities` because what makes a spec
|
||||
* invalid is that class's subject; all this arm needs is a `Validation` that answers
|
||||
* `isValid == false`.
|
||||
*/
|
||||
val INVALID = Validation.Invalid(
|
||||
message = "WebM cannot hold H.264 video.",
|
||||
suggestions = listOf(OutputSpec(Container.MKV, VideoCodec.H264, AudioCodec.AAC)),
|
||||
)
|
||||
|
||||
/** Copied from the `Waiting` arm, where it is written as two concatenated fragments. */
|
||||
const val PAUSED_PARAGRAPH =
|
||||
"Paused. Android limits background media processing, so this will " +
|
||||
"resume automatically — keeping the app open helps it along."
|
||||
}
|
||||
}
|
||||
@@ -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.junit4.v2.createComposeRule
|
||||
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.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 = createComposeRule()
|
||||
|
||||
@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,287 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import androidx.media3.common.MimeTypes
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNotNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.AudioPlan
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.ConversionPlan
|
||||
import org.libremediaconverter.model.ConversionRequest
|
||||
import org.libremediaconverter.model.ConversionRouter
|
||||
import org.libremediaconverter.model.CopyPlanner
|
||||
import org.libremediaconverter.model.DeviceCodecs
|
||||
import org.libremediaconverter.model.Engine
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.model.OutputSpec
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
import org.libremediaconverter.model.VideoPlan
|
||||
|
||||
/**
|
||||
* Guards [Media3Engine]'s two enum-to-MIME tables and the claims written above them.
|
||||
*
|
||||
* The defect: neither table was exercised at all, so nothing stood between a wrong entry and the
|
||||
* user's file. Point `H265` at `VIDEO_H264` and every hardware HEVC export writes H.264 into a
|
||||
* file the user asked to be H.265 — Transformer does exactly as told, the export succeeds, and
|
||||
* the only symptom is a codec nobody chose.
|
||||
*
|
||||
* Worse, one arm carried an assertion instead of a value:
|
||||
*
|
||||
* ```
|
||||
* // Never reached: only an Encode plan consults this, and COPY/NONE are not Encode.
|
||||
* ```
|
||||
*
|
||||
* That is a claim about *callers* parked in a branch of a callee. It happens to be true, and
|
||||
* nothing whatsoever checked it, so it would have gone on reading as true after it stopped being.
|
||||
*
|
||||
* Three kinds of test, because arm-by-arm equality alone would only pin today's answers:
|
||||
*
|
||||
* 1. Every arm of both tables, nulls included.
|
||||
* 2. The "never reached" claim, proved over every plan [CopyPlanner] can produce.
|
||||
* 3. The tables against [ConversionRouter]'s actual decisions rather than against its codec sets —
|
||||
* the comments claim behaviour ("the router routes them to FFmpeg"), and a set can be right
|
||||
* while the rule that reads it is wrong.
|
||||
*
|
||||
* A JVM test rather than an instrumented one: both tables take an enum and return a constant.
|
||||
*/
|
||||
@UnstableApi
|
||||
class Media3EngineMimeTypesTest {
|
||||
|
||||
@Test
|
||||
fun `every video codec maps to the MIME type Transformer will be given`() {
|
||||
assertEquals(
|
||||
"EXPECTED_VIDEO_MIME must name every VideoCodec, so a new one cannot arrive untested",
|
||||
VideoCodec.entries.toSet(),
|
||||
EXPECTED_VIDEO_MIME.keys,
|
||||
)
|
||||
VideoCodec.entries.forEach { codec ->
|
||||
assertEquals(
|
||||
"videoMimeTypeFor(${codec.label})",
|
||||
EXPECTED_VIDEO_MIME.getValue(codec),
|
||||
Media3Engine.videoMimeTypeFor(codec),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `every audio codec maps to the MIME type Transformer will be given`() {
|
||||
assertEquals(
|
||||
"EXPECTED_AUDIO_MIME must name every AudioCodec, so a new one cannot arrive untested",
|
||||
AudioCodec.entries.toSet(),
|
||||
EXPECTED_AUDIO_MIME.keys,
|
||||
)
|
||||
AudioCodec.entries.forEach { codec ->
|
||||
assertEquals(
|
||||
"audioMimeTypeFor(${codec.label})",
|
||||
EXPECTED_AUDIO_MIME.getValue(codec),
|
||||
Media3Engine.audioMimeTypeFor(codec),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The "never reached" claim, proved rather than repeated.
|
||||
*
|
||||
* [Media3Engine] asks these tables only for `plan.video as? VideoPlan.Encode`, and every plan
|
||||
* it sees comes from [CopyPlanner]. So the claim reduces to a property of the planner: over
|
||||
* every spec it can be handed, an `Encode` never carries `COPY` or `NONE`. That holds because
|
||||
* both codecs are answered before the `Encode` branch, and the fallback draws from
|
||||
* `ContainerCapabilities.encodableVideo`, which contains neither — but this asserts it instead
|
||||
* of trusting the reading.
|
||||
*
|
||||
* The counters are not decoration. `(plan.video as? VideoPlan.Encode)?.let { ... }` asserts
|
||||
* nothing at all for a `Drop` or `Copy` plan, so a sweep that stopped producing `Encode` plans
|
||||
* would stay green while checking nothing.
|
||||
*/
|
||||
@Test
|
||||
fun `no plan CopyPlanner can produce carries COPY or NONE inside an Encode`() {
|
||||
var videoEncodes = 0
|
||||
var audioEncodes = 0
|
||||
everyPlan().forEach { (spec, probe, plan) ->
|
||||
(plan.video as? VideoPlan.Encode)?.let {
|
||||
videoEncodes++
|
||||
assertTrue(
|
||||
"CopyPlanner produced VideoPlan.Encode(${it.codec}) for $spec against $probe",
|
||||
it.codec != VideoCodec.COPY && it.codec != VideoCodec.NONE,
|
||||
)
|
||||
}
|
||||
(plan.audio as? AudioPlan.Encode)?.let {
|
||||
audioEncodes++
|
||||
assertTrue(
|
||||
"CopyPlanner produced AudioPlan.Encode(${it.codec}) for $spec against $probe",
|
||||
it.codec != AudioCodec.COPY && it.codec != AudioCodec.NONE,
|
||||
)
|
||||
}
|
||||
}
|
||||
assertTrue("the sweep produced no video Encode plan, so it asserted nothing", videoEncodes > 0)
|
||||
assertTrue("the sweep produced no audio Encode plan, so it asserted nothing", audioEncodes > 0)
|
||||
}
|
||||
|
||||
/**
|
||||
* The video table's other claim: VP8, VP9 and AV1 targets "never reach here".
|
||||
*
|
||||
* Asked of the router rather than of its private codec set, so the rule is what is under test.
|
||||
*/
|
||||
@Test
|
||||
fun `the router sends exactly H264 and H265 video encodes to Media3`() {
|
||||
val onMedia3 = REAL_VIDEO_CODECS.filter { engineForVideoEncode(it) == Engine.MEDIA3 }
|
||||
assertEquals(listOf(VideoCodec.H264, VideoCodec.H265), onMedia3)
|
||||
}
|
||||
|
||||
/**
|
||||
* The audio table's sibling claim, and where it turned out to be incomplete.
|
||||
*
|
||||
* The comment named MP3 and FLAC. One rule — `audioEncode !in MEDIA3_AUDIO` — diverts Vorbis
|
||||
* by exactly the same logic, so three of the six encodable codecs never reach the table, not
|
||||
* two. Asserted as the whole set rather than as two memberships, which is what makes the
|
||||
* omission visible.
|
||||
*/
|
||||
@Test
|
||||
fun `the router keeps MP3 FLAC and Vorbis audio encodes off Media3`() {
|
||||
val onMedia3 = REAL_AUDIO_CODECS.filter { engineForAudioEncode(it) == Engine.MEDIA3 }
|
||||
assertEquals(listOf(AudioCodec.AAC, AudioCodec.OPUS, AudioCodec.PCM), onMedia3)
|
||||
}
|
||||
|
||||
/**
|
||||
* The binding that makes the two halves above one test rather than two coincidences.
|
||||
*
|
||||
* A codec the router starts sending to Media3 must have a MIME type here, or Transformer is
|
||||
* left to pick its own and the user gets a codec they did not choose.
|
||||
*/
|
||||
@Test
|
||||
fun `every codec the router sends to Media3 has a MIME type`() {
|
||||
REAL_VIDEO_CODECS.filter { engineForVideoEncode(it) == Engine.MEDIA3 }.forEach { codec ->
|
||||
assertNotNull(
|
||||
"${codec.label} is routed to Media3 but videoMimeTypeFor returns null",
|
||||
Media3Engine.videoMimeTypeFor(codec),
|
||||
)
|
||||
}
|
||||
REAL_AUDIO_CODECS.filter { engineForAudioEncode(it) == Engine.MEDIA3 }.forEach { codec ->
|
||||
assertNotNull(
|
||||
"${codec.label} is routed to Media3 but audioMimeTypeFor returns null",
|
||||
Media3Engine.audioMimeTypeFor(codec),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The reverse direction, which holds for video and not for audio.
|
||||
*
|
||||
* Every video codec the router withholds has a null entry, so that table is exactly the set of
|
||||
* codecs Media3 is asked to encode. Audio has one entry more than the router will ever use:
|
||||
* `VORBIS -> AUDIO_VORBIS` is correct and unreachable. Pinned deliberately — if a routing
|
||||
* change makes Vorbis live, this is the test that says the arm above stopped being dead.
|
||||
*/
|
||||
@Test
|
||||
fun `Vorbis is the one MIME type the router never asks for`() {
|
||||
REAL_VIDEO_CODECS.filter { engineForVideoEncode(it) == Engine.FFMPEG }.forEach { codec ->
|
||||
assertEquals(
|
||||
"${codec.label} never reaches Media3, so it must not name a MIME type",
|
||||
null,
|
||||
Media3Engine.videoMimeTypeFor(codec),
|
||||
)
|
||||
}
|
||||
val namedButUnrouted = REAL_AUDIO_CODECS
|
||||
.filter { Media3Engine.audioMimeTypeFor(it) != null }
|
||||
.filter { engineForAudioEncode(it) == Engine.FFMPEG }
|
||||
assertEquals(listOf(AudioCodec.VORBIS), namedButUnrouted)
|
||||
assertEquals(MimeTypes.AUDIO_VORBIS, Media3Engine.audioMimeTypeFor(AudioCodec.VORBIS))
|
||||
}
|
||||
|
||||
/**
|
||||
* Routes a video-only re-encode to [codec] and reports the engine chosen.
|
||||
*
|
||||
* `mpeg2video` is the load-bearing detail: [CopyPlanner] upgrades a request to a stream copy
|
||||
* when the source codec matches, and a `Copy` plan would answer a different question. A name
|
||||
* `CodecNames` cannot resolve forces an `Encode` for every codec, which the assertion pins so
|
||||
* that a planner change cannot quietly turn this sweep into a sweep of `Copy` plans.
|
||||
*/
|
||||
private fun engineForVideoEncode(codec: VideoCodec): Engine {
|
||||
val request = ConversionRequest(
|
||||
spec = OutputSpec(Container.MP4, codec, AudioCodec.NONE),
|
||||
probe = InputProbe(videoCodec = "mpeg2video", container = Container.MKV),
|
||||
)
|
||||
assertEquals(
|
||||
"this request no longer plans a video Encode, so its engine says nothing about $codec",
|
||||
VideoPlan.Encode(codec),
|
||||
CopyPlanner.plan(request.spec, request.probe).video,
|
||||
)
|
||||
return ConversionRouter.route(request, DeviceCodecs.PERMISSIVE).engine
|
||||
}
|
||||
|
||||
/** The audio counterpart. `ac3` is unresolvable for the same reason `mpeg2video` is. */
|
||||
private fun engineForAudioEncode(codec: AudioCodec): Engine {
|
||||
val request = ConversionRequest(
|
||||
spec = OutputSpec(Container.MP4, VideoCodec.NONE, codec),
|
||||
probe = InputProbe(audioCodec = "ac3", hasVideo = false, container = Container.MKV),
|
||||
)
|
||||
assertEquals(
|
||||
"this request no longer plans an audio Encode, so its engine says nothing about $codec",
|
||||
AudioPlan.Encode(codec),
|
||||
CopyPlanner.plan(request.spec, request.probe).audio,
|
||||
)
|
||||
return ConversionRouter.route(request, DeviceCodecs.PERMISSIVE).engine
|
||||
}
|
||||
|
||||
private fun everyPlan(): List<Triple<OutputSpec, InputProbe, ConversionPlan>> =
|
||||
ALL_SPECS.flatMap { spec -> PROBES.map { Triple(spec, it, CopyPlanner.plan(spec, it)) } }
|
||||
|
||||
private companion object {
|
||||
|
||||
/** Every arm of `videoMimeTypeFor`, including the ones the tests above prove unreachable. */
|
||||
val EXPECTED_VIDEO_MIME: Map<VideoCodec, String?> = mapOf(
|
||||
VideoCodec.H264 to MimeTypes.VIDEO_H264,
|
||||
VideoCodec.H265 to MimeTypes.VIDEO_H265,
|
||||
VideoCodec.VP8 to null,
|
||||
VideoCodec.VP9 to null,
|
||||
VideoCodec.AV1 to null,
|
||||
// Unreachable, and asserted anyway: the proof lives in another test, and a reader
|
||||
// deleting these would leave the arms themselves unexercised.
|
||||
VideoCodec.COPY to null,
|
||||
VideoCodec.NONE to null,
|
||||
)
|
||||
|
||||
val EXPECTED_AUDIO_MIME: Map<AudioCodec, String?> = mapOf(
|
||||
AudioCodec.AAC to MimeTypes.AUDIO_AAC,
|
||||
AudioCodec.OPUS to MimeTypes.AUDIO_OPUS,
|
||||
AudioCodec.VORBIS to MimeTypes.AUDIO_VORBIS,
|
||||
AudioCodec.PCM to MimeTypes.AUDIO_RAW,
|
||||
AudioCodec.MP3 to null,
|
||||
AudioCodec.FLAC to null,
|
||||
AudioCodec.COPY to null,
|
||||
AudioCodec.NONE to null,
|
||||
)
|
||||
|
||||
/** Codecs a user can actually ask to be produced: `COPY` and `NONE` are instructions. */
|
||||
val REAL_VIDEO_CODECS = VideoCodec.entries - VideoCodec.COPY - VideoCodec.NONE
|
||||
val REAL_AUDIO_CODECS = AudioCodec.entries - AudioCodec.COPY - AudioCodec.NONE
|
||||
|
||||
/** Every output a spec can name — 15 containers by 7 video codecs by 8 audio codecs. */
|
||||
val ALL_SPECS: List<OutputSpec> = Container.entries.flatMap { container ->
|
||||
VideoCodec.entries.flatMap { video ->
|
||||
AudioCodec.entries.map { audio -> OutputSpec(container, video, audio) }
|
||||
}
|
||||
}
|
||||
|
||||
/** Inputs chosen to reach each of [CopyPlanner]'s branches. */
|
||||
val PROBES = listOf(
|
||||
// Nothing known about the source at all.
|
||||
InputProbe(),
|
||||
// Identified, and the container changes: the copy upgrade applies.
|
||||
InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.MKV),
|
||||
// Identified, container unchanged: the copy upgrade deliberately does not apply.
|
||||
InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.MP4),
|
||||
// Copyable but not encodable by either engine — the fallback's reason for existing.
|
||||
InputProbe(videoCodec = "av1", audioCodec = "flac", container = Container.MKV),
|
||||
// Real codecs this app cannot name, so a copy is never proven safe.
|
||||
InputProbe(videoCodec = "mpeg2video", audioCodec = "ac3", container = Container.AVI),
|
||||
// The platform extractor could not open it.
|
||||
InputProbe(videoCodec = InputProbe.UNPARSEABLE),
|
||||
// Audio only.
|
||||
InputProbe(videoCodec = null, audioCodec = "opus", hasVideo = false, container = Container.OGG),
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,91 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* The image-demuxer rule, which looks arbitrary until it is read as a suffix.
|
||||
*
|
||||
* `MediaProbe.classify` asks [MediaProbe.isImageFormat] before anything else, so this one boolean
|
||||
* overrides everything both probes found: true and the source-info card says "Image" and a size,
|
||||
* false and it says container, codec and length. Neither mistake fails loudly.
|
||||
*
|
||||
* The rule has two halves and they are not the same shape. `image2` is a whole format name —
|
||||
* FFprobe reports it for a numbered image sequence — while the piped demuxers are named one per
|
||||
* image codec, so `_pipe` has to be matched as a *suffix*: `png_pipe`, `jpeg_pipe`, `webp_pipe`
|
||||
* and some thirty more. Widening that suffix to a substring is the tempting simplification and it
|
||||
* is wrong, because `yuv4mpegpipe` is raw video.
|
||||
*
|
||||
* The image names were measured rather than recalled. `ffprobe -show_entries format=format_name`
|
||||
* reports `png_pipe` for a `.png`, `jpeg_pipe` for a `.jpg`, `yuv4mpegpipe` for a `.y4m`, and
|
||||
* `image2` only when that demuxer is named explicitly. The container names come from
|
||||
* [MediaProbeFormatTest], and the case and spacing variants are synthetic — those exercise the
|
||||
* normalisation rather than anything FFprobe emits.
|
||||
*
|
||||
* One real format name is deliberately not asserted either way. `image2pipe` gets a false answer
|
||||
* here, being neither `image2` nor a `_pipe` suffix, and that is inert rather than a latent bug:
|
||||
* FFprobe only selects it when the demuxer is named with `-f image2pipe`, while `probeWithFFprobe`
|
||||
* forces no format at all, so a picked image arrives as `png_pipe` or its own codec's equivalent.
|
||||
* Pinning today's answer for a name this app cannot receive would be a test about FFmpeg's command
|
||||
* line rather than about this rule.
|
||||
*/
|
||||
class MediaProbeImageFormatTest {
|
||||
|
||||
@Test
|
||||
fun `a numbered image sequence is an image`() {
|
||||
assertIsImage("image2")
|
||||
}
|
||||
|
||||
/** What a picked PNG or JPEG actually reports, and the reason the suffix rule exists. */
|
||||
@Test
|
||||
fun `the per-codec piped demuxers are images`() {
|
||||
assertIsImage("png_pipe")
|
||||
assertIsImage("jpeg_pipe")
|
||||
assertIsImage("webp_pipe")
|
||||
}
|
||||
|
||||
/**
|
||||
* The half that a substring match would break.
|
||||
*
|
||||
* `yuv4mpegpipe` contains `pipe` and is not an image: it is raw uncompressed video, and
|
||||
* describing it as an image would hide its codec, its size and its length from the card while
|
||||
* leaving the file perfectly convertible.
|
||||
*/
|
||||
@Test
|
||||
fun `a format that merely contains pipe is not an image`() {
|
||||
assertNotImage("yuv4mpegpipe")
|
||||
}
|
||||
|
||||
/** The ordinary media containers, which is what the false answer is mostly for. */
|
||||
@Test
|
||||
fun `a real container is not an image`() {
|
||||
assertNotImage("mov,mp4,m4a,3gp,3g2,mj2")
|
||||
assertNotImage("matroska,webm")
|
||||
assertNotImage("mp3")
|
||||
}
|
||||
|
||||
/**
|
||||
* FFprobe names every format sharing the demuxer, so the entry that matters can be anywhere in
|
||||
* the list — and the padding and case are normalised the same way [MediaProbe.containerFrom]
|
||||
* normalises them.
|
||||
*/
|
||||
@Test
|
||||
fun `an image entry is found anywhere in the list, whatever its spacing or case`() {
|
||||
assertIsImage("PNG_PIPE")
|
||||
assertIsImage(" image2 ")
|
||||
assertIsImage("something_else, tiff_pipe")
|
||||
}
|
||||
|
||||
/** Nothing to go on is not an image; the card falls back to describing an unknown container. */
|
||||
@Test
|
||||
fun `an empty format name is not an image`() {
|
||||
assertNotImage("")
|
||||
}
|
||||
|
||||
private fun assertIsImage(formatName: String) =
|
||||
assertTrue("isImageFormat(\"$formatName\")", MediaProbe.isImageFormat(formatName))
|
||||
|
||||
private fun assertNotImage(formatName: String) =
|
||||
assertFalse("isImageFormat(\"$formatName\")", MediaProbe.isImageFormat(formatName))
|
||||
}
|
||||
@@ -0,0 +1,108 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.media.MediaFormat
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* The MIME -> short codec name table, which nothing downstream would notice going wrong.
|
||||
*
|
||||
* `MediaExtractor` answers in platform MIME spellings; the router, the copy planner and the
|
||||
* source-info card all speak FFmpeg's short names. [MediaProbe.shortName] is the one place those
|
||||
* two vocabularies meet, and most of its arms are translations rather than trimming — `video/avc`
|
||||
* is `h264`, `audio/mp4a-latm` is `aac`, `video/x-vnd.on2.vp9` is `vp9`.
|
||||
*
|
||||
* So a dropped or mistyped arm does not throw. It falls through to `substringAfter('/')` and
|
||||
* reports a different, entirely plausible-looking string. `CodecNames` carries alias lists that
|
||||
* happen to rescue some of those (`avc`, `av01`, `raw`) and not others (`mp4a-latm`,
|
||||
* `x-vnd.on2.vp9`), which is exactly why leaning on the rescue is not a plan: an unrecognised
|
||||
* codec is how a stream-copyable file quietly becomes a re-encode, and how the card ends up naming
|
||||
* a codec no user has heard of. This table is the only place those arms are pinned.
|
||||
*
|
||||
* A plain JVM test rather than Robolectric: `MediaFormat.MIMETYPE_*` are Java compile-time String
|
||||
* constants, so this test and `MediaProbe` alike carry the literals in their own bytecode and the
|
||||
* framework class is never loaded.
|
||||
*
|
||||
* Every case names its MIME in the failure message, because the MIME is the thing that has to be
|
||||
* looked up when one of these goes red.
|
||||
*/
|
||||
class MediaProbeMimeNamesTest {
|
||||
|
||||
@Test
|
||||
fun `an AVC track is reported as h264, which is what everything downstream calls it`() {
|
||||
assertShortName("h264", MediaFormat.MIMETYPE_VIDEO_AVC)
|
||||
}
|
||||
|
||||
/** On2's vendor MIME looks nothing like the codec name FFmpeg and the router use. */
|
||||
@Test
|
||||
fun `the VP8 and VP9 vendor MIMEs are reported without their vendor prefix`() {
|
||||
assertShortName("vp8", MediaFormat.MIMETYPE_VIDEO_VP8)
|
||||
assertShortName("vp9", MediaFormat.MIMETYPE_VIDEO_VP9)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `AV1 and MPEG-4 are reported by codec name rather than by MIME spelling`() {
|
||||
assertShortName("av1", MediaFormat.MIMETYPE_VIDEO_AV1)
|
||||
assertShortName("mpeg4", MediaFormat.MIMETYPE_VIDEO_MPEG4)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an AAC track is reported as aac, not as the mp4a-latm its MIME says`() {
|
||||
assertShortName("aac", MediaFormat.MIMETYPE_AUDIO_AAC)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `uncompressed audio is reported as pcm, which is not what its MIME says either`() {
|
||||
assertShortName("pcm", MediaFormat.MIMETYPE_AUDIO_RAW)
|
||||
}
|
||||
|
||||
/**
|
||||
* Four arms produce exactly what the fallback would produce anyway.
|
||||
*
|
||||
* `video/hevc` -> `hevc`, `audio/opus` -> `opus`, `audio/flac` -> `flac`,
|
||||
* `audio/vorbis` -> `vorbis`: for these the `when` arm and `substringAfter('/')` agree, so
|
||||
* deleting the arm changes no observable behaviour and no test can catch it. That is a
|
||||
* property of the code rather than a gap here, and it is reported as such rather than dressed
|
||||
* up as coverage. The assertions still earn their place — they pin the promise the router is
|
||||
* given (`hevc`, whatever the MIME happens to spell) against a later edit that changes the
|
||||
* mapping rather than deleting it.
|
||||
*/
|
||||
@Test
|
||||
fun `the arms whose MIME subtype already is the short name still map to it`() {
|
||||
assertShortName("hevc", MediaFormat.MIMETYPE_VIDEO_HEVC)
|
||||
assertShortName("opus", MediaFormat.MIMETYPE_AUDIO_OPUS)
|
||||
assertShortName("flac", MediaFormat.MIMETYPE_AUDIO_FLAC)
|
||||
assertShortName("vorbis", MediaFormat.MIMETYPE_AUDIO_VORBIS)
|
||||
}
|
||||
|
||||
/**
|
||||
* The fallback, which is what makes an unlisted codec describable at all.
|
||||
*
|
||||
* These are real `MediaFormat` MIMEs with no arm of their own. Dropping the subtype is the
|
||||
* right guess far more often than reporting the whole MIME would be — FFprobe calls the first
|
||||
* of these `ac3` too.
|
||||
*/
|
||||
@Test
|
||||
fun `a MIME with no arm of its own falls back to its subtype`() {
|
||||
assertShortName("ac3", MediaFormat.MIMETYPE_AUDIO_AC3)
|
||||
assertShortName("mpeg2", MediaFormat.MIMETYPE_VIDEO_MPEG2)
|
||||
assertShortName("dolby-vision", MediaFormat.MIMETYPE_VIDEO_DOLBY_VISION)
|
||||
}
|
||||
|
||||
/**
|
||||
* The surprising half of `substringAfter`'s contract, pinned deliberately.
|
||||
*
|
||||
* With no `/` in the string it returns the whole input rather than the empty string. Today's
|
||||
* callers gate on a `video/` or `audio/` prefix so they cannot reach this, but "report what
|
||||
* you were given" rather than "report nothing" is what would keep a malformed MIME visible on
|
||||
* the card instead of blank.
|
||||
*/
|
||||
@Test
|
||||
fun `a MIME with no subtype separator is reported unchanged`() {
|
||||
assertShortName("weird", "weird")
|
||||
assertShortName("", "")
|
||||
}
|
||||
|
||||
private fun assertShortName(expected: String, mime: String) =
|
||||
assertEquals("shortName(\"$mime\")", expected, MediaProbe.shortName(mime))
|
||||
}
|
||||
@@ -0,0 +1,79 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.media.MediaFormat
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* Reading Int track properties out of a `MediaFormat`, which is a heterogeneous map.
|
||||
*
|
||||
* [MediaProbe.intOr] guards two different failures with one expression, and only one of them is
|
||||
* obvious. A key the format does not carry is the easy half. The other is a key it *does* carry
|
||||
* with a value of another type: `getInteger` casts rather than coerces, so a frame rate stored as
|
||||
* a Float answers with a `ClassCastException`. `probeForConcat` reads `KEY_FRAME_RATE`, which the
|
||||
* platform accepts either way, and its `catch` sits outside the track loop — so without the
|
||||
* `runCatching` one oddly-typed field would discard the codec and dimensions already read from
|
||||
* that file and the join would re-encode for no reason.
|
||||
*
|
||||
* Robolectric rather than a plain JVM test, unlike the two sibling `MediaProbe` helper tests: this
|
||||
* one needs a real `MediaFormat` instance, not just its compile-time String constants.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class MediaProbeTrackFieldsTest {
|
||||
|
||||
@Test
|
||||
fun `a property the format carries as an Int is read`() {
|
||||
val format = videoFormat()
|
||||
|
||||
assertEquals(1920, with(MediaProbe) { format.intOr(MediaFormat.KEY_WIDTH) })
|
||||
assertEquals(1080, with(MediaProbe) { format.intOr(MediaFormat.KEY_HEIGHT) })
|
||||
}
|
||||
|
||||
/**
|
||||
* A track that simply does not say. `MediaExtractor` omits `KEY_FRAME_RATE` for plenty of real
|
||||
* files, and 0 is what `ConcatPlanner` reads as "cannot prove a match".
|
||||
*/
|
||||
@Test
|
||||
fun `a key the format does not carry gives the fallback`() {
|
||||
val format = videoFormat()
|
||||
|
||||
assertEquals(0, with(MediaProbe) { format.intOr(MediaFormat.KEY_FRAME_RATE) })
|
||||
assertEquals(-1, with(MediaProbe) { format.intOr(MediaFormat.KEY_FRAME_RATE, -1) })
|
||||
}
|
||||
|
||||
/**
|
||||
* The premise of the `runCatching`, pinned against the platform rather than assumed.
|
||||
*
|
||||
* If `getInteger` coerced a Float instead of throwing, the guard below would be testing
|
||||
* nothing at all — so the throw is asserted directly first.
|
||||
*/
|
||||
@Test
|
||||
fun `getInteger refuses a Float rather than coercing it`() {
|
||||
val format = videoFormat()
|
||||
format.setFloat(MediaFormat.KEY_FRAME_RATE, NON_INTEGRAL_FRAME_RATE)
|
||||
|
||||
val thrown = runCatching { format.getInteger(MediaFormat.KEY_FRAME_RATE) }.exceptionOrNull()
|
||||
|
||||
assertTrue("expected getInteger to refuse a Float, got $thrown", thrown is ClassCastException)
|
||||
}
|
||||
|
||||
/** And that refusal is answered with the fallback, not passed on to the caller. */
|
||||
@Test
|
||||
fun `a frame rate the format carries as a Float gives the fallback rather than throwing`() {
|
||||
val format = videoFormat()
|
||||
format.setFloat(MediaFormat.KEY_FRAME_RATE, NON_INTEGRAL_FRAME_RATE)
|
||||
|
||||
assertEquals(0, with(MediaProbe) { format.intOr(MediaFormat.KEY_FRAME_RATE) })
|
||||
assertEquals(-1, with(MediaProbe) { format.intOr(MediaFormat.KEY_FRAME_RATE, -1) })
|
||||
}
|
||||
|
||||
private fun videoFormat(): MediaFormat = MediaFormat.createVideoFormat(MediaFormat.MIMETYPE_VIDEO_AVC, 1920, 1080)
|
||||
|
||||
private companion object {
|
||||
/** NTSC's 30000/1001, the frame rate that cannot be stored as an Int in the first place. */
|
||||
const val NON_INTEGRAL_FRAME_RATE = 29.97f
|
||||
}
|
||||
}
|
||||
@@ -18,8 +18,10 @@ import java.util.UUID
|
||||
* the actual filesystem — the same calls `reset()` makes, without needing a ViewModel (both
|
||||
* of those construct a `WorkManager`, which is not initialised on the JVM classpath).
|
||||
*
|
||||
* The instrumented suite cannot run on the development host, so this is the only place the
|
||||
* "Start over leaks a full-size copy" defect can be caught before CI.
|
||||
* The instrumented suite could also catch the "Start over leaks a full-size copy" defect --
|
||||
* it runs on this host for API 33-36 (`tools/local-emulator/run-e2e.sh`) and on CI for
|
||||
* 33-37. Here rather than there because a real `cacheDir` is all the defect needs, and
|
||||
* finding it costs an emulator boot there and a few seconds here.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class OutputPublisherStagingTest {
|
||||
|
||||
@@ -2,13 +2,13 @@ package org.libremediaconverter.join
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.compose.ui.test.assertCountEquals
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
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
|
||||
|
||||
@@ -32,7 +32,7 @@ import org.robolectric.RobolectricTestRunner
|
||||
class JoinLeafTagsTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
private fun input(displayName: String) = InputFile(
|
||||
uri = Uri.parse("content://test/$displayName"),
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
package org.libremediaconverter.join
|
||||
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.compose.ui.test.performScrollTo
|
||||
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.model.ConcatStrategy
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* The join screen's half of the same seam, and the same two directions.
|
||||
*
|
||||
* The defect is the one `ConverterScreenContentTest` describes -- a content composable that
|
||||
* ignores the state handed to it, or renders the finished job's affordances unwired -- and it has
|
||||
* to be asked separately here because the two screens share no code. `JoinScreen` and
|
||||
* `ConverterScreen` were extracted in the same commit by the same hand, which is exactly the
|
||||
* circumstance in which one of them gets the wiring right and the other does not.
|
||||
*
|
||||
* `JoinState.Joined` is unreachable through a real `JoinViewModel` for the same reason
|
||||
* `ConversionState.Converted` is: only a `ConcatWorker` run that has already succeeded produces
|
||||
* one, carrying the strategy it chose and the name it picked.
|
||||
*
|
||||
* `JoinScreenKt` is the honest remaining coverage gap on this repo, and closing it is R38.7 (#63),
|
||||
* not this file. Which affordances each `JoinState` renders belongs there; this asserts only that
|
||||
* the injection point exists.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class JoinScreenContentTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
/** What the screen asked to save, in the order it asked. Empty until Save is tapped. */
|
||||
private val savedAs = mutableListOf<String>()
|
||||
|
||||
@Test
|
||||
fun `a finished join renders the save button`() {
|
||||
setContent(joined())
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).assertExists()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `tapping save hands back the name the finished join chose`() {
|
||||
setContent(joined())
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("joined.mp4"), savedAs)
|
||||
}
|
||||
|
||||
/** `staged` names a missing file deliberately -- see the same helper on the converter side. */
|
||||
private fun joined() = JoinState.Joined(
|
||||
staged = File("no-such-staged-output.mp4"),
|
||||
strategy = ConcatStrategy.STREAM_COPY,
|
||||
suggestedName = "joined.mp4",
|
||||
mimeType = "video/mp4",
|
||||
)
|
||||
|
||||
private fun setContent(state: JoinState) {
|
||||
composeRule.setContent {
|
||||
JoinScreenContent(
|
||||
state = state,
|
||||
actions = JoinActions(
|
||||
onPickInputs = {},
|
||||
onJoin = {},
|
||||
onCancel = {},
|
||||
onSave = { suggestedName -> savedAs += suggestedName },
|
||||
onReset = {},
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,270 @@
|
||||
package org.libremediaconverter.join
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.compose.ui.semantics.ProgressBarRangeInfo
|
||||
import androidx.compose.ui.semantics.SemanticsProperties
|
||||
import androidx.compose.ui.semantics.getOrNull
|
||||
import androidx.compose.ui.test.SemanticsMatcher
|
||||
import androidx.compose.ui.test.assertRangeInfoEquals
|
||||
import androidx.compose.ui.test.assertTextEquals
|
||||
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
|
||||
import androidx.compose.ui.test.performScrollTo
|
||||
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.convert.InputFile
|
||||
import org.libremediaconverter.model.ConcatStrategy
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* Every `JoinState` renders its own affordances, wired to its own callback.
|
||||
*
|
||||
* The defect is a branch of `JoinScreenContent`'s `when` that reads the wrong thing: a count taken
|
||||
* from a literal rather than from `inputs`, a strategy line that describes the other strategy, a
|
||||
* button wired to the neighbouring branch's callback, a `Failed` that drops the message it carries.
|
||||
* None of that is visible at compile time -- every branch of the `when` type-checks against the
|
||||
* same `JoinScreenContent` signature -- and none of it is visible from the leaf tests either, which
|
||||
* compose `FileRow` on its own and never see a state.
|
||||
*
|
||||
* `JoinScreenContentTest` deliberately asks only whether the seam exists, using `Joined`. This is
|
||||
* the matrix behind it: seven states, each pinned to what it lets the user do next.
|
||||
*
|
||||
* ### Two assertions here that nothing else in the suite makes
|
||||
*
|
||||
* **Order.** A join is the one flow where the order of the inputs is the content of the output --
|
||||
* the empty state promises "in the order you want them" -- so the rows are read back sorted by
|
||||
* their position on screen and compared as a list, not as a set. `JoinLeafTagsTest` proves a row
|
||||
* tags itself with the file it shows; nothing proved the rows come out in the order they went in.
|
||||
*
|
||||
* **Indeterminate.** The join progress bar carries no percentage, on purpose: FFmpeg reports
|
||||
* progress against one input's duration, which means nothing across a concatenation. The converter
|
||||
* screen's bar is determinate, so "it has a progress bar" is the assertion that would not notice a
|
||||
* fabricated percentage arriving here.
|
||||
*
|
||||
* ### Not asserted here, deliberately
|
||||
*
|
||||
* `JoinState.Joined.mimeType` is not rendered by this composable at all -- it is read by the entry
|
||||
* point, to open the save dialog with a type that matches the finished job. The colour of the
|
||||
* `Failed` message is `MaterialTheme.colorScheme.error`, which is theme lookup rather than state
|
||||
* logic, so it is left to the eye. The `is JoinState.Idle -> Unit` arm inside the scrolling branch
|
||||
* is unreachable by construction: the outer `when` peels `Idle` off first.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class JoinStateAffordancesTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
/** Which callback the screen invoked, in order, with what it passed. Empty until one fires. */
|
||||
private val events = mutableListOf<String>()
|
||||
|
||||
@Test
|
||||
fun `the empty state asks for files in order and offers the picker`() {
|
||||
setContent(JoinState.Idle)
|
||||
|
||||
composeRule.onNodeWithText("Pick two or more files to join, in the order you want them.").assertExists()
|
||||
// No `performScrollTo` on this one: `Idle` is the centred branch, outside the scrolling
|
||||
// column every other state renders into, so there is nothing to scroll.
|
||||
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).performClick()
|
||||
|
||||
assertEquals(listOf("pickInputs"), events)
|
||||
}
|
||||
|
||||
/**
|
||||
* The rows come out in the order the inputs went in.
|
||||
*
|
||||
* Sorted by position rather than trusting the order `fetchSemanticsNodes` happens to return, so
|
||||
* the assertion is about what the user sees down the screen. Three inputs, with names whose
|
||||
* alphabetical order is not their picked order, so a list that had been sorted anywhere on the
|
||||
* way through would not be able to pass this.
|
||||
*/
|
||||
@Test
|
||||
fun `the picked inputs are listed in the order they were picked`() {
|
||||
val picked = listOf("intro.mp4", "middle.mp4", "outro.mp4")
|
||||
setContent(JoinState.Ready(inputs = picked.map(::input)))
|
||||
|
||||
val topToBottom = composeRule.onAllNodes(isFileRow)
|
||||
.fetchSemanticsNodes()
|
||||
.sortedBy { it.positionInRoot.y }
|
||||
.map { it.config[SemanticsProperties.TestTag] }
|
||||
|
||||
assertEquals(picked.map(TestTags.Join::fileRow), topToBottom)
|
||||
}
|
||||
|
||||
/**
|
||||
* Three inputs, not two: two is the minimum a join accepts, so a button that had been
|
||||
* hardcoded to the smallest legal join would still read correctly with two on screen.
|
||||
*/
|
||||
@Test
|
||||
fun `the join button counts the files it will join`() {
|
||||
setContent(JoinState.Ready(inputs = listOf(input("intro.mp4"), input("middle.mp4"), input("outro.mp4"))))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Join.JOIN).assertTextEquals("Join 3 files")
|
||||
composeRule.onNodeWithTag(TestTags.Join.JOIN).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("join"), events)
|
||||
}
|
||||
|
||||
/** `Ready` is the one working state that still offers the picker, to replace the selection. */
|
||||
@Test
|
||||
fun `a ready join can be repicked`() {
|
||||
setContent(JoinState.Ready(inputs = listOf(input("intro.mp4"), input("outro.mp4"))))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_DIFFERENT_FILES).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("pickInputs"), events)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a running join names the count and shows a bar with no percentage`() {
|
||||
setContent(JoinState.Joining(inputs = listOf(input("intro.mp4"), input("outro.mp4"))))
|
||||
|
||||
composeRule.onNodeWithText("Joining 2 files…").assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Join.PROGRESS).assertRangeInfoEquals(ProgressBarRangeInfo.Indeterminate)
|
||||
composeRule.onNodeWithTag(TestTags.CANCEL).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("cancel"), events)
|
||||
}
|
||||
|
||||
/**
|
||||
* The paragraph is byte-identical to the converter screen's, which is the point of asserting
|
||||
* the whole of it rather than a fragment: the two branches were worded together, and a reword
|
||||
* that lands on one screen only is the failure this notices.
|
||||
*/
|
||||
@Test
|
||||
fun `a paused join explains itself and still offers cancel`() {
|
||||
setContent(JoinState.Waiting(inputs = listOf(input("intro.mp4"), input("outro.mp4"))))
|
||||
|
||||
composeRule.onNodeWithText(PAUSED_PARAGRAPH).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.CANCEL).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("cancel"), events)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a stream copied join says nothing was re-encoded`() {
|
||||
setContent(joined(ConcatStrategy.STREAM_COPY))
|
||||
|
||||
composeRule.onNodeWithText(STREAM_COPY_EXPLANATION).assertExists()
|
||||
composeRule.onNodeWithText(REENCODE_EXPLANATION).assertDoesNotExist()
|
||||
}
|
||||
|
||||
/**
|
||||
* The other half of the pair. Asserting the absence of the stream-copy line as well, because a
|
||||
* branch that had collapsed to one answer would still render *an* explanation.
|
||||
*/
|
||||
@Test
|
||||
fun `a re-encoded join says the files differed`() {
|
||||
setContent(joined(ConcatStrategy.REENCODE))
|
||||
|
||||
composeRule.onNodeWithText(REENCODE_EXPLANATION).assertExists()
|
||||
composeRule.onNodeWithText(STREAM_COPY_EXPLANATION).assertDoesNotExist()
|
||||
}
|
||||
|
||||
/** The size comes from the staged file, which is missing here, so `length()` answers `0L`. */
|
||||
@Test
|
||||
fun `a finished join reports the size of what it produced`() {
|
||||
setContent(joined(ConcatStrategy.STREAM_COPY))
|
||||
|
||||
composeRule.onNodeWithText("Joined — 0 MB.").assertExists()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a finished join offers save and start over, and they are not the same button`() {
|
||||
setContent(joined(ConcatStrategy.STREAM_COPY))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).performScrollTo().performClick()
|
||||
composeRule.onNodeWithTag(TestTags.START_OVER).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("save:joined.mp4", "reset"), events)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a saved join names the file and offers to join more`() {
|
||||
setContent(JoinState.Saved(displayName = "holiday-joined.mp4"))
|
||||
|
||||
composeRule.onNodeWithText("Saved holiday-joined.mp4.").assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Join.JOIN_MORE).assertTextEquals("Join more")
|
||||
composeRule.onNodeWithTag(TestTags.Join.JOIN_MORE).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("reset"), events)
|
||||
}
|
||||
|
||||
/**
|
||||
* The message is the whole content of this state -- it is the only thing that says why the job
|
||||
* stopped -- and it arrives as a string the failure produced, so a branch that rendered a fixed
|
||||
* apology instead would look correct on screen.
|
||||
*/
|
||||
@Test
|
||||
fun `a failed join renders the message it carries`() {
|
||||
setContent(JoinState.Failed(message = "The second file has no audio track, so joining stopped."))
|
||||
|
||||
composeRule.onNodeWithText("The second file has no audio track, so joining stopped.").assertExists()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a failed join offers start over`() {
|
||||
setContent(JoinState.Failed(message = "The second file has no audio track, so joining stopped."))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.START_OVER).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("reset"), events)
|
||||
}
|
||||
|
||||
/** Anything `FileRow` tagged, whichever file it is showing. The prefix comes from the table. */
|
||||
private val isFileRow = SemanticsMatcher("is a join file row") { node ->
|
||||
node.config.getOrNull(SemanticsProperties.TestTag)?.startsWith(TestTags.Join.fileRow("")) == true
|
||||
}
|
||||
|
||||
private fun input(displayName: String) = InputFile(
|
||||
uri = Uri.parse("content://test/$displayName"),
|
||||
displayName = displayName,
|
||||
sizeBytes = 4_000_000L,
|
||||
)
|
||||
|
||||
/** `staged` names a missing file deliberately -- see the same helper in `JoinScreenContentTest`. */
|
||||
private fun joined(strategy: ConcatStrategy) = JoinState.Joined(
|
||||
staged = File("no-such-staged-output.mp4"),
|
||||
strategy = strategy,
|
||||
suggestedName = "joined.mp4",
|
||||
mimeType = "video/mp4",
|
||||
)
|
||||
|
||||
private fun setContent(state: JoinState) {
|
||||
composeRule.setContent {
|
||||
JoinScreenContent(
|
||||
state = state,
|
||||
actions = JoinActions(
|
||||
onPickInputs = { events += "pickInputs" },
|
||||
onJoin = { events += "join" },
|
||||
onCancel = { events += "cancel" },
|
||||
onSave = { suggestedName -> events += "save:$suggestedName" },
|
||||
onReset = { events += "reset" },
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/** Byte-identical to the converter screen's, and split the same way `main` splits it. */
|
||||
const val PAUSED_PARAGRAPH =
|
||||
"Paused. Android limits background media processing, so this will " +
|
||||
"resume automatically — keeping the app open helps it along."
|
||||
|
||||
const val STREAM_COPY_EXPLANATION =
|
||||
"Files matched, so they were joined without " +
|
||||
"re-encoding — no quality loss."
|
||||
|
||||
const val REENCODE_EXPLANATION =
|
||||
"Files differed in format, so they were re-encoded " +
|
||||
"to match."
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
package org.libremediaconverter.model
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Test
|
||||
|
||||
@@ -10,6 +11,16 @@ import org.junit.Test
|
||||
* Three vocabularies meet: `MediaExtractor` MIME types, FFprobe `codec_name` strings, and the
|
||||
* enums. Stream copy depends on the round trip, so a missing alias here shows up as "we could not
|
||||
* identify the source codec" and silently costs the user a re-encode.
|
||||
*
|
||||
* Also bites on #74: `describeVideo` and `describeAudio` are one function apiece over one
|
||||
* vocabulary and had stopped matching. Only the video side special-cased
|
||||
* [InputProbe.UNPARSEABLE]; the audio side fell through to the raw name, and that sentinel opens
|
||||
* with a NUL, so the source-info card would have rendered a control character. The arms are shared
|
||||
* now, and the tests below assert both sides so the symmetric bug cannot reappear on the other one.
|
||||
*
|
||||
* The tables these read are cross-checked against the device capability check by
|
||||
* `CodecVocabularyTest` (#87). Deliberately not repeated here: this file is what each name means,
|
||||
* that one is whether the app's two copies of the vocabulary still agree.
|
||||
*/
|
||||
class CodecNamesTest {
|
||||
|
||||
@@ -48,4 +59,52 @@ class CodecNamesTest {
|
||||
// An unrecognised but real codec name is more useful shown than hidden.
|
||||
assertEquals("cinepak", CodecNames.describeVideo("cinepak"))
|
||||
}
|
||||
|
||||
/** The audio row of the same card, which had none of the above. */
|
||||
@Test
|
||||
fun `audio descriptions degrade exactly the way video ones do`() {
|
||||
assertEquals("AAC", CodecNames.describeAudio("mp4a"))
|
||||
assertEquals("Unknown", CodecNames.describeAudio(null))
|
||||
assertEquals("Unrecognised", CodecNames.describeAudio(InputProbe.UNPARSEABLE))
|
||||
assertEquals("qdm2", CodecNames.describeAudio("qdm2"))
|
||||
}
|
||||
|
||||
/**
|
||||
* #74's actual failure mode, stated as the thing the user would have seen.
|
||||
*
|
||||
* `InputProbe.UNPARSEABLE` is `"\u0000unparseable"`. Falling through to `?: name` does not
|
||||
* mislabel the track, it puts U+0000 into a `Text`.
|
||||
*/
|
||||
@Test
|
||||
fun `no description can put a control character on the card`() {
|
||||
listOf(CodecNames.describeAudio(InputProbe.UNPARSEABLE), CodecNames.describeVideo(InputProbe.UNPARSEABLE))
|
||||
.forEach { assertFalse("$it leaks the sentinel", it.contains('\u0000')) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Every alias, pinned one at a time.
|
||||
*
|
||||
* The tables became maps so `CodecVocabularyTest` could enumerate them; this is what catches a
|
||||
* key mistyped or a value pointing at the wrong enum while that rewrite happened.
|
||||
*/
|
||||
@Test
|
||||
fun `every name in the tables resolves to the codec it spells`() {
|
||||
CodecNames.VIDEO_ALIASES.forEach { (name, codec) ->
|
||||
assertEquals(name, codec, CodecNames.videoFromName(name))
|
||||
}
|
||||
CodecNames.AUDIO_ALIASES.forEach { (name, codec) ->
|
||||
assertEquals(name, codec, CodecNames.audioFromName(name))
|
||||
}
|
||||
assertEquals(VideoCodec.H264, CodecNames.videoFromName("x264"))
|
||||
assertEquals(VideoCodec.VP9, CodecNames.videoFromName("vp09"))
|
||||
assertEquals(AudioCodec.MP3, CodecNames.audioFromName("mpga"))
|
||||
assertEquals(AudioCodec.OPUS, CodecNames.audioFromName("opus"))
|
||||
}
|
||||
|
||||
/** The audio lookup reads the sentinel the same way the video one does. */
|
||||
@Test
|
||||
fun `the unparseable sentinel resolves to nothing on the audio side too`() {
|
||||
assertNull(CodecNames.audioFromName(InputProbe.UNPARSEABLE))
|
||||
assertNull(CodecNames.audioFromName(null))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -230,11 +230,17 @@ booted (`emulator_alive=yes`). The host emulator is fine; the guest is not.
|
||||
|
||||
## Can the suite run on it?
|
||||
|
||||
**Almost.** `tools/local-emulator/run-e2e.sh 37` now runs the whole suite locally, and all of it
|
||||
passes except two tests. Measured at `22c7914`: **49 tests, 2 failures, 0 errors, 2 skipped** — 45
|
||||
passed, the two `Media3EngineTest` failures dissected below, and the two `assumeTrue` skips every
|
||||
level has. It costs two deviations from how every other level is run, and both are worth
|
||||
understanding before trusting the leg.
|
||||
**Almost, and less so than it was.** `tools/local-emulator/run-e2e.sh 37` runs the whole suite
|
||||
locally. Measured at `22c7914`: **49 tests, 2 failures, 0 errors, 2 skipped** — 45 passed, the two
|
||||
`Media3EngineTest` failures dissected below, and the two `assumeTrue` skips every level has. It
|
||||
costs two deviations from how every other level is run, and both are worth understanding before
|
||||
trusting the leg.
|
||||
|
||||
**That was the high-water mark.** On 2026-08-24 a test that touches system UI joined the suite,
|
||||
and the level stopped *finishing* rather than merely failing two —
|
||||
[see below](#something-does-depend-on-system-ui-now-and-it-is-excluded-rather-than-trusted).
|
||||
Two `Media3EngineTest` failures is what **CI's gating leg** expects, because it filters on
|
||||
`notAnnotation`; a local `run-e2e.sh 37` does not filter and sees more.
|
||||
|
||||
Two things about that total before it is compared with anything. It is the size of the suite on
|
||||
the checkout that ran, not a property of API 37 — `app/src/androidTest` held 49 `@Test` methods at
|
||||
@@ -320,10 +326,60 @@ and proceeding straight to the tests fails exactly as before. The harness theref
|
||||
1. **The renderer is ANGLE, not the host GPU.** Shared with nothing else in the matrix — API
|
||||
33–36 run `-gpu host` locally, and CI runs `swiftshader_indirect`.
|
||||
2. **SystemUI is disabled.** The API 37 leg does not run the same device configuration as any
|
||||
other leg or as the Pixel. It is defensible here only because nothing in this suite touches
|
||||
system UI — these are Media3, FFmpeg and WorkManager tests — and because the alternative is no
|
||||
local API 37 coverage at all. **Anything that ever does depend on system UI must not trust
|
||||
this leg.**
|
||||
other leg or as the Pixel. It was defensible here because nothing in this suite touched
|
||||
system UI — Media3, FFmpeg and WorkManager tests — and because the alternative is no local
|
||||
API 37 coverage at all. **Anything that ever does depend on system UI must not trust this
|
||||
leg.** Something now does; see the section below.
|
||||
|
||||
### Something does depend on system UI now, and half of it is excluded
|
||||
|
||||
Added 2026-08-24, and the first entry on this page that is not a codec.
|
||||
|
||||
`SafPickerRoundTripTest` drives the real system file picker and rotates the display. Both reach
|
||||
the gralloc mapper — DocumentsUI is another app's windows, and a rotation rebuilds every surface
|
||||
on screen — and **disabling SystemUI does not help**, because it removes the *idle* trigger
|
||||
(RegionSamplingThread's nav-bar luma sampling) and not this one.
|
||||
|
||||
Measured one method per fresh emulator, `android-37.0`, `swangle_indirect`, SystemUI disabled and
|
||||
verified quiet — separately, because inferring the second from the first is the mistake this
|
||||
page's opening correction is about:
|
||||
|
||||
| test | result on android-37.0 | `hasReadColorBufferDma` aborts in the window |
|
||||
|---|---|---|
|
||||
| `thePickedInputSurvivesARealRotation` | **fails**: `INSTRUMENTATION_ABORTED: System has crashed.`, `Expected 1 tests, received 0`. The framework dies **during** it, so the JUnit XML carries a failure with no text at all. | 3 |
|
||||
| `pickingAFileThroughTheSystemPickerFillsInTheFileCard` | **passes** | 4 |
|
||||
|
||||
So a rotation, which rebuilds every surface at once, is what the mapper does not survive. Merely
|
||||
starting DocumentsUI is not. Only the rotation test carries `@FailsOnEmulatorApi37`; the picker
|
||||
test runs on the gating leg like anything else.
|
||||
|
||||
#### The correction that produced that table
|
||||
|
||||
**The first version of this section said both tests failed, and put the marker on the class.** The
|
||||
picker test had indeed failed at API 37 — with `androidx.test.uiautomator.StaleObjectException`,
|
||||
which looked like a framework restart invalidating an accessibility node, because that is exactly
|
||||
what it looks like.
|
||||
|
||||
It was the test's own bug. `UiObject2` caches the `AccessibilityNodeInfo` it was found with, and
|
||||
DocumentsUI is still settling when a node first appears; the handle went stale before `click()`.
|
||||
CI then reproduced it **deterministically** at API 33, 34 and 35 — every cold runner emulator, not
|
||||
intermittently — which is what made it obviously not an API 37 property. It had passed locally
|
||||
only because the emulator was warm.
|
||||
|
||||
The lesson is worth more than the measurement: **an annotation is a claim about an image, and a
|
||||
broken test makes every image look broken.** Re-measure after fixing a test before deciding what
|
||||
the platform did. Both the abort and the stale node produce "the run fell over", and only one of
|
||||
them was the image.
|
||||
|
||||
#### Two consequences worth stating rather than discovering
|
||||
|
||||
- **`run-e2e.sh 37` applies no annotation filter**, unlike CI, so a local API 37 run includes the
|
||||
rotation test and therefore **does not finish**: its totals come back short and which later
|
||||
tests ran is arbitrary. The summary row says so.
|
||||
- **The advisory job is still named `E2E API 37 Media3 hardware transcode (advisory)`** and now
|
||||
carries a test that is neither Media3 nor a transcode. Renaming a check is a branch-protection
|
||||
change and was deliberately not made in the same PR; the name is stale, the behaviour is
|
||||
correct.
|
||||
|
||||
### The two remaining failures are the same bug, one layer down
|
||||
|
||||
@@ -650,9 +706,15 @@ though a new API level shipped. Watch for these instead:
|
||||
- **`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.
|
||||
triggers to be noticed, not the most. When it happens, delete `@FailsOnEmulatorApi37` from
|
||||
everything carrying it rather than deleting the job — the gating leg picks them back up on its
|
||||
own, and the advisory job then runs nothing and can go.
|
||||
|
||||
**It is not two tests any more.** As of 2026-08-24 the marker is on `Media3EngineTest`'s two
|
||||
methods *and* on `SafPickerRoundTripTest` as a class, and the two groups fail for unrelated
|
||||
reasons — a codec and the gralloc mapper. They can go green independently, so check both before
|
||||
concluding the marker is done; and the job's name still says "Media3 hardware transcode", which
|
||||
half of what it runs is not.
|
||||
|
||||
## Correction owed to `CLAUDE.md`
|
||||
|
||||
|
||||
@@ -42,6 +42,18 @@ annotation = "1.+"
|
||||
junit = "4.+"
|
||||
androidxJunit = "1.+"
|
||||
espressoCore = "3.+"
|
||||
# UiAutomator. FLOATING, and the argument for it is the one the guard already makes:
|
||||
# androidx.test.uiautomator is inside `floatedGroupPrefixes` ("androidx."), so `2.+` reads
|
||||
# as "the newest RELEASED 2.x" exactly the way `work = "2.+"` does -- and this library does
|
||||
# publish alphas above its stable, so without the guard it would be a pin.
|
||||
#
|
||||
# Not pinned like ktlint/detekt/robolectric, because it is not that kind of dependency. Those
|
||||
# are pinned because a new *rule* or a new *runtime* makes untouched files fail -- the tool
|
||||
# changes its verdict on code nobody edited. UiAutomator has no verdict: it clicks what a
|
||||
# selector names, and a selector that stops matching is this repo's test to fix, in a diff
|
||||
# that says so. `2.` and not bare `+` because 3.x does not exist yet and a major is where the
|
||||
# selector API would be free to change under exactly that assumption.
|
||||
uiautomator = "2.+"
|
||||
# PINNED, unlike its neighbours. Under semver a 0.x minor is allowed to break, and
|
||||
# this library is load-bearing exactly where breakage is hardest to see: the wrapper
|
||||
# reaches for smartexception.java.Exceptions only when an FFmpeg call FAILS, so a
|
||||
@@ -139,14 +151,20 @@ junit = { group = "junit", name = "junit", version.ref = "junit" }
|
||||
androidx-junit = { group = "androidx.test.ext", name = "junit", version.ref = "androidxJunit" }
|
||||
androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-core", version.ref = "espressoCore" }
|
||||
|
||||
# The only way to touch UI this app does not own. Compose's own matchers stop at this
|
||||
# process's composition, and the system file picker is a DocumentsUI activity in another
|
||||
# process -- so a SAF round trip is unreachable without it.
|
||||
androidx-uiautomator = { group = "androidx.test.uiautomator", name = "uiautomator", version.ref = "uiautomator" }
|
||||
|
||||
# Robolectric — an Android runtime for the JVM test source set, so file-lifecycle behaviour
|
||||
# that needs a real Context can be verified without a device. The instrumented suite cannot
|
||||
# run on the development host at all (see CLAUDE.md), so an androidTest-only red test is not
|
||||
# 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.
|
||||
# Only for its `runTest`, and only so the one test that deliberately lets a coroutine error
|
||||
# escape owns the collector callback while it does -- otherwise the error is kept process-wide
|
||||
# and rethrown at whichever `runTest` starts next. See ConversionViewModelProbeFailureTest.
|
||||
kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "coroutinesTest" }
|
||||
|
||||
[plugins]
|
||||
|
||||
@@ -15,11 +15,13 @@
|
||||
# EXIT CODE: 0 only if every level was green; 1 if any level failed, wedged or could not be
|
||||
# set up; 2 if it refused to start at all. **A bare `run-e2e.sh` therefore exits 1 by design.**
|
||||
# API 37 is in the default list on purpose -- leaving it out is what left the level unlooked-at
|
||||
# for as long as it was -- and it is permanently two failures short of green, on the emulator's
|
||||
# own c2.goldfish.h264.decoder rather than on anything this app does. The summary names the two,
|
||||
# so a third is visibly new, and the last line printed says the same thing. Anything that reads a
|
||||
# non-zero exit as breakage should name the levels it wants: `run-e2e.sh 33 34 35 36` is the
|
||||
# sweep that can be green. docs/api-37-emulator-crash.md has the measurements.
|
||||
# for as long as it was -- and it is permanently short of green, on the emulator image rather
|
||||
# than on anything this app does. Since 2026-08-24 it does not even FINISH: one of its expected
|
||||
# failures kills the framework, so the totals come back short with an arbitrary tail. The summary
|
||||
# names every failure it expects, so an unnamed one is visibly new, and the last line printed
|
||||
# says the same thing. Anything that reads a non-zero exit as breakage should name the levels it
|
||||
# wants: `run-e2e.sh 33 34 35 36` is the sweep that can be green.
|
||||
# docs/api-37-emulator-crash.md has the measurements.
|
||||
#
|
||||
# WHY THIS EXISTS, AND WHAT IT DELIBERATELY DOES NOT DO
|
||||
#
|
||||
@@ -320,9 +322,20 @@ boot_emulator() {
|
||||
#
|
||||
# THIS IS A DEVIATION, and it is deliberately loud rather than silent. The API 37 leg does not
|
||||
# run the same device configuration as API 33-36 or as the Pixel. It is defensible only
|
||||
# because nothing in this suite touches SystemUI -- these are Media3, FFmpeg and WorkManager
|
||||
# tests -- and because the alternative is no API 37 coverage at all. Anything that ever does
|
||||
# depend on system UI must not trust this leg. docs/api-37-emulator-crash.md explains why.
|
||||
# because nothing in this suite touched SystemUI -- Media3, FFmpeg and WorkManager tests --
|
||||
# and because the alternative is no API 37 coverage at all. Anything that ever does depend on
|
||||
# system UI must not trust this leg. docs/api-37-emulator-crash.md explains why.
|
||||
#
|
||||
# "Touched", past tense, since 2026-08-24. SafPickerRoundTripTest drives DocumentsUI and rotates
|
||||
# the display, and both reach the gralloc mapper these images abort in -- disabling SystemUI
|
||||
# removes the IDLE trigger, not those. Measured per method on android-37.0: the ROTATION test
|
||||
# takes the framework down (INSTRUMENTATION_ABORTED) and carries @FailsOnEmulatorApi37; the
|
||||
# picker test passes.
|
||||
#
|
||||
# THIS SCRIPT APPLIES NO ANNOTATION FILTER, unlike CI, so a local `run-e2e.sh 37` runs the
|
||||
# rotation test anyway -- and because that test kills the framework rather than merely failing,
|
||||
# THE LEVEL DOES NOT FINISH. Its totals come back short and which later tests ran is arbitrary.
|
||||
# CI's gating leg never sees it.
|
||||
#
|
||||
# The retry loop is not defensive padding: at the moment boot_completed flips, the framework
|
||||
# may be in one of its restarts and `pm` is simply not published yet. The first attempt at this
|
||||
@@ -564,13 +577,25 @@ for api in "${APIS[@]}"; do
|
||||
guest_forensics "$api"
|
||||
# API 37 is in the default list on purpose, and it is expected to be red. Leaving it out would
|
||||
# put the level back where this whole exercise found it -- untested and unlooked-at -- but a
|
||||
# summary that just says "2 failures" with no explanation trains people to ignore the exit
|
||||
# code. So the row says which two, and a THIRD failure is then obviously new.
|
||||
# summary that just says "N failures" with no explanation trains people to ignore the exit
|
||||
# code. So the row NAMES the expected ones, and anything else is then obviously new.
|
||||
#
|
||||
# The list grew on 2026-08-24 and the shape of the row changed with it. The two
|
||||
# Media3EngineTest failures are a codec; the third is the gralloc bug reached through system
|
||||
# UI, and it takes the framework DOWN rather than merely failing -- so the level does not
|
||||
# finish, and the totals come back SHORT (50 of 59 when this was written) with the later
|
||||
# tests never run. A run whose totals do not add up is expected here now, which it never
|
||||
# was before.
|
||||
case "$api" in
|
||||
37 | 37.*)
|
||||
line="$line
|
||||
expected here: 2 failures, both Media3EngineTest, on c2.goldfish.h264.decoder.
|
||||
A third is new -- docs/api-37-emulator-crash.md"
|
||||
expected here: 2 Media3EngineTest failures on c2.goldfish.h264.decoder, plus
|
||||
SafPickerRoundTripTest.thePickedInputSurvivesARealRotation -- which kills the framework
|
||||
rather than merely failing, so the run ABORTS partway and the total comes back SHORT with
|
||||
an arbitrary tail. That is expected here too, and never was before. Anything else is new.
|
||||
CI's gating leg sees only the first two: the rotation test carries @FailsOnEmulatorApi37
|
||||
and this script, unlike CI, applies no annotation filter.
|
||||
docs/api-37-emulator-crash.md"
|
||||
;;
|
||||
esac
|
||||
if [ "$rc" -ne 0 ]; then
|
||||
@@ -596,8 +621,9 @@ echo "=============================================================="
|
||||
# worse than no note at all.
|
||||
if [ "$overall" -ne 0 ] && [ "$NON37_RED" -eq 0 ]; then
|
||||
echo "note: the only level that went red is API 37, which exits non-zero by design -- it is"
|
||||
echo " permanently 2 failures short of green. Confirm its row above shows exactly those"
|
||||
echo " two and nothing else; docs/api-37-emulator-crash.md says why they are the image."
|
||||
echo " permanently short of green, and since 2026-08-24 it does not even finish. Confirm"
|
||||
echo " its row above names every failure it shows; docs/api-37-emulator-crash.md says why"
|
||||
echo " each of them is the image rather than this app."
|
||||
fi
|
||||
|
||||
exit "$overall"
|
||||
|
||||
Reference in New Issue
Block a user