Compare commits
48
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0f842243b5 | ||
|
|
2fbc957119 | ||
|
|
a645442acc | ||
|
|
5761faced6 | ||
|
|
c2c0bfa848 | ||
|
|
016030f3e4 | ||
|
|
d354f6470c | ||
|
|
dbedfb4708 | ||
|
|
c6e9a480e1 | ||
|
|
d8110d7a62 | ||
|
|
6f3966cc69 | ||
|
|
7595177e81 | ||
|
|
a507736d3d | ||
|
|
1ff5c4463c | ||
|
|
70337b2c12 | ||
|
|
fea480b000 | ||
|
|
ddfb1dd78e | ||
|
|
348eaa2f22 | ||
|
|
104d02de03 | ||
|
|
90814222b7 | ||
|
|
2d4898ad44 | ||
|
|
83ac7eff2c | ||
|
|
b2c11bbfe0 | ||
|
|
3a5210ec5d | ||
|
|
5f3eda9c40 | ||
|
|
79cca0eb47 | ||
|
|
2c0bc4a583 | ||
|
|
7a47285f37 | ||
|
|
8a2bc86cac | ||
|
|
9b3b9f952b | ||
|
|
a84b24ba27 | ||
|
|
0e2525195b | ||
|
|
713d813a65 | ||
|
|
c360e82a10 | ||
|
|
699d608b47 | ||
|
|
ad47ce6c96 | ||
|
|
c60d5d54c6 | ||
|
|
d59e9acce5 | ||
|
|
324c9a4555 | ||
|
|
8a88fc4ae7 | ||
|
|
44d4c61738 | ||
|
|
44493d9943 | ||
|
|
b2790e13d9 | ||
|
|
de6d9526ba | ||
|
|
bb3358f209 | ||
|
|
04850a0415 | ||
|
|
8ab433b647 | ||
|
|
5e58334230 |
@@ -130,8 +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** — **84.9% of lines (1971/2321), 63.8% of branches**,
|
||||
measured 2026-08-26 with `./gradlew :app:jacocoTestReport`, against 454 JVM tests in 67 classes.
|
||||
- **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.
|
||||
|
||||
**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
|
||||
@@ -147,12 +148,34 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
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 not: it moved
|
||||
39 points in a single build change on 2026-08-24, then another 16 as the #52 test push and the
|
||||
Two things still hold. A floor needs a baseline that has settled, and this one has not. It moved
|
||||
39 points in a single build change on 2026-08-24; then another 16 as the #52 test push and the
|
||||
fixes it turned up landed — 69.2% -> 84.9% line, 53.2% -> 63.8% branch — while the denominator
|
||||
grew 2194 -> 2321, because that work added production code of its own. And **re-measure before
|
||||
quoting**: this entry was written quoting 81.4%, measured four hours earlier, and was already
|
||||
three points stale by the time it was ready to merge.
|
||||
grew 2194 -> 2321, because that work added production code of its own; then again on 2026-08-27
|
||||
as #132 and #133's ten children landed — 84.9% -> 87.1% line, 63.8% -> **69.1%** branch, 454 ->
|
||||
502 tests. **Branch moved four times as far as line that last time**, and that is the shape to
|
||||
expect from this kind of work rather than a surprise: those children targeted decision code —
|
||||
enum fallbacks, refusal arms, cursor shapes, a `when` over container rules — where one test
|
||||
chooses a branch the suite had never taken. Line coverage barely notices; branch coverage is the
|
||||
whole point.
|
||||
|
||||
Then #153's five children on 2026-08-29 — 87.1% -> 88.9% line, 69.1% -> **75.4%** branch, 502 ->
|
||||
546 tests.
|
||||
|
||||
**That last branch figure moved for two reasons, and only one of them is new tests.** The
|
||||
numerator rose 974 -> 1011; the denominator *fell* 1410 -> 1340. Both are the seam work. Pulling
|
||||
a `when` out of a lambda inside a `collect` deletes the coroutine state machine's synthesized
|
||||
branches around it, and what is left is a plain function whose branches a test can choose:
|
||||
`ConversionViewModel$observe$1$1` went from carrying the whole mapping to 6 branches, while the
|
||||
extracted `ConversionViewModelKt` covers 41 of 42 and `JoinViewModelKt` 38 of 39. So a seam is
|
||||
worth more than the tests it enables — it also stops the measurement counting scaffolding.
|
||||
|
||||
Be careful quoting a branch move on its own for that reason. A percentage that rises because the
|
||||
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.
|
||||
|
||||
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
|
||||
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.
|
||||
@@ -174,6 +197,32 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
|
||||
- `kotlin.code.style=official`. Gradle stays Kotlin DSL.
|
||||
|
||||
- **A stacked PR does not merge with `gh pr merge`, and `MERGED` is not proof it reached `main`.**
|
||||
Two separate traps, both measured on 2026-08-27 while landing #144-#151.
|
||||
|
||||
`gh pr merge` uses the GraphQL mutation, which refuses a stacked PR outright: *"This pull request
|
||||
is part of a stack and must be merged using the asynchronous merge REST API."* So does
|
||||
`PUT .../pulls/{n}/merge`. The one that works is
|
||||
`gh api -X PUT repos/OWNER/REPO/pulls/N/merge-async -f merge_method=merge`, which returns a uuid
|
||||
to poll at `.../merge-async/{uuid}` until `status` is `merged` or `failed`.
|
||||
|
||||
**The second trap is worse, because nothing looks wrong.** GitHub retargets a stacked PR's base
|
||||
to `main` when the PR below it merges, but *asynchronously*. Merge a stack faster than that
|
||||
settles — five PRs about thirty seconds apart, in the case that found this — and each one merges
|
||||
into its own base branch, which has itself already been merged and left behind. Every call
|
||||
returns `status: merged` and every one is true. `gh pr list --state open` comes back empty, every
|
||||
PR shows `MERGED`, and **none of the content is on `main`**.
|
||||
|
||||
What caught it was a coverage re-measure reading two points lower than the same tree had measured
|
||||
an hour earlier; a fresh `git pull` changed nothing, which is what turned it into a question.
|
||||
`git merge-base --is-ancestor <merge-sha> origin/main` answers it in one line. Do that after
|
||||
merging a stack, or merge one at a time and re-read `baseRefName` between. #160 is what the
|
||||
recovery cost.
|
||||
|
||||
The auto-retarget belongs to the stacking feature specifically. A PR opened with a plain
|
||||
`gh pr create --base some-branch` does **not** retarget when that branch merges — it is left
|
||||
pointing at a dead base and has to be moved by hand.
|
||||
|
||||
- **File one-off issues with `tools/github/file-issue.sh`, not `gh issue create`.** `gh issue
|
||||
create` does not touch the project board, so the issue exists, carries its labels, and is
|
||||
invisible in the Kanban — indistinguishable from never having been filed. Measured 2026-08-24:
|
||||
|
||||
@@ -6,6 +6,7 @@ import android.util.Log
|
||||
import androidx.lifecycle.AndroidViewModel
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import androidx.work.WorkInfo
|
||||
import androidx.work.WorkManager
|
||||
import kotlinx.coroutines.CoroutineDispatcher
|
||||
@@ -71,6 +72,119 @@ data class InputFile(
|
||||
val probe: InputProbe? = null,
|
||||
)
|
||||
|
||||
/**
|
||||
* One update about a running conversion, as WorkManager last reported it.
|
||||
*
|
||||
* Only the fields [conversionStateFrom] reads — the same shape, and for the same reason, as
|
||||
* `JobSnapshot` beside `Reattachment.choose`: the rule stays testable on the JVM because nothing
|
||||
* in it needs a `WorkInfo`, which a test cannot readily build.
|
||||
*
|
||||
* [outputData] stays a `Data` rather than being unpacked into five nullable strings. It is what a
|
||||
* test already builds with `workDataOf` everywhere in this suite, so unpacking would move the same
|
||||
* reads without making anything easier to drive.
|
||||
*/
|
||||
internal data class ConversionUpdate(
|
||||
val state: WorkInfo.State,
|
||||
val progressPercent: Int,
|
||||
val runAttemptCount: Int,
|
||||
val outputData: Data,
|
||||
)
|
||||
|
||||
/**
|
||||
* What the screen should show, given what WorkManager last said about the job.
|
||||
*
|
||||
* ## Why this is a function rather than the body of a `collect`
|
||||
*
|
||||
* It was the body of one. `workManager` is built in the constructor from `WorkManager.getInstance`,
|
||||
* `observe` is private, and nothing could hand either a chosen `WorkInfo` — so every arm below ran
|
||||
* only when a real worker happened to produce it. A real worker produces a terminal state with
|
||||
* well-formed output, which meant six of these arms had never been chosen by any test: the progress
|
||||
* read, both sides of the retry check, a success with no file, a failure with nothing to say, and
|
||||
* the two that map to a state the user cannot otherwise reach.
|
||||
*
|
||||
* That is the argument #141 made for `MediaProbe`'s track walk, against `WorkManager` instead of a
|
||||
* media fixture, and it takes the same answer: the branch matrix is a pure function, and what is
|
||||
* left needing the framework — the flow, the null check, the ownership check — is the thin edge.
|
||||
*
|
||||
* ## What is deliberately *not* in here
|
||||
*
|
||||
* The ownership check stays at the call site. Its comment is explicit that it guards the file
|
||||
* ownership the `SUCCEEDED` arm takes, not merely the assignment, so moving it inside would change
|
||||
* what it protects. And this function takes no responsibility for the staged file: it returns the
|
||||
* state, and the caller reads the file off it. A pure function that deletes files is not a seam.
|
||||
*
|
||||
* @param cancelled where a cancellation lands, which differs for a reattached job — see [observe].
|
||||
* @param fallbackSpec the current settings, read only when finished work predates the worker
|
||||
* reporting its own name and MIME type.
|
||||
*/
|
||||
@UnstableApi
|
||||
internal fun conversionStateFrom(
|
||||
update: ConversionUpdate,
|
||||
input: InputFile,
|
||||
cancelled: ConversionState,
|
||||
fallbackSpec: OutputSpec,
|
||||
): ConversionState = when (update.state) {
|
||||
WorkInfo.State.RUNNING -> ConversionState.Converting(input, update.progressPercent)
|
||||
|
||||
// ENQUEUED after a run means a retry is pending. Either the six-hour foreground budget ran out
|
||||
// mid-job, or the system refused to let the job start again while the app was in the background
|
||||
// — the second being the likelier of the two, since it needs only a process restart. Nothing
|
||||
// here can tell them apart, and nothing needs to: the answer is the same.
|
||||
WorkInfo.State.ENQUEUED ->
|
||||
if (update.runAttemptCount > 0) {
|
||||
ConversionState.Waiting(input)
|
||||
} else {
|
||||
ConversionState.Converting(input, 0)
|
||||
}
|
||||
|
||||
WorkInfo.State.SUCCEEDED -> convertedFrom(update.outputData, input, fallbackSpec)
|
||||
|
||||
// A worker that dies before it can report anything leaves no output data at all — a
|
||||
// foreground-service start refused after a process restart is one way — and an exception's
|
||||
// message can be an empty string. Both would read as a failure with nothing said, so blank
|
||||
// falls back like missing does.
|
||||
WorkInfo.State.FAILED -> ConversionState.Failed(
|
||||
update.outputData.getString(ConversionWorker.KEY_ERROR)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: ConversionWorker.GENERIC_FAILURE_MESSAGE,
|
||||
)
|
||||
|
||||
WorkInfo.State.CANCELLED -> cancelled
|
||||
WorkInfo.State.BLOCKED -> ConversionState.Converting(input, 0)
|
||||
}
|
||||
|
||||
/**
|
||||
* The `SUCCEEDED` arm, which is the only one that reads more than one field.
|
||||
*
|
||||
* Split out so [conversionStateFrom] stays a table of one line per state. A success with no output
|
||||
* path is a failure: the job said it finished and named nothing, and there is no file to offer.
|
||||
*/
|
||||
@UnstableApi
|
||||
private fun convertedFrom(outputData: Data, input: InputFile, fallbackSpec: OutputSpec): ConversionState {
|
||||
val path = outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)
|
||||
?: return ConversionState.Failed(SUCCEEDED_WITHOUT_A_FILE_MESSAGE)
|
||||
return ConversionState.Converted(
|
||||
input = input,
|
||||
staged = File(path),
|
||||
engineUsed = outputData.getString(ConversionWorker.KEY_ENGINE_USED).orEmpty(),
|
||||
routeReason = outputData.getString(ConversionWorker.KEY_ROUTE_REASON).orEmpty(),
|
||||
// Work enqueued before the worker reported this carries nothing, and WorkManager keeps
|
||||
// finished work for about a week -- so this branch is ordinary for a few days rather than a
|
||||
// corner. It is the old derivation, kept because it is the same guess the app already made
|
||||
// and there is genuinely nothing better available for such a job. New work never reaches it.
|
||||
suggestedName = outputData.getString(ConversionWorker.KEY_SUGGESTED_NAME)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: ConversionWorker.outputNameFor(input.displayName, fallbackSpec),
|
||||
mimeType = outputData.getString(ConversionWorker.KEY_MIME_TYPE)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: fallbackSpec.mimeType,
|
||||
)
|
||||
}
|
||||
|
||||
/** A job that reported success and named no file. There is nothing to offer the user to save. */
|
||||
internal const val SUCCEEDED_WITHOUT_A_FILE_MESSAGE: String =
|
||||
"Conversion reported success but produced no file."
|
||||
|
||||
sealed interface ConversionState {
|
||||
data object Idle : ConversionState
|
||||
data class Ready(val input: InputFile) : ConversionState
|
||||
@@ -442,75 +556,24 @@ class ConversionViewModel @JvmOverloads constructor(
|
||||
// that either. The state and `pendingStaged` are meant to refer to the same file
|
||||
// or to no file, and this is where that stays true.
|
||||
if (!ownership.stillHeldBy(token)) return@collect
|
||||
_state.value = when (info.state) {
|
||||
WorkInfo.State.RUNNING -> ConversionState.Converting(
|
||||
input,
|
||||
info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
|
||||
)
|
||||
|
||||
// ENQUEUED after a run means a retry is pending. Either the six-hour
|
||||
// foreground budget ran out mid-job, or the system refused to let the job
|
||||
// start again while the app was in the background — the second being the
|
||||
// likelier of the two, since it needs only a process restart. Nothing here
|
||||
// can tell them apart, and nothing needs to: the answer is the same.
|
||||
WorkInfo.State.ENQUEUED ->
|
||||
if (info.runAttemptCount > 0) {
|
||||
ConversionState.Waiting(input)
|
||||
} else {
|
||||
ConversionState.Converting(input, 0)
|
||||
}
|
||||
|
||||
WorkInfo.State.SUCCEEDED -> {
|
||||
val path = info.outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)
|
||||
if (path == null) {
|
||||
ConversionState.Failed("Conversion reported success but produced no file.")
|
||||
} else {
|
||||
val staged = File(path)
|
||||
// Take responsibility for the file at the same moment the state
|
||||
// starts referring to it, so the two cannot disagree.
|
||||
pendingStaged = staged
|
||||
ConversionState.Converted(
|
||||
input = input,
|
||||
staged = staged,
|
||||
engineUsed = info.outputData
|
||||
.getString(ConversionWorker.KEY_ENGINE_USED).orEmpty(),
|
||||
routeReason = info.outputData
|
||||
.getString(ConversionWorker.KEY_ROUTE_REASON).orEmpty(),
|
||||
suggestedName = info.outputData
|
||||
.getString(ConversionWorker.KEY_SUGGESTED_NAME)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
// Work enqueued before the worker reported this carries
|
||||
// nothing, and WorkManager keeps finished work for about a
|
||||
// week -- so this branch is ordinary for a few days rather
|
||||
// than a corner. It is the old derivation, kept because it is
|
||||
// the same guess the app already made and there is genuinely
|
||||
// nothing better available for such a job. New work never
|
||||
// reaches it.
|
||||
?: ConversionWorker.outputNameFor(
|
||||
input.displayName,
|
||||
_settings.value.spec,
|
||||
),
|
||||
mimeType = info.outputData
|
||||
.getString(ConversionWorker.KEY_MIME_TYPE)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: _settings.value.spec.mimeType,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// A worker that dies before it can report anything leaves no output data at
|
||||
// all — a foreground-service start refused after a process restart is one
|
||||
// way — and an exception's message can be an empty string. Both would read
|
||||
// as a failure with nothing said, so blank falls back like missing does.
|
||||
WorkInfo.State.FAILED -> ConversionState.Failed(
|
||||
info.outputData.getString(ConversionWorker.KEY_ERROR)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: "Conversion failed.",
|
||||
)
|
||||
|
||||
WorkInfo.State.CANCELLED -> cancelled
|
||||
WorkInfo.State.BLOCKED -> ConversionState.Converting(input, 0)
|
||||
}
|
||||
val next = conversionStateFrom(
|
||||
ConversionUpdate(
|
||||
state = info.state,
|
||||
progressPercent = info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
|
||||
runAttemptCount = info.runAttemptCount,
|
||||
outputData = info.outputData,
|
||||
),
|
||||
input = input,
|
||||
cancelled = cancelled,
|
||||
fallbackSpec = _settings.value.spec,
|
||||
)
|
||||
// Take responsibility for the file at the same moment the state starts referring
|
||||
// to it, so the two cannot disagree. Read off the result rather than assigned
|
||||
// inside the mapping: `Converted` is the only state that carries a staged file, so
|
||||
// "the state and `pendingStaged` refer to the same file or to no file" is now the
|
||||
// shape of the code rather than a rule two branches have to keep.
|
||||
if (next is ConversionState.Converted) pendingStaged = next.staged
|
||||
_state.value = next
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -565,7 +628,7 @@ class ConversionViewModel @JvmOverloads constructor(
|
||||
// than a fresh handle for the second failure's sake: a retry that fails again
|
||||
// lands back here still carrying the file, not on a bare Failed that would take
|
||||
// the offer away.
|
||||
_state.value = ConversionState.Failed(e.message ?: "Could not save the file.", pending)
|
||||
_state.value = ConversionState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -94,24 +94,61 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
state = state,
|
||||
settings = settings,
|
||||
validation = validation,
|
||||
actions = ConverterActions(
|
||||
actions = converterActions(
|
||||
viewModel = viewModel,
|
||||
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,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Which of the ViewModel's methods each affordance on the screen calls.
|
||||
*
|
||||
* ## Why this is a function rather than an argument list
|
||||
*
|
||||
* It was an argument list, inside [ConverterScreen], which no test reached: `ConverterScreenContent`
|
||||
* builds its own [ConverterActions], so every test in the suite drove the stateless inner and none
|
||||
* of them ever saw the wiring.
|
||||
*
|
||||
* Most of the list is safe without a test, and saying so is more useful than pretending otherwise:
|
||||
* `onContainer`, `onVideoCodec`, `onAudioCodec`, `onPreset`, `onSuggestion`, `onQuality` and
|
||||
* `onEnginePreference` each take a distinct type, so binding one to another's setter does not
|
||||
* compile. Verified rather than assumed — swapping `onVideoCodec` and `onAudioCodec` fails with
|
||||
* *"Inapplicable candidate(s): fun setAudioCodec(codec: AudioCodec)"*.
|
||||
*
|
||||
* **[ConverterActions.onCancel] and [ConverterActions.onReset] are the exception.** Both are
|
||||
* `() -> Unit`, so swapping them compiles silently — also verified — and ships a Cancel button that
|
||||
* throws the conversion away and a Start-over button that leaves it on screen. That pair is what
|
||||
* `ConverterWiringTest` exists for.
|
||||
*
|
||||
* The three launcher-backed actions stay parameters: they need an `ActivityResultLauncher`, which
|
||||
* is the part that genuinely needs the composition, and keeping them out means the rest can be
|
||||
* checked without one.
|
||||
*/
|
||||
@UnstableApi
|
||||
internal fun converterActions(
|
||||
viewModel: ConversionViewModel,
|
||||
onPickInput: () -> Unit,
|
||||
onConvert: () -> Unit,
|
||||
onSave: (suggestedName: String) -> Unit,
|
||||
): ConverterActions = ConverterActions(
|
||||
onPickInput = onPickInput,
|
||||
onPreset = viewModel::setPreset,
|
||||
onContainer = viewModel::setContainer,
|
||||
onVideoCodec = viewModel::setVideoCodec,
|
||||
onAudioCodec = viewModel::setAudioCodec,
|
||||
onSuggestion = viewModel::applySuggestion,
|
||||
onQuality = viewModel::setQuality,
|
||||
onEnginePreference = viewModel::setEnginePreference,
|
||||
onConvert = onConvert,
|
||||
onCancel = viewModel::cancel,
|
||||
onSave = onSave,
|
||||
onReset = viewModel::reset,
|
||||
)
|
||||
|
||||
/**
|
||||
* Everything [ConverterScreenContent] can ask for, in one value.
|
||||
*
|
||||
|
||||
@@ -92,7 +92,12 @@ object MediaProbe {
|
||||
else -> InputKind.UNPARSEABLE
|
||||
}
|
||||
|
||||
private class Extracted(
|
||||
/**
|
||||
* `internal` rather than `private` so [extractedFrom] can be named from a test. The JVM test
|
||||
* source set is a friend of `main`, so this stays invisible outside the module — the precedent
|
||||
* is `MainActivity`'s `Destination`, and [containerFrom] beside it.
|
||||
*/
|
||||
internal class Extracted(
|
||||
val videoCodec: String?,
|
||||
val audioCodec: String?,
|
||||
val durationMs: Long,
|
||||
@@ -100,33 +105,55 @@ object MediaProbe {
|
||||
val height: Int,
|
||||
)
|
||||
|
||||
/**
|
||||
* What a set of track formats says about a file.
|
||||
*
|
||||
* Split out of [probeWithExtractor] so the rules below can be tested against tracks a test
|
||||
* *chooses*, rather than against whatever the committed fixtures happen to contain. The device
|
||||
* tests exercise this through real files; none of them can construct a two-video-track input,
|
||||
* a track that omits its duration, or an audio-before-video ordering on purpose.
|
||||
*
|
||||
* Three rules live here, and each is a decision rather than plumbing:
|
||||
*
|
||||
* - **First track of a type wins.** `video == null` is the whole guard. A file with two video
|
||||
* tracks must report the first, because that is the one an engine will transcode.
|
||||
* - **Duration is the maximum across tracks**, not the first one found or the last. A file
|
||||
* whose audio outlasts its video is ordinary, and reporting the video's length would cut the
|
||||
* progress bar short.
|
||||
* - **A track that omits `KEY_DURATION` contributes nothing** rather than zero. `MediaExtractor`
|
||||
* omits it for plenty of real tracks — see `MediaProbeTrackFieldsTest` — and `maxOf` against a
|
||||
* fabricated 0 would still be correct here, but reading a key that is absent is not.
|
||||
*/
|
||||
internal fun extractedFrom(formats: List<MediaFormat>): Extracted {
|
||||
var video: String? = null
|
||||
var audio: String? = null
|
||||
var durationUs = 0L
|
||||
var width = 0
|
||||
var height = 0
|
||||
|
||||
for (format in formats) {
|
||||
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
|
||||
if (format.containsKey(MediaFormat.KEY_DURATION)) {
|
||||
durationUs = maxOf(durationUs, format.getLong(MediaFormat.KEY_DURATION))
|
||||
}
|
||||
when {
|
||||
mime.startsWith("video/") && video == null -> {
|
||||
video = shortName(mime)
|
||||
width = format.intOr(MediaFormat.KEY_WIDTH)
|
||||
height = format.intOr(MediaFormat.KEY_HEIGHT)
|
||||
}
|
||||
|
||||
mime.startsWith("audio/") && audio == null -> audio = shortName(mime)
|
||||
}
|
||||
}
|
||||
return Extracted(video, audio, durationUs / US_PER_MS, width, height)
|
||||
}
|
||||
|
||||
private fun probeWithExtractor(context: Context, uri: Uri): Extracted? {
|
||||
val extractor = MediaExtractor()
|
||||
return try {
|
||||
extractor.setDataSource(context, uri, null)
|
||||
var video: String? = null
|
||||
var audio: String? = null
|
||||
var durationUs = 0L
|
||||
var width = 0
|
||||
var height = 0
|
||||
|
||||
for (i in 0 until extractor.trackCount) {
|
||||
val format = extractor.getTrackFormat(i)
|
||||
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
|
||||
if (format.containsKey(MediaFormat.KEY_DURATION)) {
|
||||
durationUs = maxOf(durationUs, format.getLong(MediaFormat.KEY_DURATION))
|
||||
}
|
||||
when {
|
||||
mime.startsWith("video/") && video == null -> {
|
||||
video = shortName(mime)
|
||||
width = format.intOr(MediaFormat.KEY_WIDTH)
|
||||
height = format.intOr(MediaFormat.KEY_HEIGHT)
|
||||
}
|
||||
|
||||
mime.startsWith("audio/") && audio == null -> audio = shortName(mime)
|
||||
}
|
||||
}
|
||||
Extracted(video, audio, durationUs / US_PER_MS, width, height)
|
||||
extractedFrom(extractor.trackFormats())
|
||||
} catch (e: Exception) {
|
||||
Log.i(TAG, "Platform extractor could not read $uri.", e)
|
||||
null
|
||||
@@ -269,25 +296,7 @@ object MediaProbe {
|
||||
val extractor = MediaExtractor()
|
||||
return try {
|
||||
extractor.setDataSource(context, uri, null)
|
||||
var video: String? = null
|
||||
var audio: String? = null
|
||||
var width = 0
|
||||
var height = 0
|
||||
var fps = 0
|
||||
|
||||
for (i in 0 until extractor.trackCount) {
|
||||
val format = extractor.getTrackFormat(i)
|
||||
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
|
||||
if (mime.startsWith("video/") && video == null) {
|
||||
video = shortName(mime)
|
||||
width = format.intOr(MediaFormat.KEY_WIDTH)
|
||||
height = format.intOr(MediaFormat.KEY_HEIGHT)
|
||||
fps = format.intOr(MediaFormat.KEY_FRAME_RATE)
|
||||
} else if (mime.startsWith("audio/") && audio == null) {
|
||||
audio = shortName(mime)
|
||||
}
|
||||
}
|
||||
ConcatInput(video, audio, width, height, fps)
|
||||
concatInputFrom(extractor.trackFormats())
|
||||
} catch (e: Exception) {
|
||||
Log.i(TAG, "Could not probe $uri for concat; will re-encode.", e)
|
||||
ConcatInput(null, null, 0, 0, 0)
|
||||
@@ -296,6 +305,45 @@ object MediaProbe {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The join flow's read of the same track formats. See [extractedFrom] for why this is separate
|
||||
* from the extractor.
|
||||
*
|
||||
* Deliberately **not** folded into [extractedFrom] despite the overlap. This one reads frame
|
||||
* rate and does not read duration; that one reads duration and does not read frame rate. A
|
||||
* merged version would have to compute both for every caller, and `ConcatPlanner` treats an
|
||||
* unknown frame rate as "cannot prove a match" — so a field this flow does not need must not
|
||||
* start arriving as a number.
|
||||
*/
|
||||
internal fun concatInputFrom(formats: List<MediaFormat>): ConcatInput {
|
||||
var video: String? = null
|
||||
var audio: String? = null
|
||||
var width = 0
|
||||
var height = 0
|
||||
var fps = 0
|
||||
|
||||
for (format in formats) {
|
||||
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
|
||||
if (mime.startsWith("video/") && video == null) {
|
||||
video = shortName(mime)
|
||||
width = format.intOr(MediaFormat.KEY_WIDTH)
|
||||
height = format.intOr(MediaFormat.KEY_HEIGHT)
|
||||
fps = format.intOr(MediaFormat.KEY_FRAME_RATE)
|
||||
} else if (mime.startsWith("audio/") && audio == null) {
|
||||
audio = shortName(mime)
|
||||
}
|
||||
}
|
||||
return ConcatInput(video, audio, width, height, fps)
|
||||
}
|
||||
|
||||
/**
|
||||
* Every track format this extractor holds, read once.
|
||||
*
|
||||
* The thin edge the two pure functions above leave behind: a `trackCount` and a
|
||||
* `getTrackFormat` per index, which is the whole of what needs a real `MediaExtractor`.
|
||||
*/
|
||||
private fun MediaExtractor.trackFormats(): List<MediaFormat> = (0 until trackCount).map(::getTrackFormat)
|
||||
|
||||
/**
|
||||
* One track property as an Int, or [fallback] when the format has no Int to give.
|
||||
*
|
||||
|
||||
@@ -5,6 +5,7 @@ import android.net.Uri
|
||||
import android.provider.DocumentsContract
|
||||
import android.provider.OpenableColumns
|
||||
import java.io.File
|
||||
import java.io.OutputStream
|
||||
|
||||
/**
|
||||
* What a save has to say when the staged file is not there any more.
|
||||
@@ -23,6 +24,20 @@ const val STAGED_FILE_GONE_MESSAGE: String =
|
||||
"The finished file is no longer in the cache, so there is nothing left to save. " +
|
||||
"Start over to make it again."
|
||||
|
||||
/**
|
||||
* What to tell the user when the copy into their chosen destination did not finish.
|
||||
*
|
||||
* A fallback, not the usual message: `publish` throws with a real reason most of the time — the
|
||||
* volume filled, the provider revoked the grant — and that reason is better than this. This is for
|
||||
* the exception that arrives with nothing to say, which would otherwise reach the screen as an
|
||||
* empty failure.
|
||||
*
|
||||
* Kept next to [STAGED_FILE_GONE_MESSAGE] for exactly the reason that one names: **both ViewModels
|
||||
* need it**, and saving is what it is about. It was written out twice before — `ConversionViewModel`
|
||||
* and `JoinViewModel` each carried their own copy of the literal, agreeing by coincidence.
|
||||
*/
|
||||
const val SAVE_FAILED_MESSAGE: String = "Could not save the file."
|
||||
|
||||
/**
|
||||
* A staged file that is still there to be saved, and everything the save dialog needs to offer it.
|
||||
*
|
||||
@@ -170,7 +185,7 @@ open class OutputPublisher(private val context: Context) {
|
||||
open fun publish(staged: File, destination: Uri) {
|
||||
val destinationWasEmpty = destinationIsKnownEmpty(destination)
|
||||
try {
|
||||
val out = context.contentResolver.openOutputStream(destination)
|
||||
val out = openDestination(destination)
|
||||
?: error("Could not open destination for writing: $destination")
|
||||
out.use { sink -> staged.inputStream().use { source -> source.copyTo(sink) } }
|
||||
} catch (failure: Throwable) {
|
||||
@@ -179,6 +194,22 @@ open class OutputPublisher(private val context: Context) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Opens [destination] for writing, or null when the provider will not.
|
||||
*
|
||||
* A seam, and a narrow one: it exists because `openOutputStream` has **two** ways of refusing
|
||||
* and only one of them is reachable from a test otherwise. A provider that has gone away throws
|
||||
* `FileNotFoundException` from inside the call; a provider that is present and declines returns
|
||||
* null. The two are not interchangeable here — the `?: error(...)` above is the only thing that
|
||||
* turns the second into a failure rather than an NPE further down — and no fake provider can be
|
||||
* asked to produce a null return on demand.
|
||||
*
|
||||
* `protected open` rather than injected, matching `hasSpaceFor` and `createStagingFile`:
|
||||
* `WorkerStubs.kt`'s publishers already override one method to force one condition.
|
||||
*/
|
||||
protected open fun openDestination(destination: Uri): OutputStream? =
|
||||
context.contentResolver.openOutputStream(destination)
|
||||
|
||||
/**
|
||||
* True only when the destination is *positively known* to hold no bytes yet.
|
||||
*
|
||||
@@ -256,7 +287,7 @@ open class OutputPublisher(private val context: Context) {
|
||||
open fun sweepStaging(nowMs: Long = System.currentTimeMillis()) {
|
||||
val dir = stagingDir
|
||||
val listing = dir.listFiles() ?: return
|
||||
val entries = listing.map { StagingSweep.Entry(it.name, it.lastModified()) }
|
||||
val entries = snapshot(listing)
|
||||
StagingSweep.collectable(entries, nowMs).forEach { name ->
|
||||
val file = File(dir, name)
|
||||
// Re-read the timestamp rather than trusting the snapshot above. Between the
|
||||
@@ -268,6 +299,22 @@ open class OutputPublisher(private val context: Context) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The name and age of everything [sweepStaging] found, read once.
|
||||
*
|
||||
* A seam for the *race*, not for the clock — [sweepStaging] already takes `nowMs`, so the clock
|
||||
* is the caller's. What has no seam otherwise is the window between this snapshot and the
|
||||
* per-file re-read below it, and that window is the entire reason the re-read exists.
|
||||
*
|
||||
* **It has to be here and not around `listFiles()`.** A test that changes a file before the
|
||||
* listing, or during it, changes what `StagingSweep.collectable` is given — so the file is
|
||||
* never proposed for deletion and the re-read is never reached. The race being modelled is a
|
||||
* file that *was* collectable when the snapshot was taken and is not by the time the delete
|
||||
* comes round, which is exactly one worker resuming in this same process.
|
||||
*/
|
||||
protected open fun snapshot(listing: Array<File>): List<StagingSweep.Entry> =
|
||||
listing.map { StagingSweep.Entry(it.name, it.lastModified()) }
|
||||
|
||||
private fun File.canonicalOrAbsolute(): File = runCatching { canonicalFile }.getOrDefault(absoluteFile)
|
||||
|
||||
private companion object {
|
||||
|
||||
@@ -56,17 +56,38 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
|
||||
JoinScreenContent(
|
||||
state = state,
|
||||
actions = JoinActions(
|
||||
actions = joinActions(
|
||||
viewModel = viewModel,
|
||||
onPickInputs = { pickInputs.launch(arrayOf("video/*")) },
|
||||
onJoin = viewModel::join,
|
||||
onCancel = viewModel::cancel,
|
||||
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
||||
onReset = viewModel::reset,
|
||||
),
|
||||
modifier = modifier,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Which of the ViewModel's methods each affordance on the join screen calls.
|
||||
*
|
||||
* The join-side twin of `converterActions`, and the transposition risk here is worse: **three**
|
||||
* `() -> Unit` bindings rather than two. `onJoin`, `onCancel` and `onReset` are mutually
|
||||
* interchangeable as far as the compiler is concerned, so a Join button that cancels, or a Cancel
|
||||
* button that starts the job, is a swap nothing but a test would catch.
|
||||
*
|
||||
* See `converterActions` for why the launcher-backed actions stay parameters.
|
||||
*/
|
||||
@UnstableApi
|
||||
internal fun joinActions(
|
||||
viewModel: JoinViewModel,
|
||||
onPickInputs: () -> Unit,
|
||||
onSave: (suggestedName: String) -> Unit,
|
||||
): JoinActions = JoinActions(
|
||||
onPickInputs = onPickInputs,
|
||||
onJoin = viewModel::join,
|
||||
onCancel = viewModel::cancel,
|
||||
onSave = onSave,
|
||||
onReset = viewModel::reset,
|
||||
)
|
||||
|
||||
/**
|
||||
* Everything [JoinScreenContent] can ask for, in one value.
|
||||
*
|
||||
|
||||
@@ -5,6 +5,7 @@ import android.net.Uri
|
||||
import androidx.lifecycle.AndroidViewModel
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import androidx.work.WorkInfo
|
||||
import androidx.work.WorkManager
|
||||
import kotlinx.coroutines.CoroutineDispatcher
|
||||
@@ -19,6 +20,7 @@ import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.InputFile
|
||||
import org.libremediaconverter.convert.InputQuery
|
||||
import org.libremediaconverter.convert.PendingSave
|
||||
import org.libremediaconverter.convert.SAVE_FAILED_MESSAGE
|
||||
import org.libremediaconverter.convert.STAGED_FILE_GONE_MESSAGE
|
||||
import org.libremediaconverter.convert.ScreenOwnership
|
||||
import org.libremediaconverter.model.ConcatStrategy
|
||||
@@ -29,6 +31,108 @@ import org.libremediaconverter.work.jobSnapshots
|
||||
import java.io.File
|
||||
import java.util.UUID
|
||||
|
||||
/**
|
||||
* One update about a running join, as WorkManager last reported it.
|
||||
*
|
||||
* The join-side twin of `ConversionUpdate`, and deliberately the same shape: this pair of seams is
|
||||
* one refactor done twice, and letting them diverge would make the two flows harder to compare than
|
||||
* the duplication costs. There is no `progressPercent` here because `ConcatWorker` publishes none —
|
||||
* a join is indeterminate.
|
||||
*/
|
||||
internal data class JoinUpdate(val state: WorkInfo.State, val runAttemptCount: Int, val outputData: Data)
|
||||
|
||||
/**
|
||||
* What the join screen should show, given what WorkManager last said about the job.
|
||||
*
|
||||
* The join-side twin of `conversionStateFrom`, extracted for the same reason and with the same two
|
||||
* exclusions: the ownership check stays at the call site, and this takes no responsibility for the
|
||||
* staged file. See that function's KDoc for the argument in full.
|
||||
*
|
||||
* Five arms had never been chosen by any test before this was cut out, because a real `ConcatWorker`
|
||||
* only ever produces a terminal state with well-formed output.
|
||||
*
|
||||
* @param cancelled where a cancellation lands, which differs for a reattached job — see [observe].
|
||||
*/
|
||||
@UnstableApi
|
||||
internal fun joinStateFrom(update: JoinUpdate, inputs: List<InputFile>, cancelled: JoinState): JoinState =
|
||||
when (update.state) {
|
||||
// BLOCKED is a job waiting on a prerequisite, which the user has nothing to do about and
|
||||
// nothing useful to be told about. It reads as "starting", like a fresh ENQUEUED.
|
||||
WorkInfo.State.RUNNING, WorkInfo.State.BLOCKED -> JoinState.Joining(inputs)
|
||||
|
||||
// ENQUEUED after a run means a retry is pending -- the same rule, and the same reasoning, as
|
||||
// the convert side. See `conversionStateFrom`.
|
||||
WorkInfo.State.ENQUEUED ->
|
||||
if (update.runAttemptCount > 0) {
|
||||
JoinState.Waiting(inputs)
|
||||
} else {
|
||||
JoinState.Joining(inputs)
|
||||
}
|
||||
|
||||
WorkInfo.State.SUCCEEDED -> joinedFrom(update.outputData)
|
||||
|
||||
// A worker that dies before it can report anything leaves no output data at all, and an
|
||||
// exception's message can be an empty string. Both would read as a failure with nothing said,
|
||||
// so blank falls back like missing does.
|
||||
WorkInfo.State.FAILED -> JoinState.Failed(
|
||||
update.outputData.getString(ConcatWorker.KEY_ERROR)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: ConcatWorker.GENERIC_FAILURE_MESSAGE,
|
||||
)
|
||||
|
||||
WorkInfo.State.CANCELLED -> cancelled
|
||||
}
|
||||
|
||||
/**
|
||||
* The `SUCCEEDED` arm. A join that reported success and named no file has nothing to offer.
|
||||
*/
|
||||
@UnstableApi
|
||||
private fun joinedFrom(outputData: Data): JoinState {
|
||||
val path = outputData.getString(ConcatWorker.KEY_OUTPUT_PATH)
|
||||
?: return JoinState.Failed(JOINED_WITHOUT_A_FILE_MESSAGE)
|
||||
return JoinState.Joined(
|
||||
staged = File(path),
|
||||
strategy = strategyFrom(outputData.getString(ConcatWorker.KEY_STRATEGY)),
|
||||
// A join enqueued before the worker reported these carries neither, and the fallback is
|
||||
// the format such a job really used -- ConcatWorker.request has always defaulted to it,
|
||||
// and the join screen has never offered a choice.
|
||||
suggestedName = outputData.getString(ConcatWorker.KEY_SUGGESTED_NAME)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT),
|
||||
mimeType = outputData.getString(ConcatWorker.KEY_MIME_TYPE)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: ConcatWorker.DEFAULT_FORMAT.mimeType,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The strategy a finished join reported, or [ConcatStrategy.REENCODE] when it named none.
|
||||
*
|
||||
* **Looked up rather than `valueOf`, and that is a fix rather than a style choice.** `valueOf`
|
||||
* throws `IllegalArgumentException` on a name this build does not define, and this runs inside a
|
||||
* `viewModelScope` collect with no handler -- so the throw does not become a `Failed` state, it
|
||||
* takes the process down.
|
||||
*
|
||||
* Reachable for the reason `WorkerEnumFallbackTest` and `JobTags` are both written on: WorkManager
|
||||
* keeps finished work for about a week, so a downgrade or a rollback hands this build a job
|
||||
* enqueued by another one. `ConcatWorker` writes `result.strategy.name` into the output `Data`, so
|
||||
* a build that added a third strategy would leave this one crashing on its own completed joins.
|
||||
*
|
||||
* `ConcatWorker.kt` already made exactly this change for `KEY_FORMAT`, and says why in as many
|
||||
* words: *"Looked up rather than `valueOf` … a format name this build does not define used to throw
|
||||
* past the catch."* The same read on this side had not been changed with it.
|
||||
*
|
||||
* REENCODE is the safe default rather than an arbitrary one: it is the answer for inputs that do
|
||||
* not match, so a job whose strategy cannot be read is described as the more conservative of the
|
||||
* two rather than being claimed as a lossless stream copy.
|
||||
*/
|
||||
private fun strategyFrom(name: String?): ConcatStrategy =
|
||||
ConcatStrategy.entries.firstOrNull { it.name == name } ?: ConcatStrategy.REENCODE
|
||||
|
||||
/** A join that reported success and named no file. There is nothing to offer the user to save. */
|
||||
internal const val JOINED_WITHOUT_A_FILE_MESSAGE: String =
|
||||
"Joining reported success but produced no file."
|
||||
|
||||
sealed interface JoinState {
|
||||
data object Idle : JoinState
|
||||
data class Ready(val inputs: List<InputFile>) : JoinState
|
||||
@@ -194,7 +298,7 @@ class JoinViewModel @JvmOverloads constructor(
|
||||
fun onInputsPicked(uris: List<Uri>) {
|
||||
val token = ownership.claim()
|
||||
if (uris.size < 2) {
|
||||
_state.value = JoinState.Failed("Pick at least two files to join.")
|
||||
_state.value = JoinState.Failed(ConcatWorker.TOO_FEW_INPUTS_MESSAGE)
|
||||
return
|
||||
}
|
||||
viewModelScope.launch {
|
||||
@@ -250,56 +354,19 @@ class JoinViewModel @JvmOverloads constructor(
|
||||
// takes ownership of the staged file, and a superseded observation must not do
|
||||
// that either.
|
||||
if (!ownership.stillHeldBy(token)) return@collect
|
||||
_state.value = when (info.state) {
|
||||
WorkInfo.State.RUNNING, WorkInfo.State.BLOCKED -> JoinState.Joining(inputs)
|
||||
WorkInfo.State.ENQUEUED ->
|
||||
if (info.runAttemptCount > 0) {
|
||||
JoinState.Waiting(inputs)
|
||||
} else {
|
||||
JoinState.Joining(inputs)
|
||||
}
|
||||
|
||||
WorkInfo.State.SUCCEEDED -> {
|
||||
val path = info.outputData.getString(ConcatWorker.KEY_OUTPUT_PATH)
|
||||
val strategy = info.outputData.getString(ConcatWorker.KEY_STRATEGY)
|
||||
?.let(ConcatStrategy::valueOf) ?: ConcatStrategy.REENCODE
|
||||
if (path == null) {
|
||||
JoinState.Failed("Joining reported success but produced no file.")
|
||||
} else {
|
||||
val staged = File(path)
|
||||
// Take responsibility for the file at the same moment the state
|
||||
// starts referring to it, so the two cannot disagree.
|
||||
pendingStaged = staged
|
||||
JoinState.Joined(
|
||||
staged = staged,
|
||||
strategy = strategy,
|
||||
// A join enqueued before the worker reported these carries
|
||||
// neither, and the fallback is the format such a job really
|
||||
// used -- ConcatWorker.request has always defaulted to it, and
|
||||
// the join screen has never offered a choice.
|
||||
suggestedName = info.outputData
|
||||
.getString(ConcatWorker.KEY_SUGGESTED_NAME)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT),
|
||||
mimeType = info.outputData
|
||||
.getString(ConcatWorker.KEY_MIME_TYPE)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: ConcatWorker.DEFAULT_FORMAT.mimeType,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// A worker that dies before it can report anything leaves no output data at
|
||||
// all, and an exception's message can be an empty string. Both would read as
|
||||
// a failure with nothing said, so blank falls back like missing does.
|
||||
WorkInfo.State.FAILED -> JoinState.Failed(
|
||||
info.outputData.getString(ConcatWorker.KEY_ERROR)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: "Joining failed.",
|
||||
)
|
||||
|
||||
WorkInfo.State.CANCELLED -> cancelled
|
||||
}
|
||||
val next = joinStateFrom(
|
||||
JoinUpdate(
|
||||
state = info.state,
|
||||
runAttemptCount = info.runAttemptCount,
|
||||
outputData = info.outputData,
|
||||
),
|
||||
inputs = inputs,
|
||||
cancelled = cancelled,
|
||||
)
|
||||
// Read off the result rather than assigned inside the mapping -- see the same
|
||||
// three lines in ConversionViewModel for why that is the better half of the swap.
|
||||
if (next is JoinState.Joined) pendingStaged = next.staged
|
||||
_state.value = next
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -346,7 +413,7 @@ class JoinViewModel @JvmOverloads constructor(
|
||||
// than leaving "Start over" -- which deletes it -- as the only thing on offer.
|
||||
// Passing `pending` rather than rebuilding it is what keeps a retry that fails
|
||||
// again on a carrying Failed instead of a bare one.
|
||||
_state.value = JoinState.Failed(e.message ?: "Could not save the file.", pending)
|
||||
_state.value = JoinState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -39,7 +39,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
val uris = inputData.getStringArray(KEY_INPUT_URIS)?.map(Uri::parse)
|
||||
?: return Result.failure(workDataOf(KEY_ERROR to "No input files."))
|
||||
if (uris.size < 2) {
|
||||
return Result.failure(workDataOf(KEY_ERROR to "Pick at least two files to join."))
|
||||
return Result.failure(workDataOf(KEY_ERROR to TOO_FEW_INPUTS_MESSAGE))
|
||||
}
|
||||
// Absent, not zero, when the picker could not size every input -- see the same read in
|
||||
// ConversionWorker and InputQuery for why the two are no longer one number.
|
||||
@@ -107,7 +107,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
}
|
||||
FailureOutcome.FAIL -> {
|
||||
Log.e(TAG, "Joining failed.", e)
|
||||
Result.failure(workDataOf(KEY_ERROR to (e.message ?: "Joining failed.")))
|
||||
Result.failure(workDataOf(KEY_ERROR to (e.message ?: GENERIC_FAILURE_MESSAGE)))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -137,6 +137,34 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
)
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* What the user is told when a join arrives with fewer than two inputs.
|
||||
*
|
||||
* Shared with `JoinViewModel`, which refuses the same condition one layer up so the picker
|
||||
* can answer without enqueueing anything. Two copies of this sentence existed before, and
|
||||
* only the one here was pinned by a test (#139) — so the wording could drift on the screen
|
||||
* without a single test noticing, for one message the user sees from one condition.
|
||||
*
|
||||
* 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.
|
||||
*/
|
||||
const val TOO_FEW_INPUTS_MESSAGE: String = "Pick at least two files to join."
|
||||
|
||||
/**
|
||||
* The last resort when a join fails and the exception says nothing.
|
||||
*
|
||||
* Shared with `JoinViewModel`, whose `FAILED` arm falls back to the same sentence when the
|
||||
* output `Data` carries no error at all — a worker killed before it could write one. The two
|
||||
* are a chain rather than a coincidence: this is what the worker puts *in* `KEY_ERROR`, and
|
||||
* that is what the ViewModel says when `KEY_ERROR` never arrived. The user cannot tell the
|
||||
* two apart and should not have to, so they are one sentence.
|
||||
*
|
||||
* The `Log.e` above deliberately keeps its own literal. A log line has a different audience
|
||||
* and carries the exception with it; coupling it to the user-facing wording would mean
|
||||
* rewording the screen to change a log.
|
||||
*/
|
||||
const val GENERIC_FAILURE_MESSAGE: String = "Joining failed."
|
||||
|
||||
const val KEY_INPUT_URIS = "input_uris"
|
||||
const val KEY_TOTAL_BYTES = "total_bytes"
|
||||
const val KEY_FORMAT = "format"
|
||||
|
||||
@@ -313,7 +313,7 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
|
||||
}
|
||||
FailureOutcome.FAIL -> {
|
||||
Log.e(TAG, "Conversion failed.", cause)
|
||||
Result.failure(workDataOf(KEY_ERROR to (cause.message ?: "Conversion failed.")))
|
||||
Result.failure(workDataOf(KEY_ERROR to (cause.message ?: GENERIC_FAILURE_MESSAGE)))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -352,6 +352,20 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
|
||||
)
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* The last resort when a conversion fails and the exception says nothing.
|
||||
*
|
||||
* Shared with `ConversionViewModel`, whose `FAILED` arm falls back to the same sentence when
|
||||
* the output `Data` carries no error — a worker killed before it could write one, which a
|
||||
* refused foreground start after a process restart produces. The two are a chain rather than
|
||||
* a coincidence: this is what goes *into* `KEY_ERROR`, and that is what is said when
|
||||
* `KEY_ERROR` never arrived. The user cannot tell those apart and should not have to.
|
||||
*
|
||||
* See [ConcatWorker.GENERIC_FAILURE_MESSAGE] for the join-side twin, and the note there
|
||||
* about why the neighbouring `Log.e` keeps its own literal.
|
||||
*/
|
||||
const val GENERIC_FAILURE_MESSAGE: String = "Conversion failed."
|
||||
|
||||
const val KEY_INPUT_URI = "input_uri"
|
||||
const val KEY_DISPLAY_NAME = "display_name"
|
||||
const val KEY_SIZE_BYTES = "size_bytes"
|
||||
|
||||
@@ -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,238 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import androidx.work.WorkInfo
|
||||
import androidx.work.workDataOf
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.work.ConversionWorker
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* Every answer [conversionStateFrom] can give, chosen rather than stumbled into.
|
||||
*
|
||||
* ## What this revises
|
||||
*
|
||||
* The mapping is not cold code and never was: `ConversionViewModel$observe$1$1` reported 28 covered
|
||||
* lines before this file existed, because every test that drives a real worker runs it. What no
|
||||
* test did was **choose which arm it took**. A real worker reaches a terminal state with
|
||||
* well-formed output, so `SUCCEEDED`-with-a-path and `FAILED`-with-a-message were the only arms any
|
||||
* test had ever produced — the other six ran never.
|
||||
*
|
||||
* A `grep` for `WorkInfo.State.` across the JVM suite makes that look untrue: all six constants are
|
||||
* there. They are in `ReattachmentTest`, driven into **`Reattachment.choose`** — a different
|
||||
* function that encodes the same enqueued-means-retry rule. So that rule had a test in one of its
|
||||
* two homes, and the copy the user's screen reads had none.
|
||||
*
|
||||
* ## Why the seam, and why these assertions
|
||||
*
|
||||
* `WorkManager.getInstance` is called in the ViewModel's constructor and `observe` is private, so
|
||||
* nothing could hand this a chosen `WorkInfo`. Cutting the `when` out as a pure function over
|
||||
* [ConversionUpdate] is the answer #141 took for `MediaProbe`, and `JobSnapshot` beside
|
||||
* `Reattachment.choose` is the same shape again.
|
||||
*
|
||||
* The assertions are on the whole state, not on its type. `Converting(input, 40)` and
|
||||
* `Converting(input, 0)` are both `Converting`, and a mapping that dropped the progress read would
|
||||
* pass any test that only asked which class came back.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class ConversionStateMappingTest {
|
||||
|
||||
// --- running ------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `a running job reports the progress it published`() {
|
||||
// The percent is read from `progress`, not from `outputData`, and not from the settings.
|
||||
// A mapping that returned Converting(input, 0) for every RUNNING would leave the bar
|
||||
// pinned at zero for the whole conversion.
|
||||
val state = map(WorkInfo.State.RUNNING, progress = 40)
|
||||
|
||||
assertEquals(ConversionState.Converting(INPUT, 40), state)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a running job with no published progress reports zero rather than failing`() {
|
||||
// getInt's default. A worker that has started but not yet called setProgress is ordinary,
|
||||
// and must not read as an error.
|
||||
val state = map(WorkInfo.State.RUNNING, progress = null)
|
||||
|
||||
assertEquals(ConversionState.Converting(INPUT, 0), state)
|
||||
}
|
||||
|
||||
// --- enqueued: the rule that had a test only in its other home ----------
|
||||
|
||||
@Test
|
||||
fun `an enqueued job that has already run is waiting to retry`() {
|
||||
val state = map(WorkInfo.State.ENQUEUED, runAttemptCount = 1)
|
||||
|
||||
assertEquals(
|
||||
"an ENQUEUED after a run is a pending retry, which the user is told about",
|
||||
ConversionState.Waiting(INPUT),
|
||||
state,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an enqueued job that has never run is simply starting`() {
|
||||
// The other side, and the reason the test above is not enough on its own: a mapping that
|
||||
// ignored runAttemptCount and always answered Waiting would pass that one and fail this.
|
||||
val state = map(WorkInfo.State.ENQUEUED, runAttemptCount = 0)
|
||||
|
||||
assertEquals(ConversionState.Converting(INPUT, 0), state)
|
||||
}
|
||||
|
||||
// --- succeeded ----------------------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `a success that named no file is a failure, not an empty success`() {
|
||||
// The job said it finished and named nothing. There is no file to offer, so `Converted`
|
||||
// would put a Save button over a path that does not exist.
|
||||
val state = map(WorkInfo.State.SUCCEEDED, data = Data.EMPTY)
|
||||
|
||||
assertEquals(ConversionState.Failed(SUCCEEDED_WITHOUT_A_FILE_MESSAGE), state)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a success carries the worker's own name and type, not the current settings`() {
|
||||
val state = map(
|
||||
WorkInfo.State.SUCCEEDED,
|
||||
data = workDataOf(
|
||||
ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mkv",
|
||||
ConversionWorker.KEY_SUGGESTED_NAME to "holiday.mkv",
|
||||
ConversionWorker.KEY_MIME_TYPE to "video/x-matroska",
|
||||
ConversionWorker.KEY_ENGINE_USED to "FFMPEG",
|
||||
ConversionWorker.KEY_ROUTE_REASON to "container needs FFmpeg",
|
||||
),
|
||||
)
|
||||
|
||||
val converted = state as ConversionState.Converted
|
||||
assertEquals("holiday.mkv", converted.suggestedName)
|
||||
assertEquals("video/x-matroska", converted.mimeType)
|
||||
assertEquals("FFMPEG", converted.engineUsed)
|
||||
assertEquals("container needs FFmpeg", converted.routeReason)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a success from older work falls back to the current settings for name and type`() {
|
||||
// WorkManager keeps finished work about a week, so a job enqueued before the worker
|
||||
// reported these is ordinary for a few days rather than a corner case.
|
||||
val state = map(
|
||||
WorkInfo.State.SUCCEEDED,
|
||||
data = workDataOf(ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mp4"),
|
||||
)
|
||||
|
||||
val converted = state as ConversionState.Converted
|
||||
assertEquals(FALLBACK_SPEC.mimeType, converted.mimeType)
|
||||
assertEquals(
|
||||
ConversionWorker.outputNameFor(INPUT.displayName, FALLBACK_SPEC),
|
||||
converted.suggestedName,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a blank name or type falls back the same way a missing one does`() {
|
||||
// A blank string is not an answer. Without takeIf, the save dialog opens named "" and
|
||||
// registered for a MIME type of "", which no provider will accept.
|
||||
val state = map(
|
||||
WorkInfo.State.SUCCEEDED,
|
||||
data = workDataOf(
|
||||
ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mp4",
|
||||
ConversionWorker.KEY_SUGGESTED_NAME to "",
|
||||
ConversionWorker.KEY_MIME_TYPE to " ",
|
||||
),
|
||||
)
|
||||
|
||||
val converted = state as ConversionState.Converted
|
||||
assertEquals(FALLBACK_SPEC.mimeType, converted.mimeType)
|
||||
assertEquals(
|
||||
ConversionWorker.outputNameFor(INPUT.displayName, FALLBACK_SPEC),
|
||||
converted.suggestedName,
|
||||
)
|
||||
}
|
||||
|
||||
// --- failed -------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `a failure carries the reason the worker gave`() {
|
||||
val state = map(
|
||||
WorkInfo.State.FAILED,
|
||||
data = workDataOf(ConversionWorker.KEY_ERROR to "Not enough free space to convert."),
|
||||
)
|
||||
|
||||
assertEquals(ConversionState.Failed("Not enough free space to convert."), state)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a failure with nothing said still says something`() {
|
||||
// A worker killed before it could write output data leaves none at all -- a refused
|
||||
// foreground start after a process restart is one way. Failed("") would render as a blank
|
||||
// error card.
|
||||
val state = map(WorkInfo.State.FAILED, data = Data.EMPTY)
|
||||
|
||||
assertEquals(ConversionState.Failed(ConversionWorker.GENERIC_FAILURE_MESSAGE), state)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a failure whose message is blank falls back like a missing one`() {
|
||||
val state = map(
|
||||
WorkInfo.State.FAILED,
|
||||
data = workDataOf(ConversionWorker.KEY_ERROR to " "),
|
||||
)
|
||||
|
||||
assertEquals(ConversionState.Failed(ConversionWorker.GENERIC_FAILURE_MESSAGE), state)
|
||||
}
|
||||
|
||||
// --- cancelled and blocked ---------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `a cancellation lands wherever the caller said it should`() {
|
||||
// Not a fixed state: a conversion started here goes back to Ready with the picked file,
|
||||
// while one picked up by reattach goes to Idle, because the URI that job holds belongs to
|
||||
// a process that no longer exists. `observe`'s KDoc is where that distinction is set.
|
||||
val toReady = map(WorkInfo.State.CANCELLED, cancelled = ConversionState.Ready(INPUT))
|
||||
val toIdle = map(WorkInfo.State.CANCELLED, cancelled = ConversionState.Idle)
|
||||
|
||||
assertEquals(ConversionState.Ready(INPUT), toReady)
|
||||
assertEquals(ConversionState.Idle, toIdle)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a blocked job looks like one that is starting`() {
|
||||
// BLOCKED is a job waiting on a prerequisite. There is nothing useful to say about it that
|
||||
// differs from "starting", and inventing a state for it would put a word on screen the
|
||||
// user cannot act on.
|
||||
val state = map(WorkInfo.State.BLOCKED)
|
||||
|
||||
assertEquals(ConversionState.Converting(INPUT, 0), state)
|
||||
}
|
||||
|
||||
private fun map(
|
||||
state: WorkInfo.State,
|
||||
progress: Int? = null,
|
||||
runAttemptCount: Int = 0,
|
||||
data: Data = Data.EMPTY,
|
||||
cancelled: ConversionState = ConversionState.Ready(INPUT),
|
||||
): ConversionState = conversionStateFrom(
|
||||
ConversionUpdate(
|
||||
state = state,
|
||||
// Modelled on the call site, which reads `getInt(KEY_PROGRESS, 0)` -- so "no progress
|
||||
// published" is the default reaching the mapping, not a null it has to handle.
|
||||
progressPercent = progress ?: 0,
|
||||
runAttemptCount = runAttemptCount,
|
||||
outputData = data,
|
||||
),
|
||||
input = INPUT,
|
||||
cancelled = cancelled,
|
||||
fallbackSpec = FALLBACK_SPEC,
|
||||
)
|
||||
|
||||
private companion object {
|
||||
val INPUT = InputFile(Uri.parse("content://test/holiday.mov"), "holiday.mov", 4096L)
|
||||
val FALLBACK_SPEC = OutputFormat.MP4_H265.spec
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,234 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.content.ComponentName
|
||||
import android.content.ContentProvider
|
||||
import android.content.ContentValues
|
||||
import android.content.Context
|
||||
import android.content.IntentFilter
|
||||
import android.content.pm.ProviderInfo
|
||||
import android.database.Cursor
|
||||
import android.database.MatrixCursor
|
||||
import android.net.Uri
|
||||
import android.os.Bundle
|
||||
import android.provider.DocumentsContract
|
||||
import android.provider.OpenableColumns
|
||||
import org.robolectric.Robolectric
|
||||
import org.robolectric.Shadows.shadowOf
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* Content providers more than one test needs, and the registration dance they all repeat.
|
||||
*
|
||||
* Only that. A stub that serves one test stays in that test, next to the assertion it exists for —
|
||||
* the rule `work/WorkerStubs.kt` states, and the reason `UnreliableOutputStream` is still private to
|
||||
* `OutputPublisherPublishTest`.
|
||||
*
|
||||
* These started life inside `OutputPublisherPublishTest`, which is the only thing that needed a
|
||||
* provider at all. They moved here when `InputQuery`'s cursor reads turned out to need the same
|
||||
* provider answering *badly* — see [RowShape].
|
||||
*/
|
||||
|
||||
internal const val DOCUMENTS_AUTHORITY = "org.libremediaconverter.test.documents"
|
||||
internal const val PLAIN_AUTHORITY = "org.libremediaconverter.test.plain"
|
||||
|
||||
/**
|
||||
* How [FakeSafProvider] answers a metadata query.
|
||||
*
|
||||
* A provider is another app. It can be uninstalled, revoke its grant, crash, or simply answer
|
||||
* something the caller did not expect — and "answered something unexpected" is not one case but
|
||||
* several, which is why this is an enum rather than a boolean.
|
||||
*
|
||||
* The distinction that matters most to callers is **null versus missing versus zero versus
|
||||
* negative**. `InputQuery` exists to stop the last three being conflated: `hasSpaceFor(0)` is only
|
||||
* "is there 128 MB free", so a size nobody could determine must not arrive as `0`, and
|
||||
* `OutputPublisher.destinationIsKnownEmpty` must answer `false` — never "empty, go ahead and
|
||||
* delete" — for every one of them.
|
||||
*
|
||||
* Column-level granularity is deliberate. `OutputPublisher` reads only `SIZE`; `InputQuery` reads
|
||||
* both, and reaches a different answer depending on which one is bad.
|
||||
*/
|
||||
internal enum class RowShape {
|
||||
/** What a healthy provider answers: the file's real name and real length. */
|
||||
NORMAL,
|
||||
|
||||
/** A row is present and its `DISPLAY_NAME` cell is null. */
|
||||
NULL_DISPLAY_NAME,
|
||||
|
||||
/** A row is present and its `SIZE` cell is null. */
|
||||
NULL_SIZE,
|
||||
|
||||
/** The cursor carries no `DISPLAY_NAME` column at all — `getColumnIndex` gives `-1`. */
|
||||
NO_DISPLAY_NAME_COLUMN,
|
||||
|
||||
/** The cursor carries no `SIZE` column at all — `getColumnIndex` gives `-1`. */
|
||||
NO_SIZE_COLUMN,
|
||||
|
||||
/**
|
||||
* A size of `-1`.
|
||||
*
|
||||
* Not a corrupt provider: it is what anything without a fixed length reports — a pipe, or a
|
||||
* provider streaming its answer — and it is a third way of saying "unknown", distinct from a
|
||||
* null cell and from a missing column.
|
||||
*/
|
||||
NEGATIVE_SIZE,
|
||||
|
||||
/**
|
||||
* A cursor with the right columns and no rows in it.
|
||||
*
|
||||
* Distinct from returning `null`, which is what a provider that does not recognise the URI
|
||||
* does. Both mean "no answer", and code that treats one as an answer and the other as an
|
||||
* absence is wrong about one of them.
|
||||
*/
|
||||
NO_ROWS,
|
||||
|
||||
/**
|
||||
* The query itself throws.
|
||||
*
|
||||
* A resolver call is a call into another app, and that app can have been uninstalled, revoked
|
||||
* its grant, or simply crashed. `InputQuery.firstRow`'s KDoc is explicit that "a file picker is
|
||||
* not a place to bring the process down from", so this is the shape that proves the guard is
|
||||
* one.
|
||||
*/
|
||||
QUERY_THROWS,
|
||||
}
|
||||
|
||||
/**
|
||||
* A stand-in for the provider behind a SAF destination.
|
||||
*
|
||||
* It answers only what its callers ask of a document -- how many bytes are already there, what it
|
||||
* is called, and delete it -- backed by a real file so the assertions are about the filesystem
|
||||
* rather than about a mock's call log alone. The rest of the `ContentProvider` surface is stubbed.
|
||||
*
|
||||
* Writing is deliberately NOT routed through it. Robolectric's `ShadowContentResolver`
|
||||
* consults its registered-stream map before it reaches any provider, which is what lets a
|
||||
* test hand out a stream that writes some bytes and then fails -- a condition a real provider
|
||||
* cannot be asked to produce on demand.
|
||||
*/
|
||||
internal open class FakeSafProvider : ContentProvider() {
|
||||
|
||||
override fun onCreate() = true
|
||||
|
||||
override fun query(
|
||||
uri: Uri,
|
||||
projection: Array<out String>?,
|
||||
selection: String?,
|
||||
selectionArgs: Array<out String>?,
|
||||
sortOrder: String?,
|
||||
): Cursor? {
|
||||
if (rowShape == RowShape.QUERY_THROWS) throw SecurityException("provider revoked the grant")
|
||||
val file = backingFile(uri)
|
||||
if (!file.exists()) return null
|
||||
return MatrixCursor(columnsFor(rowShape)).apply {
|
||||
if (rowShape != RowShape.NO_ROWS) addRow(cellsFor(rowShape, file))
|
||||
}
|
||||
}
|
||||
|
||||
override fun call(method: String, arg: String?, extras: Bundle?): Bundle? {
|
||||
if (method != METHOD_DELETE_DOCUMENT) return null
|
||||
val target = extras?.getParcelable(EXTRA_URI, Uri::class.java) ?: return null
|
||||
deleteRequests += target
|
||||
deleteFailure?.let { throw it }
|
||||
backingFile(target).delete()
|
||||
return Bundle()
|
||||
}
|
||||
|
||||
override fun getType(uri: Uri) = "video/mp4"
|
||||
|
||||
override fun insert(uri: Uri, values: ContentValues?): Uri? = null
|
||||
|
||||
override fun delete(uri: Uri, selection: String?, selectionArgs: Array<out String>?) = 0
|
||||
|
||||
override fun update(uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array<out String>?) = 0
|
||||
|
||||
companion object {
|
||||
// DocumentsContract.METHOD_DELETE_DOCUMENT and EXTRA_URI are hidden from the public
|
||||
// SDK, so they cannot be referenced. These are the wire names
|
||||
// DocumentsContract.deleteDocument() actually sends, which is what a provider sees.
|
||||
const val METHOD_DELETE_DOCUMENT = "android:deleteDocument"
|
||||
const val EXTRA_URI = "uri"
|
||||
|
||||
/** Where the "documents" really live. Set per test to a Robolectric temp path. */
|
||||
lateinit var root: File
|
||||
|
||||
/** Every delete this provider was asked for, in order. Empty is an assertion too. */
|
||||
val deleteRequests = mutableListOf<Uri>()
|
||||
|
||||
/** Armed by the test that needs the cleanup itself to fail. */
|
||||
var deleteFailure: RuntimeException? = null
|
||||
|
||||
/**
|
||||
* How the next query answers. [reset] puts it back to [RowShape.NORMAL], so a test that
|
||||
* does not care never has to think about it.
|
||||
*/
|
||||
var rowShape: RowShape = RowShape.NORMAL
|
||||
|
||||
fun backingFile(uri: Uri) = File(root, uri.lastPathSegment.orEmpty())
|
||||
|
||||
fun reset(directory: File) {
|
||||
root = directory
|
||||
deleteRequests.clear()
|
||||
deleteFailure = null
|
||||
rowShape = RowShape.NORMAL
|
||||
}
|
||||
|
||||
private fun columnsFor(shape: RowShape): Array<String> = when (shape) {
|
||||
RowShape.NO_DISPLAY_NAME_COLUMN -> arrayOf(OpenableColumns.SIZE)
|
||||
RowShape.NO_SIZE_COLUMN -> arrayOf(OpenableColumns.DISPLAY_NAME)
|
||||
else -> arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE)
|
||||
}
|
||||
|
||||
private fun cellsFor(shape: RowShape, file: File): Array<Any?> = when (shape) {
|
||||
RowShape.NO_DISPLAY_NAME_COLUMN -> arrayOf(file.length())
|
||||
RowShape.NO_SIZE_COLUMN -> arrayOf<Any?>(file.name)
|
||||
RowShape.NULL_DISPLAY_NAME -> arrayOf(null, file.length())
|
||||
RowShape.NULL_SIZE -> arrayOf(file.name, null)
|
||||
RowShape.NEGATIVE_SIZE -> arrayOf(file.name, UNKNOWN_LENGTH)
|
||||
else -> arrayOf(file.name, file.length())
|
||||
}
|
||||
|
||||
/** What `statSize` reports for anything without a fixed length. See [RowShape.NEGATIVE_SIZE]. */
|
||||
private const val UNKNOWN_LENGTH = -1L
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The same provider, registered WITHOUT the documents-provider intent filter.
|
||||
*
|
||||
* A separate class because the package manager keys providers by component name, so two
|
||||
* authorities need two components. It exists to prove the guard is a guard: a content URI
|
||||
* from something that is not a documents provider must not be handed to `deleteDocument`.
|
||||
*/
|
||||
internal class FakePlainProvider : FakeSafProvider()
|
||||
|
||||
/**
|
||||
* Stands [provider] up on [authority] so `contentResolver` and the package manager both know it.
|
||||
*
|
||||
* `isDocumentUri()` does not look at the URI alone: it asks the package manager whether anything
|
||||
* answers `ACTION_DOCUMENTS_PROVIDER` for that authority. Registering the provider with the
|
||||
* resolver is not enough, which is the whole reason [asDocumentsProvider] is a parameter rather
|
||||
* than always true — the negative case is a test.
|
||||
*/
|
||||
internal fun registerProvider(
|
||||
context: Context,
|
||||
provider: Class<out FakeSafProvider>,
|
||||
authority: String,
|
||||
asDocumentsProvider: Boolean,
|
||||
) {
|
||||
val info = ProviderInfo().apply {
|
||||
this.authority = authority
|
||||
packageName = context.packageName
|
||||
name = provider.name
|
||||
exported = true
|
||||
grantUriPermissions = true
|
||||
}
|
||||
Robolectric.buildContentProvider(provider).create(info)
|
||||
|
||||
val packageManager = shadowOf(context.packageManager)
|
||||
packageManager.addOrUpdateProvider(info)
|
||||
if (asDocumentsProvider) {
|
||||
packageManager.addIntentFilterForProvider(
|
||||
ComponentName(context.packageName, provider.name),
|
||||
IntentFilter(DocumentsContract.PROVIDER_INTERFACE),
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,187 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.content.Context
|
||||
import android.net.Uri
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* What [InputQuery] makes of a metadata row.
|
||||
*
|
||||
* ## Why this is a separate file from `UnknownInputSizeTest`
|
||||
*
|
||||
* That test drives the case where **no provider is registered** — the query returns null and
|
||||
* `measure()` answers instead — and it drives it thoroughly. What it never does is hand `InputQuery`
|
||||
* a row. Before this file, nothing did: `firstRow`'s body, `displayNameOrNull` and `sizeOrNull` had
|
||||
* never executed in the JVM suite, so every branch inside them was untested.
|
||||
*
|
||||
* ## What is actually being pinned
|
||||
*
|
||||
* Not "does it read a cursor" — that would pass against almost any implementation. The rule is that
|
||||
* **a size nobody could determine must not arrive as a number**, and there are four separate ways a
|
||||
* provider fails to determine one: a null cell, a missing column, a negative value, and no row at
|
||||
* all. `InputQuery`'s KDoc states the stake:
|
||||
*
|
||||
* > a worker's input `Data` carries the size the *picker* found … `hasSpaceFor(0)` is only "is there
|
||||
* > 128 MB free".
|
||||
*
|
||||
* So each of those four must produce `null`, and `null` specifically — not `0`, not `-1`. A test
|
||||
* that asserted only "not the file's length" would pass on `0`, which is the exact conflation the
|
||||
* class exists to end.
|
||||
*
|
||||
* ## Why every fall-through lands on null here
|
||||
*
|
||||
* [FakeSafProvider] does not implement `openFile`, so `measure()` cannot answer for these URIs
|
||||
* either. That is deliberate: it isolates the cursor half. The other direction — the cursor says
|
||||
* nothing and `measure()` succeeds — is `UnknownInputSizeTest`'s
|
||||
* `a picked file no provider describes is measured rather than reported as empty`, and is not
|
||||
* repeated here.
|
||||
*
|
||||
* ## What the mutations say, including the one that does not bite
|
||||
*
|
||||
* Measured against `MatrixCursor`, which is what these tests drive:
|
||||
*
|
||||
* | call on a null cell | result |
|
||||
* |---|---|
|
||||
* | `getString` | returns `null` |
|
||||
* | `getLong` | returns **`0`** |
|
||||
*
|
||||
* That second row is why `sizeOrNull`'s `!isNull(it)` guard is load-bearing and why these tests
|
||||
* bite: remove it and a null size arrives as `0`, a real number indistinguishable from an empty
|
||||
* file, which is the precise conflation this class exists to end. Removing it reddens
|
||||
* `a null size is unknown rather than zero`. Removing the trailing `takeIf { it >= 0 }` reddens
|
||||
* `a negative size is unknown rather than reported`.
|
||||
*
|
||||
* **Named exemption: `displayNameOrNull`'s `!isNull(it)` guard is not pinned by anything here, and
|
||||
* cannot be.** `getString` returns null for a null cell, so the fallback applies with or without
|
||||
* the guard — removing it leaves every test in this file green. The guard is not redundant in
|
||||
* production: `Cursor.getString`'s contract states that whether it throws on a null column is
|
||||
* *implementation-defined*, and a real `ContentProvider` is free to throw where `MatrixCursor`
|
||||
* returns null. It should stay. It simply cannot be falsified with this cursor, and saying so is
|
||||
* better than implying `a null display name falls back without disturbing the size` covers it —
|
||||
* that test pins the behaviour, not the guard.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class InputQueryCursorTest {
|
||||
|
||||
private lateinit var context: Context
|
||||
private lateinit var uri: Uri
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
context = RuntimeEnvironment.getApplication()
|
||||
FakeSafProvider.reset(File(context.cacheDir, "picked").apply { mkdirs() })
|
||||
registerProvider(context, FakeSafProvider::class.java, DOCUMENTS_AUTHORITY, asDocumentsProvider = true)
|
||||
uri = Uri.parse("content://$DOCUMENTS_AUTHORITY/document/holiday.mp4")
|
||||
FakeSafProvider.backingFile(uri).writeBytes(ByteArray(PAYLOAD_BYTES))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a provider that answers properly supplies both the name and the size`() {
|
||||
val described = InputQuery.describe(context, uri)
|
||||
|
||||
assertEquals("holiday.mp4", described.displayName)
|
||||
assertEquals(PAYLOAD_BYTES.toLong(), described.sizeBytes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a null display name falls back without disturbing the size`() {
|
||||
FakeSafProvider.rowShape = RowShape.NULL_DISPLAY_NAME
|
||||
|
||||
val described = InputQuery.describe(context, uri)
|
||||
|
||||
assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
|
||||
// The two columns are read independently. A provider that cannot name the file can still
|
||||
// size it, and losing the size here would be a bug the name assertion alone would miss.
|
||||
assertEquals(PAYLOAD_BYTES.toLong(), described.sizeBytes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cursor with no display name column falls back rather than throwing`() {
|
||||
// getColumnIndex returns -1 rather than throwing, so the `it >= 0` guard is the only thing
|
||||
// between this and an IllegalArgumentException out of getString.
|
||||
FakeSafProvider.rowShape = RowShape.NO_DISPLAY_NAME_COLUMN
|
||||
|
||||
val described = InputQuery.describe(context, uri)
|
||||
|
||||
assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
|
||||
assertEquals(PAYLOAD_BYTES.toLong(), described.sizeBytes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a null size is unknown rather than zero`() {
|
||||
FakeSafProvider.rowShape = RowShape.NULL_SIZE
|
||||
|
||||
assertNull(unknownSizeMessage("a null cell"), InputQuery.sizeOf(context, uri))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cursor with no size column is unknown rather than zero`() {
|
||||
FakeSafProvider.rowShape = RowShape.NO_SIZE_COLUMN
|
||||
|
||||
assertNull(unknownSizeMessage("a missing column"), InputQuery.sizeOf(context, uri))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a negative size is unknown rather than reported`() {
|
||||
// What anything without a fixed length reports -- a pipe, or a provider streaming its
|
||||
// answer. Passing -1 through would be worse than passing 0: hasSpaceFor compares it
|
||||
// against free space, so it would read as "needs less than nothing".
|
||||
FakeSafProvider.rowShape = RowShape.NEGATIVE_SIZE
|
||||
|
||||
assertNull(unknownSizeMessage("a negative size"), InputQuery.sizeOf(context, uri))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cursor with no rows is unknown rather than zero`() {
|
||||
// Distinct from the provider returning null, which UnknownInputSizeTest covers. A cursor
|
||||
// that exists and holds nothing still has to reach the same answer.
|
||||
FakeSafProvider.rowShape = RowShape.NO_ROWS
|
||||
|
||||
val described = InputQuery.describe(context, uri)
|
||||
|
||||
assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
|
||||
assertNull(unknownSizeMessage("an empty cursor"), described.sizeBytes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a provider that throws is survived rather than propagated`() {
|
||||
// The guard firstRow's KDoc exists for: "a resolver call is a call into another app ... and
|
||||
// a file picker is not a place to bring the process down from". Without the runCatching,
|
||||
// this SecurityException reaches the caller and takes the pick with it.
|
||||
FakeSafProvider.rowShape = RowShape.QUERY_THROWS
|
||||
|
||||
val described = InputQuery.describe(context, uri)
|
||||
|
||||
assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
|
||||
assertNull(unknownSizeMessage("a provider that threw"), described.sizeBytes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a join total is unknown when any one input could not be sized`() {
|
||||
// The consequence the four cases above exist for, asserted once at the place it lands.
|
||||
// Summing the inputs that did answer would produce a lower bound indistinguishable from a
|
||||
// real total, which is what the space check cannot tell apart.
|
||||
FakeSafProvider.rowShape = RowShape.NULL_SIZE
|
||||
val unsizable = InputQuery.sizeOf(context, uri)
|
||||
FakeSafProvider.rowShape = RowShape.NORMAL
|
||||
val sizable = InputQuery.sizeOf(context, uri)
|
||||
|
||||
assertEquals(PAYLOAD_BYTES.toLong(), sizable)
|
||||
assertNull(unsizable)
|
||||
assertNull("one unknown input makes the whole total unknown", InputQuery.total(listOf(sizable, unsizable)))
|
||||
}
|
||||
|
||||
private fun unknownSizeMessage(cause: String) =
|
||||
"$cause means nobody could size the file; that must be null, not 0 -- hasSpaceFor(0) is only a headroom check"
|
||||
|
||||
private companion object {
|
||||
const val PAYLOAD_BYTES = 4096
|
||||
}
|
||||
}
|
||||
@@ -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,221 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.media.MediaFormat
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* The rules `MediaProbe` applies to a set of track formats.
|
||||
*
|
||||
* ## Why this exists, and what it revises
|
||||
*
|
||||
* Issue #84 classified `probeWithExtractor` and `probeForConcat` as device-bound and explicitly not
|
||||
* a gap:
|
||||
*
|
||||
* > These are exercised by `RemuxTest`, `ConcatEngineTest` and `RealMediaBenchmark` in
|
||||
* > `androidTest` … **Do not read their 0% as untested.**
|
||||
*
|
||||
* That was right about the measurement boundary and right about FFprobe. It was not right that
|
||||
* these are only orchestration. The track walk is a **branch matrix**, and `androidTest` reaches it
|
||||
* only through whatever the committed fixtures happen to contain — so none of the rules below is
|
||||
* *chosen* by any test there. A fixture with two video tracks, a track that omits its duration, or
|
||||
* an audio-before-video ordering is not something a device test would produce on purpose.
|
||||
*
|
||||
* The seam is the answer #133 preferred over driving `ShadowMediaExtractor`: the walk is a pure
|
||||
* function over `List<MediaFormat>`, and what is left needing a device — `setDataSource`,
|
||||
* `getTrackFormat`, `release` — is the thin edge `androidTest` should be covering. This is the
|
||||
* `work/FailureOutcome.kt` pattern `CLAUDE.md` names.
|
||||
*
|
||||
* `MediaFormat` is a real one throughout, not a stub. `MediaProbeTrackFieldsTest` records why that
|
||||
* matters: it is a heterogeneous map whose getters throw rather than coerce, and a hand-rolled
|
||||
* double would not reproduce that.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class MediaProbeTrackWalkTest {
|
||||
|
||||
// --- extractedFrom: the conversion flow's read ---------------------------
|
||||
|
||||
@Test
|
||||
fun `the first video track wins when a file carries two`() {
|
||||
// `video == null` is the entire guard. A file with two video tracks must report the first,
|
||||
// because that is the one an engine will transcode -- and the width and height must come
|
||||
// from the same track, not be mixed across them.
|
||||
val extracted = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1920, height = 1080),
|
||||
video(MediaFormat.MIMETYPE_VIDEO_HEVC, width = 640, height = 480),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals("h264", extracted.videoCodec)
|
||||
assertEquals(1920, extracted.width)
|
||||
assertEquals(1080, extracted.height)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the first audio track wins when a file carries two`() {
|
||||
val extracted = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_OPUS),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals("aac", extracted.audioCodec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `duration is the longest track, not the first or the last`() {
|
||||
// A file whose audio outlasts its video is ordinary. Taking the video's length would cut
|
||||
// the progress bar short; taking the last track's would be right only by accident of order.
|
||||
val extracted = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC, durationUs = 10_000_000),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC, durationUs = 12_500_000),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_OPUS, durationUs = 1_000_000),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals(12_500L, extracted.durationMs)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a track that does not declare its duration contributes nothing to it`() {
|
||||
// MediaExtractor omits KEY_DURATION for plenty of real tracks -- MediaProbeTrackFieldsTest
|
||||
// records the same for KEY_FRAME_RATE. Reading a key that is absent is what containsKey
|
||||
// stands between us and.
|
||||
val extracted = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC, durationUs = 7_000_000),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals(7_000L, extracted.durationMs)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `declaring audio before video changes nothing`() {
|
||||
// Track order is a property of the container, not of the content. Both orderings have to
|
||||
// reach the same answer or the same file remuxed twice would probe differently.
|
||||
val videoFirst = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1280, height = 720),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
|
||||
),
|
||||
)
|
||||
val audioFirst = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1280, height = 720),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals(videoFirst.videoCodec, audioFirst.videoCodec)
|
||||
assertEquals(videoFirst.audioCodec, audioFirst.audioCodec)
|
||||
assertEquals(videoFirst.width, audioFirst.width)
|
||||
assertEquals(videoFirst.height, audioFirst.height)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a track that is neither audio nor video is ignored`() {
|
||||
// Subtitle and timed-metadata tracks are common in MKV and MP4. Neither prefix matches, so
|
||||
// neither slot is filled -- and, importantly, a subtitle track must not be mistaken for the
|
||||
// absence of an audio track by some later `else`.
|
||||
val extracted = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
MediaFormat().apply { setString(MediaFormat.KEY_MIME, "text/vtt") },
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals("h264", extracted.videoCodec)
|
||||
assertNull(extracted.audioCodec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a file with no tracks reports nothing rather than zero-width video`() {
|
||||
val extracted = MediaProbe.extractedFrom(emptyList())
|
||||
|
||||
assertNull(extracted.videoCodec)
|
||||
assertNull(extracted.audioCodec)
|
||||
assertEquals(0L, extracted.durationMs)
|
||||
assertEquals(0, extracted.width)
|
||||
assertEquals(0, extracted.height)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an audio-only file reports no video codec at all`() {
|
||||
// The distinction MediaProbe.classify turns into InputKind.AUDIO_ONLY, and the reason
|
||||
// `hasVideo` exists: an audio file and a corrupt file must not look alike.
|
||||
val extracted = MediaProbe.extractedFrom(listOf(audio(MediaFormat.MIMETYPE_AUDIO_AAC)))
|
||||
|
||||
assertNull(extracted.videoCodec)
|
||||
assertEquals("aac", extracted.audioCodec)
|
||||
assertEquals(0, extracted.width)
|
||||
}
|
||||
|
||||
// --- concatInputFrom: the join flow's read -------------------------------
|
||||
|
||||
@Test
|
||||
fun `the join read takes frame rate from the first video track`() {
|
||||
val input = MediaProbe.concatInputFrom(
|
||||
listOf(
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1920, height = 1080, frameRate = 30),
|
||||
video(MediaFormat.MIMETYPE_VIDEO_HEVC, width = 640, height = 480, frameRate = 60),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals("h264", input.videoCodec)
|
||||
assertEquals("aac", input.audioCodec)
|
||||
assertEquals(1920, input.width)
|
||||
assertEquals(1080, input.height)
|
||||
assertEquals(30, input.frameRate)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a video track with no declared frame rate reports zero rather than guessing`() {
|
||||
// ConcatPlanner treats 0 as "cannot prove a match" and re-encodes. A guessed 30 would read
|
||||
// as agreement and produce a stream copy of clips that do not actually match -- the failure
|
||||
// its KDoc says the whole flow is arranged to avoid.
|
||||
val input = MediaProbe.concatInputFrom(listOf(video(MediaFormat.MIMETYPE_VIDEO_AVC)))
|
||||
|
||||
assertEquals(0, input.frameRate)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a file with no tracks joins as entirely unknown`() {
|
||||
val input = MediaProbe.concatInputFrom(emptyList())
|
||||
|
||||
assertNull(input.videoCodec)
|
||||
assertNull(input.audioCodec)
|
||||
assertEquals(0, input.width)
|
||||
assertEquals(0, input.height)
|
||||
assertEquals(0, input.frameRate)
|
||||
}
|
||||
|
||||
private fun video(
|
||||
mime: String,
|
||||
width: Int = 1920,
|
||||
height: Int = 1080,
|
||||
durationUs: Long? = null,
|
||||
frameRate: Int? = null,
|
||||
): MediaFormat = MediaFormat.createVideoFormat(mime, width, height).apply {
|
||||
durationUs?.let { setLong(MediaFormat.KEY_DURATION, it) }
|
||||
frameRate?.let { setInteger(MediaFormat.KEY_FRAME_RATE, it) }
|
||||
}
|
||||
|
||||
private fun audio(mime: String, durationUs: Long? = null): MediaFormat =
|
||||
MediaFormat.createAudioFormat(mime, SAMPLE_RATE, CHANNELS).apply {
|
||||
durationUs?.let { setLong(MediaFormat.KEY_DURATION, it) }
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val SAMPLE_RATE = 48_000
|
||||
const val CHANNELS = 2
|
||||
}
|
||||
}
|
||||
@@ -1,17 +1,7 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.content.ComponentName
|
||||
import android.content.ContentProvider
|
||||
import android.content.ContentValues
|
||||
import android.content.Context
|
||||
import android.content.IntentFilter
|
||||
import android.content.pm.ProviderInfo
|
||||
import android.database.Cursor
|
||||
import android.database.MatrixCursor
|
||||
import android.net.Uri
|
||||
import android.os.Bundle
|
||||
import android.provider.DocumentsContract
|
||||
import android.provider.OpenableColumns
|
||||
import org.junit.Assert.assertArrayEquals
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
@@ -20,7 +10,6 @@ import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.Robolectric
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import org.robolectric.Shadows.shadowOf
|
||||
@@ -31,90 +20,8 @@ import java.io.OutputStream
|
||||
/** What a destination volume says when it fills up mid-write. */
|
||||
private const val NO_SPACE = "No space left on device"
|
||||
|
||||
private const val DOCUMENTS_AUTHORITY = "org.libremediaconverter.test.documents"
|
||||
private const val PLAIN_AUTHORITY = "org.libremediaconverter.test.plain"
|
||||
|
||||
/**
|
||||
* A stand-in for the provider behind a SAF destination.
|
||||
*
|
||||
* It answers only what `publish()` asks of a destination -- how many bytes are already there,
|
||||
* and delete it -- backed by a real file so the assertions are about the filesystem rather
|
||||
* than about a mock's call log alone. The rest of the `ContentProvider` surface is stubbed.
|
||||
*
|
||||
* Writing is deliberately NOT routed through it. Robolectric's `ShadowContentResolver`
|
||||
* consults its registered-stream map before it reaches any provider, which is what lets a
|
||||
* test hand out a stream that writes some bytes and then fails -- the condition this whole
|
||||
* file exists for, and one a real provider cannot be asked to produce on demand.
|
||||
*/
|
||||
internal open class FakeSafProvider : ContentProvider() {
|
||||
|
||||
override fun onCreate() = true
|
||||
|
||||
override fun query(
|
||||
uri: Uri,
|
||||
projection: Array<out String>?,
|
||||
selection: String?,
|
||||
selectionArgs: Array<out String>?,
|
||||
sortOrder: String?,
|
||||
): Cursor? {
|
||||
val file = backingFile(uri)
|
||||
if (!file.exists()) return null
|
||||
return MatrixCursor(arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE)).apply {
|
||||
addRow(arrayOf<Any?>(file.name, file.length()))
|
||||
}
|
||||
}
|
||||
|
||||
override fun call(method: String, arg: String?, extras: Bundle?): Bundle? {
|
||||
if (method != METHOD_DELETE_DOCUMENT) return null
|
||||
val target = extras?.getParcelable(EXTRA_URI, Uri::class.java) ?: return null
|
||||
deleteRequests += target
|
||||
deleteFailure?.let { throw it }
|
||||
backingFile(target).delete()
|
||||
return Bundle()
|
||||
}
|
||||
|
||||
override fun getType(uri: Uri) = "video/mp4"
|
||||
|
||||
override fun insert(uri: Uri, values: ContentValues?): Uri? = null
|
||||
|
||||
override fun delete(uri: Uri, selection: String?, selectionArgs: Array<out String>?) = 0
|
||||
|
||||
override fun update(uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array<out String>?) = 0
|
||||
|
||||
companion object {
|
||||
// DocumentsContract.METHOD_DELETE_DOCUMENT and EXTRA_URI are hidden from the public
|
||||
// SDK, so they cannot be referenced. These are the wire names
|
||||
// DocumentsContract.deleteDocument() actually sends, which is what a provider sees.
|
||||
const val METHOD_DELETE_DOCUMENT = "android:deleteDocument"
|
||||
const val EXTRA_URI = "uri"
|
||||
|
||||
/** Where the "documents" really live. Set per test to a Robolectric temp path. */
|
||||
lateinit var root: File
|
||||
|
||||
/** Every delete this provider was asked for, in order. Empty is an assertion too. */
|
||||
val deleteRequests = mutableListOf<Uri>()
|
||||
|
||||
/** Armed by the test that needs the cleanup itself to fail. */
|
||||
var deleteFailure: RuntimeException? = null
|
||||
|
||||
fun backingFile(uri: Uri) = File(root, uri.lastPathSegment.orEmpty())
|
||||
|
||||
fun reset(directory: File) {
|
||||
root = directory
|
||||
deleteRequests.clear()
|
||||
deleteFailure = null
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The same provider, registered WITHOUT the documents-provider intent filter.
|
||||
*
|
||||
* A separate class because the package manager keys providers by component name, so two
|
||||
* authorities need two components. It exists to prove the guard is a guard: a content URI
|
||||
* from something that is not a documents provider must not be handed to `deleteDocument`.
|
||||
*/
|
||||
internal class FakePlainProvider : FakeSafProvider()
|
||||
/** How far a failing copy gets before the volume "fills up". Any value below the payload does. */
|
||||
private const val PARTIAL_BYTES = 512
|
||||
|
||||
/**
|
||||
* A sink that behaves like a volume filling up.
|
||||
@@ -171,6 +78,8 @@ class OutputPublisherPublishTest {
|
||||
|
||||
private val payload = ByteArray(8192) { (it % 251).toByte() }
|
||||
|
||||
/** How far a failing copy gets before the volume "fills up". Any value below the payload does. */
|
||||
|
||||
private val documentUri: Uri = Uri.parse("content://$DOCUMENTS_AUTHORITY/document/holiday.mp4")
|
||||
private val plainUri: Uri = Uri.parse("content://$PLAIN_AUTHORITY/document/holiday_plain.mp4")
|
||||
private val deadUri: Uri = Uri.parse("content://org.libremediaconverter.nonexistent/document/gone.mp4")
|
||||
@@ -179,8 +88,8 @@ class OutputPublisherPublishTest {
|
||||
fun setUp() {
|
||||
context = RuntimeEnvironment.getApplication()
|
||||
FakeSafProvider.reset(File(context.cacheDir, "destinations").apply { mkdirs() })
|
||||
register(FakeSafProvider::class.java, DOCUMENTS_AUTHORITY, asDocumentsProvider = true)
|
||||
register(FakePlainProvider::class.java, PLAIN_AUTHORITY, asDocumentsProvider = false)
|
||||
registerProvider(context, FakeSafProvider::class.java, DOCUMENTS_AUTHORITY, asDocumentsProvider = true)
|
||||
registerProvider(context, FakePlainProvider::class.java, PLAIN_AUTHORITY, asDocumentsProvider = false)
|
||||
|
||||
// SAF's CreateDocument contract hands back a document that already exists and is
|
||||
// empty, so that is the state every destination starts in here.
|
||||
@@ -299,6 +208,76 @@ class OutputPublisherPublishTest {
|
||||
assertEquals(emptyList<Uri>(), FakeSafProvider.deleteRequests)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a destination whose size cannot be determined is never deleted`() {
|
||||
// The three short-circuits in destinationIsKnownEmpty, and the reason its KDoc gives for
|
||||
// each of them answering false:
|
||||
//
|
||||
// "this decides whether a delete is allowed and 'I could not tell' must never authorise
|
||||
// one."
|
||||
//
|
||||
// The contrast is `a copy that fails partway leaves nothing at the destination` above: a
|
||||
// provider that *does* say zero gets the delete. These say nothing, so they must not.
|
||||
// Getting this backwards costs the user a file they already had, on a save that failed.
|
||||
//
|
||||
// Named exemption: of the three conjuncts, `size >= 0` cannot be falsified behaviourally.
|
||||
// Measured -- getColumnIndex returns -1 for an absent column, and isNull(-1) throws
|
||||
// CursorIndexOutOfBoundsException, which the surrounding runCatching already turns into
|
||||
// `?: false`. So relaxing it to `size >= -1` leaves this test green: same answer, reached
|
||||
// by the exception path instead. The guard should stay -- control flow through an exception
|
||||
// is worse than a comparison, and another Cursor implementation need not throw -- but no
|
||||
// assertion here pins it, and saying so beats implying the missing-column case covers it.
|
||||
// `!row.isNull(size)` and `row.moveToFirst()` do both bite.
|
||||
listOf(
|
||||
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))
|
||||
FakeSafProvider.rowShape = shape
|
||||
failMidCopy(documentUri, afterBytes = PARTIAL_BYTES)
|
||||
|
||||
assertThrows(IOException::class.java) { publisher.publish(staged, documentUri) }
|
||||
|
||||
assertEquals(
|
||||
"$description must not authorise a delete",
|
||||
emptyList<Uri>(),
|
||||
FakeSafProvider.deleteRequests,
|
||||
)
|
||||
assertTrue(
|
||||
"$description must leave the destination where it was",
|
||||
FakeSafProvider.backingFile(documentUri).exists(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a provider that declines by returning null fails with the destination named`() {
|
||||
// openOutputStream has two ways of refusing, and only one of them is otherwise reachable.
|
||||
// `a destination the provider will not open...` above drives the throwing one -- a provider
|
||||
// that has gone away. This is the other: a provider that is present, answers, and hands
|
||||
// back null. Without the `?: error(...)` that becomes an NPE inside `use`, which reaches
|
||||
// the user as "Conversion failed." with a null message.
|
||||
val nullOpening = object : OutputPublisher(context) {
|
||||
override fun openDestination(destination: Uri): OutputStream? = null
|
||||
}
|
||||
|
||||
val failure = runCatching { nullOpening.publish(staged, documentUri) }.exceptionOrNull()
|
||||
|
||||
assertTrue("a null stream must not appear to succeed, got $failure", failure != null)
|
||||
assertTrue(
|
||||
"the failure must name the destination rather than being a bare NPE; got ${failure?.message}",
|
||||
failure?.message?.contains("Could not open destination for writing") == true,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a copy that succeeds delivers every byte and deletes nothing`() {
|
||||
shadowOf(context.contentResolver).registerOutputStreamSupplier(documentUri) {
|
||||
@@ -326,28 +305,4 @@ class OutputPublisherPublishTest {
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private fun register(provider: Class<out FakeSafProvider>, authority: String, asDocumentsProvider: Boolean) {
|
||||
val info = ProviderInfo().apply {
|
||||
this.authority = authority
|
||||
packageName = context.packageName
|
||||
name = provider.name
|
||||
exported = true
|
||||
grantUriPermissions = true
|
||||
}
|
||||
Robolectric.buildContentProvider(provider).create(info)
|
||||
|
||||
// isDocumentUri() does not look at the URI alone: it asks the package manager whether
|
||||
// anything answers ACTION_DOCUMENTS_PROVIDER for that authority. Registering the
|
||||
// provider with the resolver is not enough, which is the whole reason the negative
|
||||
// case above can exist.
|
||||
val packageManager = shadowOf(context.packageManager)
|
||||
packageManager.addOrUpdateProvider(info)
|
||||
if (asDocumentsProvider) {
|
||||
packageManager.addIntentFilterForProvider(
|
||||
ComponentName(context.packageName, provider.name),
|
||||
IntentFilter(DocumentsContract.PROVIDER_INTERFACE),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.app.Application
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
@@ -26,14 +28,18 @@ import java.util.UUID
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class OutputPublisherStagingTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var cacheDir: File
|
||||
private lateinit var publisher: OutputPublisher
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
val context = RuntimeEnvironment.getApplication()
|
||||
cacheDir = context.cacheDir
|
||||
publisher = OutputPublisher(context)
|
||||
// Held as a field rather than a local: the race test below builds an anonymous
|
||||
// OutputPublisher, and inside that `object` expression a bare `context` resolves to the
|
||||
// superclass's own constructor property, which is not initialised at the super call.
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
cacheDir = app.cacheDir
|
||||
publisher = OutputPublisher(app)
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -99,4 +105,103 @@ class OutputPublisherStagingTest {
|
||||
|
||||
publisher.sweepStaging()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the sweep tolerates a staging path that is not a directory`() {
|
||||
// The other half of `listFiles() ?: return`, and not the same as the case above: a missing
|
||||
// directory is created by `stagingDir`'s own mkdirs() and lists as empty. Only a path that
|
||||
// cannot be a directory makes listFiles() answer null, and a sweep that dereferenced that
|
||||
// would take the app down on a launch rather than on a conversion -- AppStartSweepTest is
|
||||
// where this runs from.
|
||||
val stagingPath = stagingPathAsRegularFile()
|
||||
|
||||
publisher.sweepStaging()
|
||||
|
||||
assertTrue("the sweep must not have replaced the fixture", stagingPath.isFile)
|
||||
}
|
||||
|
||||
/**
|
||||
* Makes `cacheDir/conversions` a regular file, which is the whole precondition of the test
|
||||
* above -- and does it in a loop, because a single delete-then-write loses a race that CI
|
||||
* caught and this machine does not reproduce.
|
||||
*
|
||||
* `LibreMediaConverterApp.onCreate` ends with
|
||||
* `appScope.launch { OutputPublisher(...).sweepStaging() }` on `Dispatchers.IO`, and
|
||||
* `sweepStaging` reads `stagingDir`, whose getter calls `mkdirs()`. Robolectric instantiates
|
||||
* the application for every test that asks for one, so that background `mkdirs()` is in flight
|
||||
* across the whole suite, on a thread the paused main looper does not control. Between deleting
|
||||
* this path and writing it there is a window where the path does not exist and that `mkdirs()`
|
||||
* can win, which is `FileNotFoundException: ... (Is a directory)` out of `writeBytes` -- run
|
||||
* 33069641674 on #149, once, against 468 tests that pass here.
|
||||
*
|
||||
* Retrying closes it rather than narrowing it, because the race is not symmetric: `mkdirs()`
|
||||
* fails on an existing regular file, so the invariant only has to survive being *established*.
|
||||
* Once a write lands, nothing in the suite can turn this back into a directory.
|
||||
*
|
||||
* The wider problem -- application-scope IO work racing every Robolectric test that shares
|
||||
* `cacheDir` -- is #159, and is deliberately not fixed here.
|
||||
*/
|
||||
private fun stagingPathAsRegularFile(): File {
|
||||
val stagingPath = File(cacheDir, "conversions")
|
||||
repeat(FIXTURE_ATTEMPTS) {
|
||||
if (stagingPath.isFile) return stagingPath
|
||||
stagingPath.deleteRecursively()
|
||||
runCatching { stagingPath.writeBytes(ByteArray(FIXTURE_BYTES)) }
|
||||
}
|
||||
check(stagingPath.isFile) {
|
||||
"the fixture needs $stagingPath to be a regular file and it is a directory; " +
|
||||
"something recreated it $FIXTURE_ATTEMPTS times -- see #159"
|
||||
}
|
||||
return stagingPath
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a file that stops being collectable between the listing and the delete survives`() {
|
||||
// The race the second timestamp read exists for, and the only branch of it that had never
|
||||
// run. The comment in sweepStaging states the cost precisely: a worker resumed by
|
||||
// WorkManager -- in this same process -- could have started writing this very file, and
|
||||
// unlinking an inode a running job still holds open ends with the job reporting success for
|
||||
// a path that no longer exists.
|
||||
//
|
||||
// So: a file old enough to collect at listing time, touched to now before the delete is
|
||||
// reached. StagingSweep.collectable already said yes; isCollectable has to say no.
|
||||
val orphan = publisher.createStagingFile(
|
||||
StagingNames.forJob(UUID.randomUUID(), "mp4"),
|
||||
).apply { writeBytes(ByteArray(4096)) }
|
||||
assertTrue(orphan.setLastModified(System.currentTimeMillis() - StagingSweep.GRACE_PERIOD_MS - 60_000))
|
||||
|
||||
// Touched *after* the snapshot is taken, which is the only window that reaches the
|
||||
// re-read. Doing it around listFiles() instead changes what StagingSweep.collectable is
|
||||
// given, so the file is never proposed for deletion and the guard is never exercised --
|
||||
// measured, and the reason the seam sits where it does.
|
||||
val racing = object : OutputPublisher(app) {
|
||||
override fun snapshot(listing: Array<File>): List<StagingSweep.Entry> =
|
||||
super.snapshot(listing).also { orphan.setLastModified(System.currentTimeMillis()) }
|
||||
}
|
||||
|
||||
racing.sweepStaging()
|
||||
|
||||
assertTrue(
|
||||
"a file a live job started writing after the listing must not be unlinked",
|
||||
orphan.exists(),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `discarding a file with no parent at all is refused`() {
|
||||
// A relative name has no parent directory, so `staged.parentFile` is null. The handle
|
||||
// reaches the ViewModel as a path string out of WorkInfo.outputData and is turned straight
|
||||
// into a File, so this is not a shape the caller can rule out -- and the guard has to
|
||||
// answer false rather than dereference it.
|
||||
val parentless = File("holiday.mp4")
|
||||
assertNull("the fixture is supposed to have no parent", parentless.parentFile)
|
||||
|
||||
assertFalse("a file with no parent is not in staging", publisher.discardStaged(parentless))
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/** Enough to outlast a burst of application-scope sweeps; one attempt is what CI lost. */
|
||||
const val FIXTURE_ATTEMPTS = 50
|
||||
const val FIXTURE_BYTES = 8
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,241 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.app.Application
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import androidx.work.workDataOf
|
||||
import org.junit.After
|
||||
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.join.JoinState
|
||||
import org.libremediaconverter.join.JoinViewModel
|
||||
import org.libremediaconverter.join.joinActions
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.EnginePreference
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.model.QualityTier
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
import org.libremediaconverter.work.ConcatWorker
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
|
||||
/**
|
||||
* That each affordance is wired to the ViewModel method it is named after.
|
||||
*
|
||||
* ## What this covers that no other test can
|
||||
*
|
||||
* `ConverterScreenContentTest`, `ConverterStateAffordancesTest` and `JoinScreenContentTest` all
|
||||
* drive the **stateless** content composables, which build their own `ConverterActions`. So the
|
||||
* wiring — the list of `viewModel::` references the stateful outer hands down — was seen by nothing
|
||||
* in the suite.
|
||||
*
|
||||
* ## The hazard is narrower than "seventeen bindings", and this says so
|
||||
*
|
||||
* #156 was filed claiming a transposition of any two bindings would survive the suite. That is not
|
||||
* true, and it was worth checking rather than testing on the assumption:
|
||||
*
|
||||
* | swap | result |
|
||||
* |---|---|
|
||||
* | `onVideoCodec` ↔ `onAudioCodec` | **rejected by the compiler** |
|
||||
* | `onCancel` ↔ `onReset` | **compiles** |
|
||||
*
|
||||
* Every typed binding — container, both codecs, preset, suggestion, quality, engine preference —
|
||||
* takes a distinct parameter type, so the compiler is already the test. Writing assertions for
|
||||
* those would be theatre.
|
||||
*
|
||||
* **The `() -> Unit` bindings are the real gap**, because they are interchangeable to the compiler:
|
||||
* two on the converter screen (`onCancel`, `onReset`) and three on the join screen (`onJoin`,
|
||||
* `onCancel`, `onReset`). A Cancel that discards the finished file, or a Join that cancels, is a
|
||||
* one-character mistake that ships.
|
||||
*
|
||||
* ## How they are told apart
|
||||
*
|
||||
* By effect, not by a recording double. `reset()` sets the state to `Idle`; `cancel()` with no
|
||||
* active job leaves it alone (`ConversionViewModel.cancel` is `activeWorkId?.let(...)`, and
|
||||
* `SettingsEditsTest` pins that). Driving each from a non-`Idle` state is therefore enough to say
|
||||
* which one ran.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class ScreenWiringTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
ConversionDependencies.publisher = { RecordingPublisher(app) }
|
||||
ConversionDependencies.probe = { _, _ -> org.libremediaconverter.model.InputProbe() }
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
ConversionDependencies.reset()
|
||||
}
|
||||
|
||||
// --- the converter screen ----------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `Start over resets, and Cancel does not`() {
|
||||
// The transposition that compiles. If onReset were bound to cancel, this stays on Ready.
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
val pick = ParkedPickDispatcher()
|
||||
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
|
||||
val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
|
||||
viewModel.onInputPicked(INPUT_URI)
|
||||
pick.runAll()
|
||||
assertNotEquals(
|
||||
"the fixture needs a non-Idle state or neither action is observable",
|
||||
ConversionState.Idle,
|
||||
viewModel.state.value,
|
||||
)
|
||||
|
||||
actions.onReset()
|
||||
|
||||
assertEquals(ConversionState.Idle, viewModel.state.value)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `Cancel leaves the picked file on screen`() {
|
||||
// The other half. Without it, a wiring with BOTH actions bound to reset passes the test
|
||||
// above -- and that is exactly what a copy-paste of the wrong line produces.
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
val pick = ParkedPickDispatcher()
|
||||
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
|
||||
val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
|
||||
viewModel.onInputPicked(INPUT_URI)
|
||||
pick.runAll()
|
||||
val before = viewModel.state.value
|
||||
|
||||
actions.onCancel()
|
||||
|
||||
assertEquals(
|
||||
"Cancel must not throw away the pick the way Start over does",
|
||||
before,
|
||||
viewModel.state.value,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `each settings affordance reaches the setting it is named after`() {
|
||||
// The typed bindings. The compiler already rejects a transposition among these, so this is
|
||||
// not that assertion -- it is the cheaper one that each is bound to *something*, and that a
|
||||
// binding dropped to `{}` during an edit would be caught.
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
val pick = ParkedPickDispatcher()
|
||||
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
|
||||
val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
|
||||
|
||||
actions.onPreset(OutputFormat.WEBM_VP9)
|
||||
assertEquals(OutputFormat.WEBM_VP9.spec, viewModel.settings.value.spec)
|
||||
|
||||
actions.onContainer(Container.MKV)
|
||||
assertEquals(Container.MKV, viewModel.settings.value.spec.container)
|
||||
|
||||
actions.onVideoCodec(VideoCodec.H264)
|
||||
assertEquals(VideoCodec.H264, viewModel.settings.value.spec.videoCodec)
|
||||
|
||||
actions.onAudioCodec(AudioCodec.FLAC)
|
||||
assertEquals(AudioCodec.FLAC, viewModel.settings.value.spec.audioCodec)
|
||||
|
||||
actions.onQuality(QualityTier.BEST)
|
||||
assertEquals(QualityTier.BEST, viewModel.settings.value.quality)
|
||||
|
||||
actions.onEnginePreference(EnginePreference.FORCE_SOFTWARE)
|
||||
assertEquals(EnginePreference.FORCE_SOFTWARE, viewModel.settings.value.enginePreference)
|
||||
|
||||
actions.onSuggestion(OutputFormat.MP4_H264.spec)
|
||||
assertEquals(OutputFormat.MP4_H264.spec, viewModel.settings.value.spec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the launcher-backed actions are the ones the screen supplies`() {
|
||||
// Not wired to the ViewModel at all, deliberately -- they need an ActivityResultLauncher.
|
||||
// Asserted so that a later edit routing one of them at the ViewModel is noticed.
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
val pick = ParkedPickDispatcher()
|
||||
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
|
||||
val called = mutableListOf<String>()
|
||||
val actions = converterActions(
|
||||
viewModel,
|
||||
onPickInput = { called += "pick" },
|
||||
onConvert = { called += "convert" },
|
||||
onSave = { called += "save:$it" },
|
||||
)
|
||||
|
||||
actions.onPickInput()
|
||||
actions.onConvert()
|
||||
actions.onSave("holiday.mp4")
|
||||
|
||||
assertEquals(listOf("pick", "convert", "save:holiday.mp4"), called)
|
||||
}
|
||||
|
||||
// --- the join screen, where three are interchangeable -------------------
|
||||
|
||||
@Test
|
||||
fun `Start over resets the join, and Cancel does not`() {
|
||||
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
|
||||
val pick = ParkedPickDispatcher()
|
||||
val viewModel = JoinViewModel(app, pickDispatcher = pick)
|
||||
val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
|
||||
viewModel.onInputsPicked(TWO_INPUTS)
|
||||
pick.runAll()
|
||||
assertTrue(
|
||||
"the fixture needs a non-Idle state: ${viewModel.state.value}",
|
||||
viewModel.state.value !is JoinState.Idle,
|
||||
)
|
||||
|
||||
actions.onReset()
|
||||
|
||||
assertEquals(JoinState.Idle, viewModel.state.value)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `Cancel leaves the picked files on screen`() {
|
||||
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
|
||||
val pick = ParkedPickDispatcher()
|
||||
val viewModel = JoinViewModel(app, pickDispatcher = pick)
|
||||
val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
|
||||
viewModel.onInputsPicked(TWO_INPUTS)
|
||||
pick.runAll()
|
||||
val before = viewModel.state.value
|
||||
|
||||
actions.onCancel()
|
||||
|
||||
assertEquals(before, viewModel.state.value)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `Join starts the job rather than cancelling or resetting it`() {
|
||||
// The third of the join screen's interchangeable trio, and the one whose transposition is
|
||||
// worst: a Join button bound to cancel does nothing at all, which reads as a dead button.
|
||||
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
|
||||
val pick = ParkedPickDispatcher()
|
||||
val viewModel = JoinViewModel(app, pickDispatcher = pick)
|
||||
val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
|
||||
viewModel.onInputsPicked(TWO_INPUTS)
|
||||
pick.runAll()
|
||||
|
||||
actions.onJoin()
|
||||
|
||||
assertTrue(
|
||||
"Join must leave Ready for a running state, not sit still and not go Idle: " +
|
||||
"${viewModel.state.value}",
|
||||
viewModel.state.value is JoinState.Joining || viewModel.state.value is JoinState.Joined,
|
||||
)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
val INPUT_URI: Uri = Uri.parse("content://test/holiday.mov")
|
||||
val TWO_INPUTS = listOf(
|
||||
Uri.parse("content://test/a.mp4"),
|
||||
Uri.parse("content://test/b.mp4"),
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,197 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.app.Application
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNotEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.Container
|
||||
import org.libremediaconverter.model.EnginePreference
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.model.QualityTier
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
|
||||
/**
|
||||
* The seven one-line edits the settings sheet makes, and what each one leaves alone.
|
||||
*
|
||||
* ## Why these needed a file of their own
|
||||
*
|
||||
* `setPreset` was covered. The six beside it — `setContainer`, `setVideoCodec`, `setAudioCodec`,
|
||||
* `applySuggestion`, `setQuality`, `setEnginePreference` — and `cancel()` had **no coverage at
|
||||
* all**, which is the tell: they are reachable from the JVM suite by exactly the route `setPreset`
|
||||
* already takes, and nothing had asked.
|
||||
*
|
||||
* ## What is actually being asserted
|
||||
*
|
||||
* Not "the setter sets something". Each of these copies into a nested `OutputSpec`, so the failure
|
||||
* worth catching is **a setter that writes the right value into the wrong field, or that rebuilds
|
||||
* the spec and silently discards the other two**. So every test here asserts the field it changed
|
||||
* *and* that the rest of the spec survived — a `setContainer` implemented as
|
||||
* `it.copy(spec = OutputFormat.MP4_H265.spec.copy(container = container))` would pass a test that
|
||||
* only checked the container.
|
||||
*
|
||||
* `ConverterScreenContentTest` cannot cover this: it builds `ConverterActions` itself and never
|
||||
* touches the ViewModel. That the *screen* calls these is #156's, and neither implies the other.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class SettingsEditsTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var viewModel: ConversionViewModel
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
viewModel = ConversionViewModel(app)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
ConversionDependencies.reset()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `choosing a preset replaces the whole spec`() {
|
||||
viewModel.setPreset(OutputFormat.WEBM_VP9)
|
||||
|
||||
assertEquals(OutputFormat.WEBM_VP9.spec, viewModel.settings.value.spec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `changing the container leaves both codecs alone`() {
|
||||
// Moved off the default spec first, and that is load-bearing rather than tidiness. The
|
||||
// default IS `OutputFormat.MP4_H265.spec`, so a `setContainer` that rebuilt the spec from
|
||||
// that preset instead of from the current one produced an identical answer and the
|
||||
// mutation went green. Editing the codecs away from the default first is what makes
|
||||
// "the other two survived" an assertion rather than a coincidence.
|
||||
viewModel.setPreset(OutputFormat.WEBM_VP9)
|
||||
val before = viewModel.settings.value.spec
|
||||
|
||||
viewModel.setContainer(Container.MKV)
|
||||
|
||||
val after = viewModel.settings.value.spec
|
||||
assertEquals(Container.MKV, after.container)
|
||||
assertEquals("the video codec is not the container's to change", before.videoCodec, after.videoCodec)
|
||||
assertEquals("the audio codec is not the container's to change", before.audioCodec, after.audioCodec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `changing the video codec leaves the container and the audio codec alone`() {
|
||||
// The transposition this guards against is real: setVideoCodec and setAudioCodec take
|
||||
// different enum types, but a copy(...) naming the wrong field compiles wherever the types
|
||||
// happen to line up, and the picker would silently set the other one.
|
||||
val before = viewModel.settings.value.spec
|
||||
|
||||
viewModel.setVideoCodec(VideoCodec.VP9)
|
||||
|
||||
val after = viewModel.settings.value.spec
|
||||
assertEquals(VideoCodec.VP9, after.videoCodec)
|
||||
assertEquals(before.container, after.container)
|
||||
assertEquals(before.audioCodec, after.audioCodec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `changing the audio codec leaves the container and the video codec alone`() {
|
||||
val before = viewModel.settings.value.spec
|
||||
|
||||
viewModel.setAudioCodec(AudioCodec.OPUS)
|
||||
|
||||
val after = viewModel.settings.value.spec
|
||||
assertEquals(AudioCodec.OPUS, after.audioCodec)
|
||||
assertEquals(before.container, after.container)
|
||||
assertEquals(before.videoCodec, after.videoCodec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `applying a suggestion replaces the spec without disturbing quality or engine`() {
|
||||
// A suggestion comes from ContainerCapabilities when the current spec is invalid, so it is
|
||||
// a whole spec by construction. What it must not do is reset the two settings beside it.
|
||||
viewModel.setQuality(QualityTier.BEST)
|
||||
viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
|
||||
|
||||
viewModel.applySuggestion(OutputFormat.MKV_H264.spec)
|
||||
|
||||
val settings = viewModel.settings.value
|
||||
assertEquals(OutputFormat.MKV_H264.spec, settings.spec)
|
||||
assertEquals(QualityTier.BEST, settings.quality)
|
||||
assertEquals(EnginePreference.FORCE_SOFTWARE, settings.enginePreference)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `changing the quality leaves the spec and the engine preference alone`() {
|
||||
// Both neighbours are moved off their defaults first. Asserting against AUTO -- which is
|
||||
// what `ConversionSettings` starts with -- let a `setQuality` that also reset the engine
|
||||
// preference to AUTO pass, because the reset and the survival looked identical.
|
||||
viewModel.setPreset(OutputFormat.WEBM_VP9)
|
||||
viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
|
||||
val before = viewModel.settings.value.spec
|
||||
|
||||
viewModel.setQuality(QualityTier.BEST)
|
||||
|
||||
val settings = viewModel.settings.value
|
||||
assertEquals(QualityTier.BEST, settings.quality)
|
||||
assertEquals(before, settings.spec)
|
||||
assertEquals(
|
||||
"quality is not the engine preference's to change",
|
||||
EnginePreference.FORCE_SOFTWARE,
|
||||
settings.enginePreference,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `changing the engine preference leaves the spec and the quality alone`() {
|
||||
// Off the defaults for the same reason as the test above: QualityTier.FAST is the starting
|
||||
// value, so asserting it here would have been satisfied by a reset as readily as by a
|
||||
// survival.
|
||||
viewModel.setPreset(OutputFormat.WEBM_VP9)
|
||||
viewModel.setQuality(QualityTier.BEST)
|
||||
val before = viewModel.settings.value.spec
|
||||
|
||||
viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
|
||||
|
||||
val settings = viewModel.settings.value
|
||||
assertEquals(EnginePreference.FORCE_SOFTWARE, settings.enginePreference)
|
||||
assertEquals(before, settings.spec)
|
||||
assertEquals(
|
||||
"the engine preference is not the quality's to change",
|
||||
QualityTier.BEST,
|
||||
settings.quality,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `editing past every preset leaves no matching preset`() {
|
||||
// `matchingPreset` is what the settings sheet reads to decide whether to show a preset as
|
||||
// selected or to say "Custom". Editing one field of a preset must drop it out of the list
|
||||
// rather than leaving the old one highlighted.
|
||||
viewModel.setPreset(OutputFormat.MP4_H265)
|
||||
assertEquals(OutputFormat.MP4_H265, viewModel.settings.value.matchingPreset)
|
||||
|
||||
viewModel.setAudioCodec(AudioCodec.FLAC)
|
||||
|
||||
assertNull(
|
||||
"an edited spec is no longer any preset, and the sheet says Custom",
|
||||
viewModel.settings.value.matchingPreset,
|
||||
)
|
||||
assertNotEquals(OutputFormat.MP4_H265.spec, viewModel.settings.value.spec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `cancelling with no active job does nothing rather than throwing`() {
|
||||
// `activeWorkId?.let(...)` -- the null side. A user can reach Cancel through a state that
|
||||
// has already finished, and taking the app down for it would be worse than doing nothing.
|
||||
viewModel.cancel()
|
||||
|
||||
assertEquals(ConversionState.Idle, viewModel.state.value)
|
||||
}
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,200 @@
|
||||
package org.libremediaconverter.join
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import androidx.work.WorkInfo
|
||||
import androidx.work.workDataOf
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.InputFile
|
||||
import org.libremediaconverter.model.ConcatStrategy
|
||||
import org.libremediaconverter.work.ConcatWorker
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* Every answer [joinStateFrom] can give, chosen rather than stumbled into.
|
||||
*
|
||||
* The join-side twin of `ConversionStateMappingTest`, and the argument is the same one: the mapping
|
||||
* ran on every test that drove a real `ConcatWorker`, but a real worker only ever reaches a terminal
|
||||
* state with well-formed output, so five arms had never been *chosen* by anything.
|
||||
*
|
||||
* ## The one that is not just coverage
|
||||
*
|
||||
* `an unknown strategy name is read as a re-encode rather than thrown` covers a real defect this
|
||||
* seam exposed. The line it replaces was:
|
||||
*
|
||||
* ```kotlin
|
||||
* .getString(ConcatWorker.KEY_STRATEGY)?.let(ConcatStrategy::valueOf) ?: ConcatStrategy.REENCODE
|
||||
* ```
|
||||
*
|
||||
* `valueOf` throws on a name this build does not define, and this runs inside a `viewModelScope`
|
||||
* collect with no handler — so it does not become a `Failed` state, it takes the process down.
|
||||
* `ConcatWorker.kt` had already made this exact change for `KEY_FORMAT` and written down why; the
|
||||
* matching read on this side had not been changed with it.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class JoinStateMappingTest {
|
||||
|
||||
@Test
|
||||
fun `a running join is joining`() {
|
||||
assertEquals(JoinState.Joining(INPUTS), map(WorkInfo.State.RUNNING))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a blocked join looks like one that is starting`() {
|
||||
// Folded into the RUNNING arm deliberately: a job waiting on a prerequisite is nothing the
|
||||
// user can act on, and a separate word for it would be noise.
|
||||
assertEquals(JoinState.Joining(INPUTS), map(WorkInfo.State.BLOCKED))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an enqueued join that has already run is waiting to retry`() {
|
||||
assertEquals(JoinState.Waiting(INPUTS), map(WorkInfo.State.ENQUEUED, runAttemptCount = 1))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an enqueued join that has never run is simply starting`() {
|
||||
// The other side. Without it, a mapping that ignored runAttemptCount passes the test above.
|
||||
assertEquals(JoinState.Joining(INPUTS), map(WorkInfo.State.ENQUEUED, runAttemptCount = 0))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a success that named no file is a failure, not an empty success`() {
|
||||
assertEquals(
|
||||
JoinState.Failed(JOINED_WITHOUT_A_FILE_MESSAGE),
|
||||
map(WorkInfo.State.SUCCEEDED, data = Data.EMPTY),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a success carries the strategy the worker actually used`() {
|
||||
// Not cosmetic: the join screen tells the user whether their files were stream-copied or
|
||||
// re-encoded, which is the difference between lossless and lossy.
|
||||
val joined = map(
|
||||
WorkInfo.State.SUCCEEDED,
|
||||
data = workDataOf(
|
||||
ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4",
|
||||
ConcatWorker.KEY_STRATEGY to ConcatStrategy.STREAM_COPY.name,
|
||||
),
|
||||
) as JoinState.Joined
|
||||
|
||||
assertEquals(ConcatStrategy.STREAM_COPY, joined.strategy)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an unknown strategy name is read as a re-encode rather than thrown`() {
|
||||
// The defect. A build that added a third strategy leaves finished joins in the queue naming
|
||||
// it, and WorkManager keeps those about a week -- the premise WorkerEnumFallbackTest and
|
||||
// JobTags are both written on. With `valueOf` this throws IllegalArgumentException inside a
|
||||
// viewModelScope collect that has no handler, so it is not a Failed state, it is a crash.
|
||||
//
|
||||
// REENCODE rather than STREAM_COPY because it is the conservative answer: describing an
|
||||
// unknown join as lossless would be a claim the app cannot support.
|
||||
val joined = map(
|
||||
WorkInfo.State.SUCCEEDED,
|
||||
data = workDataOf(
|
||||
ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4",
|
||||
ConcatWorker.KEY_STRATEGY to "SMART_CONCAT_V2",
|
||||
),
|
||||
) as JoinState.Joined
|
||||
|
||||
assertEquals(ConcatStrategy.REENCODE, joined.strategy)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a success with no strategy at all falls back the same way`() {
|
||||
val joined = map(
|
||||
WorkInfo.State.SUCCEEDED,
|
||||
data = workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4"),
|
||||
) as JoinState.Joined
|
||||
|
||||
assertEquals(ConcatStrategy.REENCODE, joined.strategy)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a success from older work falls back to the format such a job really used`() {
|
||||
val joined = map(
|
||||
WorkInfo.State.SUCCEEDED,
|
||||
data = workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4"),
|
||||
) as JoinState.Joined
|
||||
|
||||
assertEquals(ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT), joined.suggestedName)
|
||||
assertEquals(ConcatWorker.DEFAULT_FORMAT.mimeType, joined.mimeType)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a blank name or type falls back the same way a missing one does`() {
|
||||
val joined = map(
|
||||
WorkInfo.State.SUCCEEDED,
|
||||
data = workDataOf(
|
||||
ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4",
|
||||
ConcatWorker.KEY_SUGGESTED_NAME to "",
|
||||
ConcatWorker.KEY_MIME_TYPE to " ",
|
||||
),
|
||||
) as JoinState.Joined
|
||||
|
||||
assertEquals(ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT), joined.suggestedName)
|
||||
assertEquals(ConcatWorker.DEFAULT_FORMAT.mimeType, joined.mimeType)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a failure carries the reason the worker gave`() {
|
||||
assertEquals(
|
||||
JoinState.Failed("Not enough free space to join these files."),
|
||||
map(
|
||||
WorkInfo.State.FAILED,
|
||||
data = workDataOf(
|
||||
ConcatWorker.KEY_ERROR to "Not enough free space to join these files.",
|
||||
),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a failure with nothing said still says something`() {
|
||||
assertEquals(
|
||||
JoinState.Failed(ConcatWorker.GENERIC_FAILURE_MESSAGE),
|
||||
map(WorkInfo.State.FAILED, data = Data.EMPTY),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a failure whose message is blank falls back like a missing one`() {
|
||||
assertEquals(
|
||||
JoinState.Failed(ConcatWorker.GENERIC_FAILURE_MESSAGE),
|
||||
map(WorkInfo.State.FAILED, data = workDataOf(ConcatWorker.KEY_ERROR to " ")),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cancellation lands wherever the caller said it should`() {
|
||||
// A join started here goes back to Ready with the picked files; one picked up by reattach
|
||||
// goes to Idle, because those URIs belong to a process that no longer exists.
|
||||
assertEquals(
|
||||
JoinState.Ready(INPUTS),
|
||||
map(WorkInfo.State.CANCELLED, cancelled = JoinState.Ready(INPUTS)),
|
||||
)
|
||||
assertEquals(JoinState.Idle, map(WorkInfo.State.CANCELLED, cancelled = JoinState.Idle))
|
||||
}
|
||||
|
||||
private fun map(
|
||||
state: WorkInfo.State,
|
||||
runAttemptCount: Int = 0,
|
||||
data: Data = Data.EMPTY,
|
||||
cancelled: JoinState = JoinState.Ready(INPUTS),
|
||||
): JoinState = joinStateFrom(
|
||||
JoinUpdate(state = state, runAttemptCount = runAttemptCount, outputData = data),
|
||||
inputs = INPUTS,
|
||||
cancelled = cancelled,
|
||||
)
|
||||
|
||||
private companion object {
|
||||
val INPUTS = listOf(
|
||||
InputFile(Uri.parse("content://test/a.mp4"), "a.mp4", 1024L),
|
||||
InputFile(Uri.parse("content://test/b.mp4"), "b.mp4", 2048L),
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,102 @@
|
||||
package org.libremediaconverter.join
|
||||
|
||||
import android.app.Application
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
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.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.RecordingPublisher
|
||||
import org.libremediaconverter.convert.installTestWorkManager
|
||||
import org.libremediaconverter.work.ConcatWorker
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
|
||||
/**
|
||||
* The two layers that refuse a short join, refusing it with one sentence.
|
||||
*
|
||||
* ## Why this is not "assert a constant equals itself"
|
||||
*
|
||||
* `ConcatWorker` and `JoinViewModel` both reject a join of fewer than two files, and before #158
|
||||
* each carried **its own copy of the literal**. Only the worker's was pinned — by `RefusedJobTest`,
|
||||
* added in #139 — so the wording on the screen could drift away from the wording in the job with no
|
||||
* test saying anything, for one message the user sees from one condition.
|
||||
*
|
||||
* Sharing a constant makes them agree by construction. What it does *not* do is prove that both
|
||||
* layers still reach it: a refactor that stops `JoinViewModel` refusing at all, or that gives it a
|
||||
* different message, passes any test that only reads `TOO_FEW_INPUTS_MESSAGE`. So each layer is
|
||||
* driven for real here — the ViewModel through `onInputsPicked`, the worker through `doWork` — and
|
||||
* the assertion is that the two answers are **the same string**, taken from two running layers
|
||||
* rather than from one declaration.
|
||||
*
|
||||
* That is the shape `CLAUDE.md` asks for: revert the sharing and this goes red, because the two
|
||||
* sites drift the moment they are allowed to.
|
||||
*
|
||||
* ## Scope
|
||||
*
|
||||
* The arity guard's own behaviour on the ViewModel side — that it refuses one file, that it accepts
|
||||
* two, that it claims ownership first — is #155's, and this deliberately does not duplicate it.
|
||||
* This file is about the *agreement between layers*, which is what #158 changed.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class SharedFailureMessagesTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var viewModel: JoinViewModel
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
ConversionDependencies.publisher = { RecordingPublisher(app) }
|
||||
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
|
||||
viewModel = JoinViewModel(app)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
ConversionDependencies.reset()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `both layers refuse a one-file join with the same sentence`() {
|
||||
// The ViewModel, refusing before anything is enqueued.
|
||||
viewModel.onInputsPicked(listOf(ONE_FILE))
|
||||
val fromScreen = (viewModel.state.value as JoinState.Failed).message
|
||||
|
||||
// The worker, refusing a job that reached the queue anyway -- which it can, because
|
||||
// ConcatWorker.request(...) takes a List<Uri> and checks nothing about its length.
|
||||
val result = runBlocking { worker(ONE_FILE).doWork() }
|
||||
val fromJob = (result as ListenableWorker.Result.Failure)
|
||||
.outputData.getString(ConcatWorker.KEY_ERROR)
|
||||
|
||||
assertEquals(
|
||||
"the screen and the job must say the same thing about the same refusal",
|
||||
fromScreen,
|
||||
fromJob,
|
||||
)
|
||||
// And that the shared sentence is the one either layer would have written on its own,
|
||||
// rather than both having drifted together to something else.
|
||||
assertEquals(ConcatWorker.TOO_FEW_INPUTS_MESSAGE, fromScreen)
|
||||
}
|
||||
|
||||
private fun worker(vararg inputs: Uri): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
|
||||
context = app,
|
||||
inputData = workDataOf(
|
||||
ConcatWorker.KEY_INPUT_URIS to inputs.map(Uri::toString).toTypedArray(),
|
||||
ConcatWorker.KEY_TOTAL_BYTES to 1024L,
|
||||
),
|
||||
runAttemptCount = 0,
|
||||
).build()
|
||||
|
||||
private companion object {
|
||||
val ONE_FILE: Uri = Uri.parse("content://test/holiday.mp4")
|
||||
}
|
||||
}
|
||||
@@ -358,4 +358,213 @@ class ContainerCapabilitiesTest {
|
||||
assertEquals(emptyList<VideoCodec>(), ContainerCapabilities.encodableVideo(container))
|
||||
}
|
||||
}
|
||||
|
||||
// --- the audio axis -----------------------------------------------------
|
||||
//
|
||||
// Every rule below has a video twin already tested above. The two halves of `validate` were
|
||||
// written together and only one of them was ever checked, so these are deliberately shaped like
|
||||
// their twins rather than as a fresh idea about what to assert.
|
||||
|
||||
@Test
|
||||
fun `an unidentifiable source audio codec cannot be copied`() {
|
||||
// The audio twin of `an unidentifiable source codec cannot be copied`. Never guess: a copy
|
||||
// of an unidentified codec is how you ship a file that does not play.
|
||||
val unknownAudio = InputProbe(videoCodec = "h264", audioCodec = null, container = Container.MP4)
|
||||
val spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.COPY)
|
||||
|
||||
val invalid = ContainerCapabilities.validate(spec, unknownAudio) as? Validation.Invalid
|
||||
?: throw AssertionError("copying an unidentified audio codec must be refused")
|
||||
|
||||
assertTrue(invalid.message, invalid.message.contains("could not be identified"))
|
||||
assertEverySuggestionValid(invalid, unknownAudio)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `copying an audio codec the container cannot hold is refused`() {
|
||||
// MP4 carries AAC, MP3, Opus and FLAC. Vorbis lives in Ogg and Matroska, so a stream copy
|
||||
// out of a Vorbis source into MP4 has nowhere to put the track.
|
||||
val vorbisAudio = InputProbe(videoCodec = "h264", audioCodec = "vorbis", container = Container.MKV)
|
||||
val spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.COPY)
|
||||
|
||||
val invalid = ContainerCapabilities.validate(spec, vorbisAudio) as? Validation.Invalid
|
||||
?: throw AssertionError("Vorbis copied into MP4 must be refused")
|
||||
|
||||
assertEquals("MP4 cannot hold Vorbis audio.", invalid.message)
|
||||
assertEverySuggestionValid(invalid, vorbisAudio)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an audio codec the container cannot hold is refused on the encode path too`() {
|
||||
// WAV carries PCM and nothing else. The twin is `H265 in AVI is refused`.
|
||||
val spec = OutputSpec(Container.WAV, VideoCodec.NONE, AudioCodec.AAC)
|
||||
|
||||
val invalid = ContainerCapabilities.validate(spec, mp3Source) as? Validation.Invalid
|
||||
?: throw AssertionError("AAC in WAV must be refused")
|
||||
|
||||
assertEquals("WAV cannot hold AAC audio.", invalid.message)
|
||||
assertEverySuggestionValid(invalid, mp3Source)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an audio codec this app cannot encode is refused, and copying is offered instead`() {
|
||||
// Matroska carries Vorbis; nothing here encodes it. The refusal has to say so *and* say
|
||||
// what would work, which is the audio twin of `copying is offered as the fix when the codec
|
||||
// is right but unencodable`.
|
||||
val spec = OutputSpec(Container.MKV, VideoCodec.H264, AudioCodec.VORBIS)
|
||||
|
||||
val invalid = ContainerCapabilities.validate(spec, h264Source) as? Validation.Invalid
|
||||
?: throw AssertionError("encoding Vorbis must be refused")
|
||||
|
||||
assertEquals(
|
||||
"This app cannot encode Vorbis audio. It can still be copied from a Vorbis source.",
|
||||
invalid.message,
|
||||
)
|
||||
assertEverySuggestionValid(invalid, h264Source)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `copying a video codec the container cannot hold is refused`() {
|
||||
// Not the audio axis, but the one video refusal with no test: AVI predates H.265, so a
|
||||
// stream copy out of an HEVC source into AVI has nowhere to put the track. `H265 in AVI is
|
||||
// refused` covers the matrix; this covers what validate() does with it.
|
||||
val h265Source = InputProbe(videoCodec = "hevc", audioCodec = "mp3", container = Container.MP4)
|
||||
val spec = OutputSpec(Container.AVI, VideoCodec.COPY, AudioCodec.MP3)
|
||||
|
||||
val invalid = ContainerCapabilities.validate(spec, h265Source) as? Validation.Invalid
|
||||
?: throw AssertionError("H.265 copied into AVI must be refused")
|
||||
|
||||
assertEquals("AVI cannot hold H.265 video.", invalid.message)
|
||||
assertEverySuggestionValid(invalid, h265Source)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `no audio track is accepted by every container in both modes`() {
|
||||
// The audio twin of VideoCodec.NONE -> true. A container that refused "no audio" would make
|
||||
// every video-only output invalid.
|
||||
Container.entries.forEach { container ->
|
||||
listOf(CodecMode.COPY, CodecMode.ENCODE).forEach { mode ->
|
||||
assertTrue(
|
||||
"$container should accept no audio track ($mode)",
|
||||
ContainerCapabilities.accepts(container, AudioCodec.NONE, mode),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
// identical: silently answering "false" would refuse a perfectly good remux.
|
||||
runCatching { ContainerCapabilities.accepts(Container.MP4, AudioCodec.COPY, CodecMode.COPY) }
|
||||
.onSuccess { throw AssertionError("expected audio COPY to be rejected by the matrix") }
|
||||
}
|
||||
|
||||
/**
|
||||
* Every alternative a refusal offers has to be one the same input could actually take.
|
||||
*
|
||||
* `Validation.Invalid` promises exactly this and names this class as the proof. The global
|
||||
* property test walks the presets; these paths reach `suggestions()` through `validateAudio`,
|
||||
* which no preset does.
|
||||
*/
|
||||
private fun assertEverySuggestionValid(invalid: Validation.Invalid, probe: InputProbe) {
|
||||
invalid.suggestions.forEach {
|
||||
assertTrue(
|
||||
"suggestion $it is itself invalid, so the chip leads to a second error",
|
||||
ContainerCapabilities.validate(it, probe).isValid,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,9 +27,6 @@ import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import java.io.File
|
||||
import java.util.UUID
|
||||
import java.util.concurrent.ExecutionException
|
||||
import java.util.concurrent.Executor
|
||||
import java.util.concurrent.TimeUnit
|
||||
|
||||
/**
|
||||
* That a refused foreground-service start does not end the job.
|
||||
@@ -125,6 +122,40 @@ class DeniedForegroundStartTest {
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a join denied past the attempt bound fails with a message the user can act on`() {
|
||||
// The join twin of the conversion case above. ConcatWorker reaches the same FailureOutcome
|
||||
// through its own `when`, and that arm was the only one of its three with no test -- so a
|
||||
// join that gave up silently, or gave up with an empty Data, would have looked identical to
|
||||
// one that retried.
|
||||
val worker = concatWorker(runAttemptCount = FailureOutcome.MAX_FOREGROUND_START_ATTEMPTS)
|
||||
|
||||
val result = runBlocking { worker.doWork() }
|
||||
|
||||
assertEquals(
|
||||
ListenableWorker.Result.failure(
|
||||
workDataOf(ConcatWorker.KEY_ERROR to FailureOutcome.FOREGROUND_DENIED_MESSAGE),
|
||||
),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a join that gives up collects the partial it had already staged`() {
|
||||
// The delete lives on ConcatWorker's `catch (e: Throwable)` path, which every give-up goes
|
||||
// through. Written first so a missing delete cannot pass by asking whether a file nobody
|
||||
// wrote is absent.
|
||||
concatStagedFile().writeBytes(ByteArray(PARTIAL_BYTES))
|
||||
|
||||
runBlocking { concatWorker(runAttemptCount = FailureOutcome.MAX_FOREGROUND_START_ATTEMPTS).doWork() }
|
||||
|
||||
assertEquals(
|
||||
"a join that gave up must not orphan what it staged",
|
||||
emptyList<String>(),
|
||||
stagedNames(),
|
||||
)
|
||||
}
|
||||
|
||||
private fun conversionWorker(runAttemptCount: Int = 0): ConversionWorker =
|
||||
TestListenableWorkerBuilder<ConversionWorker>(
|
||||
context = app,
|
||||
@@ -141,18 +172,22 @@ class DeniedForegroundStartTest {
|
||||
.setForegroundUpdater(DenyingForegroundUpdater)
|
||||
.build()
|
||||
|
||||
private fun concatWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
|
||||
private fun concatWorker(runAttemptCount: Int = 0): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
|
||||
context = app,
|
||||
inputData = workDataOf(
|
||||
ConcatWorker.KEY_INPUT_URIS to arrayOf(INPUT.toString(), "content://test/second.mp4"),
|
||||
ConcatWorker.KEY_TOTAL_BYTES to INPUT_BYTES,
|
||||
ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
|
||||
ConcatWorker.KEY_FORMAT to CONCAT_FORMAT.name,
|
||||
),
|
||||
runAttemptCount = 0,
|
||||
runAttemptCount = runAttemptCount,
|
||||
).setId(CONCAT_ID)
|
||||
.setForegroundUpdater(DenyingForegroundUpdater)
|
||||
.build()
|
||||
|
||||
/** The staging path the join will compute, asked for rather than spelled out here. */
|
||||
private fun concatStagedFile(): File =
|
||||
publisher.createStagingFile(StagingNames.forJob(CONCAT_ID, CONCAT_FORMAT.extension))
|
||||
|
||||
/** The staging path the worker will compute, asked for rather than spelled out here. */
|
||||
private fun stagedFile(): File = publisher.createStagingFile(StagingNames.forJob(CONVERSION_ID, SPEC.extension))
|
||||
|
||||
@@ -164,6 +199,7 @@ class DeniedForegroundStartTest {
|
||||
const val INPUT_BYTES = 1024L
|
||||
const val PARTIAL_BYTES = 2048
|
||||
val SPEC = OutputFormat.MP4_H265.spec
|
||||
val CONCAT_FORMAT = OutputFormat.MP4_H264
|
||||
val CONVERSION_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000001")
|
||||
val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000002")
|
||||
}
|
||||
@@ -182,18 +218,3 @@ private object DenyingForegroundUpdater : ForegroundUpdater {
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* An already-failed future, written out rather than pulled from a futures library.
|
||||
*
|
||||
* `await()` takes the `isDone` fast path and unwraps the `ExecutionException`, which is what puts
|
||||
* the platform's own exception in front of the worker's catch rather than a wrapper.
|
||||
*/
|
||||
private class FailedFuture(private val failure: Throwable) : ListenableFuture<Void> {
|
||||
override fun addListener(listener: Runnable, executor: Executor): Unit = executor.execute(listener)
|
||||
override fun cancel(mayInterruptIfRunning: Boolean): Boolean = false
|
||||
override fun isCancelled(): Boolean = false
|
||||
override fun isDone(): Boolean = true
|
||||
override fun get(): Void = throw ExecutionException(failure)
|
||||
override fun get(timeout: Long, unit: TimeUnit): Void = throw ExecutionException(failure)
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,276 @@
|
||||
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.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.OutputPublisher
|
||||
import org.libremediaconverter.convert.SoftwareTranscoder
|
||||
import org.libremediaconverter.convert.installTestWorkManager
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.ContainerCapabilities
|
||||
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.libremediaconverter.model.OutputSpec
|
||||
import org.libremediaconverter.model.Validation
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import java.io.File
|
||||
import java.util.UUID
|
||||
|
||||
/**
|
||||
* Jobs the worker refuses before it converts anything, and what it says about them.
|
||||
*
|
||||
* Two exits, both cold before this file, and both reachable for the same underlying reason: **a job
|
||||
* does not have to come from the picker.** WorkManager keeps queued and finished work for about a
|
||||
* week, so a downgrade or a rollback hands this build a job enqueued by another one — the premise
|
||||
* `WorkerEnumFallbackTest` and `JobTags` are both written on — and `ConversionWorker.request(...)`
|
||||
* is callable directly.
|
||||
*
|
||||
* What makes these worth their own file rather than another case in an existing one is that both
|
||||
* are about the *message*. A refusal that fails with empty output `Data` renders the UI's generic
|
||||
* "Conversion failed." with nothing else to say, which is the defect shape `DeniedForegroundStartTest`
|
||||
* records from the device pass. Asserting the verdict alone would pass against exactly that.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class RefusedJobTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var publisher: OutputPublisher
|
||||
private lateinit var engine: RefusingTranscoder
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
publisher = AlwaysRoomPublisher(app)
|
||||
engine = RefusingTranscoder()
|
||||
ConversionDependencies.publisher = { publisher }
|
||||
ConversionDependencies.software = { engine }
|
||||
// Neither test is about probing or about this machine's codecs; both would otherwise decide
|
||||
// the outcome for reasons no assertion mentions. See WorkerCancellationTest's setUp.
|
||||
ConversionDependencies.probe = { _, _ -> InputProbe() }
|
||||
ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE }
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
ConversionDependencies.reset()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a job with no input URI fails with a message rather than a bare failure`() {
|
||||
val result = runBlocking { workerWithout(ConversionWorker.KEY_INPUT_URI).doWork() }
|
||||
|
||||
// `Failure.equals` compares output data, so this pins the message and the verdict together.
|
||||
assertEquals(
|
||||
ListenableWorker.Result.failure(workDataOf(ConversionWorker.KEY_ERROR to "No input file.")),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a job with no input URI stages nothing`() {
|
||||
// The URI read is the first thing doWork does -- above the space check, above the staging
|
||||
// name, above the try. A refusal there must not have reserved anything.
|
||||
runBlocking { workerWithout(ConversionWorker.KEY_INPUT_URI).doWork() }
|
||||
|
||||
assertEquals("a job refused for having no input must not stage a file", emptyList<String>(), stagedNames())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a spec the picker would never have allowed is refused with the reason`() {
|
||||
// WAV carries PCM and nothing else. The picker cannot produce this combination today, which
|
||||
// is exactly why the worker checks: the job can arrive from a queue written before the
|
||||
// settings changed, or from a direct request(...) call.
|
||||
val expected = ContainerCapabilities.validate(REFUSED_SPEC, InputProbe()) as? Validation.Invalid
|
||||
?: throw AssertionError("the fixture spec is supposed to be invalid; ContainerCapabilities disagrees")
|
||||
|
||||
val result = runBlocking { worker(REFUSED_SPEC).doWork() }
|
||||
|
||||
assertEquals(
|
||||
ListenableWorker.Result.failure(workDataOf(ConversionWorker.KEY_ERROR to expected.message)),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a refused spec never reaches an engine`() {
|
||||
// The half that says it failed *before* converting rather than during. Without this, a
|
||||
// worker that ran the job and then reported the validation message would pass the test
|
||||
// above -- and would have spent the user's battery on a file it was going to refuse.
|
||||
runBlocking { worker(REFUSED_SPEC).doWork() }
|
||||
|
||||
assertTrue("a refused spec must be refused before any engine runs", engine.invocations.isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a valid spec is not refused`() {
|
||||
// The control. Every assertion above is about a refusal, so without this they would all
|
||||
// still pass against a worker that refused everything.
|
||||
val result = runBlocking { worker(OutputFormat.MP4_H265.spec).doWork() }
|
||||
|
||||
assertEquals(ListenableWorker.Result.success(), stripOutput(result))
|
||||
assertEquals(listOf(OutputFormat.MP4_H265.spec), engine.invocations)
|
||||
}
|
||||
|
||||
// --- the same refusal, on the join side ----------------------------------
|
||||
|
||||
@Test
|
||||
fun `a join of a single file is refused with a message rather than joined`() {
|
||||
// The arm beside it -- a job with no URI array at all -- is covered on the device by
|
||||
// `UnopenableUriTest.aJoinWithNoInputArrayFailsWithAMessage`. This one was covered by
|
||||
// nothing in either source set, which a coverage report cannot say because it cannot see
|
||||
// androidTest: the two arms are adjacent lines and only one of them had a test.
|
||||
//
|
||||
// Reachable for the reason this file's header gives, plus one of its own: `request(...)`
|
||||
// takes a `List<Uri>` and checks nothing about its length, so a single-item join is a
|
||||
// well-formed call, not a corrupted queue entry.
|
||||
val result = runBlocking { joinWorker(INPUT).doWork() }
|
||||
|
||||
assertEquals(
|
||||
ListenableWorker.Result.failure(
|
||||
workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.TOO_FEW_INPUTS_MESSAGE),
|
||||
),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a join of two files is not refused for its count`() {
|
||||
// The control, and the half that makes the test above bite on the boundary rather than on
|
||||
// the message: without it, `uris.size < 3` passes everything here.
|
||||
//
|
||||
// It refuses the space instead of letting the job run, because the next thing past the
|
||||
// count guard is `ConcatEngine`, which is native -- `NamingPublisher`'s KDoc records that
|
||||
// no JVM test gets past it. A refusal with the *space* message is proof that execution
|
||||
// reached line 57, which is proof it got past line 42, and it costs no engine to say so.
|
||||
val noRoom = NamingPublisher(app).apply { refuseSpace = true }
|
||||
ConversionDependencies.publisher = { noRoom }
|
||||
|
||||
val result = runBlocking { joinWorker(INPUT, SECOND_INPUT).doWork() }
|
||||
|
||||
assertEquals(
|
||||
ListenableWorker.Result.failure(
|
||||
workDataOf(ConcatWorker.KEY_ERROR to "Not enough free space to join these files."),
|
||||
),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
/** [ListenableWorker.Result.Success] compares its output data, which these tests do not pin. */
|
||||
private fun stripOutput(result: ListenableWorker.Result): ListenableWorker.Result =
|
||||
if (result is ListenableWorker.Result.Success) ListenableWorker.Result.success() else result
|
||||
|
||||
private fun worker(spec: OutputSpec): ConversionWorker = build(
|
||||
workDataOf(
|
||||
ConversionWorker.KEY_INPUT_URI to INPUT.toString(),
|
||||
ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME,
|
||||
ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES,
|
||||
ConversionWorker.KEY_CONTAINER to spec.container.name,
|
||||
ConversionWorker.KEY_VIDEO_CODEC to spec.videoCodec.name,
|
||||
ConversionWorker.KEY_AUDIO_CODEC to spec.audioCodec.name,
|
||||
ConversionWorker.KEY_ENGINE_PREFERENCE to EnginePreference.FORCE_SOFTWARE.name,
|
||||
),
|
||||
)
|
||||
|
||||
/**
|
||||
* The ordinary input `Data`, less one key.
|
||||
*
|
||||
* Built by removal rather than by spelling out a shorter map, so the test cannot drift into
|
||||
* omitting something else as well and passing for a reason it does not name.
|
||||
*/
|
||||
private fun workerWithout(key: String): ConversionWorker {
|
||||
val full = OutputFormat.MP4_H265.spec
|
||||
val entries = mapOf(
|
||||
ConversionWorker.KEY_INPUT_URI to INPUT.toString(),
|
||||
ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME,
|
||||
ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES,
|
||||
ConversionWorker.KEY_CONTAINER to full.container.name,
|
||||
ConversionWorker.KEY_VIDEO_CODEC to full.videoCodec.name,
|
||||
ConversionWorker.KEY_AUDIO_CODEC to full.audioCodec.name,
|
||||
ConversionWorker.KEY_ENGINE_PREFERENCE to EnginePreference.FORCE_SOFTWARE.name,
|
||||
) - key
|
||||
return build(Data.Builder().putAll(entries).build())
|
||||
}
|
||||
|
||||
private fun build(data: Data): ConversionWorker =
|
||||
TestListenableWorkerBuilder<ConversionWorker>(context = app, inputData = data, runAttemptCount = 0)
|
||||
.setId(JOB_ID)
|
||||
.build()
|
||||
|
||||
/**
|
||||
* A join job carrying [inputs], a declared total, and a format.
|
||||
*
|
||||
* The total is declared so `hasRoomFor` takes its `hasSpaceFor` branch: the other branch is
|
||||
* `hasSpaceForUnknownSize`, which `NamingPublisher` does not override and which would measure
|
||||
* this machine's real disk.
|
||||
*/
|
||||
private fun joinWorker(vararg inputs: Uri): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
|
||||
context = app,
|
||||
inputData = workDataOf(
|
||||
ConcatWorker.KEY_INPUT_URIS to inputs.map(Uri::toString).toTypedArray(),
|
||||
ConcatWorker.KEY_TOTAL_BYTES to INPUT_BYTES * inputs.size,
|
||||
ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
|
||||
),
|
||||
runAttemptCount = 0,
|
||||
).setId(JOB_ID).build()
|
||||
|
||||
private fun stagedNames(): List<String> =
|
||||
publisher.createStagingFile("anything").parentFile?.listFiles().orEmpty().map { it.name }.sorted()
|
||||
|
||||
private companion object {
|
||||
val INPUT: Uri = Uri.parse("file:///tmp/holiday.mp4")
|
||||
const val DISPLAY_NAME = "holiday.mp4"
|
||||
const val INPUT_BYTES = 1024L
|
||||
|
||||
/** A join needs two, and "two" is the boundary the count guard is about. */
|
||||
val SECOND_INPUT: Uri = Uri.parse("file:///tmp/holiday-2.mp4")
|
||||
|
||||
/** WAV carries PCM and nothing else, so AAC in WAV has nowhere to go. */
|
||||
val REFUSED_SPEC = OutputSpec(
|
||||
org.libremediaconverter.model.Container.WAV,
|
||||
VideoCodec.NONE,
|
||||
AudioCodec.AAC,
|
||||
)
|
||||
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000005")
|
||||
}
|
||||
}
|
||||
|
||||
/** An engine that records what it was asked for and writes an output, so a success is a success. */
|
||||
private class RefusingTranscoder : SoftwareTranscoder {
|
||||
|
||||
/** Every spec that actually reached an engine. Empty is the assertion for a refused job. */
|
||||
val invocations = mutableListOf<OutputSpec>()
|
||||
|
||||
override suspend fun run(
|
||||
request: ConversionRequest,
|
||||
inputPath: String,
|
||||
output: File,
|
||||
durationMs: Long,
|
||||
onProgress: (Int) -> Unit,
|
||||
) {
|
||||
invocations += request.spec
|
||||
output.writeBytes(ByteArray(OUTPUT_BYTES))
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val OUTPUT_BYTES = 512
|
||||
}
|
||||
}
|
||||
@@ -1,12 +1,16 @@
|
||||
package org.libremediaconverter.work
|
||||
|
||||
import android.app.Application
|
||||
import android.content.Context
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import androidx.work.ForegroundInfo
|
||||
import androidx.work.ForegroundUpdater
|
||||
import androidx.work.ListenableWorker
|
||||
import androidx.work.testing.TestListenableWorkerBuilder
|
||||
import androidx.work.workDataOf
|
||||
import com.google.common.util.concurrent.ListenableFuture
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import org.junit.After
|
||||
@@ -18,6 +22,7 @@ import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.OutputPublisher
|
||||
import org.libremediaconverter.convert.SoftwareTranscoder
|
||||
import org.libremediaconverter.convert.StagingNames
|
||||
import org.libremediaconverter.convert.installTestWorkManager
|
||||
import org.libremediaconverter.model.ConversionRequest
|
||||
import org.libremediaconverter.model.DeviceCodecs
|
||||
@@ -108,6 +113,54 @@ class WorkerCancellationTest {
|
||||
assertEquals("a failed attempt must not leave its partial behind", emptyList<String>(), stagedNames())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cancelled join propagates instead of being turned into a Result`() {
|
||||
val thrown = runCatching { runBlocking { concatWorker().doWork() } }.exceptionOrNull()
|
||||
|
||||
assertTrue(
|
||||
"cancellation must leave doWork as cancellation, not as a Result; got $thrown",
|
||||
thrown is CancellationException,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cancelled join still deletes the partial it had already staged`() {
|
||||
// Written first, so a missing delete cannot pass by asking whether a file nobody wrote is
|
||||
// absent -- the same reason PartialThenFailingTranscoder writes before it throws.
|
||||
concatStagedFile().writeBytes(ByteArray(PARTIAL_STAGED_BYTES))
|
||||
|
||||
runCatching { runBlocking { concatWorker().doWork() } }
|
||||
|
||||
assertEquals("a cancelled join must not leave its partial behind", emptyList<String>(), stagedNames())
|
||||
}
|
||||
|
||||
/**
|
||||
* A join whose foreground start is cancelled rather than denied.
|
||||
*
|
||||
* The conversion twin cancels *inside the engine*, which is the honest shape there because
|
||||
* `ConversionDependencies` has a seam for it. `ConcatWorker` calls `ConcatEngine` directly and
|
||||
* has no such seam -- it is native, and nothing here gets past it -- so the cancellation is
|
||||
* injected at the only other point inside the `try`: `setForeground`. That is not a contrivance.
|
||||
* A job cancelled while WorkManager is promoting it to the foreground is precisely when the
|
||||
* window is open, and what is being tested is the `catch` arm, which cannot tell where in the
|
||||
* `try` the cancellation came from.
|
||||
*/
|
||||
private fun concatWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
|
||||
context = app,
|
||||
inputData = workDataOf(
|
||||
ConcatWorker.KEY_INPUT_URIS to arrayOf(INPUT.toString(), "file:///tmp/second.mp4"),
|
||||
ConcatWorker.KEY_TOTAL_BYTES to INPUT_BYTES,
|
||||
ConcatWorker.KEY_FORMAT to CONCAT_FORMAT.name,
|
||||
),
|
||||
runAttemptCount = 0,
|
||||
).setId(CONCAT_ID)
|
||||
.setForegroundUpdater(CancellingForegroundUpdater)
|
||||
.build()
|
||||
|
||||
/** The staging path the join will compute, asked for rather than spelled out here. */
|
||||
private fun concatStagedFile(): File =
|
||||
publisher.createStagingFile(StagingNames.forJob(CONCAT_ID, CONCAT_FORMAT.extension))
|
||||
|
||||
/**
|
||||
* A worker routed to the software engine, which is [failure] and nothing else.
|
||||
*
|
||||
@@ -142,7 +195,10 @@ class WorkerCancellationTest {
|
||||
const val DISPLAY_NAME = "holiday.mp4"
|
||||
const val INPUT_BYTES = 1024L
|
||||
val SPEC = OutputFormat.MP4_H265.spec
|
||||
val CONCAT_FORMAT = OutputFormat.MP4_H264
|
||||
const val PARTIAL_STAGED_BYTES = 2048
|
||||
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000003")
|
||||
val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000004")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -167,3 +223,19 @@ private class PartialThenFailingTranscoder(private val failure: () -> Nothing) :
|
||||
const val PARTIAL_BYTES = 2048
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Stands in for a job cancelled while WorkManager is promoting it to the foreground.
|
||||
*
|
||||
* The mechanism `DeniedForegroundStartTest` documents, carrying a different exception:
|
||||
* `WorkForegroundUpdater` propagates whatever the future failed with, and
|
||||
* `ListenableFuture.await()` unwraps the `ExecutionException`, so the worker meets a bare
|
||||
* `CancellationException` exactly where a real cancellation would put one.
|
||||
*/
|
||||
private object CancellingForegroundUpdater : ForegroundUpdater {
|
||||
override fun setForegroundAsync(
|
||||
context: Context,
|
||||
id: UUID,
|
||||
foregroundInfo: ForegroundInfo,
|
||||
): ListenableFuture<Void> = FailedFuture(CancellationException("cancelled while going foreground"))
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@ import org.libremediaconverter.model.DeviceCodecs
|
||||
import org.libremediaconverter.model.EnginePreference
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.model.OutputSpec
|
||||
import org.libremediaconverter.model.QualityTier
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
@@ -109,6 +110,50 @@ class WorkerEnumFallbackTest {
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a container this build does not define falls back to the default spec`() {
|
||||
assertFallsBackToDefault(container = "HOLOTAPE")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a video codec this build does not define falls back to the default spec`() {
|
||||
assertFallsBackToDefault(video = "H267")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an audio codec this build does not define falls back to the default spec`() {
|
||||
assertFallsBackToDefault(audio = "SUPER_AAC")
|
||||
}
|
||||
|
||||
/**
|
||||
* Drives a job whose spec is [NOT_THE_FALLBACK] on every axis but the one named, and asserts the
|
||||
* whole spec came back as [DEFAULT_SPEC].
|
||||
*
|
||||
* **The baseline is the point.** `readSpec` returns the *entire* fallback spec the moment any
|
||||
* one axis fails to resolve, so a test starting from `MP4_H265` -- which is itself the fallback
|
||||
* -- could not tell a worker that read the spec correctly from one that gave up on it. Starting
|
||||
* from MKV/H.264 makes the difference visible on two axes at once.
|
||||
*
|
||||
* Asserting the spec that *ran*, rather than only that a `Result` came back, is the other half:
|
||||
* the defect these three are written for threw out of `doWork` entirely, so "a Result at all"
|
||||
* would pass against a fallback to something arbitrary.
|
||||
*/
|
||||
private fun assertFallsBackToDefault(
|
||||
container: String = NOT_THE_FALLBACK.container.name,
|
||||
video: String = NOT_THE_FALLBACK.videoCodec.name,
|
||||
audio: String = NOT_THE_FALLBACK.audioCodec.name,
|
||||
) {
|
||||
val transcoder = RequestRecordingTranscoder()
|
||||
ConversionDependencies.software = { transcoder }
|
||||
|
||||
val result = runBlocking {
|
||||
conversionWorker(container = container, video = video, audio = audio).doWork()
|
||||
}
|
||||
|
||||
assertEquals(ListenableWorker.Result.success(), stripOutput(result))
|
||||
assertEquals(listOf(DEFAULT_SPEC), transcoder.specs)
|
||||
}
|
||||
|
||||
/** [ListenableWorker.Result.Success] compares its output data, which these tests do not pin. */
|
||||
private fun stripOutput(result: ListenableWorker.Result): ListenableWorker.Result =
|
||||
if (result is ListenableWorker.Result.Success) ListenableWorker.Result.success() else result
|
||||
@@ -116,15 +161,18 @@ class WorkerEnumFallbackTest {
|
||||
private fun conversionWorker(
|
||||
quality: String = QualityTier.FAST.name,
|
||||
preference: String = EnginePreference.FORCE_SOFTWARE.name,
|
||||
container: String = SPEC.container.name,
|
||||
video: String = SPEC.videoCodec.name,
|
||||
audio: String = SPEC.audioCodec.name,
|
||||
): ConversionWorker = TestListenableWorkerBuilder<ConversionWorker>(
|
||||
context = app,
|
||||
inputData = workDataOf(
|
||||
ConversionWorker.KEY_INPUT_URI to INPUT.toString(),
|
||||
ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME,
|
||||
ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES,
|
||||
ConversionWorker.KEY_CONTAINER to SPEC.container.name,
|
||||
ConversionWorker.KEY_VIDEO_CODEC to SPEC.videoCodec.name,
|
||||
ConversionWorker.KEY_AUDIO_CODEC to SPEC.audioCodec.name,
|
||||
ConversionWorker.KEY_CONTAINER to container,
|
||||
ConversionWorker.KEY_VIDEO_CODEC to video,
|
||||
ConversionWorker.KEY_AUDIO_CODEC to audio,
|
||||
ConversionWorker.KEY_QUALITY to quality,
|
||||
ConversionWorker.KEY_ENGINE_PREFERENCE to preference,
|
||||
),
|
||||
@@ -146,6 +194,12 @@ class WorkerEnumFallbackTest {
|
||||
const val DISPLAY_NAME = "holiday.mp4"
|
||||
const val INPUT_BYTES = 1024L
|
||||
val SPEC = OutputFormat.MP4_H265.spec
|
||||
|
||||
/** What `readSpec` returns when any axis fails to resolve. */
|
||||
val DEFAULT_SPEC = OutputFormat.MP4_H265.spec
|
||||
|
||||
/** A spec that differs from [DEFAULT_SPEC] on container *and* video codec. See the helper. */
|
||||
val NOT_THE_FALLBACK = OutputFormat.MKV_H264.spec
|
||||
val CONVERSION_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000021")
|
||||
val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000022")
|
||||
}
|
||||
@@ -156,6 +210,9 @@ private class RequestRecordingTranscoder : SoftwareTranscoder {
|
||||
|
||||
val qualities = mutableListOf<QualityTier>()
|
||||
|
||||
/** The spec each run was asked for. Which one ran is what the three readSpec tests assert. */
|
||||
val specs = mutableListOf<OutputSpec>()
|
||||
|
||||
override suspend fun run(
|
||||
request: ConversionRequest,
|
||||
inputPath: String,
|
||||
@@ -164,6 +221,7 @@ private class RequestRecordingTranscoder : SoftwareTranscoder {
|
||||
onProgress: (Int) -> Unit,
|
||||
) {
|
||||
qualities += request.quality
|
||||
specs += request.spec
|
||||
output.writeBytes(ByteArray(OUTPUT_BYTES))
|
||||
}
|
||||
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
package org.libremediaconverter.work
|
||||
|
||||
import android.content.Context
|
||||
import com.google.common.util.concurrent.ListenableFuture
|
||||
import org.libremediaconverter.convert.OutputPublisher
|
||||
import org.libremediaconverter.convert.SoftwareTranscoder
|
||||
import org.libremediaconverter.model.ConversionRequest
|
||||
import java.io.File
|
||||
import java.util.concurrent.ExecutionException
|
||||
import java.util.concurrent.Executor
|
||||
import java.util.concurrent.TimeUnit
|
||||
|
||||
/**
|
||||
* Scaffolding more than one worker test needs.
|
||||
@@ -68,3 +72,25 @@ object WritingTranscoder : SoftwareTranscoder {
|
||||
|
||||
private const val OUTPUT_BYTES = 512
|
||||
}
|
||||
|
||||
/**
|
||||
* An already-failed future, written out rather than pulled from a futures library.
|
||||
*
|
||||
* `await()` takes the `isDone` fast path and unwraps the `ExecutionException`, which is what puts
|
||||
* the original exception in front of the worker's `catch` rather than a wrapper. That is the whole
|
||||
* mechanism behind driving a `ForegroundUpdater` to fail: `WorkForegroundUpdater` propagates
|
||||
* whatever the future failed with rather than swallowing it, so `setForeground()` throws exactly
|
||||
* what is handed here.
|
||||
*
|
||||
* Shared because two tests inject two different failures through it -- a denied foreground start
|
||||
* and a cancellation -- and Kotlin will not take two file-private top-level classes of one name in
|
||||
* one package.
|
||||
*/
|
||||
internal class FailedFuture(private val failure: Throwable) : ListenableFuture<Void> {
|
||||
override fun addListener(listener: Runnable, executor: Executor): Unit = executor.execute(listener)
|
||||
override fun cancel(mayInterruptIfRunning: Boolean): Boolean = false
|
||||
override fun isCancelled(): Boolean = false
|
||||
override fun isDone(): Boolean = true
|
||||
override fun get(): Void = throw ExecutionException(failure)
|
||||
override fun get(timeout: Long, unit: TimeUnit): Void = throw ExecutionException(failure)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user