Compare commits
21
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
dbedfb4708 | ||
|
|
c6e9a480e1 | ||
|
|
d8110d7a62 | ||
|
|
6f3966cc69 | ||
|
|
7595177e81 | ||
|
|
a507736d3d | ||
|
|
1ff5c4463c | ||
|
|
70337b2c12 | ||
|
|
fea480b000 | ||
|
|
ddfb1dd78e | ||
|
|
348eaa2f22 | ||
|
|
104d02de03 | ||
|
|
90814222b7 | ||
|
|
2d4898ad44 | ||
|
|
83ac7eff2c | ||
|
|
b2c11bbfe0 | ||
|
|
3a5210ec5d | ||
|
|
5f3eda9c40 | ||
|
|
79cca0eb47 | ||
|
|
a84b24ba27 | ||
|
|
b2790e13d9 |
@@ -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
|
- 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
|
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.
|
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**,
|
- **Coverage is reported, not gated** — **88.9% of lines (2087/2348), 75.4% of branches
|
||||||
measured 2026-08-26 with `./gradlew :app:jacocoTestReport`, against 454 JVM tests in 67 classes.
|
(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.**
|
**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
|
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
|
disproportionately Robolectric, so each one added denominator and no numerator — the measurement
|
||||||
was punishing exactly the tests that were hardest to write.
|
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
|
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
|
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
|
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
|
grew 2194 -> 2321, because that work added production code of its own; then again on 2026-08-27
|
||||||
quoting**: this entry was written quoting 81.4%, measured four hours earlier, and was already
|
as #132 and #133's ten children landed — 84.9% -> 87.1% line, 63.8% -> **69.1%** branch, 454 ->
|
||||||
three points stale by the time it was ready to merge.
|
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
|
- **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 —
|
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.
|
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.
|
- `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
|
- **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
|
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:
|
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.AndroidViewModel
|
||||||
import androidx.lifecycle.viewModelScope
|
import androidx.lifecycle.viewModelScope
|
||||||
import androidx.media3.common.util.UnstableApi
|
import androidx.media3.common.util.UnstableApi
|
||||||
|
import androidx.work.Data
|
||||||
import androidx.work.WorkInfo
|
import androidx.work.WorkInfo
|
||||||
import androidx.work.WorkManager
|
import androidx.work.WorkManager
|
||||||
import kotlinx.coroutines.CoroutineDispatcher
|
import kotlinx.coroutines.CoroutineDispatcher
|
||||||
@@ -71,6 +72,119 @@ data class InputFile(
|
|||||||
val probe: InputProbe? = null,
|
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 {
|
sealed interface ConversionState {
|
||||||
data object Idle : ConversionState
|
data object Idle : ConversionState
|
||||||
data class Ready(val input: InputFile) : 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
|
// 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.
|
// or to no file, and this is where that stays true.
|
||||||
if (!ownership.stillHeldBy(token)) return@collect
|
if (!ownership.stillHeldBy(token)) return@collect
|
||||||
_state.value = when (info.state) {
|
val next = conversionStateFrom(
|
||||||
WorkInfo.State.RUNNING -> ConversionState.Converting(
|
ConversionUpdate(
|
||||||
input,
|
state = info.state,
|
||||||
info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
|
progressPercent = info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
|
||||||
)
|
runAttemptCount = info.runAttemptCount,
|
||||||
|
outputData = info.outputData,
|
||||||
// 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
|
input = input,
|
||||||
// start again while the app was in the background — the second being the
|
cancelled = cancelled,
|
||||||
// likelier of the two, since it needs only a process restart. Nothing here
|
fallbackSpec = _settings.value.spec,
|
||||||
// can tell them apart, and nothing needs to: the answer is the same.
|
)
|
||||||
WorkInfo.State.ENQUEUED ->
|
// Take responsibility for the file at the same moment the state starts referring
|
||||||
if (info.runAttemptCount > 0) {
|
// to it, so the two cannot disagree. Read off the result rather than assigned
|
||||||
ConversionState.Waiting(input)
|
// inside the mapping: `Converted` is the only state that carries a staged file, so
|
||||||
} else {
|
// "the state and `pendingStaged` refer to the same file or to no file" is now the
|
||||||
ConversionState.Converting(input, 0)
|
// shape of the code rather than a rule two branches have to keep.
|
||||||
}
|
if (next is ConversionState.Converted) pendingStaged = next.staged
|
||||||
|
_state.value = next
|
||||||
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)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -565,7 +628,7 @@ class ConversionViewModel @JvmOverloads constructor(
|
|||||||
// than a fresh handle for the second failure's sake: a retry that fails again
|
// 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
|
// lands back here still carrying the file, not on a bare Failed that would take
|
||||||
// the offer away.
|
// 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,
|
state = state,
|
||||||
settings = settings,
|
settings = settings,
|
||||||
validation = validation,
|
validation = validation,
|
||||||
actions = ConverterActions(
|
actions = converterActions(
|
||||||
|
viewModel = viewModel,
|
||||||
onPickInput = { pickInput.launch(arrayOf("*/*")) },
|
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) },
|
onConvert = { requestNotifications.launch(Manifest.permission.POST_NOTIFICATIONS) },
|
||||||
onCancel = viewModel::cancel,
|
|
||||||
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
||||||
onReset = viewModel::reset,
|
|
||||||
),
|
),
|
||||||
modifier = modifier,
|
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.
|
* Everything [ConverterScreenContent] can ask for, in one value.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -92,7 +92,12 @@ object MediaProbe {
|
|||||||
else -> InputKind.UNPARSEABLE
|
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 videoCodec: String?,
|
||||||
val audioCodec: String?,
|
val audioCodec: String?,
|
||||||
val durationMs: Long,
|
val durationMs: Long,
|
||||||
@@ -100,33 +105,55 @@ object MediaProbe {
|
|||||||
val height: Int,
|
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? {
|
private fun probeWithExtractor(context: Context, uri: Uri): Extracted? {
|
||||||
val extractor = MediaExtractor()
|
val extractor = MediaExtractor()
|
||||||
return try {
|
return try {
|
||||||
extractor.setDataSource(context, uri, null)
|
extractor.setDataSource(context, uri, null)
|
||||||
var video: String? = null
|
extractedFrom(extractor.trackFormats())
|
||||||
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)
|
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Log.i(TAG, "Platform extractor could not read $uri.", e)
|
Log.i(TAG, "Platform extractor could not read $uri.", e)
|
||||||
null
|
null
|
||||||
@@ -269,25 +296,7 @@ object MediaProbe {
|
|||||||
val extractor = MediaExtractor()
|
val extractor = MediaExtractor()
|
||||||
return try {
|
return try {
|
||||||
extractor.setDataSource(context, uri, null)
|
extractor.setDataSource(context, uri, null)
|
||||||
var video: String? = null
|
concatInputFrom(extractor.trackFormats())
|
||||||
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)
|
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Log.i(TAG, "Could not probe $uri for concat; will re-encode.", e)
|
Log.i(TAG, "Could not probe $uri for concat; will re-encode.", e)
|
||||||
ConcatInput(null, null, 0, 0, 0)
|
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.
|
* One track property as an Int, or [fallback] when the format has no Int to give.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -24,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. " +
|
"The finished file is no longer in the cache, so there is nothing left to save. " +
|
||||||
"Start over to make it again."
|
"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.
|
* A staged file that is still there to be saved, and everything the save dialog needs to offer it.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -56,17 +56,38 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
|||||||
|
|
||||||
JoinScreenContent(
|
JoinScreenContent(
|
||||||
state = state,
|
state = state,
|
||||||
actions = JoinActions(
|
actions = joinActions(
|
||||||
|
viewModel = viewModel,
|
||||||
onPickInputs = { pickInputs.launch(arrayOf("video/*")) },
|
onPickInputs = { pickInputs.launch(arrayOf("video/*")) },
|
||||||
onJoin = viewModel::join,
|
|
||||||
onCancel = viewModel::cancel,
|
|
||||||
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
||||||
onReset = viewModel::reset,
|
|
||||||
),
|
),
|
||||||
modifier = modifier,
|
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.
|
* Everything [JoinScreenContent] can ask for, in one value.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import android.net.Uri
|
|||||||
import androidx.lifecycle.AndroidViewModel
|
import androidx.lifecycle.AndroidViewModel
|
||||||
import androidx.lifecycle.viewModelScope
|
import androidx.lifecycle.viewModelScope
|
||||||
import androidx.media3.common.util.UnstableApi
|
import androidx.media3.common.util.UnstableApi
|
||||||
|
import androidx.work.Data
|
||||||
import androidx.work.WorkInfo
|
import androidx.work.WorkInfo
|
||||||
import androidx.work.WorkManager
|
import androidx.work.WorkManager
|
||||||
import kotlinx.coroutines.CoroutineDispatcher
|
import kotlinx.coroutines.CoroutineDispatcher
|
||||||
@@ -19,6 +20,7 @@ import org.libremediaconverter.convert.ConversionDependencies
|
|||||||
import org.libremediaconverter.convert.InputFile
|
import org.libremediaconverter.convert.InputFile
|
||||||
import org.libremediaconverter.convert.InputQuery
|
import org.libremediaconverter.convert.InputQuery
|
||||||
import org.libremediaconverter.convert.PendingSave
|
import org.libremediaconverter.convert.PendingSave
|
||||||
|
import org.libremediaconverter.convert.SAVE_FAILED_MESSAGE
|
||||||
import org.libremediaconverter.convert.STAGED_FILE_GONE_MESSAGE
|
import org.libremediaconverter.convert.STAGED_FILE_GONE_MESSAGE
|
||||||
import org.libremediaconverter.convert.ScreenOwnership
|
import org.libremediaconverter.convert.ScreenOwnership
|
||||||
import org.libremediaconverter.model.ConcatStrategy
|
import org.libremediaconverter.model.ConcatStrategy
|
||||||
@@ -29,6 +31,108 @@ import org.libremediaconverter.work.jobSnapshots
|
|||||||
import java.io.File
|
import java.io.File
|
||||||
import java.util.UUID
|
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 {
|
sealed interface JoinState {
|
||||||
data object Idle : JoinState
|
data object Idle : JoinState
|
||||||
data class Ready(val inputs: List<InputFile>) : JoinState
|
data class Ready(val inputs: List<InputFile>) : JoinState
|
||||||
@@ -194,7 +298,7 @@ class JoinViewModel @JvmOverloads constructor(
|
|||||||
fun onInputsPicked(uris: List<Uri>) {
|
fun onInputsPicked(uris: List<Uri>) {
|
||||||
val token = ownership.claim()
|
val token = ownership.claim()
|
||||||
if (uris.size < 2) {
|
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
|
return
|
||||||
}
|
}
|
||||||
viewModelScope.launch {
|
viewModelScope.launch {
|
||||||
@@ -250,56 +354,19 @@ class JoinViewModel @JvmOverloads constructor(
|
|||||||
// takes ownership of the staged file, and a superseded observation must not do
|
// takes ownership of the staged file, and a superseded observation must not do
|
||||||
// that either.
|
// that either.
|
||||||
if (!ownership.stillHeldBy(token)) return@collect
|
if (!ownership.stillHeldBy(token)) return@collect
|
||||||
_state.value = when (info.state) {
|
val next = joinStateFrom(
|
||||||
WorkInfo.State.RUNNING, WorkInfo.State.BLOCKED -> JoinState.Joining(inputs)
|
JoinUpdate(
|
||||||
WorkInfo.State.ENQUEUED ->
|
state = info.state,
|
||||||
if (info.runAttemptCount > 0) {
|
runAttemptCount = info.runAttemptCount,
|
||||||
JoinState.Waiting(inputs)
|
outputData = info.outputData,
|
||||||
} else {
|
),
|
||||||
JoinState.Joining(inputs)
|
inputs = inputs,
|
||||||
}
|
cancelled = cancelled,
|
||||||
|
)
|
||||||
WorkInfo.State.SUCCEEDED -> {
|
// Read off the result rather than assigned inside the mapping -- see the same
|
||||||
val path = info.outputData.getString(ConcatWorker.KEY_OUTPUT_PATH)
|
// three lines in ConversionViewModel for why that is the better half of the swap.
|
||||||
val strategy = info.outputData.getString(ConcatWorker.KEY_STRATEGY)
|
if (next is JoinState.Joined) pendingStaged = next.staged
|
||||||
?.let(ConcatStrategy::valueOf) ?: ConcatStrategy.REENCODE
|
_state.value = next
|
||||||
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
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -346,7 +413,7 @@ class JoinViewModel @JvmOverloads constructor(
|
|||||||
// than leaving "Start over" -- which deletes it -- as the only thing on offer.
|
// 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
|
// Passing `pending` rather than rebuilding it is what keeps a retry that fails
|
||||||
// again on a carrying Failed instead of a bare one.
|
// 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)
|
val uris = inputData.getStringArray(KEY_INPUT_URIS)?.map(Uri::parse)
|
||||||
?: return Result.failure(workDataOf(KEY_ERROR to "No input files."))
|
?: return Result.failure(workDataOf(KEY_ERROR to "No input files."))
|
||||||
if (uris.size < 2) {
|
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
|
// 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.
|
// 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 -> {
|
FailureOutcome.FAIL -> {
|
||||||
Log.e(TAG, "Joining failed.", e)
|
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 {
|
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_INPUT_URIS = "input_uris"
|
||||||
const val KEY_TOTAL_BYTES = "total_bytes"
|
const val KEY_TOTAL_BYTES = "total_bytes"
|
||||||
const val KEY_FORMAT = "format"
|
const val KEY_FORMAT = "format"
|
||||||
|
|||||||
@@ -313,7 +313,7 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
|
|||||||
}
|
}
|
||||||
FailureOutcome.FAIL -> {
|
FailureOutcome.FAIL -> {
|
||||||
Log.e(TAG, "Conversion failed.", cause)
|
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 {
|
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_INPUT_URI = "input_uri"
|
||||||
const val KEY_DISPLAY_NAME = "display_name"
|
const val KEY_DISPLAY_NAME = "display_name"
|
||||||
const val KEY_SIZE_BYTES = "size_bytes"
|
const val KEY_SIZE_BYTES = "size_bytes"
|
||||||
|
|||||||
@@ -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,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
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -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,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")
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -146,7 +146,7 @@ class RefusedJobTest {
|
|||||||
|
|
||||||
assertEquals(
|
assertEquals(
|
||||||
ListenableWorker.Result.failure(
|
ListenableWorker.Result.failure(
|
||||||
workDataOf(ConcatWorker.KEY_ERROR to "Pick at least two files to join."),
|
workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.TOO_FEW_INPUTS_MESSAGE),
|
||||||
),
|
),
|
||||||
result,
|
result,
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user