Compare commits
12
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
da344b0fe4 | ||
|
|
aa7e1d8b01 | ||
|
|
794cef7b34 | ||
|
|
eded47d666 | ||
|
|
b41341a1cb | ||
|
|
0f842243b5 | ||
|
|
2fbc957119 | ||
|
|
a645442acc | ||
|
|
5761faced6 | ||
|
|
c2c0bfa848 | ||
|
|
016030f3e4 | ||
|
|
d354f6470c |
@@ -130,9 +130,9 @@ 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** — **88.9% of lines (2087/2348), 75.4% of branches
|
||||
(1011/1340)**, measured 2026-08-29 with `./gradlew :app:jacocoTestReport`, against 546 JVM tests
|
||||
in 76 classes.
|
||||
- **Coverage is reported, not gated** — **92.8% of lines (2183/2352), 81.3% of branches
|
||||
(1091/1342)**, measured 2026-09-02 with `./gradlew :app:jacocoTestReport`, against 584 JVM tests
|
||||
in 87 classes.
|
||||
|
||||
**Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.**
|
||||
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
|
||||
@@ -174,6 +174,66 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
denominator shrank is not the same claim as one that rises because more branches are tested, and
|
||||
this entry has a history of explaining its own numbers wrongly.
|
||||
|
||||
Then wave 3 (#167-#178) on 2026-09-02 — 88.9% -> 92.8% line, 75.4% -> **81.3%** branch, 546 ->
|
||||
584 tests, in ten PRs from #179 to #188.
|
||||
|
||||
**Its shape is different from the two before it, and the difference is the thing to carry
|
||||
forward.** Waves 1 and 2 were finding uncovered code. By wave 3 there was not much of that left,
|
||||
so the gaps were sorted into two kinds before any test was written:
|
||||
|
||||
- **coverage gaps** — the line never executes. Filtered to sites where JaCoCo reports `mi > 0`, a
|
||||
concrete instruction no test runs, which is what separates a real gap from a partial branch on
|
||||
a compound condition. That filter cut the candidate list roughly in half and was right to.
|
||||
- **assertion gaps** — JaCoCo is green and nothing checks the answer. `MainActivity`'s rail and
|
||||
bottom bar were both *executed* by `AppRootRestorationTest` and **transposing them passed the
|
||||
entire suite**; so did swapping the two progress-notification strings, and swapping `Content`'s
|
||||
two destinations. No coverage number would ever have found any of the three.
|
||||
|
||||
So **every ticket named the mutation that had to go red, and that was its acceptance criterion
|
||||
rather than a coverage delta**. It caught **two vacuous tests written in the same session**,
|
||||
before either shipped:
|
||||
|
||||
- a `firstContainerHolding` test asserting a refusal still offered *something*. True, and
|
||||
useless: the source container is a candidate in its own right, so the list stays non-empty
|
||||
whatever the fallback does. What it actually buys is the codec the user asked for.
|
||||
- a staged-delete test scanning for a `"join-"` prefix `StagingNames.forJob` does not produce —
|
||||
it names files `<jobId>.<ext>`, so the assertion was true of everything.
|
||||
|
||||
It also corrected a *third* test that was not vacuous: `probeForConcat`'s KDoc claimed to drive
|
||||
the `catch` arm, and rethrowing from that catch left it green. That is how the arm turned out to
|
||||
be unreachable — see the next paragraph. A passing test with a wrong explanation is its own
|
||||
failure mode.
|
||||
|
||||
**A green mutation is only evidence when the mutation is a real change**, which is the mirror
|
||||
trap: one `classify` mutation stayed green because reordering two arms was semantically
|
||||
equivalent for every reachable input. A bad mutation and a weak test look identical in the output.
|
||||
|
||||
Three things came back **not as the ticket described them**, which is a result rather than a
|
||||
shortfall:
|
||||
|
||||
- `ContainerCapabilities:282`'s `exclude` filter **cannot drop anything**. `repair` always
|
||||
changes a codec on the shared container — a codec it left alone is one `validate` would not
|
||||
have refused — and the one non-default `exclude` carries `COPY` while every candidate carries
|
||||
`NONE`. F4-shaped.
|
||||
- `probeForConcat`'s catch arm is **unreachable on this runtime**. Robolectric's `MediaExtractor`
|
||||
never throws from `setDataSource`, measured across an unregistered `content://` authority, a
|
||||
missing `file://`, a file of garbage bytes and an `http://` URL — all four returned with
|
||||
`trackCount = 0`. It stays device-only.
|
||||
- `JobSnapshots:31`'s missed arm was **not** the `!isFile` one the ticket named — that is already
|
||||
covered by the `reclaimed` fixture. It was `path == null`: a job carrying no output path at
|
||||
all. Read the report, not the ticket, when the two disagree.
|
||||
|
||||
One item was **included against** the F4 rule rather than exempted by it, and the distinction is
|
||||
worth having written down since both live in the same function: `ContainerCapabilities:94`
|
||||
(`accepts(container, VideoCodec.NONE, mode)`) is dead in production today — every caller guards
|
||||
`NONE` first — and was tested anyway, because its audio twin at `:101` has had a test since #136
|
||||
and the asymmetry was the argument. The `COPY -> error(...)` arms beside it stay exempt, because
|
||||
a second line of defence that can be provoked is not one.
|
||||
|
||||
Denominators moved here too, in both directions and for two different reasons: 1340 -> 1342
|
||||
branches from `MediaProbe.merge`, 2348 -> 2352 lines from the `ConcatJoiner` interface. Neither
|
||||
is new untested code.
|
||||
|
||||
And **re-measure before quoting**: this entry was once written quoting 81.4%, measured four hours
|
||||
earlier, and was already three points stale by the time it was ready to merge.
|
||||
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
|
||||
|
||||
@@ -24,9 +24,11 @@ import androidx.compose.runtime.saveable.Saver
|
||||
import androidx.compose.runtime.saveable.rememberSaveable
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.platform.testTag
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.libremediaconverter.convert.ConverterScreen
|
||||
import org.libremediaconverter.join.JoinScreen
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.libremediaconverter.ui.theme.LibreMediaConverterTheme
|
||||
|
||||
/**
|
||||
@@ -114,7 +116,7 @@ internal fun AppRoot(
|
||||
|
||||
if (useRail) {
|
||||
Row(modifier = Modifier.fillMaxSize()) {
|
||||
NavigationRail {
|
||||
NavigationRail(modifier = Modifier.testTag(TestTags.Shell.NAVIGATION_RAIL)) {
|
||||
Destination.entries.forEach { item ->
|
||||
NavigationRailItem(
|
||||
selected = destination == item,
|
||||
@@ -132,7 +134,7 @@ internal fun AppRoot(
|
||||
Scaffold(
|
||||
modifier = Modifier.fillMaxSize(),
|
||||
bottomBar = {
|
||||
NavigationBar {
|
||||
NavigationBar(modifier = Modifier.testTag(TestTags.Shell.NAVIGATION_BAR)) {
|
||||
Destination.entries.forEach { item ->
|
||||
NavigationBarItem(
|
||||
selected = destination == item,
|
||||
|
||||
@@ -50,19 +50,42 @@ object MediaProbe {
|
||||
)
|
||||
|
||||
fun probe(context: Context, uri: Uri): InputProbe {
|
||||
val extracted = probeWithExtractor(context, uri)
|
||||
val info = probeWithFFprobe(context, uri)
|
||||
val merged = merge(probeWithExtractor(context, uri), probeWithFFprobe(context, uri))
|
||||
if (merged.kind == InputKind.UNPARSEABLE) {
|
||||
// Not a failure: an unparseable input is a strong signal that this job belongs on
|
||||
// FFmpeg. Reporting an unknown codec makes the router say so.
|
||||
Log.i(TAG, "Neither MediaExtractor nor FFprobe could read $uri; routing to FFmpeg.")
|
||||
}
|
||||
return merged
|
||||
}
|
||||
|
||||
/**
|
||||
* What the two probes together say about one input.
|
||||
*
|
||||
* A pure function, and `internal` for the same reason [extractedFrom] is: the precedence rules
|
||||
* below are the answer to "which probe wins", and until this was pulled out of [probe] the only
|
||||
* way to ask was to have a real `MediaExtractor` and a real FFprobe **disagree**, which nothing
|
||||
* on any source set can arrange. `RemuxTest` drives this on a device against committed
|
||||
* fixtures, but only ever with one probe answering and the other agreeing or also failing --
|
||||
* so every elvis here was taken in one direction and never the other.
|
||||
*
|
||||
* The rules, each of which is a decision rather than an accident:
|
||||
*
|
||||
* - **The extractor wins on codecs.** It is the platform's own view of what it can decode,
|
||||
* which is the thing the router is about to ask about. FFprobe's name for the same track can
|
||||
* differ, and the copy planner keys off these strings.
|
||||
* - **FFprobe alone reports the container.** `MediaExtractor` cannot, which is why [InputProbe]
|
||||
* carries a nullable one and `CopyPlanner` treats null as "container unknown".
|
||||
* - **Duration is the larger of the two**, not the first non-zero. Either probe can report zero
|
||||
* for a file the other times correctly, and a zero duration makes the FFmpeg progress
|
||||
* percentage undefined.
|
||||
*/
|
||||
internal fun merge(extracted: Extracted?, info: FFprobeInfo?): InputProbe {
|
||||
val videoCodec = extracted?.videoCodec ?: info?.videoCodec
|
||||
val audioCodec = extracted?.audioCodec ?: info?.audioCodec
|
||||
val kind = classify(extracted, info)
|
||||
|
||||
if (kind == InputKind.UNPARSEABLE) {
|
||||
// Not a failure: an unparseable input is a strong signal that this job belongs on
|
||||
// FFmpeg. Reporting an unknown codec makes the router say so.
|
||||
Log.i(TAG, "Neither MediaExtractor nor FFprobe could read $uri; routing to FFmpeg.")
|
||||
return UNREADABLE
|
||||
}
|
||||
if (kind == InputKind.UNPARSEABLE) return UNREADABLE
|
||||
|
||||
return InputProbe(
|
||||
videoCodec = videoCodec,
|
||||
@@ -83,7 +106,7 @@ object MediaProbe {
|
||||
* audio file and a corrupt file indistinguishable. The source-info card cannot describe either
|
||||
* honestly until they are separate, and neither can the copy planner.
|
||||
*/
|
||||
private fun classify(extracted: Extracted?, info: FFprobeInfo?): InputKind = when {
|
||||
internal fun classify(extracted: Extracted?, info: FFprobeInfo?): InputKind = when {
|
||||
info?.isImage == true -> InputKind.IMAGE
|
||||
extracted == null && info == null -> InputKind.UNPARSEABLE
|
||||
(extracted?.videoCodec ?: info?.videoCodec) != null -> InputKind.VIDEO
|
||||
@@ -162,7 +185,11 @@ object MediaProbe {
|
||||
}
|
||||
}
|
||||
|
||||
private class FFprobeInfo(
|
||||
/**
|
||||
* `internal` rather than `private` for the same reason [Extracted] is, and it should have been
|
||||
* from the start: [merge] cannot be named from a test while half its signature is private.
|
||||
*/
|
||||
internal class FFprobeInfo(
|
||||
val container: Container?,
|
||||
val videoCodec: String?,
|
||||
val audioCodec: String?,
|
||||
|
||||
@@ -4,6 +4,7 @@ import android.content.Context
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.libremediaconverter.codec.AndroidDeviceCodecs
|
||||
import org.libremediaconverter.ffmpeg.ConcatEngine
|
||||
import org.libremediaconverter.ffmpeg.FFmpegEngine
|
||||
import org.libremediaconverter.model.ConversionRequest
|
||||
import org.libremediaconverter.model.DeviceCodecs
|
||||
@@ -40,6 +41,27 @@ interface SoftwareTranscoder {
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The join path. Implemented by [org.libremediaconverter.ffmpeg.ConcatEngine].
|
||||
*
|
||||
* Added last of the three, and the gap it closes was measured rather than guessed:
|
||||
* `PerJobStagingTest`'s KDoc records that reverting `ConcatWorker` to a constant staging name left
|
||||
* all 257 tests green, because nothing in the JVM suite can get past a `ConcatEngine` constructed
|
||||
* in place. Everything after that line -- the failure mapping, the message fallback, the staged
|
||||
* delete -- was untested on every source set.
|
||||
*
|
||||
* The result type stays nested in the implementation rather than being lifted here. Moving it would
|
||||
* touch every call site to buy nothing: what a test needs is the ability to *not* run FFmpeg, and
|
||||
* that is the method, not the type.
|
||||
*/
|
||||
interface ConcatJoiner {
|
||||
suspend fun join(
|
||||
inputs: List<Uri>,
|
||||
output: File,
|
||||
format: OutputFormat = OutputFormat.MP4_H264,
|
||||
): ConcatEngine.Result
|
||||
}
|
||||
|
||||
/**
|
||||
* The seam that lets tests force failure paths.
|
||||
*
|
||||
@@ -69,6 +91,9 @@ object ConversionDependencies {
|
||||
@Volatile
|
||||
var software: () -> SoftwareTranscoder = { FFmpegEngine() }
|
||||
|
||||
@Volatile
|
||||
var concat: (Context) -> ConcatJoiner = { ConcatEngine(it) }
|
||||
|
||||
@Volatile
|
||||
var publisher: (Context) -> OutputPublisher = { OutputPublisher(it) }
|
||||
|
||||
@@ -103,6 +128,7 @@ object ConversionDependencies {
|
||||
fun reset() {
|
||||
hardware = { Media3Engine(it) }
|
||||
software = { FFmpegEngine() }
|
||||
concat = { ConcatEngine(it) }
|
||||
publisher = { OutputPublisher(it) }
|
||||
deviceCodecs = { AndroidDeviceCodecs.get() }
|
||||
probe = { context, uri -> MediaProbe.probe(context, uri) }
|
||||
|
||||
@@ -7,6 +7,7 @@ import com.arthenica.ffmpegkit.FFmpegKit
|
||||
import com.arthenica.ffmpegkit.FFmpegKitConfig
|
||||
import com.arthenica.ffmpegkit.ReturnCode
|
||||
import kotlinx.coroutines.suspendCancellableCoroutine
|
||||
import org.libremediaconverter.convert.ConcatJoiner
|
||||
import org.libremediaconverter.convert.MediaProbe
|
||||
import org.libremediaconverter.convert.StagingNames
|
||||
import org.libremediaconverter.model.ConcatPlanner
|
||||
@@ -24,11 +25,11 @@ import kotlin.coroutines.resumeWithException
|
||||
* reliably fail when they differ — it can emit a file whose later segments are
|
||||
* garbled. See [ConcatPlanner].
|
||||
*/
|
||||
class ConcatEngine(private val context: Context) {
|
||||
class ConcatEngine(private val context: Context) : ConcatJoiner {
|
||||
|
||||
data class Result(val strategy: ConcatStrategy, val output: File)
|
||||
|
||||
suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat = OutputFormat.MP4_H264): Result {
|
||||
override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): Result {
|
||||
require(inputs.size >= 2) { "Joining needs at least two files." }
|
||||
|
||||
val paths = inputs.map { uri ->
|
||||
|
||||
@@ -56,6 +56,20 @@ object TestTags {
|
||||
*/
|
||||
const val RETRY_SAVE: String = "action.retrySave"
|
||||
|
||||
/**
|
||||
* The adaptive shell around both screens -- `AppRoot`'s two layouts.
|
||||
*
|
||||
* Named because there is no other way to tell them apart from a test. Both render the same two
|
||||
* destinations with the same labels and the same selection state, so every assertion that could
|
||||
* be written without these tags is satisfied by either layout, and transposing the two bodies
|
||||
* passed the whole suite. Exactly one of the two exists at a time, which is what makes
|
||||
* `assertExists` / `assertDoesNotExist` on this pair a statement about the width class.
|
||||
*/
|
||||
object Shell {
|
||||
const val NAVIGATION_RAIL: String = "shell.navigationRail"
|
||||
const val NAVIGATION_BAR: String = "shell.navigationBar"
|
||||
}
|
||||
|
||||
/** `ConverterScreen`. */
|
||||
object Converter {
|
||||
const val CHOOSE_FILE: String = "converter.chooseFile"
|
||||
|
||||
@@ -15,7 +15,6 @@ import kotlinx.coroutines.CancellationException
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.InputQuery
|
||||
import org.libremediaconverter.convert.StagingNames
|
||||
import org.libremediaconverter.ffmpeg.ConcatEngine
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
|
||||
/**
|
||||
@@ -37,7 +36,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
|
||||
override suspend fun doWork(): Result {
|
||||
val uris = inputData.getStringArray(KEY_INPUT_URIS)?.map(Uri::parse)
|
||||
?: return Result.failure(workDataOf(KEY_ERROR to "No input files."))
|
||||
?: return Result.failure(workDataOf(KEY_ERROR to NO_INPUTS_MESSAGE))
|
||||
if (uris.size < 2) {
|
||||
return Result.failure(workDataOf(KEY_ERROR to TOO_FEW_INPUTS_MESSAGE))
|
||||
}
|
||||
@@ -77,7 +76,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
),
|
||||
)
|
||||
|
||||
val result = ConcatEngine(applicationContext).join(uris, staged, format)
|
||||
val result = ConversionDependencies.concat(applicationContext).join(uris, staged, format)
|
||||
Result.success(
|
||||
workDataOf(
|
||||
KEY_OUTPUT_PATH to staged.absolutePath,
|
||||
@@ -148,6 +147,16 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
* Here rather than in the ViewModel because the rule is the worker's: `request(...)` takes
|
||||
* a `List<Uri>` and checks nothing about its length, so this is the guard that always runs.
|
||||
*/
|
||||
/**
|
||||
* A job carrying no input array at all -- a downgrade, or a queue entry from a build that
|
||||
* spelled the key differently.
|
||||
*
|
||||
* A constant rather than the literal it was, for the convention #158 established: a message
|
||||
* the user can see is named once, so a test asserts the same string the worker writes
|
||||
* rather than a copy of it that can drift.
|
||||
*/
|
||||
const val NO_INPUTS_MESSAGE: String = "No input files."
|
||||
|
||||
const val TOO_FEW_INPUTS_MESSAGE: String = "Pick at least two files to join."
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,129 @@
|
||||
package org.libremediaconverter
|
||||
|
||||
import androidx.activity.ComponentActivity
|
||||
import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass
|
||||
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
||||
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 androidx.work.Data
|
||||
import org.junit.After
|
||||
import org.junit.Before
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.installTestWorkManager
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
|
||||
/**
|
||||
* Which navigation affordance the shell actually renders, and which screen it actually shows.
|
||||
*
|
||||
* Assertion gaps rather than coverage gaps, both of them, and that is why they lasted.
|
||||
* `AppRootRestorationTest` already drives `AppRoot` at `Compact` and `Expanded`, so JaCoCo is green
|
||||
* on `useRail` -- but it asserts only that the *selected tab* survives recreation, through a stub
|
||||
* `content` composable. Nothing anywhere queried for a rail or a bar, and nothing rendered the real
|
||||
* screens. Two consequences, both measured before this file existed:
|
||||
*
|
||||
* - **Transposing the `NavigationRail` and `NavigationBar` bodies passed the entire suite.**
|
||||
* - **Transposing `Content`'s two arms passed it too** -- a tablet showing the phone chrome, or the
|
||||
* Convert tab opening the Join screen, and 546 tests with nothing to say about either.
|
||||
*
|
||||
* `AppRoot`'s own KDoc is why this matters more than it looks: from targetSdk 37 the app is resized
|
||||
* and rotated whether or not it is ready, so the width class is not a preference, it is whatever
|
||||
* the system hands over.
|
||||
*
|
||||
* ## Two things this needed that the rest of the suite does not
|
||||
*
|
||||
* **`createAndroidComposeRule`, not `createComposeRule`.** Rendering `AppRoot` with its *default*
|
||||
* content reaches `ConverterScreen`'s `viewModel = viewModel()`, which needs a
|
||||
* `ViewModelStoreOwner`; the plain rule supplies none. It works because both ViewModels are
|
||||
* `@JvmOverloads constructor(app: Application, …)`, so `AndroidViewModelFactory` can build them,
|
||||
* and because `app/build.gradle.kts` already puts `ui-test-manifest`'s `ComponentActivity` in the
|
||||
* merged manifest the unit tests build against -- which that file says in terms.
|
||||
*
|
||||
* **Tags on the two bars.** They are in `TestTags`, applied inside `main`, for the reason that
|
||||
* file's KDoc gives: a tag the test hands down proves only that the test set it.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class AdaptiveShellTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createAndroidComposeRule<ComponentActivity>()
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
val app = RuntimeEnvironment.getApplication()
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
// The real screens are composed here, so their ViewModels are real too. Neither test is
|
||||
// about probing or publishing; left alone they would reach the FFprobe loader and this
|
||||
// machine's codec list, and decide things no assertion mentions.
|
||||
ConversionDependencies.probe = { _, _ -> InputProbe() }
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
ConversionDependencies.reset()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a phone gets the bottom bar and a tablet gets the rail`() {
|
||||
setShell(WindowWidthSizeClass.Compact)
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertDoesNotExist()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an expanded window gets the rail`() {
|
||||
setShell(WindowWidthSizeClass.Expanded)
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertDoesNotExist()
|
||||
}
|
||||
|
||||
/**
|
||||
* The width class no test had ever passed.
|
||||
*
|
||||
* `useRail` is `!= Compact`, so Medium takes the rail with Expanded. Narrowing it to
|
||||
* `== Expanded` is a one-character change that breaks every tablet and unfolded foldable and
|
||||
* nothing else -- and until this test, nothing in either source set used `Medium` at all.
|
||||
*/
|
||||
@Test
|
||||
fun `a medium window is a rail window, not a phone`() {
|
||||
setShell(WindowWidthSizeClass.Medium)
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertDoesNotExist()
|
||||
}
|
||||
|
||||
/**
|
||||
* The mapping every other test stubs out: which screen each destination actually opens.
|
||||
*
|
||||
* Matched on each screen's own "choose a file" affordance rather than on a title, because those
|
||||
* tags are applied by the screens themselves -- so this fails if the destinations are
|
||||
* transposed, and it fails for the right reason.
|
||||
*/
|
||||
@Test
|
||||
fun `Convert opens the converter and Join opens the join screen`() {
|
||||
setShell(WindowWidthSizeClass.Compact)
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).assertDoesNotExist()
|
||||
|
||||
composeRule.onNodeWithText(Destination.JOIN.label).performClick()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).assertExists()
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertDoesNotExist()
|
||||
}
|
||||
|
||||
/** [AppRoot] with its real content, which is the half nothing else composes. */
|
||||
private fun setShell(width: WindowWidthSizeClass) {
|
||||
composeRule.setContent { AppRoot(width) }
|
||||
}
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import org.libremediaconverter.model.CodecNames
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
|
||||
/**
|
||||
@@ -133,6 +134,38 @@ class CodecVocabularyTest {
|
||||
* 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.
|
||||
*/
|
||||
/**
|
||||
* The sentinel is not just another unknown name, and the difference is the whole guard.
|
||||
*
|
||||
* `canDecode` ends `?: true` -- a name neither table knows keeps the permissive answer, because
|
||||
* the app would rather try than refuse a file it might handle. `InputProbe.UNPARSEABLE` has to
|
||||
* be the exception: the platform has *already* failed to parse the input, so there is nothing
|
||||
* for a decoder to be permissive about, and waving it through spends a Media3 attempt on a job
|
||||
* that cannot start.
|
||||
*
|
||||
* The `cinepak` line is what makes the sentinel line mean something. Without it, deleting the
|
||||
* early return leaves this test green -- both names would fall through to the same `?: true`.
|
||||
* The pair is the assertion.
|
||||
*
|
||||
* `DeviceCodecs.PERMISSIVE` carries the same rule and `ConversionRouterTest` pins its routing
|
||||
* consequence. This is the implementation that runs on a device.
|
||||
*/
|
||||
@Test
|
||||
fun `the unparseable sentinel is refused even where an unknown name is waved through`() {
|
||||
val everything = AndroidDeviceCodecs.forTesting(
|
||||
encoders = emptySet(),
|
||||
decoders = setOf("video/avc", "video/hevc"),
|
||||
)
|
||||
assertFalse(
|
||||
"the platform could not parse this input, so there is nothing to decode with",
|
||||
everything.canDecode(InputProbe.UNPARSEABLE),
|
||||
)
|
||||
assertTrue(
|
||||
"a merely unknown name still keeps the permissive answer",
|
||||
everything.canDecode("cinepak"),
|
||||
)
|
||||
}
|
||||
|
||||
@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"))
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.withTimeout
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNotEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.AudioPlan
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.ConversionRequest
|
||||
import org.libremediaconverter.model.CopyPlanner
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.model.OutputSpec
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
import org.libremediaconverter.model.VideoPlan
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import java.io.File
|
||||
import java.util.concurrent.CancellationException
|
||||
|
||||
/**
|
||||
* A job that reached Media3 with a container Media3 cannot mux.
|
||||
*
|
||||
* [Media3Muxers]' own KDoc names the defect this guards: *"the router claimed five containers while
|
||||
* the engine silently wrote MP4 for all of them."* `factoryFor` answers null for fourteen of the
|
||||
* app's containers, and `buildTransformer` turns that null into a failed job rather than letting
|
||||
* `Transformer` fall back to its default muxer.
|
||||
*
|
||||
* The guard had never fired. `Media3Engine$buildTransformer$3` -- the `requireNotNull` message
|
||||
* lambda -- was four lines and four branches at 0%, which is to say the entire repair for a defect
|
||||
* the codebase went to the trouble of writing down was untested. Weakening it would restore that
|
||||
* bug silently, because the wrong output is a *playable file with the wrong container*, not a crash.
|
||||
*
|
||||
* Same harness and same two disciplines as [Media3EngineEmptyCompositionTest]: assert the plan
|
||||
* really is the one the test needs before driving the engine, and rule out
|
||||
* `CancellationException` so an unresumed continuation cannot read as a pass.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class Media3MuxerGuardTest {
|
||||
|
||||
@Test
|
||||
fun `a container Media3 cannot mux fails the job rather than silently writing MP4`() {
|
||||
val context = RuntimeEnvironment.getApplication()
|
||||
val engine = Media3Engine(context)
|
||||
val request = ConversionRequest(
|
||||
spec = OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.OPUS),
|
||||
probe = InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.MP4),
|
||||
)
|
||||
|
||||
// The premise, asserted rather than assumed -- three separate ways this test could pass
|
||||
// over a path it never entered.
|
||||
val plan = CopyPlanner.plan(request.spec, request.probe)
|
||||
assertEquals("the plan has to still be WebM by the time the engine sees it", Container.WEBM, plan.container)
|
||||
assertNull("...and Media3 really has no muxer for it", Media3Muxers.factoryFor(plan.container))
|
||||
// Not the empty-composition refusal, which fires earlier and is a different test's subject.
|
||||
assertNotEquals(VideoPlan.Drop, plan.video)
|
||||
assertNotEquals(AudioPlan.Drop, plan.audio)
|
||||
|
||||
val failure = try {
|
||||
runCatching {
|
||||
runBlocking {
|
||||
withTimeout(TIMEOUT_MS) {
|
||||
engine.transcode(Uri.parse("file:///dev/null"), File(context.cacheDir, "guard.webm"), request) {
|
||||
}
|
||||
}
|
||||
}
|
||||
}.exceptionOrNull()
|
||||
} finally {
|
||||
engine.close()
|
||||
}
|
||||
|
||||
assertFalse(
|
||||
"the continuation was never resumed -- the refusal escaped instead of failing the job: $failure",
|
||||
failure is CancellationException,
|
||||
)
|
||||
// Type *and* message, and the message half is the load-bearing one. Replacing the
|
||||
// requireNotNull with a fallback factory does not make the export succeed here: it lets it
|
||||
// run on and fail some other way, which a bare type assertion would happily accept.
|
||||
assertTrue("expected the muxer guard to refuse the job, got $failure", failure is IllegalArgumentException)
|
||||
assertTrue(
|
||||
"the refusal has to name the container it could not mux, got: ${failure?.message}",
|
||||
failure?.message.orEmpty().contains("cannot mux") &&
|
||||
failure?.message.orEmpty().contains(Container.WEBM.name),
|
||||
)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/** Nothing is decoded or muxed on this path -- the guard refuses before any of that. */
|
||||
const val TIMEOUT_MS = 10_000L
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,166 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.InputKind
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
|
||||
/**
|
||||
* Which of the two probes wins, when they disagree.
|
||||
*
|
||||
* [MediaProbe.probe] runs `MediaExtractor` and FFprobe independently and then merges the two, and
|
||||
* every rule in that merge is a decision. None of them had a test, for a reason that is structural
|
||||
* rather than an oversight: `RemuxTest` drives the whole thing on a device against committed
|
||||
* fixtures, but only ever with **one probe answering and the other agreeing or also failing**.
|
||||
* Nothing on any source set can arrange for a real extractor and a real FFprobe to disagree, so
|
||||
* every elvis in the merge was taken in one direction and never the other.
|
||||
*
|
||||
* Cutting `merge` out of `probe` is what makes the question askable. Both halves of its signature
|
||||
* had to become `internal` for that -- `Extracted` already was, with a KDoc giving this exact
|
||||
* reason; `FFprobeInfo` simply never got the same treatment.
|
||||
*/
|
||||
class MediaProbeMergeTest {
|
||||
|
||||
/**
|
||||
* The rule with the loudest failure mode, and `isImageFormat`'s own KDoc names it: a false
|
||||
* positive here "makes the source-info card describe a video as an image". So the image verdict
|
||||
* has to beat a real video codec from the extractor, and the ordering that makes it do so is
|
||||
* the first arm of `classify` rather than anything a reader would infer from the fields.
|
||||
*/
|
||||
@Test
|
||||
fun `an image verdict from FFprobe beats a video codec from the extractor`() {
|
||||
val merged = MediaProbe.merge(
|
||||
extracted = extracted(video = "h264"),
|
||||
info = info(video = "mjpeg", isImage = true),
|
||||
)
|
||||
|
||||
assertEquals(InputKind.IMAGE, merged.kind)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the extractor wins on codecs, because it is the view the router will act on`() {
|
||||
val merged = MediaProbe.merge(
|
||||
extracted = extracted(video = "h264", audio = "aac"),
|
||||
info = info(video = "hevc", audio = "mp3"),
|
||||
)
|
||||
|
||||
assertEquals("h264", merged.videoCodec)
|
||||
assertEquals("aac", merged.audioCodec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `FFprobe answers for a file the extractor could not open`() {
|
||||
val merged = MediaProbe.merge(extracted = null, info = info(video = "vp9", audio = "opus"))
|
||||
|
||||
assertEquals("vp9", merged.videoCodec)
|
||||
assertEquals("opus", merged.audioCodec)
|
||||
assertEquals(InputKind.VIDEO, merged.kind)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the extractor answers for a file FFprobe could not read`() {
|
||||
val merged = MediaProbe.merge(extracted = extracted(video = "h264", audio = "aac"), info = null)
|
||||
|
||||
assertEquals("h264", merged.videoCodec)
|
||||
assertEquals("aac", merged.audioCodec)
|
||||
assertNull("only FFprobe can name the container, so it stays unknown here", merged.container)
|
||||
}
|
||||
|
||||
/**
|
||||
* The larger of the two, not the first non-zero.
|
||||
*
|
||||
* Either probe can report zero for a file the other times correctly, and a zero duration makes
|
||||
* the FFmpeg progress percentage undefined -- `FFmpegEngine` divides by it. Both orderings are
|
||||
* asserted because "take the extractor's" and "take the larger" agree in one direction and not
|
||||
* the other, and only one of them is the rule.
|
||||
*/
|
||||
@Test
|
||||
fun `duration is the longer of the two readings, whichever probe supplied it`() {
|
||||
assertEquals(
|
||||
5_000L,
|
||||
MediaProbe.merge(extracted(duration = 0L), info(duration = 5_000L)).durationMs,
|
||||
)
|
||||
assertEquals(
|
||||
5_000L,
|
||||
MediaProbe.merge(extracted(duration = 5_000L), info(duration = 0L)).durationMs,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `dimensions come from the extractor, and from FFprobe only when it has none`() {
|
||||
assertEquals(1920, MediaProbe.merge(extracted(width = 1920), info(width = 640)).width)
|
||||
assertEquals(640, MediaProbe.merge(extracted = null, info = info(width = 640)).width)
|
||||
assertEquals(0, MediaProbe.merge(extracted(width = 0), info(width = 0)).width)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the container comes from FFprobe, which is the only probe that can name one`() {
|
||||
val merged = MediaProbe.merge(extracted(video = "h264"), info(container = Container.MKV))
|
||||
|
||||
assertEquals(Container.MKV, merged.container)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a file with audio and no video is audio-only, not unparseable`() {
|
||||
val merged = MediaProbe.merge(extracted(video = null, audio = "mp3"), info = null)
|
||||
|
||||
assertEquals(InputKind.AUDIO_ONLY, merged.kind)
|
||||
assertFalse(merged.hasVideo)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a file neither probe could open is the one unreadable answer`() {
|
||||
val merged = MediaProbe.merge(extracted = null, info = null)
|
||||
|
||||
assertEquals(MediaProbe.UNREADABLE, merged)
|
||||
assertEquals(InputProbe.UNPARSEABLE, merged.videoCodec)
|
||||
}
|
||||
|
||||
/**
|
||||
* The arm the ticket was filed for: parsed, and carrying no stream either probe recognised.
|
||||
*
|
||||
* Distinct from "neither probe could open it" -- here the extractor opened the file happily and
|
||||
* found nothing convertible, which is what a container holding only subtitles looks like. It
|
||||
* has to reach the same [MediaProbe.UNREADABLE] answer, because the router keys off that and
|
||||
* there is nothing here for Media3 to do either way.
|
||||
*
|
||||
* Its input was already being built elsewhere in the suite -- `MediaProbeTrackWalkTest` calls
|
||||
* `extractedFrom(emptyList())` and gets exactly this -- and had simply never been handed to the
|
||||
* merge.
|
||||
*/
|
||||
@Test
|
||||
fun `a file that parsed but carries no recognised stream is unreadable too`() {
|
||||
val merged = MediaProbe.merge(extracted = MediaProbe.extractedFrom(emptyList()), info = null)
|
||||
|
||||
assertEquals(InputKind.UNPARSEABLE, merged.kind)
|
||||
assertEquals(MediaProbe.UNREADABLE, merged)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `hasVideo follows the codec that survived the merge, not either probe alone`() {
|
||||
assertTrue(MediaProbe.merge(extracted(video = null), info(video = "vp9")).hasVideo)
|
||||
assertFalse(MediaProbe.merge(extracted(video = null, audio = "aac"), info(video = null)).hasVideo)
|
||||
}
|
||||
|
||||
private fun extracted(
|
||||
video: String? = "h264",
|
||||
audio: String? = "aac",
|
||||
duration: Long = 1_000L,
|
||||
width: Int = 1280,
|
||||
height: Int = 720,
|
||||
) = MediaProbe.Extracted(video, audio, duration, width, height)
|
||||
|
||||
private fun info(
|
||||
container: Container? = null,
|
||||
video: String? = "h264",
|
||||
audio: String? = "aac",
|
||||
duration: Long = 1_000L,
|
||||
width: Int = 1280,
|
||||
height: Int = 720,
|
||||
isImage: Boolean = false,
|
||||
) = MediaProbe.FFprobeInfo(container, video, audio, duration, width, height, isImage)
|
||||
}
|
||||
@@ -232,6 +232,12 @@ class OutputPublisherPublishTest {
|
||||
RowShape.NO_SIZE_COLUMN to "a cursor with no SIZE column",
|
||||
RowShape.NULL_SIZE to "a cursor whose SIZE cell is null",
|
||||
RowShape.NO_ROWS to "a cursor holding no rows",
|
||||
// The third case the KDoc names -- "a resolver call that throws" -- and the one the
|
||||
// list was missing. It reaches `?: false` through `runCatching` rather than through a
|
||||
// cursor answer, so it is the only one of the four that proves the catch is load
|
||||
// bearing: a provider that revokes its grant between the picker and the write must not
|
||||
// have its document deleted on the way out.
|
||||
RowShape.QUERY_THROWS to "a provider that throws out of query",
|
||||
).forEach { (shape, description) ->
|
||||
FakeSafProvider.deleteRequests.clear()
|
||||
FakeSafProvider.backingFile(documentUri).writeBytes(ByteArray(0))
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.net.Uri
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.model.ConcatPlanner
|
||||
import org.libremediaconverter.model.ConcatStrategy
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
|
||||
/**
|
||||
* A clip in a join that nothing could read, from the probe all the way to the strategy.
|
||||
*
|
||||
* Both halves of this are covered already, and separately: `MediaProbeTrackWalkTest` pins what
|
||||
* `concatInputFrom` makes of a track list, and `ConcatPlannerTest`'s
|
||||
* `an unknown codec is not treated as a match` pins what the planner does with a hand-built
|
||||
* `ConcatInput(video = null)`. **Nothing spanned the two**, and the span is the load-bearing part:
|
||||
* the planner's safety rests on the probe really producing that shape, and the hand-built fixture
|
||||
* would go on passing if it stopped.
|
||||
*
|
||||
* Measured rather than asserted: mutating `concatInputFrom`'s initial `video` to a non-null
|
||||
* placeholder leaves `ConcatPlannerTest` green and turns this red.
|
||||
*
|
||||
* ## The asymmetry this protects
|
||||
*
|
||||
* `ConcatPlanner` guards its video check against a null codec (`ConcatStrategy.kt:51`) and its
|
||||
* audio check not at all (`:54`). **That is correct, not an oversight.** `MediaProbe.shortName`
|
||||
* returns a non-null `String`, so in `concatInputFrom` a null `audioCodec` means the track is
|
||||
* *absent* — and two clips with no audio genuinely do match. A null `videoCodec` carries both
|
||||
* meanings, absent or unreadable, which is why only that one is guarded.
|
||||
*
|
||||
* So the audio check is safe *because* the video guard fires first on a clip nothing could read.
|
||||
* Nothing wrote that coupling down and nothing held it.
|
||||
*
|
||||
* ## What this deliberately does not cover
|
||||
*
|
||||
* `probeForConcat`'s `catch` arm (`MediaProbe.kt:300-302`). It is **not reachable on the JVM**:
|
||||
* Robolectric's `MediaExtractor` never throws from `setDataSource`, measured across an
|
||||
* unregistered `content://` authority, a missing `file://`, a file of garbage bytes and an `http://`
|
||||
* URL — all four returned normally with `trackCount = 0`. So the failure arrives here as an empty
|
||||
* track list rather than as an exception, which reaches the same `ConcatInput(null, null, 0, 0, 0)`
|
||||
* by the other road. The catch stays device-only, and this file does not pretend otherwise.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class UnreadableJoinInputTest {
|
||||
|
||||
@Test
|
||||
fun `a clip nothing could read probes as unknown, and an unknown clip is re-encoded`() {
|
||||
val unreadable = MediaProbe.probeForConcat(RuntimeEnvironment.getApplication(), UNREADABLE)
|
||||
|
||||
assertNull("an unreadable clip proves nothing about its video codec", unreadable.videoCodec)
|
||||
assertNull("nor about its audio codec", unreadable.audioCodec)
|
||||
assertEquals("nor about its dimensions", 0, unreadable.width)
|
||||
assertEquals(0, unreadable.height)
|
||||
assertEquals(0, unreadable.frameRate)
|
||||
|
||||
assertEquals(
|
||||
"a clip nothing could read is not evidence of a match with anything",
|
||||
ConcatStrategy.REENCODE,
|
||||
ConcatPlanner.plan(listOf(unreadable, unreadable)),
|
||||
)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/** `content://` so the probe takes the SAF branch a real pick takes. Nothing answers it. */
|
||||
val UNREADABLE: Uri = Uri.parse("content://test/vanished.mp4")
|
||||
}
|
||||
}
|
||||
@@ -166,6 +166,30 @@ class FFmpegCommandBuilderTest {
|
||||
assertPair(cmd(OutputFormat.OPUS), "-c:a", "libopus")
|
||||
}
|
||||
|
||||
/**
|
||||
* The arm most conversions actually take, and the only one in `audioArgs` with no test.
|
||||
*
|
||||
* `flac wav and opus select the right encoders` above covers the three named arms; MP3 has its
|
||||
* own. AAC arrives through the `else`, so nothing named it and nothing pinned either half of
|
||||
* what it emits -- neither `aac` nor `192k` appeared anywhere in this file. Both are shipped
|
||||
* defaults: MP4 and M4A are the formats the picker offers first, so this is the audio
|
||||
* every ordinary conversion gets.
|
||||
*
|
||||
* The bitrate is asserted as well as the encoder because it is the half a refactor is likelier
|
||||
* to lose. An `-b:a` that quietly changed would not fail anything, would not look wrong in a
|
||||
* command line, and would show up only as files that sound different from the ones the app
|
||||
* produced last month.
|
||||
*/
|
||||
@Test
|
||||
fun `aac is the default encoder, at the bitrate the app ships`() {
|
||||
assertPair(cmd(OutputFormat.MP4_H264), "-c:a", "aac")
|
||||
assertPair(cmd(OutputFormat.MP4_H264), "-b:a", "192k")
|
||||
// Through the `else` rather than through a named arm, so an AAC branch added above it later
|
||||
// has to keep answering the same way.
|
||||
assertPair(cmd(OutputFormat.M4A_AAC), "-c:a", "aac")
|
||||
assertPair(cmd(OutputFormat.M4A_AAC), "-b:a", "192k")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `audio only formats never carry a video encoder`() {
|
||||
listOf(OutputFormat.MP3, OutputFormat.FLAC, OutputFormat.WAV, OutputFormat.OPUS)
|
||||
|
||||
@@ -451,6 +451,99 @@ class ContainerCapabilitiesTest {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The video twin of `no audio track is accepted by every container in both modes`.
|
||||
*
|
||||
* Dead in production today, and deliberately so: every caller guards `NONE` before asking the
|
||||
* matrix, so nothing reaches this arm through the app. **The asymmetry is the argument, not the
|
||||
* reachability** -- its audio counterpart at the top of the same `when` has had a dedicated
|
||||
* test since #136, and one of a matched pair being covered is how a later reader concludes the
|
||||
* other was considered and exempted. It was not; it was simply missed.
|
||||
*
|
||||
* Not the same shape as the two `COPY -> error(...)` arms, which `docs/coverage-read-findings.md`
|
||||
* records as a named exemption (F4). Those are guards that must not be provokable. This is a
|
||||
* documented answer -- "no video track fits anywhere" -- and an answer is a thing to pin.
|
||||
*/
|
||||
@Test
|
||||
fun `no video track is accepted by every container in both modes`() {
|
||||
Container.entries.forEach { container ->
|
||||
listOf(CodecMode.COPY, CodecMode.ENCODE).forEach { mode ->
|
||||
assertTrue(
|
||||
"$container should accept no video track ($mode)",
|
||||
ContainerCapabilities.accepts(container, VideoCodec.NONE, mode),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A suggestion that keeps the codec the user asked for, rather than falling back to the
|
||||
* container's first encodable one.
|
||||
*
|
||||
* `repairVideo`'s third arm -- "the request is not a copy, and this container can encode it" --
|
||||
* is the one that preserves intent, and it was the only arm of the four nothing reached. The
|
||||
* property test above executes `repairVideo` on every case it walks and lands elsewhere each
|
||||
* time: an explicit COPY that works, a source the container can carry untouched, or no video
|
||||
* track at all.
|
||||
*
|
||||
* The route is indirect because it is the only one the app has. VP9 into WebM is a perfectly
|
||||
* good video request; what makes it invalid is the *audio* -- WebM carries Opus and Vorbis, not
|
||||
* AAC. So `validateAudio` refuses, `suggestions` looks for a container that can hold what was
|
||||
* asked for, and MP4 can encode VP9. The suggestion has to come back carrying VP9: swapping to
|
||||
* the container's first encodable codec would discard the choice the user made.
|
||||
*/
|
||||
@Test
|
||||
fun `a repaired suggestion keeps the video codec the user chose`() {
|
||||
val invalid = ContainerCapabilities.validate(
|
||||
OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.AAC),
|
||||
h264Source,
|
||||
)
|
||||
|
||||
assertTrue("WebM cannot hold AAC, so this spec is invalid", invalid is Validation.Invalid)
|
||||
val suggestions = (invalid as Validation.Invalid).suggestions
|
||||
assertTrue(
|
||||
"expected a suggestion that still encodes VP9, got $suggestions",
|
||||
suggestions.any { it.videoCodec == VideoCodec.VP9 },
|
||||
)
|
||||
assertEverySuggestionValid(invalid, h264Source)
|
||||
}
|
||||
|
||||
/**
|
||||
* The fallback in `firstContainerHolding`: when the input's own container cannot hold the
|
||||
* codec the user asked for, any container that can will do.
|
||||
*
|
||||
* The preferred half -- "the container the input already uses" -- is what every other case
|
||||
* reaches, because they all start from a file whose own container carries the codec in
|
||||
* question. The elvis after it had never run.
|
||||
*
|
||||
* AVI is the input that makes it run: AVI predates H.265 and has no mapping for it, so asking
|
||||
* an AVI for H.265 is refused, and the container the input already uses cannot be part of the
|
||||
* answer. Without the fallback the only candidates left are AVI itself and the container
|
||||
* holding the *source* codec -- also AVI -- so the refusal still offers something, but what it
|
||||
* offers is H.264: the app quietly declines the codec the user asked for instead of moving them
|
||||
* to a container that supports it.
|
||||
*
|
||||
* That is why this asserts the codec survives rather than that the list is non-empty. A
|
||||
* non-empty assertion passes with the fallback deleted -- measured, not assumed.
|
||||
*/
|
||||
@Test
|
||||
fun `an input whose container cannot hold the requested codec is moved, not downgraded`() {
|
||||
val aviSource = InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.AVI)
|
||||
|
||||
val invalid = ContainerCapabilities.validate(
|
||||
OutputSpec(Container.AVI, VideoCodec.H265, AudioCodec.AAC),
|
||||
aviSource,
|
||||
)
|
||||
|
||||
assertTrue("AVI has no mapping for H.265", invalid is Validation.Invalid)
|
||||
val suggestions = (invalid as Validation.Invalid).suggestions
|
||||
assertTrue(
|
||||
"expected a container that can actually hold H.265, got $suggestions",
|
||||
suggestions.any { it.videoCodec == VideoCodec.H265 },
|
||||
)
|
||||
assertEverySuggestionValid(invalid, aviSource)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `resolving audio COPY before asking the matrix is required`() {
|
||||
// The audio twin of `resolving COPY before asking the matrix is required`, and the reason is
|
||||
|
||||
@@ -28,6 +28,7 @@ class TagTableUniquenessTest {
|
||||
fun `every tag constant has its own value`() {
|
||||
val tags = tagsIn(
|
||||
TestTags::class.java,
|
||||
TestTags.Shell::class.java,
|
||||
TestTags.Converter::class.java,
|
||||
TestTags.Join::class.java,
|
||||
)
|
||||
|
||||
@@ -0,0 +1,237 @@
|
||||
package org.libremediaconverter.work
|
||||
|
||||
import android.app.Application
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import androidx.work.ListenableWorker
|
||||
import androidx.work.testing.TestListenableWorkerBuilder
|
||||
import androidx.work.workDataOf
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.ConcatJoiner
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.StagingNames
|
||||
import org.libremediaconverter.convert.installTestWorkManager
|
||||
import org.libremediaconverter.ffmpeg.ConcatEngine
|
||||
import org.libremediaconverter.model.ConcatStrategy
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import java.io.File
|
||||
import java.util.UUID
|
||||
|
||||
/**
|
||||
* What a join does when the engine fails partway.
|
||||
*
|
||||
* Everything past `ConcatWorker`'s `setForeground` was untested on **every** source set, and the
|
||||
* repo had already measured the cost: `PerJobStagingTest`'s KDoc records that reverting
|
||||
* `ConcatWorker` to a constant staging name left all 257 tests green, because nothing in the JVM
|
||||
* suite can get past a `ConcatEngine` constructed in place. `RefusedJobTest` says the same from the
|
||||
* other side -- "the next thing past the count guard is `ConcatEngine`, which is native".
|
||||
* `ConcatEngineTest` on a device tests the engine directly, bypassing the worker, and
|
||||
* `ConcatWorkerTest` covers only the too-few-inputs guard and the happy path.
|
||||
*
|
||||
* `ConversionDependencies.concat` is the seam that closes it, added here to sit beside the
|
||||
* `.hardware` and `.software` that `ConversionWorker` has had all along -- the asymmetry between the
|
||||
* two workers was the whole reason one of them had a tested failure path and the other did not.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class ConcatFailureTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var publisher: AlwaysRoomPublisher
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
publisher = AlwaysRoomPublisher(app)
|
||||
ConversionDependencies.publisher = { publisher }
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
ConversionDependencies.reset()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a join whose engine fails reports the engine's own reason`() {
|
||||
ConversionDependencies.concat = { FailingJoiner { error(DEMUXER_MESSAGE) } }
|
||||
|
||||
val result = runBlocking { joinWorker().doWork() }
|
||||
|
||||
assertEquals(
|
||||
ListenableWorker.Result.failure(workDataOf(ConcatWorker.KEY_ERROR to DEMUXER_MESSAGE)),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A failure carrying no message at all, which Kotlin and Java both allow and FFmpegKit's
|
||||
* wrappers can produce.
|
||||
*
|
||||
* Without the fallback the user is shown an empty error, and `JoinViewModel` cannot tell that
|
||||
* from a job that reported nothing -- the two would be one blank screen with different causes.
|
||||
*/
|
||||
@Test
|
||||
fun `a failure with no message of its own still says something`() {
|
||||
ConversionDependencies.concat = { FailingJoiner { throw RuntimeException() } }
|
||||
|
||||
val result = runBlocking { joinWorker().doWork() }
|
||||
|
||||
assertEquals(
|
||||
ListenableWorker.Result.failure(
|
||||
workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.GENERIC_FAILURE_MESSAGE),
|
||||
),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The staged file is deleted on the way out.
|
||||
*
|
||||
* The joiner writes before it fails, exactly as `PartialThenFailingTranscoder` does on the
|
||||
* conversion side: a stub that only threw would let a missing `staged.delete()` pass. What it
|
||||
* costs to lose is a full-size partial per failed join, sitting in cache until the sweep is old
|
||||
* enough to be sure nobody is coming back for it.
|
||||
*
|
||||
* Asserted against the file the joiner was actually handed rather than by scanning the staging
|
||||
* directory for a name. The first draft did scan, for a `"join-"` prefix that
|
||||
* `StagingNames.forJob` does not produce -- it names files `<jobId>.<ext>` -- so the assertion
|
||||
* was trivially true and the mutation walked straight through it.
|
||||
*/
|
||||
@Test
|
||||
fun `a failed join leaves nothing behind in staging`() {
|
||||
val joiner = FailingJoiner { error(DEMUXER_MESSAGE) }
|
||||
ConversionDependencies.concat = { joiner }
|
||||
|
||||
runBlocking { joinWorker().doWork() }
|
||||
|
||||
val staged = requireNotNull(joiner.lastOutput) { "the joiner never ran, so this proves nothing" }
|
||||
assertEquals(
|
||||
"the fixture has to write before it fails, or the delete is unobservable",
|
||||
PARTIAL_BYTES,
|
||||
joiner.bytesWritten,
|
||||
)
|
||||
assertFalse("a failed join must not leave its partial behind: $staged", staged.exists())
|
||||
}
|
||||
|
||||
/**
|
||||
* The success path, and the staging name #159's fixture and `PerJobStagingTest` both care about.
|
||||
*
|
||||
* Worth its place rather than a happy-path formality: `PerJobStagingTest`'s KDoc records that
|
||||
* **reverting `ConcatWorker` to a constant staging name left all 257 tests green**, because
|
||||
* nothing could reach the line that names the file. This is the test that was missing when that
|
||||
* was written -- the join's output `Data` had never been read by anything on the JVM.
|
||||
*
|
||||
* The staged path is asserted to carry the job id, not a constant: two joins of the same format
|
||||
* sharing one name is the defect, and `ConcatEngine`'s list file collided harder still.
|
||||
*/
|
||||
@Test
|
||||
fun `a join that works reports its own staged file, strategy and name`() {
|
||||
val joiner = SucceedingJoiner()
|
||||
ConversionDependencies.concat = { joiner }
|
||||
|
||||
val result = runBlocking { joinWorker().doWork() }
|
||||
|
||||
assertTrue("got $result", result is ListenableWorker.Result.Success)
|
||||
val data = (result as ListenableWorker.Result.Success).outputData
|
||||
assertEquals(
|
||||
"the staged file has to be this job's, not a name every join shares",
|
||||
File(publisherStagingDir(), StagingNames.forJob(JOB_ID, OutputFormat.MP4_H264.extension)).absolutePath,
|
||||
data.getString(ConcatWorker.KEY_OUTPUT_PATH),
|
||||
)
|
||||
assertEquals(ConcatStrategy.STREAM_COPY.name, data.getString(ConcatWorker.KEY_STRATEGY))
|
||||
assertEquals(OutputFormat.MP4_H264.mimeType, data.getString(ConcatWorker.KEY_MIME_TYPE))
|
||||
assertTrue(
|
||||
"the save dialog needs a name with the right extension, got ${data.getString(
|
||||
ConcatWorker.KEY_SUGGESTED_NAME,
|
||||
)}",
|
||||
data.getString(ConcatWorker.KEY_SUGGESTED_NAME).orEmpty().endsWith(".${OutputFormat.MP4_H264.extension}"),
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The arm beside the count guard: no URI array at all.
|
||||
*
|
||||
* Covered today only by `UnopenableUriTest` on a device, although it runs before staging and
|
||||
* before any native code. It is the exact sibling of `RefusedJobTest`'s ConversionWorker twin,
|
||||
* and it belongs on the JVM with it -- a device test for a branch that needs no device is a
|
||||
* slower test that reports later.
|
||||
*/
|
||||
@Test
|
||||
fun `a join with no input array at all is refused with a message`() {
|
||||
val result = runBlocking {
|
||||
TestListenableWorkerBuilder<ConcatWorker>(
|
||||
context = app,
|
||||
inputData = workDataOf(ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name),
|
||||
runAttemptCount = 0,
|
||||
).setId(JOB_ID).build().doWork()
|
||||
}
|
||||
|
||||
assertEquals(
|
||||
ListenableWorker.Result.failure(workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.NO_INPUTS_MESSAGE)),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
private fun publisherStagingDir(): File? = publisher.createStagingFile("probe").parentFile
|
||||
|
||||
private fun joinWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
|
||||
context = app,
|
||||
inputData = workDataOf(
|
||||
ConcatWorker.KEY_INPUT_URIS to arrayOf(FIRST.toString(), SECOND.toString()),
|
||||
ConcatWorker.KEY_TOTAL_BYTES to TOTAL_BYTES,
|
||||
ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
|
||||
),
|
||||
runAttemptCount = 0,
|
||||
).setId(JOB_ID).build()
|
||||
|
||||
private companion object {
|
||||
val FIRST: Uri = Uri.parse("file:///tmp/one.mp4")
|
||||
val SECOND: Uri = Uri.parse("file:///tmp/two.mp4")
|
||||
const val TOTAL_BYTES = 2048L
|
||||
const val DEMUXER_MESSAGE = "the demuxer rejected the input list"
|
||||
const val PARTIAL_BYTES = 2048
|
||||
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000b")
|
||||
}
|
||||
}
|
||||
|
||||
/** A joiner that writes something and then fails, so a missing `staged.delete()` cannot pass. */
|
||||
private class FailingJoiner(private val failure: () -> Nothing) : ConcatJoiner {
|
||||
|
||||
/** The handle the worker created, kept so a test can ask whether it survived the failure. */
|
||||
var lastOutput: File? = null
|
||||
var bytesWritten = 0
|
||||
|
||||
override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): ConcatEngine.Result {
|
||||
lastOutput = output
|
||||
output.writeBytes(ByteArray(PARTIAL_BYTES))
|
||||
bytesWritten = PARTIAL_BYTES
|
||||
failure()
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val PARTIAL_BYTES = 2048
|
||||
}
|
||||
}
|
||||
|
||||
/** The joiner that finishes, so the success path and the output `Data` can be read on the JVM. */
|
||||
private class SucceedingJoiner : ConcatJoiner {
|
||||
override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): ConcatEngine.Result {
|
||||
output.writeBytes(ByteArray(OUTPUT_BYTES))
|
||||
return ConcatEngine.Result(ConcatStrategy.STREAM_COPY, output)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val OUTPUT_BYTES = 4096
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,88 @@
|
||||
package org.libremediaconverter.work
|
||||
|
||||
import android.content.pm.ServiceInfo
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.annotation.Config
|
||||
|
||||
/**
|
||||
* [ConversionForegroundType.current] answers differently on each of the three API regimes, and
|
||||
* until this file only one of them was ever executed.
|
||||
*
|
||||
* `app/src/test/resources/robolectric.properties` pins the whole JVM suite to `sdk=36`, so every
|
||||
* Robolectric test that reaches a `ForegroundInfo` takes the `mediaProcessing` arm and no other.
|
||||
* The 33 and 34 arms were cold: 3 lines and 3 of 4 branches, measured on `main` at `d354f64`.
|
||||
*
|
||||
* **The instrumented test is not a substitute, and the reason is specific.**
|
||||
* `ConversionWorkerTest.foregroundTypeMatchesTheRunningApiLevel` asserts against whichever API the
|
||||
* leg happens to be — one arm per leg, never the other two — and the legs that would cover 33 and
|
||||
* 34 are the ones issue #122 wedges. `docs/coverage-read-findings.md` records an API 33 run that
|
||||
* reported `received: 60` and `failed: unknown`: the regime *was* exercised, and that leg could
|
||||
* not have said so if it had broken. Four `@Config` classes here pin all three arms
|
||||
* deterministically, in the same `./gradlew` invocation as everything else.
|
||||
*
|
||||
* `minSdk` is 33, so none of these is dead code — each is a device someone is running the app on.
|
||||
*
|
||||
* **SDK 35 is in the list for the boundary, not for the answer.** It shares its answer with 36,
|
||||
* which would make it look redundant. It is not: relaxing `>= VANILLA_ICE_CREAM` to `>` is invisible
|
||||
* at every level except exactly 35, so without this class that mutation survives the suite.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@Config(sdk = [33])
|
||||
class ForegroundTypeApi33Test {
|
||||
|
||||
/**
|
||||
* Zero rather than a named constant because there is no constant to name: API 33 does not
|
||||
* require a type, and `mediaProcessing` does not exist here to pass. `ForegroundInfo` reads 0
|
||||
* as "no type at all", which is what this regime wants.
|
||||
*/
|
||||
@Test
|
||||
fun `api 33 asks for no foreground service type`() {
|
||||
assertEquals(0, ConversionForegroundType.current())
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* API 34 makes a type mandatory and still has no `mediaProcessing`, so `dataSync` is the only
|
||||
* sensible fit. See [ForegroundTypeApi33Test] for why this file exists.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@Config(sdk = [34])
|
||||
class ForegroundTypeApi34Test {
|
||||
|
||||
@Test
|
||||
fun `api 34 falls back to dataSync, the only type that fits`() {
|
||||
assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC, ConversionForegroundType.current())
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The first level with `mediaProcessing`, and therefore the one that tells `>=` from `>`.
|
||||
* See [ForegroundTypeApi33Test].
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@Config(sdk = [35])
|
||||
class ForegroundTypeApi35Test {
|
||||
|
||||
@Test
|
||||
fun `api 35 is the first level that takes mediaProcessing`() {
|
||||
assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROCESSING, ConversionForegroundType.current())
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The level the rest of the suite runs at, asserted here rather than assumed — it is the one arm
|
||||
* that was already covered, and leaving it out would make this file look like it is about the old
|
||||
* levels rather than about all three regimes. See [ForegroundTypeApi33Test].
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@Config(sdk = [36])
|
||||
class ForegroundTypeApi36Test {
|
||||
|
||||
@Test
|
||||
fun `api 36 keeps mediaProcessing`() {
|
||||
assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROCESSING, ConversionForegroundType.current())
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,226 @@
|
||||
package org.libremediaconverter.work
|
||||
|
||||
import android.app.Application
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import androidx.work.ListenableWorker
|
||||
import androidx.work.testing.TestListenableWorkerBuilder
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertThrows
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.HardwareTranscoder
|
||||
import org.libremediaconverter.convert.SoftwareTranscoder
|
||||
import org.libremediaconverter.convert.installTestWorkManager
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.ConversionRequest
|
||||
import org.libremediaconverter.model.DeviceCodecs
|
||||
import org.libremediaconverter.model.EnginePreference
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import java.io.File
|
||||
import java.util.UUID
|
||||
|
||||
/**
|
||||
* What happens when the hardware engine does not finish the job.
|
||||
*
|
||||
* `runMedia3OrFallBack` was eleven lines at 0% on the JVM and `isCancellation` had never been
|
||||
* called by any unit test at all. Its own KDoc calls the fallback the protection against vendor
|
||||
* hardware encoders that "cannot be tested for correctness", so it is the branch most likely to
|
||||
* matter on a device nobody here owns — and it was reachable the whole time through
|
||||
* `ConversionDependencies.hardware`, which no unit test had ever used.
|
||||
*
|
||||
* The sharp one is cancellation. `runMedia3OrFallBack` catches `Throwable`, so without the
|
||||
* `isCancellation` re-throw a user cancelling a hardware transcode would have the app quietly
|
||||
* start a *second* conversion in software — the one thing cancelling is supposed to prevent.
|
||||
*
|
||||
* `ForcedFailureTest` covers the failure half on a device. It does not cover the cancellation half,
|
||||
* and this host cannot run it either way.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class HardwareFallbackTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var hardware: RecordingHardwareTranscoder
|
||||
private lateinit var software: RecordingSoftwareTranscoder
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
hardware = RecordingHardwareTranscoder()
|
||||
software = RecordingSoftwareTranscoder()
|
||||
ConversionDependencies.publisher = { AlwaysRoomPublisher(app) }
|
||||
ConversionDependencies.hardware = { hardware }
|
||||
ConversionDependencies.software = { software }
|
||||
// A probe with real codecs, not the default: `InputProbe()` reports UNPARSEABLE, which
|
||||
// PERMISSIVE.canDecode refuses, and the router would send every job here straight to
|
||||
// FFmpeg without any of these tests mentioning why.
|
||||
ConversionDependencies.probe = { _, _ -> H264_SOURCE }
|
||||
ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE }
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
ConversionDependencies.reset()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a hardware failure runs the job again in software, on a clean staging file`() {
|
||||
hardware.failWith = { error("the vendor encoder produced nothing usable") }
|
||||
|
||||
val result = runBlocking { worker().doWork() }
|
||||
|
||||
assertTrue("the job should still succeed, got $result", result is ListenableWorker.Result.Success)
|
||||
assertEquals("the hardware engine gets exactly one attempt", 1, hardware.attempts)
|
||||
assertEquals("and the job then goes to software", 1, software.attempts)
|
||||
// The `staged.delete()` between the two, asserted where it is observable: FFmpeg must not
|
||||
// find a half-written hardware output sitting at the path it is about to write.
|
||||
assertFalse(
|
||||
"the partial hardware output must be gone before FFmpeg starts",
|
||||
software.outputExistedOnEntry,
|
||||
)
|
||||
assertEquals("the hardware engine is closed either way", 1, hardware.closes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cancelled hardware transcode is not quietly retried in software`() {
|
||||
hardware.failWith = { throw CancellationException("the user pressed Cancel") }
|
||||
|
||||
assertThrows(CancellationException::class.java) { runBlocking { worker().doWork() } }
|
||||
|
||||
assertEquals("the hardware engine ran", 1, hardware.attempts)
|
||||
assertEquals(
|
||||
"cancelling must not start a second conversion -- that is the whole point of cancelling",
|
||||
0,
|
||||
software.attempts,
|
||||
)
|
||||
assertEquals("and the engine is still closed on the way out", 1, hardware.closes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a hardware transcode that works never reaches the software engine`() {
|
||||
val result = runBlocking { worker().doWork() }
|
||||
|
||||
assertTrue("got $result", result is ListenableWorker.Result.Success)
|
||||
assertEquals(1, hardware.attempts)
|
||||
assertEquals("the fallback is a fallback, not a second pass", 0, software.attempts)
|
||||
assertEquals(1, hardware.closes)
|
||||
}
|
||||
|
||||
/**
|
||||
* #169: the display-name fallback, which reaches further than the notification title.
|
||||
*
|
||||
* `inputData.getString(KEY_DISPLAY_NAME) ?: "input"` had never taken its right-hand side. The
|
||||
* value is not only the foreground notification's title: it feeds `outputNameFor`, so it is
|
||||
* also the filename offered in the user's save dialog. A job enqueued by an older build, or
|
||||
* built by hand, carries no such key.
|
||||
*/
|
||||
@Test
|
||||
fun `a job that names no input file still suggests an output name`() {
|
||||
val result = runBlocking { worker(displayName = null).doWork() }
|
||||
|
||||
assertTrue("got $result", result is ListenableWorker.Result.Success)
|
||||
val suggested = (result as ListenableWorker.Result.Success)
|
||||
.outputData.getString(ConversionWorker.KEY_SUGGESTED_NAME)
|
||||
assertTrue(
|
||||
"expected a name built from the fallback, got $suggested",
|
||||
suggested.orEmpty().startsWith("input"),
|
||||
)
|
||||
}
|
||||
|
||||
private fun worker(displayName: String? = DISPLAY_NAME): ConversionWorker {
|
||||
val spec = OutputFormat.MP4_H265.spec
|
||||
val entries = buildMap<String, Any> {
|
||||
put(ConversionWorker.KEY_INPUT_URI, INPUT.toString())
|
||||
displayName?.let { put(ConversionWorker.KEY_DISPLAY_NAME, it) }
|
||||
put(ConversionWorker.KEY_SIZE_BYTES, INPUT_BYTES)
|
||||
put(ConversionWorker.KEY_CONTAINER, spec.container.name)
|
||||
put(ConversionWorker.KEY_VIDEO_CODEC, spec.videoCodec.name)
|
||||
put(ConversionWorker.KEY_AUDIO_CODEC, spec.audioCodec.name)
|
||||
// AUTO rather than FORCE_SOFTWARE, which is what every other worker test uses and is
|
||||
// exactly why this path had no coverage: forcing software never enters the function.
|
||||
put(ConversionWorker.KEY_ENGINE_PREFERENCE, EnginePreference.AUTO.name)
|
||||
}
|
||||
return TestListenableWorkerBuilder<ConversionWorker>(
|
||||
context = app,
|
||||
inputData = Data.Builder().putAll(entries).build(),
|
||||
runAttemptCount = 0,
|
||||
).setId(JOB_ID).build()
|
||||
}
|
||||
|
||||
private companion object {
|
||||
val INPUT: Uri = Uri.parse("file:///tmp/holiday.mp4")
|
||||
const val DISPLAY_NAME = "holiday.mp4"
|
||||
const val INPUT_BYTES = 1024L
|
||||
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000009")
|
||||
val H264_SOURCE = InputProbe(
|
||||
videoCodec = "h264",
|
||||
audioCodec = "aac",
|
||||
container = Container.MP4,
|
||||
durationMs = 1_000,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A hardware engine that writes something before it fails, and remembers being closed.
|
||||
*
|
||||
* Writing first is the point, exactly as it is for `PartialThenFailingTranscoder`: an engine that
|
||||
* only threw would let a missing `staged.delete()` pass unnoticed.
|
||||
*/
|
||||
@UnstableApi
|
||||
private class RecordingHardwareTranscoder : HardwareTranscoder {
|
||||
|
||||
var attempts = 0
|
||||
var closes = 0
|
||||
var failWith: (() -> Unit)? = null
|
||||
|
||||
override suspend fun transcode(input: Uri, output: File, request: ConversionRequest, onProgress: (Int) -> Unit) {
|
||||
attempts++
|
||||
output.writeBytes(ByteArray(PARTIAL_BYTES))
|
||||
failWith?.invoke()
|
||||
}
|
||||
|
||||
override fun close() {
|
||||
closes++
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val PARTIAL_BYTES = 2048
|
||||
}
|
||||
}
|
||||
|
||||
/** The software engine, recording whether the hardware attempt's leftovers were cleared first. */
|
||||
private class RecordingSoftwareTranscoder : SoftwareTranscoder {
|
||||
|
||||
var attempts = 0
|
||||
var outputExistedOnEntry = false
|
||||
|
||||
override suspend fun run(
|
||||
request: ConversionRequest,
|
||||
inputPath: String,
|
||||
output: File,
|
||||
durationMs: Long,
|
||||
onProgress: (Int) -> Unit,
|
||||
) {
|
||||
attempts++
|
||||
outputExistedOnEntry = output.exists()
|
||||
output.writeBytes(ByteArray(OUTPUT_BYTES))
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val OUTPUT_BYTES = 512
|
||||
}
|
||||
}
|
||||
@@ -16,6 +16,7 @@ import androidx.work.testing.WorkManagerTestInitHelper
|
||||
import androidx.work.workDataOf
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
@@ -125,6 +126,39 @@ class JobSnapshotsTest {
|
||||
assertEquals(newer.absolutePath, Reattachment.choose(snapshots)?.job?.outputPath)
|
||||
}
|
||||
|
||||
/**
|
||||
* A job in the tag query that never recorded an output path at all.
|
||||
*
|
||||
* Distinct from the three cases above, which all *have* a path and differ in what it names. A
|
||||
* job still running, or one that finished without writing its result key, carries no path at
|
||||
* all -- and `getWorkInfosByTagFlow` returns it alongside the finished ones, because the tag is
|
||||
* the worker class and every attempt ever enqueued carries it.
|
||||
*
|
||||
* The guard is the `?.` in `path?.let(::File)`. Without it the null goes straight into a `File`
|
||||
* constructor. What this pins is the consequence rather than the null check: such a job must
|
||||
* not be offered as a result, so `Reattachment.choose` has to walk past it to the job that
|
||||
* really produced a file. Choosing it would put a Converted screen in front of the user with a
|
||||
* Save button that has nothing to save.
|
||||
*/
|
||||
@Test
|
||||
fun `a job that recorded no output path is not offered as a result`() {
|
||||
val real = stagedFile("real.mp4", bytes = 4096)
|
||||
finishedWithOutput(real)
|
||||
finishedWithNoOutput()
|
||||
|
||||
val snapshots = snapshots()
|
||||
|
||||
assertEquals("both jobs carry the tag, so both come back", 2, snapshots.size)
|
||||
val silent = snapshots.single { it.outputPath == null }
|
||||
assertFalse("no path means no output, not an empty one", silent.outputExists)
|
||||
assertEquals("and no time either, for the same reason", 0L, silent.outputModifiedAt)
|
||||
assertEquals(
|
||||
"the reattachment has to walk past it to the job that really produced a file",
|
||||
real.absolutePath,
|
||||
Reattachment.choose(snapshots)?.job?.outputPath,
|
||||
)
|
||||
}
|
||||
|
||||
private fun snapshots(): List<JobSnapshot> = runBlocking {
|
||||
workManager.jobSnapshots(
|
||||
tag = ConversionWorker::class.java.name,
|
||||
@@ -152,6 +186,11 @@ class JobSnapshotsTest {
|
||||
).result.get()
|
||||
}
|
||||
|
||||
/** A job that carries the tag and no result key -- still running, or finished without one. */
|
||||
private fun finishedWithNoOutput() {
|
||||
workManager.enqueue(OneTimeWorkRequestBuilder<ConversionWorker>().build()).result.get()
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/** Two fixed moments a day apart, so the ordering is stated rather than raced for. */
|
||||
const val OLDER_MS = 1_700_000_000_000L
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
package org.libremediaconverter.work
|
||||
|
||||
import android.app.Notification
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNotEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.installTestWorkManager
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import java.util.UUID
|
||||
|
||||
/**
|
||||
* The two things a progress notification can say, and that they are not the same thing.
|
||||
*
|
||||
* An assertion gap rather than a coverage one, and the distinction is the reason this file exists.
|
||||
* JaCoCo is green on `build`'s `if (indeterminate)`, because `ProgressNotificationTest` drives it
|
||||
* through a real worker -- but that test reads only the notification id and
|
||||
* `Notification.EXTRA_PROGRESS`. **Nothing had ever read the text.** Swapping the two branches, or
|
||||
* collapsing them into one string, passed the entire suite.
|
||||
*
|
||||
* What it costs to get wrong is small and constant: a conversion that has been running for four
|
||||
* minutes still saying "Preparing", or one that has not started reporting yet claiming 0%. Neither
|
||||
* is a crash, and neither would be found by anything else here -- which is exactly the kind of
|
||||
* thing that survives for a long time.
|
||||
*
|
||||
* Nothing else in the suite constructs [ConversionNotifications] directly.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class NotificationProgressTextTest {
|
||||
|
||||
/**
|
||||
* `build` reaches `WorkManager.getInstance` for the Cancel action's PendingIntent, so the
|
||||
* notification cannot be built at all without one. That coupling is why nothing had ever
|
||||
* constructed this class directly and read what it produced.
|
||||
*/
|
||||
@Before
|
||||
fun setUp() {
|
||||
installTestWorkManager(RuntimeEnvironment.getApplication(), Data.EMPTY)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an indeterminate notification says something different from a measured one`() {
|
||||
val context = RuntimeEnvironment.getApplication()
|
||||
val notifications = ConversionNotifications(context)
|
||||
|
||||
val preparing = notifications.build(JOB_ID, TITLE, percent = 0, indeterminate = true).text()
|
||||
val measured = notifications.build(JOB_ID, TITLE, percent = 42, indeterminate = false).text()
|
||||
|
||||
assertNotEquals(
|
||||
"the two states have to read differently, or the text says nothing at all",
|
||||
preparing,
|
||||
measured,
|
||||
)
|
||||
assertTrue(
|
||||
"a measured notification has to carry its percentage, got \"$measured\"",
|
||||
measured.contains("42"),
|
||||
)
|
||||
assertTrue(
|
||||
"an indeterminate one must not invent one, got \"$preparing\"",
|
||||
!preparing.contains("42") && !preparing.contains("0"),
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The title is the caller's, not the builder's -- it is the file the user picked, and it is what
|
||||
* tells two simultaneous conversions apart in the shade.
|
||||
*/
|
||||
@Test
|
||||
fun `the notification is titled with the file it is converting`() {
|
||||
val context = RuntimeEnvironment.getApplication()
|
||||
|
||||
val built = ConversionNotifications(context).build(JOB_ID, TITLE, percent = 10)
|
||||
|
||||
assertEquals(TITLE, built.extras.getString(Notification.EXTRA_TITLE))
|
||||
}
|
||||
|
||||
private fun Notification.text(): String = extras.getString(Notification.EXTRA_TEXT).orEmpty()
|
||||
|
||||
private companion object {
|
||||
const val TITLE = "holiday.mp4"
|
||||
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000a")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user